552248 - handle EvalInFrame where frame is in saved callstack [@ JS_Assert][@ BindNameToSlot]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[timeless]

baseline: http://hg.mozilla.org/mozilla-central/rev/0ab4f3a39bb9
+1 patch to certManager.js

steps:
1. install nightly tester tools +restart
2. install venkman (force install, restart)
3. open venkman
4. show interactive session
5. /fbreak certManager 1
6. open preferences>advanced>encryption>view certificates

roughly by here i'm dead, although i typically do 6 before 5 and interact with the search field.

0x0000000103b1f0ad in JS_Assert (s=0x103ba9cb0 "currentCallStack && !currentCallStack->isSaved()", file=0x103ba2f00 "/Users/timeless/devel/mozilla.org/mozilla-central/js/src/jscntxt.h", ln=1407) at /Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsutil.cpp:73
73	    *((int *) NULL) = 0;  /* To continue from here in GDB: "return" then "continue". */

#0   JS_Assert (s=0x103ba9cb0 "currentCallStack && !currentCallStack->isSaved()", file=0x103ba2f00 "mozilla-central/js/src/jscntxt.h", ln=1407) at mozilla-central/js/src/jsutil.cpp:73
#1   JSContext::activeCallStack
#2   JSStackFrame::varobj
#3   BindNameToSlot
#4   EmitNameOp
#5   js_EmitTree
#6   js_EmitTree
#7   js_EmitTree
#8   JSCompiler::compileScript
#9   JS_EvaluateUCInStackFrame
#10  jsd_EvaluateUCScriptInStackFrame
#11  JSD_AttemptUCScriptInStackFrame
#12  jsdStackFrame::Eval
#13  NS_InvokeByIndex_P
#14  XPCWrappedNative::CallMethod

Comment 1

[timeless]

I'm going to tentatively blame:
http://hg.mozilla.org/mozilla-central/rev/5d8801fe08f5

Comment 2

[Luke Wagner [:luke]]

Looking at the callstack, I am guessing that the problem is that the varobj-removal patch assumes that the frame passed to JS_EvaluateUCInStackFrame is in the current (active) callstack (i.e., not a callstack that has been set aside by JS_SaveFrameChain).  Do you think that is happening here?  On way to test would be to print the following expression in gdb for frame 3 (in the above list):

  currentCallStack->isSaved() && currentCallStack->contains(caller)

Comment 3

[timeless]

timeless-mbp:bin timeless$ ./jsdb 
NewRuntime: 0x100866e00
NewRuntime: 0x1008a7600
NewRuntime: 0x1008e7800
successfully loaded debugger.js for depth 3
successfully loaded debugger.js for depth 2
successfully loaded debugger.js for depth 1
js> help()
JavaScript-C 1.8.0 pre-release 1 2007-10-03
...
js> function a(){null.null}
js> a()
............................
msg = null has no properties
filename = typein
lineno = 2
lineBuf = null
tokenOffset = null
............................
[E]at [i]gnore [p]ass along [d]ebug ?d

hit debug break hook

 >line  2 of JSDScript: typein:a:2:1
  line  3 of JSDScript: typein:_TOP_LEVEL_:3:1

unable to find source for typein

---- at this point, it's time to have a gdb ----
---- I don't know how to get cx->fp to be null from jsdb, but it is in venkman because of how jsdxpc interacts with running scripts/xpconnect. ----

(gdb) b JS_EvaluateUCInStackFrame 
Breakpoint 1 at 0x1000892ba: file /Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsdbgapi.cpp, line 1351.
(gdb) c
Continuing.

jsdb1> jsd.EvaluateScriptInStackFrame(jsd.GetStackFrame(),"print(1)")

