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

RESOLVED FIXED in Firefox 53

Status

()

P1
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: decoder, Assigned: lth)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla53
x86_64
Linux
assertion, testcase
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
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
[...]
(Reporter)

Comment 1

2 years ago
Created attachment 8817053 [details]
Testcase
(Assignee)

Comment 2

2 years ago
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
(Assignee)

Comment 3

2 years ago
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
(Assignee)

Comment 4

2 years ago
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.
(Assignee)

Comment 5

2 years ago
Created attachment 8817346 [details] [diff] [review]
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.
Attachment #8817346 - Flags: review?(bbouvier)
(Assignee)

Updated

2 years ago
Component: JavaScript Engine → JavaScript Engine: JIT
Priority: -- → P1
(Assignee)

Comment 6

2 years ago
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.
Attachment #8817346 - Flags: review?(bbouvier)
(Assignee)

Comment 7

2 years ago
Created attachment 8817421 [details] [diff] [review]
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.
Attachment #8817346 - Attachment is obsolete: true
Attachment #8817421 - Flags: review?(bbouvier)
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?
Attachment #8817421 - Flags: review?(bbouvier) → review+
(Assignee)

Comment 9

2 years ago
(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.)
(Assignee)

Comment 11

2 years ago
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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/748148e8d6b8
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.