1633355 - Hit MOZ_CRASH(temporarily unsupported conversion of typeref to JS value) at wasm/WasmInstance.cpp:272 with --wasm-gc

Status: VERIFIED FIXED

(Core :: JavaScript: WebAssembly, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20200426-df251f2e0320 (debug build, run with --fuzzing-safe --no-threads --wasm-gc):

var g23 = newGlobal({newCompartment: true});
g23.parent = this;
g23.eval(`
    var dbg = new Debugger(parent);
    dbg.onEnterFrame = function(frame) {}
`);
let bin = wasmTextToBinary(`
     (type $wabbit (struct
        (field $x (mut i32))
        (field $left (mut (ref opt $wabbit)))
        (field $right (mut (ref opt $wabbit)))
     ))
     (global $g (mut (ref opt $wabbit)) (ref.null))
     (func (export "init") (param $n i32)
       (global.set $g (call $make (local.get $n)))
     )
     (func $make (param $n i32) (result (ref opt $wabbit))
       (local $tmp i32)
       (struct.new $wabbit (local.get $tmp) (ref.null) (ref.null))
     )
`);
let mod = new WebAssembly.Module(bin);
let ins = new WebAssembly.Instance(mod).exports;
ins.init(6);

Backtrace:

received signal SIGSEGV, Segmentation fault.
0x00005555568c77f5 in bool ToJSValue<DebugCodegenVal>(JSContext*, void const*, js::wasm::ValType, JS::MutableHandle<JS::Value>) ()
#0  0x00005555568c77f5 in bool ToJSValue<DebugCodegenVal>(JSContext*, void const*, js::wasm::ValType, JS::MutableHandle<JS::Value>) ()
#1  0x00005555568c6df0 in js::wasm::ResultsToJSValue(JSContext*, js::wasm::ResultType, void*, mozilla::Maybe<char*>, JS::MutableHandle<JS::Value>) ()
#2  0x000055555695e139 in js::wasm::DebugFrame::updateReturnJSValue(JSContext*) ()
#3  0x000055555683343a in WasmHandleDebugTrap() ()
#4  0x00000ae7e59cf6fe in ?? ()
#5  0x00007fffffffb350 in ?? ()
#6  0x00007fffffffb378 in ?? ()
#7  0x0000000000000000 in ?? ()
rax	0x555557050114	93825020526868
rbx	0x7fffffffb468	140737488335976
rcx	0x555557f9c908	93825036568840
rdx	0x0	0
rsi	0x7ffff6efd770	140737336301424
rdi	0x7ffff6efc540	140737336296768
rbp	0x7fffffffb1c0	140737488335296
rsp	0x7fffffffb190	140737488335248
r8	0x7ffff6efd770	140737336301424
r9	0x7ffff7f9bd00	140737353727232
r10	0x58	88
r11	0x7ffff6ba47a0	140737332791200
r12	0x7fffffffb468	140737488335976
r13	0x7ffff5e27000	140737318645760
r14	0x7fffffffb470	140737488335984
r15	0x0	0
rip	0x5555568c77f5 <bool ToJSValue<DebugCodegenVal>(JSContext*, void const*, js::wasm::ValType, JS::MutableHandle<JS::Value>)+917>
=> 0x5555568c77f5 <_ZL9ToJSValueI15DebugCodegenValEbP9JSContextPKvN2js4wasm7ValTypeEN2JS13MutableHandleINS8_5ValueEEE+917>:	movl   $0x110,0x0
   0x5555568c7800 <_ZL9ToJSValueI15DebugCodegenValEbP9JSContextPKvN2js4wasm7ValTypeEN2JS13MutableHandleINS8_5ValueEEE+928>:	callq  0x555555818636 <abort>

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Lars T Hansen [:lth]]

This is probably a result of the multi-value related refactoring of this code. Types of the kind (ref T) and (optref T) aren't allowed to escape to JS yet so ToJSValue cannot be allowed on them, and in this case ToJSValue is not the correct thing to call when encountering such a value in a debug frame. Here it's the code in the debug frame that needs a guard, I think, though I'm not yet sure what that should look like.

Comment 3

[Andy Wingo [:wingo]]

It is a result of computing cachedReturnJSValue. Used to be we'd let this one escape as a JSObject: https://hg.mozilla.org/mozilla-central/diff/e7f852ba3cd19120f17ad65c5a43e3d40db595aa/js/src/wasm/WasmTypes.cpp#l1.48

Should we do the same as bug 1632113 or what?

Comment 4

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200428100141-a99c73301874.
The bug appears to have been introduced in the following build range:
> Start: 2fd61eb5c69ce9ac806048a35c7a7a88bf4b9652 (20200421094220)
> End: 263963426b561b2aa687aeeaeddc4fd93fff9e57 (20200421222503)
> Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2fd61eb5c69ce9ac806048a35c7a7a88bf4b9652&tochange=263963426b561b2aa687aeeaeddc4fd93fff9e57

Comment 5

[Lars T Hansen [:lth]]

(In reply to Andy Wingo [:wingo] from comment #3)

It is a result of computing cachedReturnJSValue. Used to be we'd let this one escape as a JSObject: https://hg.mozilla.org/mozilla-central/diff/e7f852ba3cd19120f17ad65c5a43e3d40db595aa/js/src/wasm/WasmTypes.cpp#l1.48

Should we do the same as bug 1632113 or what?

Well, we absolutely cannot allow ToWebAssemblyValue in this situation (this in reference to the earlier patch) because we don't have the machinery for the type check. ToJSValue is arguably safe. That said, the bug here is really ToJSValue_typeref which just crashes when (IIUC) it could throw, could it not?

Comment 6

[Andy Wingo [:wingo]]

Attachment: Bug 1633355 - Throw instead of crashing when debugging wasm typeref-valued functions

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef4071208f60
Throw instead of crashing when debugging wasm typeref-valued functions r=lth

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ef4071208f60

Comment 9

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20200518152416-a627b6676824.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.