1282741 - 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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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.

Comment 2

[Andrew Overholt [:overholt]]

Any progress here?

Comment 3

[Jan de Mooij [:jandem]]

Looking at this now.

Comment 4

[Jan de Mooij [:jandem]]

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.

Comment 5

[Shu-yu Guo [:shu]]

Attachment: Fix assertion involving debug mode toggle cycles in debug mode OSR.

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

Comment 6

[Andrew Overholt [:overholt]]

Can we land this now, Shu?

Comment 7

[Shu-yu Guo [:shu]]

(In reply to Andrew Overholt [:overholt] from comment #6)
> Can we land this now, Shu?

I completely forgot, sorry.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d88bb4cf62f
Fix assertion involving debug mode toggle cycles in debug mode OSR. r=jandem

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5d88bb4cf62f

Comment 10

[David Bolter [:davidb] (NeedInfo me for attention)]

Is this safe to uplift to 51?

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to David Bolter [:davidb] from comment #10)
> Is this safe to uplift to 51?

Sorry. I mean 50.

Comment 12

[Shu-yu Guo [:shu]]

(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.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Could you fill the uplift request? Thanks

Comment 14

[Shu-yu Guo [: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

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8779072 [details] [diff] [review]
Fix assertion involving debug mode toggle cycles in debug mode OSR.

Low risk, Beta50+

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f79842c6d155