Closed Bug 1300546 Opened 3 years ago Closed 3 years ago

[wasm] Assertion failure: *def->output() == alloc, at js/src/jit/RegisterAllocator.cpp:217

Categories

(Core :: JavaScript Engine, defect, critical)

ARM
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox50 --- affected
firefox52 --- fixed

People

(Reporter: decoder, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

The attached binary WebAssembly testcase crashes on mozilla-inbound revision 9805030e34ef+ (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 --target=i686-pc-linux-gnu --enable-simulator=arm). 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');
Wasm.instantiateModule(new Uint8Array(data.buffer));



Backtrace:

==20911==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000 (pc 0x093f4c04 bp 0xffc4c6d8 sp 0xffc4c620 T0)
    #0 0x93f4c03 in js::jit::AllocationIntegrityState::checkIntegrity(js::jit::LBlock*, js::jit::LInstruction*, unsigned int, js::jit::LAllocation, bool) js/src/jit/RegisterAllocator.cpp:217:17
    #1 0x93ecf0c in js::jit::AllocationIntegrityState::check(bool) js/src/jit/RegisterAllocator.cpp:169:22
    #2 0x8ce7f3f in js::jit::GenerateLIR(js::jit::MIRGenerator*) js/src/jit/Ion.cpp:1962:18
    #3 0xb789466 in js::wasm::IonCompileFunction(js::wasm::IonCompileTask*) js/src/asmjs/WasmIonCompile.cpp:3693:25
    #4 0xb7b8578 in js::wasm::CompileFunction(js::wasm::IonCompileTask*) js/src/asmjs/WasmIonCompile.cpp:3715:16
    #5 0xb726e78 in js::wasm::ModuleGenerator::finishFuncDef(unsigned int, js::wasm::FunctionGenerator*) js/src/asmjs/WasmGenerator.cpp:871:14
    #6 0xb6b0b04 in DecodeFunctionBody(js::wasm::Decoder&, js::wasm::ModuleGenerator&, unsigned int) js/src/asmjs/WasmCompile.cpp:1243:12
    #7 0xb6b0b04 in DecodeCodeSection(js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/asmjs/WasmCompile.cpp:1303
    #8 0xb6b0b04 in js::wasm::Compile(js::wasm::ShareableBytes const&, js::wasm::CompileArgs const&, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/asmjs/WasmCompile.cpp:1586
    #9 0x830997e in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) js/src/asmjs/WasmJS.cpp:247:27
    #10 0x8230fcf in WasmLoop(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:5364:14
[...]
Attached file Testcase
I couldn't transform the test case after a few attempts, so here are my thoughts:

Regalloc issue with i64.xor reproducible on ARM or x86 debug builds, so it might be an issue specific to 32 bits in general.

This check ensures that the reused input actually is reused; it says it's failing, but according to the IONFLAGS regalloc output, it is correct, so there might an issue with the check itself.

There seems to be a mismatch between low and high here:

(rr) p i
$25 = 0
(rr) p ins->getDef(0)->output()->toRegister()
$22 = {static Total = 32, static Invalid = 4294967295, code_ = 2}
(rr) p ins->getDef(1)->output()->toRegister()
$23 = {static Total = 32, static Invalid = 4294967295, code_ = 1}
(rr) p alloc.toRegister()
$24 = {static Total = 32, static Invalid = 4294967295, code_ = 1}

So I suspect the issue is in AllocationIntegrityState::record(), but really not sure about all this. Hannes, any idea?
Flags: needinfo?(hv1989)
Attached patch test5.jsSplinter Review
textual testcase. It needs wasm.js from jit-tests/lib/
Fallout from bug 1290450. All (not reused) uses to an instruction using MUST_REUSE_INPUT (with MIRType_INT64) should be marked without "atStart". Else the regalloc can and will give registers for inputs which happen to overlap the output. This is troublesome since we need to have the reused inputs in the output registers already.
Flags: needinfo?(hv1989)
Attachment #8791666 - Flags: review?(bbouvier)
Depends on: 1290450
Comment on attachment 8791666 [details] [diff] [review]
bug1300546-regalloc-int64

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

Thanks!

::: js/src/jit-test/tests/wasm/bug1300546.js
@@ +1,1 @@
> +// |jit-test| error: TypeError

Can you put the test case under the regress/ directory? That's where we have been putting the fuzzbugs so far.

@@ +3,5 @@
> +load(libdir + "wasm.js");
> +
> +setJitCompilerOption('wasm.test-mode', 1);
> +
> +print( wasmEvalText(`

Can you remove the call to print?

@@ +17,5 @@
> +      (current_memory)
> +      (current_memory)
> +      (current_memory)
> +      (current_memory)
> +      (i64.rem_s (i64.const 17) (i64.xor (i64.const 17) (i64.xor (i64.const 17) (i64.xor (i64.xor (i64.const 17) (i64.const 17)) (i64.xor (i64.const 17) (i64.const 17))))))

Can you indent the code a bit more, please? Also, any chances the test case can be reduced?
Attachment #8791666 - Flags: review?(bbouvier) → review+
(In reply to Benjamin Bouvier [:bbouvier] from comment #5)
> Comment on attachment 8791666 [details] [diff] [review]
> bug1300546-regalloc-int64
> 
> Review of attachment 8791666 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks!
> 
> ::: js/src/jit-test/tests/wasm/bug1300546.js
> @@ +1,1 @@
> > +// |jit-test| error: TypeError
> 
> Can you put the test case under the regress/ directory? That's where we have
> been putting the fuzzbugs so far.

Sure, the directory didn't caught my attention while searching for such a folder.
> 
> @@ +3,5 @@
> > +load(libdir + "wasm.js");
> > +
> > +setJitCompilerOption('wasm.test-mode', 1);
> > +
> > +print( wasmEvalText(`
> 
> Can you remove the call to print?

Sure.

> 
> @@ +17,5 @@
> > +      (current_memory)
> > +      (current_memory)
> > +      (current_memory)
> > +      (current_memory)
> > +      (i64.rem_s (i64.const 17) (i64.xor (i64.const 17) (i64.xor (i64.const 17) (i64.xor (i64.xor (i64.const 17) (i64.const 17)) (i64.xor (i64.const 17) (i64.const 17))))))
> 
> Can you indent the code a bit more, please? Also, any chances the test case
> can be reduced?

This is already reduced from just the translation. It could do try some more, but I was already seeming diminishing returns on reducing.
> This is already reduced from just the translation. I could try some
> more, but I was already seeing diminishing returns on reducing.
Pushed by hv1989@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73ac0ad42d63
IonMonkey: Make sure the registers of lhs and rhs don't overlap during ALU, r=bbouvier
Flags: needinfo?(hv1989)
Pushed by hv1989@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a71ed072a2b
IonMonkey: Make sure the registers of lhs and rhs don't overlap during ALU, r=bbouvier
Depends on: 1304628
Pushed by hv1989@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/371a46d8334c
IonMonkey: Make sure the registers of lhs and rhs don't overlap during ALU, r=bbouvier
https://hg.mozilla.org/mozilla-central/rev/371a46d8334c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.