1331064 - Assertion failure: throwing, at js/src/jscntxt.cpp:946 with wasm and Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 97d6f7364394 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

var lfModule = new WebAssembly.Module(wasmTextToBinary("(module)"));
var g = newGlobal();
g.parent = this;
g.eval("new Debugger(parent).onExceptionUnwind = function () { hits++; };");
lfModule = new WebAssembly.Module(wasmTextToBinary(`
(module
    (import $imp "a" "b" (result i32))
    (memory 1 1)
    (table 2 2 anyfunc)
    (elem (i32.const 0) $imp $def)
    (func $def (result i32) (i32.load (i32.const 0)))
    (type $v2i (func (result i32)))
    (func $call (param i32) (result i32) (call_indirect $v2i (get_local 0)))
    (export "call" $call)
)
`));
processCode("jsTestDriverEnd();");
function processCode(lfVarx) {
    processModule(lfModule, lfVarx);
}
function processModule(module, jscode) {
    imports = {}
    for (let descriptor of WebAssembly.Module.imports(module)) {
        imports[descriptor.module] = {}
        imports[descriptor.module][descriptor.name] = new Function("x", "y", "z", jscode);
            instance = new WebAssembly.Instance(module, imports);
    }
    for (let descriptor of WebAssembly.Module.exports(module)) {
        switch (descriptor.kind) {
            case "function":
                print(instance.exports[descriptor.name]())
        }
    }
}



Backtrace:

 received signal SIGSEGV, Segmentation fault.
JSContext::getPendingException (this=<optimized out>, rval=..., rval@entry=...) at js/src/jscntxt.cpp:946
#0  JSContext::getPendingException (this=<optimized out>, rval=..., rval@entry=...) at js/src/jscntxt.cpp:946
#1  0x0000000000a94656 in js::Debugger::fireExceptionUnwind (this=this@entry=0x7ffff693d800, cx=0x7ffff695f000, vp=..., vp@entry=...) at js/src/vm/Debugger.cpp:1764
#2  0x0000000000a951a6 in js::Debugger::<lambda(js::Debugger*)>::operator() (dbg=0x7ffff693d800, __closure=<synthetic pointer>) at js/src/vm/Debugger.cpp:1026
#3  js::Debugger::dispatchHook<js::Debugger::slowPathOnExceptionUnwind(JSContext*, js::AbstractFramePtr)::<lambda(js::Debugger*)>, js::Debugger::slowPathOnExceptionUnwind(JSContext*, js::AbstractFramePtr)::<lambda(js::Debugger*)> > (fireHook=..., cx=0x7ffff695f000, hookIsEnabled=...) at js/src/vm/Debugger.cpp:1893
#4  js::Debugger::slowPathOnExceptionUnwind (cx=cx@entry=0x7ffff695f000, frame=...) at js/src/vm/Debugger.cpp:1027
#5  0x0000000000de7690 in js::Debugger::onExceptionUnwind (frame=..., cx=0x7ffff695f000) at js/src/vm/Debugger-inl.h:66
#6  WasmHandleDebugThrow () at js/src/wasm/WasmTypes.cpp:152
#7  0x00007ffff7ff4c30 in ?? ()
[...]
#10 0x0000000000000000 in ?? ()
rax	0x2061520	33953056
rbx	0x7fffffffbd38	140737488338232
rcx	0x118e79e	18409374
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffbcc0	140737488338112
rsp	0x7fffffffbc90	140737488338064
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x0	0
r11	0x0	0
r12	0x7ffff693d800	140737330272256
r13	0x7fffffffbd80	140737488338304
r14	0x7fffffffbe20	140737488338464
r15	0x7fffffffbd60	140737488338272
rip	0x925ef9 <JSContext::getPendingException(JS::MutableHandle<JS::Value>)+313>
=> 0x925ef9 <JSContext::getPendingException(JS::MutableHandle<JS::Value>)+313>:	movl   $0x0,0x0
   0x925f04 <JSContext::getPendingException(JS::MutableHandle<JS::Value>)+324>:	ud2

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

So, there's an exception here that's getting cleared, and then we try to reuse it.

Using rr shows that onExceptionUnwind is called here the first time: http://searchfox.org/mozilla-central/source/js/src/vm/Interpreter.cpp#1269

At some point, it clears the exception, much lower in the call stack:
- Debugger::reportUncaughtException(Maybe<AutoCompartment>& ac):
- in the call to PrepareScriptEnvironmentAndInvoke:
- cx->runtime()->scriptEnvironmentPreparer->invoke(scope, closure):
- which calls EnvironmentPreparer::invoke(HandleObject scope, Closure& closure)
- there's an AutoReportException there that clears the pending exception

When we get back from the FFI back to JS, since the function has throw, its return value is false, we get to the throw label which tries to call the onExceptionUnwind hook again, but there's no more exception pending on the context!

Would a correct fix be just returning early in WasmHandleTrap if there's no pending exception?

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Actually, it seems that there's an error in the JS code being called, *and* in the debugger handler (undefined references in both cases).

Comment 3

[Yury Delendik (:yury)]

Smaller test (call_indirect matters):

var g = newGlobal();
g.parent = this;
g.eval("new Debugger(parent).onExceptionUnwind = function () { some_error; };");

var module = new WebAssembly.Module(wasmTextToBinary(`
(module
    (import $imp "a" "b" (result i32))
    (memory 1 1)
    (table 2 2 anyfunc)
    (elem (i32.const 0) $imp $def)
    (func $def (result i32) (i32.load (i32.const 0)))
    (type $v2i (func (result i32)))
    (func $call (param i32) (result i32) (call_indirect $v2i (get_local 0)))
    (export "call" $call)
)`));

var instance = new WebAssembly.Instance(module, { a: { b: () => {
  some_other_error;
}}});
instance.exports.call();

Comment 4

[Yury Delendik (:yury)]

Attachment: Bug 1331064 - Ignore wasm onExceptionUnwind events in JSTRAP_ERROR state.

Review commit: https://reviewboard.mozilla.org/r/105612/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/105612/

Comment 5

[Luke Wagner [:luke]]

Comment on attachment 8828104 [details]
Bug 1331064 - Ignore wasm onExceptionUnwind events in JSTRAP_ERROR state.

https://reviewboard.mozilla.org/r/105612/#review106866

(Stealing)

::: js/src/wasm/WasmTypes.cpp:160
(Diff revision 1)
> +        // no onExceptionUnwind handlers must be fired.
> +        JSTrapStatus status;
> +        if (!cx->isExceptionPending())
> +            status = JSTRAP_ERROR;
> +        else
> +            status = Debugger::onExceptionUnwind(cx, frame);

Could this be
```
if (cx->isExceptionPendiong()) {
    JSTrapStatus status = Debugger::onExceptionUnwind(...);
    if (status == JSTRAP_RETURN) {
       ...
    }
}
```

Comment 6

[Yury Delendik (:yury)]

Comment on attachment 8828104 [details]
Bug 1331064 - Ignore wasm onExceptionUnwind events in JSTRAP_ERROR state.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/105612/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49e6eba10d32
Ignore wasm onExceptionUnwind events in JSTRAP_ERROR state. r=luke

Comment 8

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/0e0b0668aa16
user:        Yury Delendik
date:        Sat Jan 07 10:40:38 2017 -0600
summary:     Bug 1286948 - onEnterFrame/onLeaveFrame wasm events and callstack. r=shu

This iteration took 264.147 seconds to run.

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/49e6eba10d32