Closed Bug 1385844 Opened 3 years ago Closed 11 months ago

Assertion failure: frame.isDebuggee(), at js/src/vm/Debugger-inl.h:18

Categories

(Core :: JavaScript Engine, defect, P2)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- wontfix
firefox70 --- wontfix
firefox71 --- wontfix
firefox72 --- fixed

People

(Reporter: decoder, Assigned: jimb)

Details

(4 keywords, Whiteboard: [jsbugmon:testComment=8,origRev=c9f0730a57a6])

Attachments

(1 file)

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

var g = newGlobal([]);
var dbg = Debugger(g);
dbg.onExceptionUnwind = function(f, x) {
    var o = {}
    var global = this;
    var p = new Proxy(o, {
        "deleteProperty": function(await, key) {
            var g = newGlobal();
            g.parent = global;
            g.eval("var dbg = new Debugger(parent); dbg.onEnterFrame = function(frame) {};");
        }
    })
    assertEq(delete p.foo, true);
};
dbg.onDebuggerStatement = function(f) {
    assertEq(f.eval('throw 42'), null);
};
g.eval('debugger');


Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0000000000545636 in js::Debugger::onLeaveFrame (cx=0x7ffff6924000, frame=..., pc=0x7ffff44dd789 "\231\230\220\004ِ\t\224\377\377\377", <incomplete sequence \367\234>, ok=true) at js/src/vm/Debugger-inl.h:22
#0  0x0000000000545636 in js::Debugger::onLeaveFrame (cx=0x7ffff6924000, frame=..., pc=0x7ffff44dd789 "\231\230\220\004ِ\t\224\377\377\377", <incomplete sequence \367\234>, ok=true) at js/src/vm/Debugger-inl.h:22
#1  0x000000000053793e in Interpret (cx=0x7ffff6924000, state=...) at js/src/vm/Interpreter.cpp:4325
#2  0x000000000053d373 in js::RunScript (cx=0x7ffff6924000, state=...) at js/src/vm/Interpreter.cpp:409
#3  0x000000000053d8c6 in js::InternalCallOrConstruct (cx=cx@entry=0x7ffff6924000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:487
#4  0x000000000053db78 in InternalCall (cx=cx@entry=0x7ffff6924000, args=...) at js/src/vm/Interpreter.cpp:514
#5  0x000000000053dcad in js::Call (cx=cx@entry=0x7ffff6924000, fval=..., fval@entry=..., thisv=..., thisv@entry=..., args=..., rval=..., rval@entry=...) at js/src/vm/Interpreter.cpp:533
#6  0x0000000000aa4878 in js::Call (cx=0x7ffff6924000, fval=..., thisObj=<optimized out>, arg0=..., rval=...) at js/src/vm/Interpreter.h:112
#7  0x0000000000b47031 in js::Debugger::fireDebuggerStatement (this=this@entry=0x7ffff693a000, cx=cx@entry=0x7ffff6924000, vp=..., vp@entry=...) at js/src/vm/Debugger.cpp:1784
#8  0x0000000000b47514 in js::Debugger::<lambda(js::Debugger*)>::operator() (dbg=0x7ffff693a000, __closure=<synthetic pointer>) at js/src/vm/Debugger.cpp:1045
#9  js::Debugger::dispatchHook<js::Debugger::slowPathOnDebuggerStatement(JSContext*, js::AbstractFramePtr)::<lambda(js::Debugger*)>, js::Debugger::slowPathOnDebuggerStatement(JSContext*, js::AbstractFramePtr)::<lambda(js::Debugger*)> > (fireHook=..., cx=0x7ffff6924000, hookIsEnabled=...) at js/src/vm/Debugger.cpp:1925
#10 js::Debugger::slowPathOnDebuggerStatement (cx=0x7ffff6924000, frame=...) at js/src/vm/Debugger.cpp:1046
#11 0x000000000053a358 in js::Debugger::onDebuggerStatement (frame=..., frame@entry=..., cx=<optimized out>) at js/src/vm/Debugger-inl.h:58
#12 Interpret (cx=0x7ffff6924000, state=...) at js/src/vm/Interpreter.cpp:3942
#13 0x000000000053d373 in js::RunScript (cx=0x7ffff6924000, state=...) at js/src/vm/Interpreter.cpp:409
#14 0x0000000000540209 in js::ExecuteKernel (cx=cx@entry=0x7ffff6924000, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x7fffffffc920) at js/src/vm/Interpreter.cpp:698
#15 0x0000000000576dd6 in EvalKernel (cx=cx@entry=0x7ffff6924000, v=..., evalType=evalType@entry=INDIRECT_EVAL, caller=..., env=..., pc=pc@entry=0x0, vp=...) at js/src/builtin/Eval.cpp:328
#16 0x0000000000577168 in js::IndirectEval (cx=0x7ffff6924000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/Eval.cpp:421
#17 0x00000000005486db in js::CallJSNative (cx=cx@entry=0x7ffff6924000, native=0x5770a0 <js::IndirectEval(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:293
#18 0x000000000053d7ab in js::InternalCallOrConstruct (cx=cx@entry=0x7ffff6924000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:469
#19 0x000000000053db78 in InternalCall (cx=cx@entry=0x7ffff6924000, args=...) at js/src/vm/Interpreter.cpp:514
#20 0x000000000053dcad in js::Call (cx=cx@entry=0x7ffff6924000, fval=..., fval@entry=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:533
#21 0x0000000000aa397d in js::Wrapper::call (this=this@entry=0x1f1b070 <js::CrossCompartmentWrapper::singleton>, cx=cx@entry=0x7ffff6924000, proxy=..., proxy@entry=..., args=...) at js/src/proxy/Wrapper.cpp:169
#22 0x0000000000a8ac82 in js::CrossCompartmentWrapper::call (this=0x1f1b070 <js::CrossCompartmentWrapper::singleton>, cx=0x7ffff6924000, wrapper=..., args=...) at js/src/proxy/CrossCompartmentWrapper.cpp:359
#23 0x0000000000a9b56a in js::Proxy::call (cx=cx@entry=0x7ffff6924000, proxy=proxy@entry=..., args=...) at js/src/proxy/Proxy.cpp:481
#24 0x0000000000a9b65c in js::proxy_Call (cx=0x7ffff6924000, argc=<optimized out>, vp=<optimized out>) at js/src/proxy/Proxy.cpp:741
#25 0x00000000005486db in js::CallJSNative (cx=cx@entry=0x7ffff6924000, native=0xa9b5e0 <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:293
#26 0x000000000053dab9 in js::InternalCallOrConstruct (cx=0x7ffff6924000, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:451
[...]
#38 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8559
rax	0x0	0
rbx	0x7fffffffaa60	140737488333408
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffaaa0	140737488333472
rsp	0x7fffffffaa60	140737488333408
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x0	0
r11	0x0	0
r12	0x1	1
r13	0x1	1
r14	0x7ffff6924000	140737330167808
r15	0x7ffff44dd789	140737292130185
rip	0x545636 <js::Debugger::onLeaveFrame(JSContext*, js::AbstractFramePtr, unsigned char*, bool)+454>
=> 0x545636 <js::Debugger::onLeaveFrame(JSContext*, js::AbstractFramePtr, unsigned char*, bool)+454>:	movl   $0x0,0x0
   0x545641 <js::Debugger::onLeaveFrame(JSContext*, js::AbstractFramePtr, unsigned char*, bool)+465>:	ud2
