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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

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?

Comment 3

[Hannes Verschore [:h4writer]]

Attachment: test5.js

textual testcase. It needs wasm.js from jit-tests/lib/

Comment 4

[Hannes Verschore [:h4writer]]

Attachment: 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.

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

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?

Comment 6

[Hannes Verschore [:h4writer]]

(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

[Hannes Verschore [:h4writer]]

> This is already reduced from just the translation. I could try some
> more, but I was already seeing diminishing returns on reducing.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
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

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

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

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

[Pulsebot]

Pushed by [e-mail]:
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

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/371a46d8334c