Assertion failure: size_t(np) == countedPointers, at wasm/WasmBaselineCompile.cpp:2667
Categories
(Core :: JavaScript: WebAssembly, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox75 | --- | disabled |
firefox76 | --- | disabled |
firefox77 | --- | verified |
People
(Reporter: decoder, Assigned: wingo)
Details
(4 keywords, Whiteboard: [bugmon:update,bisect])
Attachments
(2 files)
The following testcase crashes on mozilla-central revision 20200408-6663d3dc883b (build with --enable-debug, run with --fuzzing-safe --no-threads):
var g63 = newGlobal({newCompartment: true});
g63.parent = this;
g63.eval("var dbg = new Debugger(parent);");
function testAddNextThreeInts(text, imports) {
new WebAssembly.Instance(
new WebAssembly.Module(wasmTextToBinary(text)
), { imports });
}
testAddNextThreeInts(`
(func $boxNextInt (import "imports" "boxNextInt")
(result anyref))
(func $boxNextThreeInts (result anyref anyref anyref)
call $boxNextInt
call $boxNextThreeInts
)
`, {});
Backtrace:
received signal SIGSEGV, Segmentation fault.
0x0000555556808b95 in js::wasm::StackMapGenerator::createStackMap(char const*, mozilla::Vector<bool, 32ul, js::SystemAllocPolicy> const&, unsigned int, js::wasm::HasRefTypedDebugFrame, mozilla::Vector<js::wasm::Stk, 0ul, js::SystemAllocPolicy> const&) ()
#0 0x0000555556808b95 in js::wasm::StackMapGenerator::createStackMap(char const*, mozilla::Vector<bool, 32ul, js::SystemAllocPolicy> const&, unsigned int, js::wasm::HasRefTypedDebugFrame, mozilla::Vector<js::wasm::Stk, 0ul, js::SystemAllocPolicy> const&) ()
#1 0x00005555567c23d3 in js::wasm::BaseCompiler::emitCall() ()
#2 0x00005555567dae1d in js::wasm::BaseCompiler::emitBody() ()
#3 0x00005555567e8b2f in js::wasm::BaselineCompileFunctions(js::wasm::ModuleEnvironment const&, js::LifoAlloc&, mozilla::Vector<js::wasm::FuncCompileInput, 8ul, js::SystemAllocPolicy> const&, js::wasm::CompiledCode*, mozilla::UniquePtr<char [], JS::FreePolicy>*) ()
#4 0x000055555686d6cc in ExecuteCompileTask(js::wasm::CompileTask*, mozilla::UniquePtr<char [], JS::FreePolicy>*) ()
#5 0x000055555686e5b7 in js::wasm::ModuleGenerator::finishFuncDefs() ()
#6 0x00005555567fa81e in bool DecodeCodeSection<js::wasm::Decoder>(js::wasm::ModuleEnvironment const&, js::wasm::Decoder&, js::wasm::ModuleGenerator&) ()
#7 0x00005555567fa3b9 in js::wasm::CompileBuffer(js::wasm::CompileArgs const&, js::wasm::ShareableBytes const&, mozilla::UniquePtr<char [], JS::FreePolicy>*, mozilla::Vector<mozilla::UniquePtr<char [], JS::FreePolicy>, 0ul, js::SystemAllocPolicy>*, JS::OptimizedEncodingListener*) ()
#8 0x00005555568d1596 in js::WasmModuleObject::construct(JSContext*, unsigned int, JS::Value*) ()
#9 0x0000555555904ef2 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
[...]
#21 0x000055555578827d in main ()
rax 0x555556ff2c3d 93825020144701
rbx 0x2 2
rcx 0x555557f46850 93825036216400
rdx 0x0 0
rsi 0x7ffff6efd770 140737336301424
rdi 0x7ffff6efc540 140737336296768
rbp 0x7fffffff9050 140737488326736
rsp 0x7fffffff8fe0 140737488326624
r8 0x7ffff6efd770 140737336301424
r9 0x7ffff7f9bd00 140737353727232
r10 0x58 88
r11 0x7ffff6ba47a0 140737332791200
r12 0x7ffff5e28410 140737318650896
r13 0x3 3
r14 0xc 12
r15 0x0 0
rip 0x555556808b95 <js::wasm::StackMapGenerator::createStackMap(char const*, mozilla::Vector<bool, 32ul, js::SystemAllocPolicy> const&, unsigned int, js::wasm::HasRefTypedDebugFrame, mozilla::Vector<js::wasm::Stk, 0ul, js::SystemAllocPolicy> const&)+1701>
=> 0x555556808b95 <_ZN2js4wasm17StackMapGenerator14createStackMapEPKcRKN7mozilla6VectorIbLm32ENS_17SystemAllocPolicyEEEjNS0_21HasRefTypedDebugFrameERKNS5_INS0_3StkELm0ES6_EE+1701>: movl $0xa6b,0x0
0x555556808ba0 <_ZN2js4wasm17StackMapGenerator14createStackMapEPKcRKN7mozilla6VectorIbLm32ENS_17SystemAllocPolicyEEEjNS0_21HasRefTypedDebugFrameERKNS5_INS0_3StkELm0ES6_EE+1712>: callq 0x555555814706 <abort>
Reporter | ||
Comment 1•4 years ago
|
||
Comment 2•4 years ago
|
||
Andy, related to multi-value most likely, could be a dup of another we've seen this weekend.
Assignee | ||
Comment 3•4 years ago
|
||
FYI the debugger bits just serve to force usage of the baseline compiler; this is a bug in the baseline compiler. Which unfortunately has existed for a while! Patch will have details.
Assignee | ||
Comment 4•4 years ago
|
||
The baseline compiler needs to adjust its virtual stack to match the
machine stack at control-flow joins, and also when preparing stack
result areas for calls. In the latter case, there may be temporary
values on the stack, so the stack height before pushing stack results is
not the current block's stack height.
Assignee | ||
Comment 5•4 years ago
•
|
||
The patch above fixes the issue. The problem was that the captured Stk entries were incorrectly placed as if they had been pushed from the block's stackHeight, instead of the stackHeight before the call -- i.e. two Stk entries were actually at the same location. So, the MST pointer addition at https://searchfox.org/mozilla-central/source/js/src/wasm/WasmBaselineCompile.cpp#2604 happened twice for the same location.
To me it would seem that you wouldn't ever want to add one location two times, but adding an assert in MachineStackTracker::setGCPointer that the entry is currently false causes a couple test case failures:
FAIL - wasm/gc/anyref-boxing-struct.js
FAIL - wasm/gc/stackmaps4-params-n-locals.js
Probably worth looking into.
Similarly I tried added an assert in pushBlockResults that the incoming fr.stackHeight()
was the block stack height, but it was not an invariant (many non-multi-value cases fail), for reasons I don't fully understand.
Reporter | ||
Comment 6•4 years ago
|
||
This is a nightly-only s-s bug as per comment 3 and IRC discussion. No sec-approval required to land the fix.
Updated•4 years ago
|
Assignee | ||
Comment 7•4 years ago
|
||
Similar test case identified by :decoder:, also fails in a similar place:
setJitCompilerOption("wasm.ion", 0);
new WebAssembly.Module(wasmTextToBinary(`
(module
(func (result anyref f64 anyref)
(call 0)
(call 0)
(call 0)
(unreachable)))`));
Am triaging to see if this fix is incomplete.
Assignee | ||
Comment 8•4 years ago
|
||
Yaaargh, this is the same bug but in a different place. The stack result locations are correctly pushed onto the virtual stack before the call, but not after. Recall that in general there will be some machine stack space taken up by arguments, unless all arguments are constants. Stack result locations are pushed on after the arguments have been computed but before the call, so after the call they may need to be shuffled down. Therefore their offsets change, in general. (Not in this case, as there are no arguments, but that's not important.)
The bug comes in where we capture the results on the stack. At blocks, we do this at the block stack height. Here we need to take into account any values pushed during the course of this block, but just as we were failing to take this into account before a call, we also fail to do so after.
Comment 9•4 years ago
|
||
Hmmm. I worry slightly that we have a breakdown of abstractions in the compiler / we need better abstractions to protect us from these types of errors.
Assignee | ||
Comment 10•4 years ago
|
||
Yeah I hear what you are saying! Updated patch does slightly rework some of the abstractions.
Comment 11•4 years ago
|
||
https://hg.mozilla.org/integration/autoland/rev/248ff2b5590595ba24a03690ec13b8a14753b3b3
https://hg.mozilla.org/mozilla-central/rev/248ff2b55905
Updated•4 years ago
|
Updated•4 years ago
|
Comment 12•4 years ago
|
||
Bugmon Analysis: Verified bug as fixed on rev mozilla-central 20200423145559-03626342f6e6. Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.
Updated•4 years ago
|
Description
•