Closed Bug 1282741 Opened 3 years ago Closed 3 years ago

Assertion failure: kind == ICEntry::Kind_CallVM || kind == ICEntry::Kind_WarmupCounter|| kind == ICEntry::Kind_StackCheck || kind == ICEntry::Kind_EarlyStackCheck, at js/src/jit/BaselineDebugModeOSR.cpp:48

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: decoder, Assigned: shu)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 0e3f8401b804 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe):

function removeAdd(dbg, g) {
    dbg.removeDebuggee(g);
    dbg.addDebuggee(g);
    switch (dbg.removeDebuggee(g)) {}
}
function newGlobalDebuggerPair(toggleSeq) {
    var g = newGlobal();
    var dbg = new Debugger;
    dbg.addDebuggee(g);
    g.eval("" + function f() {
        for (var i = 0; i < 100; i++) interruptIf(i == 95);
    });
    setInterruptCallback(function() {
        return true;
    });
    return [g, dbg];
}
function testEpilogue(toggleSeq) {
    var [g, dbg] = newGlobalDebuggerPair(toggleSeq);
    dbg.onEnterFrame = function(f) {
        f.onPop = function() {
            toggleSeq(dbg, g);
        }
    };
    assertEq(g.f(), 100);
}
testEpilogue(removeAdd);


Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x00000000005ec4b2 in PatchBaselineFramesForDebugMode (start=<synthetic pointer>, entries=..., activation=..., obs=..., cx=0x7ffff6965000) at js/src/jit/BaselineDebugModeOSR.cpp:481
#0  0x00000000005ec4b2 in PatchBaselineFramesForDebugMode (start=<synthetic pointer>, entries=..., activation=..., obs=..., cx=0x7ffff6965000) at js/src/jit/BaselineDebugModeOSR.cpp:481
#1  js::jit::RecompileOnStackBaselineScriptsForDebugMode (cx=cx@entry=0x7ffff6965000, obs=..., observing=observing@entry=js::Debugger::NotObserving) at js/src/jit/BaselineDebugModeOSR.cpp:918
#2  0x0000000000a6747d in js::Debugger::updateExecutionObservabilityOfFrames (cx=0x7ffff6965000, obs=..., observing=js::Debugger::NotObserving) at js/src/vm/Debugger.cpp:2329
#3  0x0000000000a99449 in js::Debugger::removeDebuggee (cx=cx@entry=0x7ffff6965000, argc=<optimized out>, vp=<optimized out>) at js/src/vm/Debugger.cpp:3524
#4  0x0000000000aef14e in js::CallJSNative (cx=cx@entry=0x7ffff6965000, native=0xa99130 <js::Debugger::removeDebuggee(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:227
#5  0x0000000000ae8109 in js::InternalCallOrConstruct (cx=0x7ffff6965000, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:452
#6  0x0000000000adc1af in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:503
#7  Interpret (cx=0x7ffff6965000, state=...) at js/src/vm/Interpreter.cpp:2872
#8  0x0000000000ae7eee in js::RunScript (cx=cx@entry=0x7ffff6965000, state=...) at js/src/vm/Interpreter.cpp:398
#9  0x0000000000ae8208 in js::InternalCallOrConstruct (cx=cx@entry=0x7ffff6965000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:470
#10 0x0000000000ae8456 in InternalCall (cx=cx@entry=0x7ffff6965000, args=...) at js/src/vm/Interpreter.cpp:497
#11 0x0000000000ae85ae in js::Call (cx=cx@entry=0x7ffff6965000, fval=..., fval@entry=..., thisv=..., thisv@entry=..., args=..., rval=..., rval@entry=...) at js/src/vm/Interpreter.cpp:516
#12 0x0000000000a12505 in js::Call (cx=0x7ffff6965000, fval=..., thisObj=<optimized out>, arg0=..., rval=...) at js/src/vm/Interpreter.h:114
#13 0x0000000000a811f0 in js::Debugger::slowPathOnLeaveFrame (cx=cx@entry=0x7ffff6965000, frame=..., pc=pc@entry=0x7ffff69a37cc "\231\220\210\r\210\004&*\033\063\216\025\327\210\377\377\377\373Έ\377\377\377\367ϐ\210\004", frameOk=frameOk@entry=true) at js/src/vm/Debugger.cpp:916
#14 0x000000000083665a in js::Debugger::onLeaveFrame (ok=true, pc=0x7ffff69a37cc "\231\220\210\r\210\004&*\033\063\216\025\327\210\377\377\377\373Έ\377\377\377\367ϐ\210\004", frame=..., cx=0x7ffff6965000) at js/src/vm/Debugger-inl.h:24
#15 js::jit::DebugEpilogue (cx=cx@entry=0x7ffff6965000, frame=0x7fffffffbba8, pc=0x7ffff69a37cc "\231\220\210\r\210\004&*\033\063\216\025\327\210\377\377\377\373Έ\377\377\377\367ϐ\210\004", ok=ok@entry=true) at js/src/jit/VMFunctions.cpp:703
#16 0x0000000000836c04 in js::jit::DebugEpilogueOnBaselineReturn (cx=0x7ffff6965000, frame=<optimized out>, pc=<optimized out>) at js/src/jit/VMFunctions.cpp:685
#17 0x00007ffff7fe7f25 in ?? ()
[...]
#33 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x4	4
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffa870	140737488332912
rsp	0x7fffffffa330	140737488331568
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fdc740	140737353992000
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff7e9c230	140737352679984
r13	0x7ffff698a500	140737330586880
r14	0x9	9
r15	0x7ffff69cd200	140737330860544
rip	0x5ec4b2 <js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving)+7858>
=> 0x5ec4b2 <js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving)+7858>:	movl   $0x0,0x0
   0x5ec4bd <js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving)+7869>:	ud2
