Closed
Bug 1300546
Opened 9 years ago
Closed 9 years ago
[wasm] Assertion failure: *def->output() == alloc, at js/src/jit/RegisterAllocator.cpp:217
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla52
People
(Reporter: decoder, Unassigned)
References
Details
(Keywords: assertion, testcase)
Attachments
(3 files)
355 bytes,
application/octet-stream
|
Details | |
2.28 KB,
patch
|
Details | Diff | Splinter Review | |
3.03 KB,
patch
|
bbouvier
:
review+
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
Comment 2•9 years ago
|
||
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)
Comment 3•9 years ago
|
||
textual testcase. It needs wasm.js from jit-tests/lib/
Comment 4•9 years ago
|
||
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)
Comment 5•9 years ago
|
||
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+
Comment 6•9 years ago
|
||
(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.
Comment 7•9 years ago
|
||
> 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
![]() |
||
Comment 9•9 years ago
|
||
Backed out for failing own test on arm64:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1988bdcb4fe4
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=416c73ba796b2d0a5f4c6ce8f8cd4ab6029b9f81
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=36141618&repo=mozilla-inbound
TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/wasm/regress/bug1300546.js | Unknown (code 0, args "")
Flags: needinfo?(hv1989)
Updated•9 years ago
|
Flags: needinfo?(hv1989)
Comment 10•9 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
Comment 11•9 years ago
|
||
backout bugherder landing |
Backed out because something from this push broke android tests like https://treeherder.mozilla.org/logviewer.html#?job_id=36245856&repo=mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5b7c94b3244
Comment 12•9 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•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 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.
Description
•