Closed Bug 1322288 Opened 7 years ago Closed 7 years ago

Assertion failure: isAvailable(r), at js/src/wasm/WasmBaselineCompile.cpp:806


(Core :: JavaScript Engine: JIT, defect, P1)




Tracking Status
firefox53 --- fixed


(Reporter: decoder, Assigned: lth)


(Keywords: assertion, testcase)


(2 files, 1 obsolete file)

The attached binary WebAssembly testcase crashes on mozilla-inbound revision 1e7ee9c3b114+ (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2 --without-intl-api --enable-debug). To reproduce, you can run the following code in the JS shell (running with --wasm-always-baseline might be necessary):

var data = os.file.readFile(file, 'binary');
new WebAssembly.Instance(new WebAssembly.Module(data.buffer));


==16794==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000004501fb0 bp 0x7ffcbdad0700 sp 0x7ffcbdad06d0 T0)
    #0 0x4501faf in js::jit::SpecializedRegSet<js::jit::AllocatableSetAccessors<js::jit::TypedRegisterSet<js::jit::FloatRegister> >, js::jit::TypedRegisterSet<js::jit::FloatRegister> >::take(js::jit::FloatRegister) js/src/jit/RegisterSets.h:750:9
    #1 0x4501faf in js::wasm::BaseCompiler::allocFPU(js::jit::FloatRegister) js/src/wasm/WasmBaselineCompile.cpp:807
    #2 0x44d80ba in js::wasm::BaseCompiler::needF32(js::wasm::BaseCompiler::RegF32) js/src/wasm/WasmBaselineCompile.cpp:1045:9
    #3 0x44d80ba in js::wasm::BaseCompiler::popF32(js::wasm::BaseCompiler::RegF32) js/src/wasm/WasmBaselineCompile.cpp:1676
    #4 0x44cf50f in js::wasm::BaseCompiler::popJoinRegUnlessVoid(js::wasm::ExprType) js/src/wasm/WasmBaselineCompile.cpp:1741:27
    #5 0x44cbc10 in void js::wasm::BaseCompiler::jumpConditionalWithJoinReg<js::jit::AssemblerX86Shared::DoubleCondition, js::wasm::BaseCompiler::RegF64, js::wasm::BaseCompiler::RegF64>(js::wasm::BaseCompiler::BranchState*, js::jit::AssemblerX86Shared::DoubleCondition, js::wasm::BaseCompiler::RegF64, js::wasm::BaseCompiler::RegF64) js/src/wasm/WasmBaselineCompile.cpp:3773:20
    #6 0x44892c3 in js::wasm::BaseCompiler::emitBranchPerform(js::wasm::BaseCompiler::BranchState*) js/src/wasm/WasmBaselineCompile.cpp:5133:9
    #7 0x448ea14 in js::wasm::BaseCompiler::emitBrIf() js/src/wasm/WasmBaselineCompile.cpp:5461:5
    #8 0x44ad6e8 in js::wasm::BaseCompiler::emitBody() js/src/wasm/WasmBaselineCompile.cpp:6760:13
    #9 0x44bacc8 in js::wasm::BaseCompiler::emitFunction() js/src/wasm/WasmBaselineCompile.cpp:7373:10
    #10 0x44bfff9 in js::wasm::BaselineCompileFunction(js::wasm::CompileTask*) js/src/wasm/WasmBaselineCompile.cpp:7634:10
    #11 0xcc023b in js::wasm::CompileFunction(js::wasm::CompileTask*) js/src/wasm/WasmGenerator.cpp:1147:16
    #12 0xcbface in js::wasm::ModuleGenerator::finishFuncDef(unsigned int, js::wasm::FunctionGenerator*) js/src/wasm/WasmGenerator.cpp:924:14
    #13 0xc6cbed in DecodeFunctionBody(js::wasm::Decoder&, js::wasm::ModuleGenerator&, unsigned int) js/src/wasm/WasmCompile.cpp:60:12
    #14 0xc6cbed in DecodeCodeSection(js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/wasm/WasmCompile.cpp:89
    #15 0xc6cbed in js::wasm::Compile(js::wasm::ShareableBytes const&, js::wasm::CompileArgs const&, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/wasm/WasmCompile.cpp:190
    #16 0xe2c069 in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) js/src/wasm/WasmJS.cpp:388:27
    #17 0x605cf0 in WasmLoop(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:5656:14
Attached file Testcase
For the time being we're going to assume that this is a dup of bug 1321521, which we're actively investigating.
Assignee: nobody → lhansen
Depends on: 1321521
This is not a bug with the float register allocator; instead it looks like we're perhaps leaking a float register somewhere.
No longer depends on: 1321521
This is a problem that was introduced with branch optimization.  The conditional branch wants to push the join register of the appropriate type.  This is a fixed register.  However, due to the structure of the code the argument registers, which are no longer needed, have not been freed yet, and one of them may be the join register.

The appropriate fix here is actually to make the conditional branch abstraction free the operands, so that they become available once they are no longer needed.
Actually even simpler: just a missing generalization of how we handle the join register while we set up the optimized conditional branch, we must also handle float types.
Attachment #8817346 - Flags: review?(bbouvier)
Component: JavaScript Engine → JavaScript Engine: JIT
Priority: -- → P1
Comment on attachment 8817346 [details] [diff] [review]

I believe there are further cleanups possible / desirable / necessary here; pulling this for now.
Attachment #8817346 - Flags: review?(bbouvier)
Further generalization makes this even cleaner.  We can't remove the old maybeReserveJoinRegI() because that should do nothing for non-integer types, so there's a little bit of duplication here with the introduction of maybeReserveJoinReg(), but with the removal of the popWhateverNotJoinReg() functions this still seems like a net win.
Attachment #8817346 - Attachment is obsolete: true
Attachment #8817421 - Flags: review?(bbouvier)
Comment on attachment 8817421 [details] [diff] [review]

Review of attachment 8817421 [details] [diff] [review]:

It looks simpler, thanks. Can you include a test case, please?
Attachment #8817421 - Flags: review?(bbouvier) → review+
(In reply to Benjamin Bouvier [:bbouvier] from comment #8)
> Can you include a test case, please?

I can do that.  However, like the test case you created for bug 1322450 it will in practice depend on internal details of the register allocator and compiler (such as, the order in which registers is allocated and which register is designated the join register), so if those details change it will not remain an actual test case for the problem.  (This type of problem keeps me up at night, and general solutions are complex.)
Bug 1322288 - wasm baseline, reserve float join registers in optimized control flow.  r=bbouvier
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.