1543590 - Crash [@ js::FrameIter::FrameIter] with Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision bdaf1b36c442 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off --more-compartments):

var g = newGlobal();
dbg = Debugger(g);
dbg.onDebuggerStatement = frame => {
    frame.onPop = completion => {
        dbg.removeDebuggee(g);
    };
};
g.eval('function* f() { debugger; yield 2; debugger; yield 3; debugger; }');
dbg = Debugger(g);
genObj = g.f();
dbg.onDebuggerStatement = frame => {
  frame.onPop = completion => {};
};
for (x of genObj) {}

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  js::FrameIter::FrameIter (this=0x7fffffffb280, data=...) at js/src/vm/Stack.cpp:783
#1  0x0000555555a00bc2 in js::DebuggerFrame::getReferent (frame=...) at js/src/vm/Debugger.cpp:9448
#2  0x0000555555a07d55 in js::ScriptedOnPopHandler::onPop (this=0x7ffff5f1af60, cx=<optimized out>, frame=..., resumeMode=@0x7fffffffb87c: js::ResumeMode::Return, vp=...) at js/src/vm/Debugger.cpp:8827
#3  0x0000555555a3b79c in js::Debugger::slowPathOnLeaveFrame (cx=<optimized out>, cx@entry=0x7ffff5f19000, frame=..., pc=pc@entry=0x7ffff58e1e42 "\313\001", frameOk=frameOk@entry=true) at js/src/vm/Debugger.cpp:1070
#4  0x00005555558eba53 in js::Debugger::onLeaveFrame (cx=0x7ffff5f19000, frame=..., pc=0x7ffff58e1e42 "\313\001", ok=true) at js/src/vm/Debugger-inl.h:30
#5  0x00005555558d3a4b in Interpret (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:2089
#6  0x00005555558e2426 in js::RunScript (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:422
#7  0x00005555558e2c7f in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f19000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:562
#8  0x00005555558e30fd in InternalCall (cx=cx@entry=0x7ffff5f19000, args=...) at js/src/vm/Interpreter.cpp:589
#9  0x00005555558e3270 in js::Call (cx=cx@entry=0x7ffff5f19000, fval=..., fval@entry=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:605
#10 0x0000555555e6a680 in js::ForwardingProxyHandler::call (this=<optimized out>, cx=0x7ffff5f19000, proxy=..., args=...) at js/src/proxy/Wrapper.cpp:162
#11 0x0000555555e55193 in js::CrossCompartmentWrapper::call (this=0x555557bf5c60 <js::CrossCompartmentWrapper::singleton>, cx=<optimized out>, wrapper=..., args=...) at js/src/proxy/CrossCompartmentWrapper.cpp:238
#12 0x0000555555e618e5 in js::Proxy::call (cx=0x7ffff5f19000, proxy=proxy@entry=..., args=...) at js/src/proxy/Proxy.cpp:503
#13 0x00005555558e2ec6 in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f19000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:508
#14 0x00005555558e30fd in InternalCall (cx=0x7ffff5f19000, args=...) at js/src/vm/Interpreter.cpp:589
#15 0x00005555558d49b1 in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:593
#16 Interpret (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:3075
#17 0x00005555558e2426 in js::RunScript (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:422
[...]
#26 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:11288
rax	0x2c8b514951b8	48976875835832
rbx	0x7fffffffb280	140737488335488
rcx	0x2c8b5148a200	48976875790848
rdx	0x0	0
rsi	0x50	80
rdi	0x7fffffffb2d0	140737488335568
rbp	0x7fffffffb270	140737488335472
rsp	0x7fffffffb250	140737488335440
r8	0x7fffffffb940	140737488337216
r9	0x10	16
r10	0x10	16
r11	0xf	15
r12	0x1	1
r13	0x0	0
r14	0x7fffffffb87c	140737488337020
r15	0x7fffffffb9a0	140737488337312
rip	0x555555c9806b <js::FrameIter::FrameIter(js::FrameIter::Data const&)+27>
=> 0x555555c9806b <js::FrameIter::FrameIter(js::FrameIter::Data const&)+27>:	mov    -0x50(%rsi),%rax
   0x555555c9806f <js::FrameIter::FrameIter(js::FrameIter::Data const&)+31>:	mov    %rax,-0x50(%rdi)

Comment 1

[Fuzzing Team]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 2

[Jason Orendorff [:jorendorff]]

var g = newGlobal({newCompartment: true});
g.eval('function* f() { debugger; yield 1; }');
var genObj = g.f();

var dbg1 = Debugger(g);
dbg1.onDebuggerStatement = frame => {
  frame.onPop = completion => {
    dbg2.removeDebuggee(g);
  };
};

var dbg2 = Debugger(g);
dbg2.onDebuggerStatement = frame => {
  frame.onPop = completion => {};
};

genObj.next();

Comment 3

[Jason Orendorff [:jorendorff]]

The two onDebuggerStatement callbacks both fire, so we create Frame objects for that frame in both dbg1 and dbg2.

The crash happens while we're firing onPop callbacks. This call to getDebuggerFrames() gets two frames, one from dbg1 and one from dbg2. Then we go into the loop to fire callbacks.

1. The first frame.onPop callback removes the debuggee from dbg2. This has the effect of killing all dbg2's frame objects (their .live property becomes false). But for some reason this doesn't free the frame object's onPop handler.

2. On this line, the second time through the loop, we're looking at dbg2's frame object, and it has an onPop handler attached. So we try to fire a callback.

That crashes because the frame object is dead (we crash trying to get an AbstractFramePtr from the FrameIter data, which is null).

Comment 4

[Jason Orendorff [:jorendorff]]

On second thought, maybe it's deliberate that we don't kill the onPop handler. But in that case we need to add a check to the if-statement. It should not fire if the frame was killed by another onPop handler.

This is pretty in-the-weeds; P3 was right :-P

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1543590 - Don't crash trying to fire a dead frame's onPop handler. r?jimb

Mutating Debugger state between the time a callback-triggering event is
reported to js::Debugger::onSomeEventSlowPath and the time the
callback is actually called can invalidate assumptions, and multiple Debuggers
are a way to do that, part 183.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de9c7f8fd959
Don't crash trying to fire a dead frame's onPop handler. r=jimb

Comment 7

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/de9c7f8fd959