This seems to go back beyond m-c rev dc4b163f7db7 (early Nov 2014).

Setting needinfo? from Jan as a start, since Baseline stuff is at the top of the stack, please feel free to move the NI? on where necessary.
Flags: needinfo?(jdemooij)
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Any progress here?
Flags: needinfo?(nihsanullah)
Looking at this now.
Flags: needinfo?(nihsanullah)
Something like this happens:

* We call DebugEpilogueOnBaselineReturn, at this point frame->script()->baselineScript()->hasDebugInstrumentation() is true.

* Then we remove the global as debuggee, so we recompile the script without debug instrumentation.

* We add and remove the global again as debuggee?

* We fail this assert, with kind == ICEntry::Kind_DebugEpilogue:

                // Case F, should only need to undo case B, I or J.
                MOZ_ASSERT_IF(!script->baselineScript()->hasDebugInstrumentation(),
                              kind == ICEntry::Kind_CallVM ||
                              kind == ICEntry::Kind_WarmupCounter||
                              kind == ICEntry::Kind_StackCheck ||
                              kind == ICEntry::Kind_EarlyStackCheck);

Shu, can you take a look? I looked a bit but it's not clear where the bug is.
Flags: needinfo?(jdemooij) → needinfo?(shu)
It's been a while, but I think the assertion was too tight. I don't know why I
thought that the cases that causes On -> Off -> On would be different from
cases that can cause Off -> On -> Off, because you can just extend the chain
once and get Off -> On -> Off -> On...
Attachment #8779072 - Flags: review?(jdemooij)
Attachment #8779072 - Flags: review?(jdemooij) → review+
Flags: needinfo?(shu)
Can we land this now, Shu?
Assignee: nobody → shu
Flags: needinfo?(shu)
(In reply to Andrew Overholt [:overholt] from comment #6)
> Can we land this now, Shu?

I completely forgot, sorry.
Flags: needinfo?(shu)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d88bb4cf62f
Fix assertion involving debug mode toggle cycles in debug mode OSR. r=jandem
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5d88bb4cf62f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Is this safe to uplift to 51?
Flags: needinfo?(shu)
(In reply to David Bolter [:davidb] from comment #10)
> Is this safe to uplift to 51?

Sorry. I mean 50.
(In reply to David Bolter [:davidb] from comment #11)
> (In reply to David Bolter [:davidb] from comment #10)
> > Is this safe to uplift to 51?
> 
> Sorry. I mean 50.

Yeah, should be fine.
Flags: needinfo?(shu)
Could you fill the uplift request? Thanks
Flags: needinfo?(shu)
Comment on attachment 8779072 [details] [diff] [review]
Fix assertion involving debug mode toggle cycles in debug mode OSR.

Approval Request Comment
[Feature/regressing bug #]: 716647
[User impact if declined]: none, this is an assertion fix in DEBUG builds only
[Describe test coverage new/current, TreeHerder]: on m-c
[Risks and why]: low, bug fix only
[String/UUID change made/needed]: none
Flags: needinfo?(shu)
Attachment #8779072 - Flags: approval-mozilla-beta?
Comment on attachment 8779072 [details] [diff] [review]
Fix assertion involving debug mode toggle cycles in debug mode OSR.

Low risk, Beta50+
Attachment #8779072 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.