[@ JS_Assert] JS_GetFrameScopeChain aborts when asked about a native frame

ASSIGNED
Assigned to

Status

()

Core
JavaScript Engine
--
critical
ASSIGNED
8 years ago
7 years ago

People

(Reporter: timeless, Assigned: dmandelin)

Tracking

({assertion, crash})

Trunk
x86
Mac OS X
assertion, crash
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 obsolete attachment)

(Reporter)

Description

8 years ago
with luke's fix for bug 552248, and using the general steps from bug 552248 comment 11, i now hit this gem:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x0000000103b20065 in JS_Assert (s=0x103bbbee0 "FUN_INTERPRETED(this)", file=0x103ba44b8 "jsfun.h", ln=173)
73	    *((int *) NULL) = 0;  /* To continue from here in GDB: "return" then "continue". */
(gdb) bt
#0 JS_Assert (s=0x103bbbee0 "FUN_INTERPRETED(this)", file=0x103ba44b8, ln=173)
#1 JSFunction::countArgsAndVars (this=0x1210c7af0)
#2 NewCallObject
#3 js_GetCallObject
#4 JS_GetFrameCallObject
#5 JS_GetFrameScopeChain
#6 jsd_GetScopeChainForStackFrame
#7 JSD_GetScopeChainForStackFrame

I believe the right regression for this is:
http://hg.mozilla.org/mozilla-central/rev/910ee7db07de

filed confidential per gal's request. there's a 1 week expiry window.
(Reporter)

Comment 1

8 years ago
Created attachment 432771 [details] [diff] [review]
proposal

with this, i can play with the native frame in venkman without crashing, and the results seem fairly reasonable.

note that 'this' can easily be different between the caller frame and the native frame, and being able to see the other this is fairly helpful.
Assignee: general → timeless
Status: NEW → ASSIGNED
Attachment #432771 - Flags: review?(dmandelin)
(Reporter)

Comment 2

8 years ago
(note that i can't get bugmail for this bug while it's grouped, so if you have comments, please contact me by irc).
Timeless, you shouldn't have to patch this. Patching to whack bug-moles is no good either, for instance the jsfun.h changes are wrong and should not be committed -- but again this is no slam on you, since you were doing best effort under duress.

JS_EvaluateUCInStackFrame passes JS_DISPLAY_SIZE to JSCompiler::compileScript to try to shut off use of the display, but this doesn't satisfy the assertion and assumptions in the code hg blame'ed on dmandelin.

I think this bug should not be restricted. Let's fix it in the open. I'll leave that decision to sayrer.

/be
Flags: in-testsuite?

Comment 4

8 years ago
dmandelin has the blame, to assigning there, but delegation is obv ok.
Assignee: timeless → dmandelin
Group: core-security
(Assignee)

Comment 5

8 years ago
Comment on attachment 432771 [details] [diff] [review]
proposal

Looks good to me. I assume you know what you are doing with the JSFunction members being called on debugger-only stuff, since I don't know anything about that.
Attachment #432771 - Flags: review?(dmandelin) → review+
Comment on attachment 432771 [details] [diff] [review]
proposal

The jsfun.h changes are not wanted. We do not want to paper over bugs where a native function is used as if it were a scripted function.

Isn't the problem here that a native function call frame can be presented to JS_EvaluateUCInStackFrame?

/be
Attachment #432771 - Flags: review-
(Assignee)

Comment 7

8 years ago
I tried to repro this with the steps in bug 552248 comment 11, but at step 12, it doesn't crash, and it doesn't stop in venkman. Is '/fbreak cert 1' an abbreviation or something? Or are the steps a bit different for this bug?
(Reporter)

Comment 8

8 years ago
http://www.hacksrus.com/~ginda/venkman/faq/venkman-faq.html#2.19
(Reporter)

Comment 9

8 years ago
brendan: it needs to be legal to do this, it's actually quite useful.
Timeless: can you say why it's useful? What about a native function call's frame is useful for eval-in-stack-frame? Is it a particular native (e.g., obj_eval) or any/many? More details would be helpful.

/be
(Reporter)

Comment 11

8 years ago
brendan: sorry, yes, being able to evaluate with the scope/object are useful, as well as poking the object's parent and getting other properties.

roughly, the useful things are:

scope + 'this object' + all properties relating to 'this object' (including parents)
(Reporter)

Comment 12

7 years ago
Comment on attachment 432771 [details] [diff] [review]
proposal

while this patch has been shifting in my tree, i think it's now reached the completely rotted state, only two hunks have survived:


@@ -2267,9 +2267,12 @@ BindNameToSlot(JSContext *cx, JSCodeGene
-            JS_ASSERT(cg->staticLevel >= fun->u.i.script->staticLevel);
+            if (cg->staticLevel < fun->u.i.script->staticLevel)
+                return JS_TRUE;


@@ -187,7 +187,7 @@ Parser::Parser(JSContext *cx, JSPrincipa
-    JS_ASSERT_IF(cfp, cfp->isScriptFrame());
+    /* The debugger might ask us to operate on a native caller frame pointer */

I'm going to drop the patch from my queue. We'll see if I get around to using venkman this year...
Attachment #432771 - Attachment is obsolete: true
Crash Signature: [@ JS_Assert]
You need to log in before you can comment on or make changes to this bug.