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

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[timeless]

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.

Comment 1

[timeless]

Attachment: 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.

Comment 2

[timeless]

(note that i can't get bugmail for this bug while it's grouped, so if you have comments, please contact me by irc).

Comment 3

[Brendan Eich [:brendan]]

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

Comment 4

[Robert Sayre]

dmandelin has the blame, to assigning there, but delegation is obv ok.

Comment 5

[David Mandelin [:dmandelin]]

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.

Comment 6

[Brendan Eich [:brendan]]

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

Comment 7

[David Mandelin [:dmandelin]]

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?

Comment 8

[timeless]

http://www.hacksrus.com/~ginda/venkman/faq/venkman-faq.html#2.19

Comment 9

[timeless]

brendan: it needs to be legal to do this, it's actually quite useful.

Comment 10

[Brendan Eich [:brendan]]

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

Comment 11

[timeless]

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)

Comment 12

[timeless]

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...

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Closing because no crash reported since 12 weeks.