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

Status: RESOLVED FIXED

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

Description

[Christian Holler (:decoder)]

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));



Backtrace:

==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
[...]

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Lars T Hansen [:lth]]

For the time being we're going to assume that this is a dup of bug 1321521, which we're actively investigating.

Comment 3

[Lars T Hansen [:lth]]

This is not a bug with the float register allocator; instead it looks like we're perhaps leaking a float register somewhere.

Comment 4

[Lars T Hansen [:lth]]

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.

Comment 5

[Lars T Hansen [:lth]]

Attachment: bug1322288-reserve-float-joinreg.patch

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.

Comment 6

[Lars T Hansen [:lth]]

Comment on attachment 8817346 [details] [diff] [review]
bug1322288-reserve-float-joinreg.patch

I believe there are further cleanups possible / desirable / necessary here; pulling this for now.

Comment 7

[Lars T Hansen [:lth]]

Attachment: bug1322288-reserve-float-joinreg.patch

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.

Comment 8

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8817421 [details] [diff] [review]
bug1322288-reserve-float-joinreg.patch

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

It looks simpler, thanks. Can you include a test case, please?

Comment 9

[Lars T Hansen [:lth]]

(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.)

Comment 10

[Lars T Hansen [:lth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fa6fc6fa46b

Comment 11

[Lars T Hansen [:lth]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/748148e8d6b84aa8c07194951a530e141e64187e
Bug 1322288 - wasm baseline, reserve float join registers in optimized control flow.  r=bbouvier

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/748148e8d6b8