1655695 - Hit MOZ_CRASH(We do not expect the stackmap to cover inbound args) at js/src/wasm/cranelift/src/compile.rs:250 or Crash [@ RefPtr<js::wasm::Code const>::get]

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20200727-932240e49142 (debug build, run with --fuzzing-safe --no-threads --wasm-compiler=cranelift test.js):

See attachment.

Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000555556c8c235 in RustMozCrash ()
#0  0x0000555556c8c235 in RustMozCrash ()
#1  0x0000555556c8c1e8 in mozglue_static::panic_hook::hcf93afeab0f1f120 ()
#2  0x0000555556c8bd1c in core::ops::function::Fn::call::h71908f84f4fbb781 ()
#3  0x0000555556f8dbc5 in std::panicking::rust_panic_with_hook::hb976084785e50594 ()
#4  0x0000555556cbff9f in std::panicking::begin_panic::h8ab20339796cc1e5 ()
#5  0x0000555556cc8317 in baldrdash::compile::BatchCompiler::compile::h1b7a7c205594c7cf ()
#6  0x0000555556cd139d in cranelift_compile_function ()
#7  0x00005555568b4930 in js::wasm::CraneliftCompileFunctions(js::wasm::ModuleEnvironment const&, js::LifoAlloc&, mozilla::Vector<js::wasm::FuncCompileInput, 8ul, js::SystemAllocPolicy> const&, js::wasm::CompiledCode*, mozilla::UniquePtr<char [], JS::FreePolicy>*) ()
#8  0x0000555556927a69 in ExecuteCompileTask(js::wasm::CompileTask*, mozilla::UniquePtr<char [], JS::FreePolicy>*) ()
#9  0x0000555556928087 in js::wasm::ModuleGenerator::launchBatchCompile() ()
#10 0x0000555556928391 in js::wasm::ModuleGenerator::compileFuncDef(unsigned int, unsigned int, unsigned char const*, unsigned char const*, mozilla::Vector<unsigned int, 8ul, js::SystemAllocPolicy>&&) ()
#11 0x00005555568b29a8 in bool DecodeCodeSection<js::wasm::Decoder>(js::wasm::ModuleEnvironment const&, js::wasm::Decoder&, js::wasm::ModuleGenerator&) ()
#12 0x00005555568b2643 in js::wasm::CompileBuffer(js::wasm::CompileArgs const&, js::wasm::ShareableBytes const&, mozilla::UniquePtr<char [], JS::FreePolicy>*, mozilla::Vector<mozilla::UniquePtr<char [], JS::FreePolicy>, 0ul, js::SystemAllocPolicy>*, JS::OptimizedEncodingListener*) ()
#13 0x0000555556990404 in js::WasmModuleObject::construct(JSContext*, unsigned int, JS::Value*) ()
#14 0x0000555555948482 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
[...]
#49 0x0000000000000000 in ?? ()
rax	0x5555583e3840	93825041053760
rbx	0x7fffffff8baa	140737488325546
rcx	0x7ffff6e362dd	140737335485149
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffff8990	140737488325008
rsp	0x7fffffff8980	140737488324992
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f9bd40	140737353727296
r10	0x58	88
r11	0x7ffff6dac7a0	140737334921120
r12	0xfa	250
r13	0x24	36
r14	0xfa	250
r15	0x555558001eeb	93825036984043
rip	0x555556c8c235 <RustMozCrash+37>
=> 0x555556c8c235 <RustMozCrash+37>:	mov    %r14d,0x0
   0x555556c8c23d <RustMozCrash+45>:	callq  0x55555584d42e <abort>

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Christian Holler (:decoder)]

This is a fuzzblocker, happening with high frequency.

Comment 3

[Lars T Hansen [:lth]]

The assert suggests that some logic is missing in cranelift; we do indeed expect stack maps to cover inbound args. (Haven't looked at the code.)

Comment 4

[Julian Seward [:jseward]]

Routing to Chris for initial assessment.

Comment 5

[Chris Fallin [:cfallin]]

Interesting; I'll look into this right away.

Regarding stackmaps and inbound args, here is the logic that went into that: in our prologues, we copy inbound args to temporaries (for registers these usually get coalesced to the ABI-specified register; for stack args they would either live in regs or be spilled to a spillslot if necessary, which is suboptimal but much simpler for now). Because of this, we effectively consider the inbound arg storage on the stack to be dead after these copies. The copies happen before any safepoint, and so the GC will always see the arg values via the temporaries, and if it moves any object, it will edit the references that are live and are used by the function body. In the ABI, the argument area on the stack is volatile (free for the callee to trash) so we need not consider the values to be live references for that reason either.

So, basically, it's a simplification: avoid reasoning about reference args by reducing them to the solved case of reference temporaries.

:lth, if any of that is missing something that we need to handle, please do let me know; happy to rework as needed!

Now, regarding this fuzzbug, it seems the stackmap that's being created is too long, and overlaps with the args; will figure out what's going on.

Comment 6

[Lars T Hansen [:lth]]

If they're truly dead then that's fine, though yeah, that's suboptimal. Won't matter much in the short run but x86-32 customers will eventually be unhappy :-)

Comment 7

[Chris Fallin [:cfallin]]

:decoder, I'm not able to reproduce -- it seems there may be a missing dependency (a fuzzing library?) or I'm missing a flag of some sort.

I built (on an aarch64/Linux machine) with:

../configure --enable-debug --disable-optimize --enable-fuzzing

And then I ran:

cfallin@arm1-ci:~/moz/mozilla-unified/js/src/BUILD_DBG.OBJ$ dist/bin/js --fuzzing-safe --no-threads --wasm-compiler=cranelift --shared-memory=off ./test.js
./test.js:2:16 TypeError: import object field 'fuzzing-support' is not an Object
Stack:
  @./test.js:2:16

(This is almost the same as your flags, except I added --shared-memory=off which is needed to select Cranelift until atomics land; but I got the same error when omitting this flag.)

Comment 8

[Christian Holler (:decoder)]

This reproduces cleanly for me on x86_64 (no ARM64 required) and the attached test.zip should contain everything required.

Can you try reproducing with a build from TC first? pip install fuzzfetch && python -mfuzzfetch --target js --debug --fuzzing

Edit: Btw, with --fuzzing-safe, you no longer need the additional flags like --shared-memory=off for Cranelift, everything will be selected properly automatically, thanks to Lars!

Comment 9

[Benjamin Bouvier [:bbouvier] (inactive)]

Considering the src location + it's happening on x86, it means this is an old-backend issue, so it might have been there for some time. The fix might be to just wait for the new x86 backend to land in bug 1654813, which should be only a few days away.

Comment 10

[Lars T Hansen [:lth]]

Sounds good. Assigning, just so that it doesn't get lost.

Comment 11

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Unable to reproduce bug using the following builds:
> mozilla-central 20200804091327-7cb90fa4f485
> mozilla-central 20200727203201-932240e49142
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Comment 12

[Chris Fallin [:cfallin]]

:decoder, can you confirm this was fixed by the new-backend landing?

Comment 13

[Christian Holler (:decoder)]

I can no longer reproduce this, so let's assume it's fixed.