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

RESOLVED FIXED in Firefox 52

Status

()

--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: decoder, Unassigned)

Tracking

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

Trunk
mozilla52
ARM
Linux
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 affected, firefox52 fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

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

Comment 1

2 years ago
Created attachment 8788171 [details]
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)
Created attachment 8791632 [details] [diff] [review]
test5.js

textual testcase. It needs wasm.js from jit-tests/lib/
Created attachment 8791666 [details] [diff] [review]
bug1300546-regalloc-int64

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.

Comment 8

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

Comment 10

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

Updated

2 years ago
Depends on: 1304628

Comment 12

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

Comment 13

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