Closed Bug 1620748 Opened 4 months ago Closed 3 months ago

Wasm debugging crash in DebuggerEnvironment::getVariable

Categories

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

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox-esr68 --- unaffected
firefox74 --- wontfix
firefox75 --- wontfix
firefox76 --- fixed

People

(Reporter: lth, Assigned: lth)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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.)

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.

Flags: needinfo?(ydelendik)

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?

Flags: needinfo?(ydelendik) → needinfo?(lhansen)

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.

Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Flags: needinfo?(lhansen)
Has STR: --- → yes
Regressed by: 1598009

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

Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/195f1fd6cc2b
Decode already-read byte for type for debugger.  r=rhunt
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
You need to log in before you can comment on or make changes to this bug.