Breakpoint 1, JS_EvaluateUCInStackFrame (cx=0x10085fe00, fp=0x7fff5fbfcc70, chars=0x1005a6d20, length=8, filename=0x10002eaea "jsdb_show", lineno=1, rval=0x7fff5fbfa788) at /Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsdbgapi.cpp:1351
1351	    JS_ASSERT_NOT_ON_TRACE(cx);
(gdb) 
1357	    scobj = JS_GetFrameScopeChain(cx, fp);
(gdb) 
1358	    if (!scobj)
(gdb) 
1369	                                       filename, lineno, NULL, JS_DISPLAY_SIZE);
(gdb) n
1371	    if (!script)
(gdb) n
1375	    if (cx->fp != fp) {
(gdb) p cx->fp
$1 = (JSStackFrame *) 0x7fff5fbfcc70
(gdb) p fp
$2 = (JSStackFrame *) 0x7fff5fbfcc70
(gdb) b 1417
Breakpoint 2 at 0x100089519: file /Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsdbgapi.cpp, line 1417.
(gdb) p cx->fp
$3 = (JSStackFrame *) 0x7fff5fbfcc70
(gdb) set cx->fp=0
(gdb) p cx->fp
$4 = (JSStackFrame *) 0x0
(gdb) n
1376	        memcpy(displayCopy, cx->display, sizeof displayCopy);
(gdb) c

Assertion failure: cx->fp, at /Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsinterp.cpp:1478

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x00000001001890bd in JS_Assert (s=0x10020cfac "cx->fp", file=0x100219778 "/Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsinterp.cpp", ln=1478) at /Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsutil.cpp:73

73	    *((int *) NULL) = 0;  /* To continue from here in GDB: "return" then "continue". */
(gdb) 
#0  0x00000001001890bd in JS_Assert (s=0x10020cfac "cx->fp", file=0x100219778 "/Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsinterp.cpp", ln=1478) at /Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsutil.cpp:73
#1  js_ContainingCallStack (cx=0x10085fe00, target=0x7fff5fbfcc70) at jsinterp.cpp:1478
#2  js_Execute () at jsinterp.cpp:1556
#3  JS_EvaluateUCInStackFrame (cx=0x10085fe00, fp=0x7fff5fbfcc70, chars=0x1005a6d20, length=8, filename=0x10002eaea "jsdb_show", lineno=1, rval=0x7fff5fbfa788) at /Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsdbgapi.cpp:1412
#4  JS_EvaluateInStackFrame (cx=0x10085fe00, fp=0x7fff5fbfcc70, bytes=0x10058ed70 "print(1)", length=8, filename=0x10002eaea "jsdb_show", lineno=1, rval=0x7fff5fbfa788) at /Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsdbgapi.cpp:1435
#5  jsd_EvaluateScriptInStackFrame ()
#6  JSD_EvaluateScriptInStackFrame ()
#7  EvaluateScriptInStackFrame ()
#8  js_Invoke (cx=0x100873c00, argc=2, vp=0x10089c520, flags=2)
#9  js_Interpret (cx=0x100873c00)
#10 js_Execute ()
#11 JS_EvaluateUCScriptForPrincipals (cx=0x100873c00, obj=0x1007d0000, principals=0x0, chars=0x100633210, length=64, filename=0x10058e210 "jsdb_evalLoop1", lineno=30, rval=0x7fff5fbfb630)
#12 JS_EvaluateUCScript (cx=0x100873c00, obj=0x1007d0000, chars=0x100633210, length=64, filename=0x10058e210 "jsdb_evalLoop1", lineno=30, rval=0x7fff5fbfb630)
#13 JS_EvaluateScript (cx=0x100873c00, obj=0x1007d0000, bytes=0x1006331a0 "jsd.EvaluateScriptInStackFrame(jsd.GetStackFrame(),\"print(1)\")\n\n", nbytes=64, filename=0x10058e210 "jsdb_evalLoop1", lineno=30, rval=0x7fff5fbfb630)
#14 SafeEval
#15 js_Invoke
#16 js_Interpret
#17 js_Invoke
#18 js_InternalInvoke
#19 JS_CallFunction
#20 jsdb_ExecHookHandler
#21 jsd_CallExecutionHook
#22 jsd_InterruptHandler
#23 js_Interpret
#24 js_Execute
#25 JS_EvaluateUCInStackFrame <- a more accurate comparison to jsdxpc would be to trigger the code here instead
#26 JS_EvaluateInStackFrame (cx=0x10085fe00, fp=0x1009b1450, bytes=0x10058ed70 "print(1)", length=8, filename=0x10002eaea "jsdb_show", lineno=1, rval=0x7fff5fbfd0b8)
#27 jsd_EvaluateScriptInStackFrame
#28 JSD_EvaluateScriptInStackFrame
#29 EvaluateScriptInStackFrame
#30 js_Invoke
#31 js_Interpret (cx=0x100873c00)
#32 js_Execute
#33 JS_EvaluateUCScriptForPrincipals
#34 JS_EvaluateUCScript
#35 JS_EvaluateScript
#36 SafeEval
#37 js_Invoke
#38 js_Interpret
#39 js_Invoke
#40 js_InternalInvoke
#41 JS_CallFunction
#42 jsdb_ExecHookHandler
#43 jsd_CallExecutionHook
#44 jsd_ThrowHandler
#45 js_Interpret
#46 js_Execute
#47 JS_ExecuteScript
#48 Process
#49 ProcessArgs
#50 main

afaict JS_EvaluateUCInStackFrame assumes that it can pass in an fp and not put away the original cx->fp:

JS_EvaluateUCInStackFrame...
    if (cx->fp != fp) {
        memcpy(displayCopy, cx->display, sizeof displayCopy);
        ...
    }
    ok = js_Execute(cx, scobj, script, fp, JSFRAME_DEBUGGER | JSFRAME_EVAL,
                    rval);

But the code in js_ContainingCallStack seems to assume that cx and fp are related. Note that I've hit 3 or so different but related crashes while using venkman and trying to make the crash go away. 

If I apply the following patch, i get one of the other crashes i got w/ venkman

diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp
--- a/js/src/jsdbgapi.cpp
+++ b/js/src/jsdbgapi.cpp
@@ -1352,2 +1352,3 @@ JS_EvaluateUCInStackFrame(JSContext *cx,
 
+    JSStackFrame *fp2 = NULL;
     JSObject *scobj;
@@ -1374,2 +1375,3 @@ JS_EvaluateUCInStackFrame(JSContext *cx,
     JSStackFrame *displayCopy[JS_DISPLAY_SIZE];
+    CallStack cs(cx);
     if (cx->fp != fp) {
@@ -1388,3 +1390,4 @@ JS_EvaluateUCInStackFrame(JSContext *cx,
          */
-        JSStackFrame *fp2 = fp, *last = NULL;
+        JSStackFrame *last = NULL;
+        fp2 = fp;
         while (fp2) {
@@ -1408,2 +1411,6 @@ JS_EvaluateUCInStackFrame(JSContext *cx,
         }
+        fp2 = cx->fp;
+        cx->fp = fp;
+        cs.setInitialFrame(fp);
+        cx->pushCallStack(&cs);
     }
@@ -1413,4 +1420,7 @@ JS_EvaluateUCInStackFrame(JSContext *cx,
 
-    if (cx->fp != fp)
+    if (fp2) {
+        cx->popCallStack();
         memcpy(cx->display, displayCopy, sizeof cx->display);
+        cx->fp = fp2;
+    }
     js_DestroyScript(cx, script);

Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x00000001001890b5 in JS_Assert (s=0x100215508 "cx->activeCallStack()->contains(this)", file=0x10020cf08 "/Users/timeless/devel/mozilla.org/mozilla-central/js/src/jscntxt.h", ln=1682) at /Users/timeless/devel/mozilla.org/mozilla-central/js/src/jsutil.cpp:73
73	    *((int *) NULL) = 0;  /* To continue from here in GDB: "return" then "continue". */

It also doesn't work if i don't use the CallStack object.

Comment 4

[timeless]

Luke: in other words, yes. That's what's happening in the jsdxpc case.

Comment 5

[timeless]

Attachment: this fixes my crash

Comment 6

[timeless]

actually, it doesn't, i just hit one of the other crashes shortly after...

Comment 7

[Luke Wagner [:luke]]

Attachment: fix

I added evalInFrame to the js shell which optionally wraps the JS_EvalUCInStackFrame with calls to JS_{Save,Restore}FrameChain.  This let me reproduce both of the crashes described above with the attached test case.  The fix is to remove the assumption described in comment 2, which means slightly generalizing js_ContainingCallStack and using the CallStack version of JSStackFrame::varobj() in jsemit.cpp.

timeless: does this fix the problems you are seeing in Venkman?

Comment 8

[Luke Wagner [:luke]]

Attachment: rebased for m-c

Comment 9

[Luke Wagner [:luke]]

Attachment: rebased for m-c (after hg qref)

oops, forgot to qref

Comment 10

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/e7e05fc7d120

Comment 11

[timeless]

fwiw, w/ this, venkman works -- sorta, i'm still crashing because of 2 or 3 other older regressions. for reference, below are my general steps.

steps:
1. install minefield (3.7a3)
2. install nightly tester tools
https://addons.mozilla.org/en-US/firefox/addon/6543
3. restart
4. install a jsd consumer compatible with minefield
https://addons.mozilla.org/en-US/firefox/addon/216 claims compatibility with 3.7a1 force override
5. restart
6. tools>JavaScript Debugger
7. if you don't see "Interactive Session", View>Show/Hide>Interactive Session
8. /fbreak cert 1
9. open preferences (on os-x, cmd-,)
10. advanced>encryption
11. view certificates
12. if you haven't crashed yet, you should be stopped in venkman
13. if you don't see "Call Stack", View>Show/Hide>Call Stack
14. if you don't see "Loaded Scripts", View>Show/Hide>Loaded Scripts
14. "Continue" until you hit 'enhanceSpace'
15. in call stack, double click "init", "setTree", and "view"

You should crash when you double click "view".

bp-4f20a2fa-5761-4eff-bd74-6fb022100315

reproducible: sometimes (there's some other variable that i'm not able to control for, unfortunately)

the end result of this is that using a debugger without this fix is very likely to crash

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x0000002a
0x02309b71 in BindNameToSlot ()
(gdb) bt
#0  0x02309b71 in BindNameToSlot ()
#1  0x0230c696 in EmitNameOp ()
#2  0x0230f7b5 in js_EmitTree ()
#3  0x0230f781 in js_EmitTree ()
#4  0x0230ea03 in js_EmitTree ()
#5  0x02380fde in JSCompiler::compileScript ()
#6  0x02300317 in JS_EvaluateUCInStackFrame ()
#7  0x00bdf199 in jsd_EvaluateUCScriptInStackFrame ()
#8  0x00bdb5ea in JSD_AttemptUCScriptInStackFrame ()
#9  0x00be98fe in jsdStackFrame::Eval ()

I believe this specific crash is the one luke's fixing.

Comment 12

[Luke Wagner [:luke]]

(In reply to comment #11)
> I believe this specific crash is the one luke's fixing.

I would be interested to hear of any further problems with this fix that warrant the present progressive.

Comment 13

[timeless]

oh, that's an unpatched build, the comment was originally written in an effort to request that this patch be forced into 1.9.3a3 (explaining how it would happen, and what the stack signature would look like), until i discovered that even w/ your fix there are more problems along that path.

Comment 14

[Brendan Eich [:brendan]]

(In reply to comment #13)
> oh, that's an unpatched build, the comment was originally written in an effort
> to request that this patch be forced into 1.9.3a3 (explaining how it would
> happen, and what the stack signature would look like), until i discovered that
> even w/ your fix there are more problems along that path.

What you wrote was not clear. Let's file the other regressions and try to fix them too. In the mean time, this bug has a patch, and the patch reliably fixes some but not all of the crashes. Right?

/be

Comment 15

[timeless]

right

Comment 16

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/e7e05fc7d120