Closed Bug 678086 Opened 8 years ago Closed 8 years ago

[jsdbg2] Assertion failure: fp->isScriptFrame(), at vm/Debugger.cpp:393

Categories

(Core :: JavaScript Engine, defect, critical)

Other Branch
x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: decoder, Assigned: jorendorff)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(1 file)

The following code crashes on jsdbg2 branch (revision 48e43edc8834, options -j -m):


function f() {
    frame = dbg.getNewestFrame();
}
var g = newGlobal('new-compartment');
g.f = function (a, b) { return a + "/" + this . f( ) ; };
var dbg = Debugger(g);
dbg.onDebuggerStatement = function (frame) {
    var f = frame.eval("f").return;
    assertEq(f.call(null, "a", "b").return, "a/b");
};
g.eval("debugger;");
Reduced:

var g = newGlobal('new-compartment');
g.f = function () { dbg.getNewestFrame(); };
var dbg = new Debugger;
dbg.addDebuggee(g).getOwnPropertyDescriptor("f").value.call();

Boring patch coming.
Attached patch v1Splinter Review
The bug is that Debugger::observesFrame doesn't know we never want to observe dummy frames. It has three call sites, only one of which correctly calls !fp->isDummyFrame() first. In particular, the asserting test case has us calling Debugger::getYoungestFrame, which does not check for dummy frames.

So: Add an isDummyFrame() call in observesFrame and remove the now-redundant one in Debugger::getOlder.

*yawn*
Assignee: general → jorendorff
Attachment #552541 - Flags: review?(jimb)
Comment on attachment 552541 [details] [diff] [review]
v1

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

It would be nice to have a comment in Debugger::observesFrame.
Attachment #552541 - Flags: review?(jimb) → review+
Can we get this landed?
Philor backed bug 695922, bug 678086, and bug 719841 out of mozilla-inbound for causing Tp crashes.
https://hg.mozilla.org/mozilla-central/rev/117f2280bd37
Of course I was unable to reproduce the crash by running talos locally. But looking closer at the patch made it clear. The test case to reproduce this crash turns out to be the following smiley face:

    ({c:

and the fix is like this:

diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -7013,26 +7013,26 @@ Parser::primaryExpr(TokenKind tt, bool a
                 reportErrorNumber(NULL, JSREPORT_ERROR, JSMSG_BAD_PROP_ID);
                 return NULL;
             }
 
             op = JSOP_INITPROP;
             tt = tokenStream.getToken();
             if (tt == TOK_COLON) {
                 pnval = assignExpr();
+                if (!pnval)
+                    return NULL;
 
                 /*
                  * Treat initializers which mutate __proto__ as non-constant,
                  * so that we can later assume singleton objects delegate to
                  * the default Object.prototype.
                  */
-                if ((pnval && !pnval->isConstant()) ||
-                    atom == context->runtime->atomState.protoAtom) {
+                if (!pnval->isConstant() || atom == context->runtime->atomState.protoAtom)
                     pn->pn_xflags |= PNX_NONCONST;
-                }
             }
 #if JS_HAS_DESTRUCTURING_SHORTHAND
             else if (ltok == TOK_NAME && (tt == TOK_COMMA || tt == TOK_RC)) {
                 /*
                  * Support, e.g., |var {x, y} = o| as destructuring shorthand
                  * for |var {x: x, y: y} = o|, per proposed JS2/ES4 for JS1.8.
                  */
                 tokenStream.ungetToken();
diff --git a/js/src/jit-test/tests/basic/bug678086-syntax.js b/js/src/jit-test/tests/basic/bug678086-syntax.js
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug678086-syntax.js
@@ -0,0 +1,2 @@
+load(libdir + "asserts.js");
+assertThrowsInstanceOf(function () { eval("({p:"); }, SyntaxError);  // don't crash
https://hg.mozilla.org/mozilla-central/rev/ee98e4d71995
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/2e891e0db397
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.