1437501 - Crash [@ js::jit::JSJitProfilingFrameIterator::moveToNextFrame] with wasm

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 2b7d42d527af (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu --enable-simulator=arm, run with --fuzzing-safe):

oomTest(new Function(`
  enableGeckoProfiling();
  enableSingleStepProfiling();
  let module = new WebAssembly.Module(wasmTextToBinary(\`
    (module
        (table 2 2 anyfunc)
        (type \$v2i (func (result i32)))
        (func \$call (param i32) (result i32) (call_indirect \$v2i (get_local 0)))
        (export "call" \$call)
    )
  \`));
  let instance = new WebAssembly.Instance(module, {});
  instance.exports.call(0);
`));


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x08383226 in js::jit::JSJitProfilingFrameIterator::moveToNextFrame (this=0xffffb94c, frame=0xbac) at js/src/jit/JSJitFrameIter.cpp:675
#0  0x08383226 in js::jit::JSJitProfilingFrameIterator::moveToNextFrame (this=0xffffb94c, frame=0xbac) at js/src/jit/JSJitFrameIter.cpp:675
#1  0x0883f5f0 in JS::ProfilingFrameIterator::settleFrames (this=0xffffb934) at js/src/vm/Stack.cpp:2015
#2  0x0883f67c in JS::ProfilingFrameIterator::settle (this=0xffffb934) at js/src/vm/Stack.cpp:2025
#3  0x0883f721 in JS::ProfilingFrameIterator::operator++ (this=0xffffb934) at js/src/vm/Stack.cpp:1993
#4  0x080a86a3 in SingleStepCallback (arg=0xf6e1d800, sim=<optimized out>, pc=<optimized out>) at js/src/shell/js.cpp:5559
#5  0x08574baa in js::jit::Simulator::execute<false> (this=0xf6e58000) at js/src/jit/arm/Simulator-arm.cpp:4948
#6  js::jit::Simulator::callInternal (this=0xf6e58000, entry=0x36268810 "\360O-\351\004\320M\342\020\212-\355\r\200\240\341h\220\235\345\r\260\240\341t\240\235", <incomplete sequence \345>) at js/src/jit/arm/Simulator-arm.cpp:5037
#7  0x08574ee1 in js::jit::Simulator::call (this=<optimized out>, entry=<optimized out>, argument_count=<optimized out>) at js/src/jit/arm/Simulator-arm.cpp:5120
#8  0x083b039a in EnterJit (cx=cx@entry=0xf6e1d800, state=..., code=0x36275848 "\004\340-\345\a") at js/src/jit/Jit.cpp:101
#9  0x083b0bfe in js::jit::MaybeEnterJit (cx=0xf6e1d800, state=...) at js/src/jit/Jit.cpp:163
#10 0x081a2f05 in js::RunScript (cx=0xf6e1d800, state=...) at js/src/vm/Interpreter.cpp:408
#11 0x081a3525 in js::InternalCallOrConstruct (cx=0xf6e1d800, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:495
#12 0x081a3820 in InternalCall (cx=cx@entry=0xf6e1d800, args=...) at js/src/vm/Interpreter.cpp:522
#13 0x081a39da in js::Call (cx=0xf6e1d800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:541
#14 0x085a8c29 in JS_CallFunction (cx=0xf6e1d800, obj=..., fun=..., args=..., rval=...) at js/src/jsapi.cpp:2996
#15 0x08499282 in OOMTest (cx=0xf6e1d800, argc=1, vp=0xf5675058) at js/src/builtin/TestingFunctions.cpp:1654
[...]
#30 main (argc=3, argv=0xffffce24, envp=0xffffce34) at js/src/shell/js.cpp:9317
eax	0xffffb94c	-18100
ebx	0x8dcfff4	148701172
ecx	0xbac	2988
edx	0xffffb94c	-18100
esi	0xffffb934	-18124
edi	0xbac	2988
ebp	0xffffb828	4294948904
esp	0xffffb820	4294948896
eip	0x8383226 <js::jit::JSJitProfilingFrameIterator::moveToNextFrame(js::jit::CommonFrameLayout*)+22>
=> 0x8383226 <js::jit::JSJitProfilingFrameIterator::moveToNextFrame(js::jit::CommonFrameLayout*)+22>:	mov    0x4(%ecx),%eax
   0x8383229 <js::jit::JSJitProfilingFrameIterator::moveToNextFrame(js::jit::CommonFrameLayout*)+25>:	mov    %eax,%esi


Frequent bug, marking fuzzblocker.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: errorfp.patch

Fun: the value of FP in the jit iterator was 0xbac, which is the FailFP value ~0x1, hinting me pretty quickly at what went wrong.

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 8950212 [details] [diff] [review]
errorfp.patch

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

::: js/src/wasm/WasmStubs.cpp
@@ +687,5 @@
> +
> +    // FP may have been set to FailFP; reset it.
> +    masm.movePtr(sp, ScratchIonEntry);
> +    masm.addPtr(Imm32(masm.framePushed()), ScratchIonEntry);
> +    masm.movePtr(ScratchIonEntry, FramePointer);

Does this need to be set or are you just trying to reduce the window in which the profiling frame iterator drops the stack?  I would think that, since this is the exception path, it's probably not worth it.  But if it is necessary, could you comment why?

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Right, we'll just lose a few instructions of frame information, as long as we ignore FailFP in the profiling frame iterator. Will remove it and update the test. Thanks!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e1e3fb7d63a
Ignore wasm::FailFP when unwinding jit->wasm frames; r=luke

Comment 5

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/0e1e3fb7d63a