Two notes about the smaller TC:
- it crashes without any flags at all in a release build, `js tc.js`
- in a debug build it will eventually (< 1min) abort with the expected assertion failure, `exn.isObject() && exn.toObject().is<ErrorObject>(), at /home/lhansen/m-u/js/src/wasm/WasmBuiltins.cpp:595` , with --fuzzing-safe (alone); without flags, it signals OOM
With rr and a debug build things become a little easier. We're in WasmHandleTrap with Trap::OutOfBounds as expected. We grab the activation and then the context from the activation. Down in ReportError, we find that the `exn` value is not an object (which is why the assertion fails), it is a string, "out of memory". This string is being thrown by js::ReportOutOfMemory in response to an allocation failure in js. But that OOM failure is itself a result of trying to report a wasm failure:
```
#0 js::ReportOutOfMemory (cx=0x7f4b70df5000) at js/src/vm/JSContext.cpp:257
#1 0x0000563e3895fb00 in js::gc::GCRuntime::tryNewTenuredThing<JSObject, (js::AllowGC)1> (cx=0x7f4b70df5000,
kind=js::gc::AllocKind::OBJECT12, thingSize=120) at js/src/gc/Allocator.cpp:367
...
#21 0x0000563e38532a93 in JS_ReportErrorNumberUTF8 (cx=0x7f4b70df5000,
errorCallback=0x563e37f98e20 <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=356)
at /home/lhansen/m-u/js/src/jsapi.cpp:4752
#22 0x0000563e391d1ff8 in ReportError (cx=0x7f4b70df5000, errorNumber=356) at js/src/wasm/WasmBuiltins.cpp:587
#23 0x0000563e391bc17e in WasmHandleTrap () at js/src/wasm/WasmBuiltins.cpp:642
...
I think the bug here is an assumption in the wasm code that when JS_ReportErrorNumberUTF8 is called by ReportError in WasmBuiltins.cpp, either there will be no pending exception or the exception will be an exception object. But OOM breaks those rules because in that case a static string is thrown. This code was recently rewritten as part of the exception handling patch,
```
# HG changeset patch
# User Asumu Takikawa <asumu@igalia.com>
# Date 1612326572 0
# Wed Feb 03 04:29:32 2021 +0000
# Node ID 0d79003d2bab8f54911d8bf7780307ab0d466895
# Parent 81bb9db5e1392947bb8cb07f9f9a585574e7c931
Bug 1335652 - wasm exceptions part 12: stack walking for exceptions r=rhunt
```
In this patch, there's a more sophisticated check in HasCatchableException, it will not blithely assume that the exn is an exn object. At a minimum we need the same check here.
Bug 1708124 Comment 8 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Two notes about the smaller TC:
- it crashes without any flags at all in a release build, `js tc.js`
- in a debug build it will eventually (< 1min) abort with the expected assertion failure, `exn.isObject() && exn.toObject().is<ErrorObject>(), at /home/lhansen/m-u/js/src/wasm/WasmBuiltins.cpp:595` , with --fuzzing-safe (alone); without flags, it signals OOM
With rr and a debug build things become a little easier. We're in WasmHandleTrap with Trap::OutOfBounds as expected. We grab the activation and then the context from the activation. Down in ReportError, we find that the `exn` value is not an object (which is why the assertion fails), it is a string, "out of memory". This string is being thrown by js::ReportOutOfMemory in response to an allocation failure in js. But that OOM failure is itself a result of trying to report a wasm failure:
```
#0 js::ReportOutOfMemory (cx=0x7f4b70df5000) at js/src/vm/JSContext.cpp:257
#1 0x0000563e3895fb00 in js::gc::GCRuntime::tryNewTenuredThing<JSObject, (js::AllowGC)1> (cx=0x7f4b70df5000,
kind=js::gc::AllocKind::OBJECT12, thingSize=120) at js/src/gc/Allocator.cpp:367
...
#21 0x0000563e38532a93 in JS_ReportErrorNumberUTF8 (cx=0x7f4b70df5000,
errorCallback=0x563e37f98e20 <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=356)
at /home/lhansen/m-u/js/src/jsapi.cpp:4752
#22 0x0000563e391d1ff8 in ReportError (cx=0x7f4b70df5000, errorNumber=356) at js/src/wasm/WasmBuiltins.cpp:587
#23 0x0000563e391bc17e in WasmHandleTrap () at js/src/wasm/WasmBuiltins.cpp:642
```
I think the bug here is an assumption in the wasm code that when JS_ReportErrorNumberUTF8 is called by ReportError in WasmBuiltins.cpp, either there will be no pending exception or the exception will be an exception object. But OOM breaks those rules because in that case a static string is thrown. This code was recently rewritten as part of the exception handling patch,
```
# HG changeset patch
# User Asumu Takikawa <asumu@igalia.com>
# Date 1612326572 0
# Wed Feb 03 04:29:32 2021 +0000
# Node ID 0d79003d2bab8f54911d8bf7780307ab0d466895
# Parent 81bb9db5e1392947bb8cb07f9f9a585574e7c931
Bug 1335652 - wasm exceptions part 12: stack walking for exceptions r=rhunt
```
In this patch, there's a more sophisticated check in HasCatchableException, it will not blithely assume that the exn is an exn object. At a minimum we need the same check here.