Jason please take a look for post 57. Thank you.
Assignee: nobody → jorendorff
Flags: needinfo?(jorendorff)
Priority: -- → P2
Reproduces cleanly. Debugging.
Flags: needinfo?(jorendorff)
I reduced it slightly to:

var g1 = this;
var g2 = newGlobal();
var dbg = Debugger(g2);
dbg.onExceptionUnwind = function(f, x) {
    var h = newGlobal();
    h.parent = g1;
    h.eval("var dbg = new Debugger(parent); dbg.onEnterFrame = function(frame) {};");
};
dbg.onDebuggerStatement = function(f) {
    assertEq(f.eval('throw 42').throw, 42);
};
g2.eval('debugger');

Currently building something else, but my guess is that Jan's patch in bug 1385843 fixes this too.
Nope. Still asserts.

I have a few other bugs to fix before getting back to this.
Wontfix for 64. We could still take a patch for 66 and uplift for 65.
Assignee: jorendorff → nobody
Flags: needinfo?(jorendorff)
Priority: P1 → P2
Whiteboard: [jsbugmon:update,testComment=5,origRev=31724aea10ca] → [jsbugmon:update,testComment=5,origRev=31724aea10ca,ignore]
var g1 = this;
var g2 = newGlobal();
var dbg = Debugger(g2);
dbg.onExceptionUnwind = function(f, x) {
    var h = newGlobal();
    h.parent = g1;
    h.eval("var dbg = new Debugger(parent); dbg.onEnterFrame = function(frame) {};");
};
dbg.onDebuggerStatement = function(f) {
    assertEq(f.eval('throw 42').throw, 42);
};
g2.eval('debugger');

