Closed Bug 1486731 Opened 2 years ago Closed 2 years ago

Assertion failure: !unwoundIonCallerFP_, at js/src/wasm/WasmFrameIter.cpp:1174 with Profiling

Categories

(Core :: Javascript: WebAssembly, defect)

ARM
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- fixed

People

(Reporter: decoder, Assigned: bbouvier)

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 768eef11f5ff (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 --cpu-count=2 --ion-offthread-compile=off):

var lfLogBuffer = `
Object.prototype[3] = 3;
enableGeckoProfiling();
enableSingleStepProfiling();
setJitCompilerOption("ion.warmup.trigger", 50);
`;
var lfModule = new WebAssembly.Module(wasmTextToBinary(`
    (module
        (import "global" "func" (result i32))
        (func (export "func_0") (result i32)
         call 0 ;; calls the import, which is func #0
        )
    )
`));
processModule(lfModule, lfLogBuffer);
for (let i = 0; i < 100; ++i)
  processModule(lfModule, "");
function processModule(module, jscode) {
    imports = {}
    for (let descriptor of WebAssembly.Module.imports(module)) {
        imports[descriptor.module] = {}
        switch (descriptor.kind) {
          case "function":
            imports[descriptor.module][descriptor.name] = new Function("x", "y", "z", jscode);
            instance = new WebAssembly.Instance(module, imports);
            for (dmod in imports)
              for (dname in imports[dmod])
                if (imports[dmod][dname] == undefined) {}
        }
    }
    for (let descriptor of WebAssembly.Module.exports(module)) {
       switch (descriptor.kind) {
         case "function":
           print(instance.exports[descriptor.name]())
       }
    }
}


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x08a3da21 in js::wasm::ProfilingFrameIterator::ProfilingFrameIterator (this=0xffffbddc, activation=..., state=...) at js/src/wasm/WasmFrameIter.cpp:1174
#0  0x08a3da21 in js::wasm::ProfilingFrameIterator::ProfilingFrameIterator (this=0xffffbddc, activation=..., state=...) at js/src/wasm/WasmFrameIter.cpp:1174
#1  0x088e2a8d in JS::ProfilingFrameIterator::iteratorConstruct (this=0xffffbdc4, state=...) at js/src/vm/Stack.cpp:1968
#2  0x088e32bd in JS::ProfilingFrameIterator::ProfilingFrameIterator (this=0xffffbdc4, cx=0xf6e1b800, state=..., samplePositionInProfilerBuffer=...) at js/src/vm/Stack.cpp:1888
#3  0x080a7c0b in SingleStepCallback (arg=0xf6e1b800, sim=<optimized out>, pc=<optimized out>) at js/src/shell/js.cpp:5892
#4  0x08688bf1 in js::jit::Simulator::execute<false> (this=0xf6e3f000) at js/src/jit/arm/Simulator-arm.cpp:4900
#5  js::jit::Simulator::callInternal (this=0xf6e3f000, entry=0x5e15f800 "\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:4982
#6  0x08688dd9 in js::jit::Simulator::call (this=<optimized out>, entry=<optimized out>, argument_count=<optimized out>) at js/src/jit/arm/Simulator-arm.cpp:5065
#7  0x0835c71c in EnterBaseline (data=..., cx=0xf6e1b800) at js/src/jit/BaselineJIT.cpp:161
#8  js::jit::EnterBaselineAtBranch (cx=0xf6e1b800, fp=0xf623f018, pc=0xf6219d73 "\343\201V") at js/src/jit/BaselineJIT.cpp:236
#9  0x0821b989 in Interpret (cx=0xf6e1b800, state=...) at js/src/vm/Interpreter.cpp:2186
[...]
#20 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9963
eax	0x0	0
ebx	0xffffbddc	-16932
ecx	0xf7d90864	-136771484
edx	0x0	0
esi	0xffffbd88	-17016
edi	0xffffc188	-15992
ebp	0xffffbcc8	4294950088
esp	0xffffbc80	4294950016
eip	0x8a3da21 <js::wasm::ProfilingFrameIterator::ProfilingFrameIterator(js::jit::JitActivation const&, JS::ProfilingFrameIterator::RegisterState const&)+609>
=> 0x8a3da21 <js::wasm::ProfilingFrameIterator::ProfilingFrameIterator(js::jit::JitActivation const&, JS::ProfilingFrameIterator::RegisterState const&)+609>:	movl   $0x0,0x0
   0x8a3da2b <js::wasm::ProfilingFrameIterator::ProfilingFrameIterator(js::jit::JitActivation const&, JS::ProfilingFrameIterator::RegisterState const&)+619>:	ud2
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/79e9e6a709b0
user:        Benjamin Bouvier
date:        Fri Aug 24 15:27:20 2018 +0200
summary:     Bug 1437065: Inline monomorphic calls to wasm; r=luke, r=jandem

This iteration took 287.330 seconds to run.
Attached patch fix.patchSplinter Review
Nice catch.

In the generic JIT entry, before we set FP := SP, FP can contain trash, in particular, something that looks like a tagged FP (in the test case it contained 3, which I guess relates to the Object.prototype[3] = 3 line in the test case).

So before considering the value of FP in the profiling frame iterator, we need to make sure it's valid.

The one case we're actually interested in is function calls. There are four ways to call into a function:
- from the interpreter, which sets FP (and doesn't tag it).
- from the jit entry (ditto).
- from another wasm function (ditto).
- from an inlined jit caller, which sets the low tag of FP.

So the test was correct for functions, but not for a jit entry. I think for other codeRange kinds, it was also correct (because they're all called from wasm), so we could also just make the test `codeRange->kind() != JitEntry`, but I found it less specific.
Assignee: nobody → bbouvier
Status: NEW → ASSIGNED
Attachment #9004542 - Flags: review?(luke)
Comment on attachment 9004542 [details] [diff] [review]
fix.patch

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

Nice fix and comment.
Attachment #9004542 - Flags: review?(luke) → review+
Pushed by bbouvier@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/df8bd3f469f9
wasm profiling: Don't look at FP's tagged bit in non-function call situations; r=luke
Pushed by bbouvier@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5202cfbf8d60
Guard test against profiling mode enabled; r=me on a CLOSED TREE
https://hg.mozilla.org/mozilla-central/rev/df8bd3f469f9
https://hg.mozilla.org/mozilla-central/rev/5202cfbf8d60
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.