1282518 - Assertion failure: isBaselineFrame(), at js/src/vm/Stack.h:178 with Debugger and interrupt callback

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 --ion-offthread-compile=off --ion-eager):

function testResumptionVal(resumptionVal) {
    g = newGlobal();
    dbg = new Debugger;
    setInterruptCallback(function() {
        dbg.addDebuggee(g);
        dbg.getNewestFrame().onStep = function() {
            dbg.removeDebuggee(g);
            return resumptionVal;
        }
        return true;
    })
    g.eval("(" + function f() {
        invokeInterruptCallback(function() f());
    } + ")()")
}
testResumptionVal({
    return: ""
});



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0000000000600338 in js::AbstractFramePtr::asBaselineFrame (this=<optimized out>) at js/src/vm/Stack.h:178
#0  0x0000000000600338 in js::AbstractFramePtr::asBaselineFrame (this=<optimized out>) at js/src/vm/Stack.h:178
#1  0x0000000000a9d50d in js::AbstractFramePtr::setReturnValue (this=<optimized out>, rval=...) at js/src/vm/Stack-inl.h:440
#2  0x0000000000a55748 in js::Debugger::propagateForcedReturn (cx=cx@entry=0x7ffff6965000, frame=..., rval=..., rval@entry=...) at js/src/vm/Debugger.cpp:6205
#3  0x0000000000b299c5 in InvokeInterruptCallback (cx=cx@entry=0x7ffff6965000) at js/src/vm/Runtime.cpp:597
#4  0x0000000000b29bb5 in JSRuntime::handleInterrupt (this=<optimized out>, cx=cx@entry=0x7ffff6965000) at js/src/vm/Runtime.cpp:674
#5  0x000000000044a2a8 in js::CheckForInterrupt (cx=0x7ffff6965000) at js/src/jscntxt.h:623
#6  InvokeInterruptCallbackWrapper (cx=0x7ffff6965000, argc=<optimized out>, vp=0x7fffffff8ee8) at js/src/shell/js.cpp:3342
#7  0x00007ffff7fc1fdd in ?? ()
#8  0x00007ffff7e967c0 in ?? ()
#9  0x00007fffffff8ec0 in ?? ()
#10 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7fffffff8900	140737488324864
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffff8880	140737488324736
rsp	0x7fffffff8880	140737488324736
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fdc740	140737353992000
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7fffffff8910	140737488324880
r13	0x0	0
r14	0x7ffff6965008	140737330434056
r15	0x7fffffff88f0	140737488324848
rip	0x600338 <js::AbstractFramePtr::asBaselineFrame() const+56>
=> 0x600338 <js::AbstractFramePtr::asBaselineFrame() const+56>:	movl   $0x0,0x0
   0x600343 <js::AbstractFramePtr::asBaselineFrame() const+67>:	ud2

Comment 1

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

This seems to have existed since early Nov 2014 (m-c rev dc4b163f7db7). Since it involves Debugger, setting needinfo? from Jim and Nick as a start.

Comment 2

[Jim Blandy :jimb]

I can reproduce on current Mozilla Central (63cc31d6cc1c). Taking.

Comment 3

[Jim Blandy :jimb]

The crash occurs because the test case ends up trying to set the return value of a RematerializedFrame, and AbstractFramePtr::setReturnValue doesn't handle that case, because RematerializedFrame has no setReturnValue method.

Indeed, the returnValue_ field of RematerializedFrame is set, but never used. This sort of makes sense, since they're only used to store the effects of debugging operations until control actually returns to that frame, and we build some actually executable frame type from it.

The test creates a situation where we're returning to the frame only to force it to return. I'm unclear on how this stuff actually works. I could imagine the solution being either to build a BaselineFrame first, and then set its return value; or to add a setReturnValue method and hasReturnValue_ flag to RematerializedFrame, and then use that somehow when building a BaselineFrame.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Thanks for taking, Jim!

Comment 5

[Jim Blandy :jimb]

I talked with Shu about this. He says that he probably just hadn't gotten around to finishing the code, but when we're actually building a Baseline frame from a RematerializedFrame, returnValue_ should be transferred just like all of the other fields. This seems straightforward, so I'll give it a shot.

Comment 7

[Jim Blandy :jimb]

Attachment: Propagate return values from RematerializedFrames to BaselineFrames.

This fixes the crash here and for the test cases in bug 1285939.

Comment 8

[Jim Blandy :jimb]

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a2db0f5678d

Comment 9

[Shu-yu Guo [:shu]]

Comment on attachment 8773962 [details] [diff] [review]
Propagate return values from RematerializedFrames to BaselineFrames.

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

Wunderbar, sorry I missed this during the initial implementation and thanks for the patch!

Comment 10

[Jim Blandy :jimb]

Thanks for the review!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/737a34eee8e6
Propagate return values from RematerializedFrames to BaselineFrames. r=shu

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/737a34eee8e6