asserts js shell compiled with --enable-debug on m-c rev 8ec327de0ba7 using --fuzzing-safe --no-threads --no-baseline --no-ion --more-compartments at Assertion failure: frame.isDebuggee(), at js/src/vm/Debugger-inl.h:21

Whiteboard: [jsbugmon:update,testComment=5,origRev=31724aea10ca,ignore] → [jsbugmon:update,testComment=8,origRev=8ec327de0ba7]
Whiteboard: [jsbugmon:update,testComment=8,origRev=8ec327de0ba7] → [jsbugmon:testComment=8,origRev=8ec327de0ba7]

I think we can drop this from regression triage.

Whiteboard: [jsbugmon:testComment=8,origRev=8ec327de0ba7] → [jsbugmon:testComment=8,origRev=c9f0730a57a6,update]
Whiteboard: [jsbugmon:testComment=8,origRev=c9f0730a57a6,update] → [jsbugmon:testComment=8,origRev=c9f0730a57a6]

Jim mentioned over Slack that he's still the only one taking Debugger fuzzbugs.

Flags: needinfo?(jimb)

I can reproduce this. This should be quick.

Assignee: nobody → jimb
Flags: needinfo?(jimb)

Here's a further simplified test case:

var g1 = this;

var h = newGlobal();
h.parent = g1;
h.eval(`
  var hdbg = new Debugger(parent);
  function j() {
    hdbg.onEnterFrame = function(frame) {};
  }
`);

var g2 = newGlobal();
g2.j = h.j;

var dbg = new Debugger(g2);
var g2DO = dbg.addDebuggee(g2);

dbg.onDebuggerStatement = function(f) {
  f.eval('j()');
};
g2.eval('debugger');

Since an onEnterFrame hook detects calls anywhere in any debuggee realm, setting such a hook entails setting the isDebuggee flag on all stack frames in the debuggee realms. This is the job of Debugger::updateExecutionObservabilityOfFrames.

Unfortunately, that function uses FrameIter to walk the stack. FrameIter respects 'debugger eval prev' links, which make the parent of a frame for a call to Debugger.Frame.prototype.eval to appear to be the Debugger.Frame's referent, not the actual youngest debuggee frame. (Debugger eval prev links are somewhat nonsensical, and perhaps should be removed, but they predate the Debugger API.)

In the test case, the function h.f sets an onEnterFrame hook on a Debugger whose debuggee is the main global running the test script. At that point, the JavaScript stack looks like this (youngest to oldest):

  1. In global h, a call to h.j , setting the onEnterFrame hook
  2. In global g2, a debugger eval frame evaluating the expression j()
  3. In the main global, the onDebuggerStatement handler
  4. In global g2, an eval frame evaluating the statement debugger;
  5. In the main global, a frame running the test script top level code.

As a debugger eval frame, frame 2 has a debugger eval prev link pointing to frame 4. The FrameIter in Debugger::updateExecutionObservabilityOfFrames follows that link, skipping over frame 3. When we return from frame 3, the assertion notices that the frame's script is marked as a debuggee (setting the onEnterFrame hook set its realm's DebuggerObservesAllExecution flag), but that the frame itself is not. This violates the Debugger's invariant that all frames running debuggee scripts must be themselves debuggee frames.

Setting a hook on a Debugger may expand the set of behaviors it observes, so
that new scripts and stack frames must have their isDebuggee flags set. The
Debugger::updateExecutionObservabilityOfFrames function is supposed to walk
the stack and sets the flag where necessary.

However, the old code performed that stack walk using FrameIter, which follows
'debugger eval prevlinks, potentially skipping over stack frames that need to be flagged. This patch changes the code to useAllFramesIter, which differs fromFrameIter` only in that it ignores 'debugger eval prev' links.

Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1f35e218386e
Ignore 'debugger eval prev' links when marking frames as debuggees. r=jorendorff
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.