Closed
Bug 847405
Opened 11 years ago
Closed 11 years ago
[jsdbg2] Assertion failure: caller->compartment() == caller->callee().compartment(), at builtin/Eval.cpp:416
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: decoder, Assigned: jimb)
Details
(Keywords: assertion, testcase, Whiteboard: [jsbugmon:update])
Attachments
(2 files)
|
586 bytes,
text/plain
|
Details | |
|
4.37 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
The following testcase asserts on mozilla-central revision f99a075a5bce (no options required):
eval("(function() { " + '\
var g = newGlobal("new-compartment");\
var dbg = new Debugger;\
var gw = dbg.addDebuggee(g);\
gw.evalInGlobalWithBindings("eval(\'var x = d1;\');var y = d2; x;",{}).return\
' + " })();");
|
Reporter | |
Updated•11 years ago
|
Whiteboard: [jsbugmon:update,bisect]
|
|
Reporter | |
Updated•11 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
|
|
Reporter | |
Comment 1•11 years ago
|
||
JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 107152:6852b4928efa user: Bill McCloskey date: Fri Sep 14 17:19:53 2012 -0700 summary: Bug 790865 - Add more compartment assertions (r=terrence) This iteration took 90.624 seconds to run.
|
|
Reporter | |
Comment 2•11 years ago
|
||
I guess this is a problem with the debugger and not with the assertions added, right?
Could you post a stack, Christian? Thanks.
|
||
Comment 4•11 years ago
|
||
Yes, this is a problem with the debugger. The evalInGlobal frame thinks it is a function frame. I think it clearly isn't.
|
|
Reporter | |
Updated•11 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:]
|
|
Reporter | |
Comment 5•11 years ago
|
||
JSBugMon: Cannot process bug: Unknown exception (check manually)
|
|
Reporter | |
Updated•11 years ago
|
Whiteboard: [jsbugmon:] → [jsbugmon:update]
|
|
Reporter | |
Comment 6•11 years ago
|
||
(In reply to Bill McCloskey (:billm) from comment #3) > Could you post a stack, Christian? Thanks. Stack from revision 55f9e3e3dae7 (debug+opt): Program received signal SIGSEGV, Segmentation fault. scopeChain (this=<optimized out>) at ../vm/Stack-inl.h:47 47 JS_ASSERT_IF(!(flags_ & HAS_SCOPECHAIN), isFunctionFrame()); (gdb) bt #0 scopeChain (this=<optimized out>) at ../vm/Stack-inl.h:47 #1 scopeChain (this=<optimized out>) at ../vm/Stack-inl.h:635 #2 scopeChain (this=<optimized out>) at /srv/repos/mozilla-central/js/src/builtin/Eval.cpp:407 #3 js::DirectEval (cx=0xf38790, args=...) at /srv/repos/mozilla-central/js/src/builtin/Eval.cpp:413 #4 0x000000000053bc5c in js::Interpret (cx=<optimized out>, entryFrame=0x7ffff62f2208, interpMode=js::JSINTERP_NORMAL, useNewType=<optimized out>) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:2340 #5 0x0000000000545b93 in js::RunScript (cx=0xf38790, fp=0x7ffff62f2208) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:365 #6 0x0000000000547add in js::ExecuteKernel (cx=0xf38790, script=..., scopeChainArg=..., thisv=..., type=<optimized out>, evalInFrame=..., result=0x7fffffffb8d0) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:553 #7 0x00000000006667ab in js::EvaluateInEnv (cx=0xf38790, env=(JSObject * const) 0x7ffff604f080 [object Object], thisv=$jsval((JSObject *) 0x7ffff604d060 [object global] delegate), frame=..., chars=..., length=<optimized out>, filename=0xb68cfd "debugger eval code", lineno=1, rval=JSVAL_VOID) at /srv/repos/mozilla-central/js/src/vm/Debugger.cpp:3892 #8 0x0000000000680de3 in DebuggerGenericEval (cx=0xf38790, fullMethodName=<optimized out>, code=..., bindings=0x7ffff62f21f0, vp=$jsval((JSObject *) 0x7ffff603cf80 [object Function "evalInGlobalWithBindings"]), dbg= 0xfe1d20, scope=(JSObject * const) 0x7ffff604d060 [object global] delegate, iter=0x0) at /srv/repos/mozilla-central/js/src/vm/Debugger.cpp:3983 #9 0x000000000068107d in DebuggerObject_evalInGlobalWithBindings (cx=0xf38790, argc=<optimized out>, vp=<optimized out>) at /srv/repos/mozilla-central/js/src/vm/Debugger.cpp:4763 #10 0x0000000000545fa1 in CallJSNative (args=..., native=<optimized out>, cx=0xf38790) at ../jscntxtinlines.h:338 #11 js::InvokeKernel (cx=0xf38790, args=..., construct=js::NO_CONSTRUCT) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:408 #12 0x0000000000536e0d in js::Interpret (cx=<optimized out>, entryFrame=0x7ffff62f20d0, interpMode=js::JSINTERP_NORMAL, useNewType=<optimized out>) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:2393 #13 0x0000000000545b93 in js::RunScript (cx=0xf38790, fp=0x7ffff62f20d0) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:365 #14 0x0000000000547add in js::ExecuteKernel (cx=0xf38790, script=..., scopeChainArg=..., thisv=..., type=<optimized out>, evalInFrame=..., result=0x7ffff62f20a8) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:553 #15 0x0000000000740689 in EvalKernel (cx=0xf38790, args=..., evalType=DIRECT_EVAL, caller=..., scopeobj=(JSObject * const) 0x7ffff6035060 [object global] delegate, pc=0xf91f75 "{") at /srv/repos/mozilla-central/js/src/builtin/Eval.cpp:303 #16 0x0000000000741174 in js::DirectEval (cx=0xf38790, args=...) at /srv/repos/mozilla-central/js/src/builtin/Eval.cpp:422 #17 0x000000000053bc5c in js::Interpret (cx=<optimized out>, entryFrame=0x7ffff62f2038, interpMode=js::JSINTERP_NORMAL, useNewType=<optimized out>) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:2340 #18 0x0000000000545b93 in js::RunScript (cx=0xf38790, fp=0x7ffff62f2038) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:365 #19 0x000000000054816e in ExecuteKernel (result=0x0, evalInFrame=..., thisv=..., scopeChainArg=..., script=0x7ffff6039160, cx=0xf38790, type=<optimized out>) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:553 #20 js::Execute (cx=0xf38790, script=0x7ffff6039160, scopeChainArg=..., rval=0x0) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:593 #21 0x000000000043d335 in JS_ExecuteScript (cx=0xf38790, objArg=(JSObject *) 0x7ffff6035060 [object global] delegate, scriptArg=<optimized out>, rval=0x0) at /srv/repos/mozilla-central/js/src/jsapi.cpp:5615 #22 0x00000000004125de in Process (cx=0xf38790, obj_=<optimized out>, filename=<optimized out>, forceTTY=<optimized out>) at /srv/repos/mozilla-central/js/src/shell/js.cpp:469 #23 0x000000000041d86f in ProcessArgs (op=0x7fffffffdcf0, obj_=(JSObject *) 0x7ffff6035060 [object global] delegate, cx=0xf38790) at /srv/repos/mozilla-central/js/src/shell/js.cpp:5102 #24 Shell (cx=0xf38790, op=0x7fffffffdcf0, envp=<optimized out>) at /srv/repos/mozilla-central/js/src/shell/js.cpp:5139 #25 0x0000000000404c61 in main (argc=<optimized out>, argv=<optimized out>, envp=0x7fffffffdee0) at /srv/repos/mozilla-central/js/src/shell/js.cpp:5369
|
|
Reporter | |
Comment 7•11 years ago
|
||
|
|
||
Comment 8•11 years ago
|
||
(function () {
var g = newGlobal("new-compartment");
var dbg = new Debugger;
var gw = dbg.addDebuggee(g);
gw.evalInGlobalWithBindings("eval('Math')",{}).return
})();
|
Assignee | |
Updated•11 years ago
|
Assignee: general → jimb
|
|
Assignee | |
Comment 9•11 years ago
|
||
Both the assertion and the ExecuteType computation that we were suspicious of were added in bug 800586.
|
|
Assignee | |
Comment 10•11 years ago
|
||
Okay, so I think I know what's going on here:
First, the DEBUG flag is really only essential for eval-in-frame; but it does enable consistency checks of various sorts, and tells IonMonkey that the frame is probably not worth compiling. All the frames Debugger pushes should be DEBUG frames.
Next, the FUNCTION flag should be set on direct eval frames when the call is in a function body, and on eval-in-frame frames if the frame we're evaluating in is itself a FUNCTION frame. The frame machinery ensures our eval-in-frame frames inherit the FUNCTION flag as needed, even though we don't set it in EvaluateInEnv.
All the frames Debugger creates for eval-in-frame or eval-in-global should have the EVAL flag. This seems obvious.
Finally, the GLOBAL flag seems to be the opposite of the FUNCTION flag. If I evaluate the following at the top level:
eval('{ let x = 4; dumpstack(); }');
then that eval's frame has the GLOBAL and EVAL flags set, even though the scope object is not a global object.
So, I think evalInGlobalWithBindings should always have the GLOBAL flag set. As we suspected, this line:
ExecuteType type = !frame && env->is<GlobalObject>() ? EXECUTE_DEBUG_GLOBAL : EXECUTE_DEBUG;
should not be checking the type of env. Even though we've extended the scope chain with a bindings object, it's still a GLOBAL frame.|
|
Assignee | |
Comment 11•11 years ago
|
||
Patch, with regression test and comments.
Attachment #801067 -
Flags: superreview?(luke)
Attachment #801067 -
Flags: review?(jorendorff)
|
|
Assignee | |
Comment 12•11 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=a5bcc2ddc8b5
|
|
Assignee | |
Comment 13•11 years ago
|
||
(Try run looks good.)
|
||
Comment 14•11 years ago
|
||
Comment on attachment 801067 [details] [diff] [review] Ensure that evalInGlobal never creates frames with the 'FUNCTION' flag set. Review of attachment 801067 [details] [diff] [review]: ----------------------------------------------------------------- Nice job tracking this down. IIUC, the root cause of the bug is that, if the GLOBAL bit is not set going into initExecuteFrame and !evalInFramePrev, we just copy the flags from the innermost stack frame (which is completely unrelated in the case of evalInGlobal). Given that, your fix seems to be the right one. I filed bug 914173 to do some refactoring in this area to make it all more sensible; feel free to steal, otherwise hopefully Jan or I will get to it before long.
|
|
||
Comment 15•11 years ago
|
||
Comment on attachment 801067 [details] [diff] [review] Ensure that evalInGlobal never creates frames with the 'FUNCTION' flag set. Review of attachment 801067 [details] [diff] [review]: ----------------------------------------------------------------- A+++++ ::: js/src/vm/Stack.h @@ +247,5 @@ > + * - If evalInFramePrev_ is set, then this frame was created for an > + * "eval in frame" call, which can push a successor to any live > + * frame; so its logical "prev" frame is not necessarily the > + * previous frame in memory. Iteration should treat > + * evalInFramePrev_ as this frame's previous frame. Wait, is that last thing right? Why should iteration skip frames that exist on the stack and are part of the current frame's continuation? I think it shouldn't. We should know better than to distort the stack chain to make it look like the scope chain. They're fairly independent. @@ +320,5 @@ > + > + /* > + * For an eval-in-frame DEBUGGER frame, the frame in whose scope we're > + * evaluating code. Iteration treats this as our previous frame. > + */ Thanks so much for improving the comments here. If evalInFramePrev_ is uninitialized (i.e. it's garbage bits) for frames without the DEBUGGER flag set, please say so!
Attachment #801067 -
Flags: superreview?(luke)
Attachment #801067 -
Flags: review?(jorendorff)
Attachment #801067 -
Flags: review+
|
|
||
Comment 16•11 years ago
|
||
(In reply to Jason Orendorff [:jorendorff] from comment #15) > Wait, is that last thing right? Why should iteration skip frames that exist > on the stack and are part of the current frame's continuation? I think it > shouldn't. We should know better than to distort the stack chain to make it > look like the scope chain. They're fairly independent. In case it isn't clear, this is, if anything, fodder for a follow-up bug. Feel free to land this patch without addressing it.
|
|
Assignee | |
Comment 17•11 years ago
|
||
Thanks! https://hg.mozilla.org/integration/mozilla-inbound/rev/3ffdc887c562
|
||
Comment 18•11 years ago
|
||
(In reply to Jason Orendorff [:jorendorff] from comment #15) > Wait, is that last thing right? Why should iteration skip frames that exist > on the stack and are part of the current frame's continuation? I think it > shouldn't. We should know better than to distort the stack chain to make it > look like the scope chain. They're fairly independent. So if the stack looks like this, old to new: - frame 1 - frame 2 - frame 3 And we perform an evalInFrame("print(Error().stack)") in frame 2, it should print frames 1-3? When I refactored a lot of vm/Stack I kept the current behavior (hide frame 3), but if we decide we don't need evalInFramePrev that could simplify ScriptFrameIter quite a bit and makes it easier to baseline-compile debugger frames...
|
|
Assignee | |
Comment 19•11 years ago
|
||
(In reply to Jason Orendorff [:jorendorff] from comment #15) > Wait, is that last thing right? Why should iteration skip frames that exist > on the stack and are part of the current frame's continuation? I think it > shouldn't. We should know better than to distort the stack chain to make it > look like the scope chain. They're fairly independent. Yes, Debugger.Frame.prototype.older does indeed follow the evalInFrame parent. I'm not sure this is a dynamic / static chain confusion, but it does seem like confusion of some sort. Filed as bug 914286, with a test case. > If evalInFramePrev_ is uninitialized (i.e. it's garbage bits) for frames > without the DEBUGGER flag set, please say so! I'll check on that.
|
|
Assignee | |
Comment 20•11 years ago
|
||
(Replying to jandem in bug 914286.)
|
|
Assignee | |
Updated•11 years ago
|
Status: NEW → ASSIGNED
|
|
Assignee | |
Updated•11 years ago
|
Flags: in-testsuite+
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla26
|
|
Reporter | |
Updated•11 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
|
|
Reporter | |
Comment 21•11 years ago
|
||
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 680c89c76100).
|
|
||
Comment 22•11 years ago
|
||
This landed on m-c: http://hg.mozilla.org/mozilla-central/rev/3ffdc887c562
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update]
You need to log in
before you can comment on or make changes to this bug.
Description
•