1620748 - Wasm debugging crash in DebuggerEnvironment::getVariable

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P2)

Description

[Lars T Hansen [:lth]]

Steps to repro: https://bugzilla.mozilla.org/show_bug.cgi?id=1618495#c12

Crash report: https://crash-stats.mozilla.org/report/index/69f72f06-0b29-4981-ac2a-4d4780200307

This is almost certainly related to buggy / missing support for wasm reference types in the debugger.

(I'm keeping this bug in the open since it is almost certainly only a problem in the devtools and not actually security sensitive.)

Comment 1

[Lars T Hansen [:lth]]

Yury, are you able to take a look at this? When I build Nightly locally and try to repro, the debugger + tab crash when I reach the breakpoint, so I can't really repro.

Comment 2

[Yury Delendik (:yury)]

The stack is:

#0  0x00007fd0f4ca1e4d in js::wasm::RefType::RefType(js::wasm::RefType::Kind) (this=0x7fffb0b91690, kind=4)
    at /home/yury/Work/mozilla-unified/js/src/wasm/WasmTypes.h:379
#1  0x00007fd0f4ce99ca in js::wasm::RefType::fromTypeCode(js::wasm::TypeCode) (tc=4)
    at /home/yury/Work/mozilla-unified/js/src/wasm/WasmTypes.h:393
#2  0x00007fd0f4e1e5bf in js::wasm::Decoder::uncheckedReadValType() (this=0x7fffb0b917f0)
    at /home/yury/Work/mozilla-unified/js/src/wasm/WasmValidate.h:680
#3  0x00007fd0f4e08f4d in js::wasm::DecodeValidatedLocalEntries(js::wasm::Decoder&, mozilla::Vector<js::wasm::ValType, 16ul, js::SystemAllocPolicy>*) (d=..., locals=0x7fffb0b91920)
    at /home/yury/Work/mozilla-unified/js/src/wasm/WasmValidate.cpp:440
#4  0x00007fd0f4d132e5 in js::wasm::DebugState::debugGetLocalTypes(unsigned int, mozilla::Vector<js::wasm::ValType, 16ul, js::SystemAllocPolicy>*, unsigned long*, js::wasm::StackResults*) (this=0x7fd0db1ba900, funcIndex=25, locals=0x7fffb0b91920, argsLength=0x7fffb0b918d0, stackResults=0x7fffb0b918cc)
    at /home/yury/Work/mozilla-unified/js/src/wasm/WasmDebug.cpp:367
#5  0x00007fd0f3df4e34 in js::WasmFunctionScope::create(JSContext*, JS::Handle<js::Scope*>, unsigned int) (cx=0x7fd0dd831000, enclosing=..., funcIndex=25) at /home/yury/Work/mozilla-unified/js/src/vm/Scope.cpp:1582
#6  0x00007fd0f4dac70d in js::WasmInstanceObject::getFunctionScope(JSContext*, JS::Handle<js::WasmInstanceObject*>, unsigned int) (cx=0x7fd0dd831000, instanceObj=..., funcIndex=25)
    at /home/yury/Work/mozilla-unified/js/src/wasm/WasmJS.cpp:1816
#7  0x00007fd0f3aa5a3e in js::GetFrameEnvironmentAndScope(JSContext*, js::AbstractFramePtr, unsigned char*, JS::MutableHandle<JSObject*>, JS::MutableHandle<js::Scope*>) (cx=0x7fd0dd831000, frame=..., pc=0x0, env=..., scope=...) at /home/yury/Work/mozilla-unified/js/src/vm/EnvironmentObject.cpp:3832
#8  0x00007fd0f3aa5524 in js::DebugEnvironments::updateLiveEnvironments(JSContext*) (cx=0x7fd0dd831000)
    at /home/yury/Work/mozilla-unified/js/src/vm/EnvironmentObject.cpp:2983
#9  0x00007fd0f3aa6743 in js::GetDebugEnvironmentForFrame(JSContext*, js::AbstractFramePtr, unsigned char*) (cx=0x7fd0dd831000, frame=..., pc=0x7fd0d4beb4e1 "\273")
    at /home/yury/Work/mozilla-unified/js/src/vm/EnvironmentObject.cpp:3284
#10 0x00007fd0f41274dc in js::DebuggerFrame::getEnvironment(JSContext*, JS::Handle<js::DebuggerFrame*>, JS::MutableHandle<js::DebuggerEnvironment*>) (cx=0x7fd0dd831000, frame=..., result=...)
    at /home/yury/Work/mozilla-unified/js/src/debugger/Frame.cpp:550
#11 0x00007fd0f412c13c in js::DebuggerFrame::CallData::environmentGetter() (this=0x7fffb0b927f8)
    at /home/yury/Work/mozilla-unified/js/src/debugger/Frame.cpp:1452
#12 0x00007fd0f4152a7e in js::DebuggerFrame::CallData::ToNative<&js::DebuggerFrame::CallData::environmentGetter>(JSContext*, unsigned int, JS::Value*) (cx=0x7fd0dd831000, argc=0, vp=0x7fffb0b92c20)
    at /home/yury/Work/mozilla-unified/js/src/debugger/Frame.cpp:1347

Crash caused by reading function (with index 25) locals:

(; 05 01 6F _04_ 7F 01 6F 01 7F 01 6F ;)
(local $var1 anyref) (local $var2 i32) (local $var3 i32) (local $var4 i32) (local $var5 i32) (local $var6 anyref) (local $var7 i32) (local $var8 anyref)

Code at https://searchfox.org/mozilla-central/source/js/src/wasm/WasmValidate.h#680 does not look right. Changing that to return RefType::fromTypeCode(TypeCode(code)); maybe addresses the issue.

Was the locals encoding changed for ref types?

Comment 3

[Lars T Hansen [:lth]]

Yes, I changed this code back in mid-December as we generalized the representation of types to accomodate the nulltype and future types. I think your proposed fix is right, as shown by readValType() directly below. WILLFIX.

Comment 4

[Lars T Hansen [:lth]]

Attachment: Bug 1620748 - Decode already-read byte for type for debugger. r?rhunt

This looks like a paste-o to me, the code in readValType() below clearly
shows we should use the byte we've already read.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/195f1fd6cc2b
Decode already-read byte for type for debugger.  r=rhunt

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/195f1fd6cc2b