Closed
Bug 472450
Opened 16 years ago
Closed 16 years ago
TM: Crash [@ JS_GetGlobalForObject] or [@ js_PutBlockObject] or [@ js_Interpret]
Categories
(Core :: JavaScript Engine, defect, P2)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla1.9.2a1
People
(Reporter: gkw, Assigned: mrbkap)
References
Details
(4 keywords, Whiteboard: fixed-in-tracemonkey)
Crash Data
Attachments
(1 file)
(deleted),
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
After bug 470388 was fixed, the following testcases remain unfixed and crash at the following locations in opt: ( from https://bugzilla.mozilla.org/show_bug.cgi?id=470388#c6 )
JS_GetGlobalForObject at 0x000000000000000c:
for each (let x in [function(){}, {}, {}, function(){}, function(){},
function(){}]) { ([<y/> for (y in x)]); }
======
js_PutBlockObject at null:
for each (let x in [function(){}, {}, {}, function(){}, function(){}]) { ([<y/>
for (y in x)]); }
======
Both testcases assert dbg at:
Assertion failure: StackBase(fp) + blockDepth == regs.sp, at ../jsinterp.cpp:6732
in TM.
Flags: blocking1.9.1?
Updated•16 years ago
|
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Comment 1•16 years ago
|
||
Bug 472941 has another testcase that triggers this assertion.
Should you need an additional test case -- http://apps.goonfleet.com/evetools/route.html and uncheck/recheck the "Use normal jump bridges" checkbox. Crashes on checking it, e.g.: http://crash-stats.mozilla.com/report/index/83ba79ed-8663-4074-8f85-3c1d32090111
Updated•16 years ago
|
Assignee: general → jim
Updated•16 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 3•16 years ago
|
||
These JS_GetGlobalForObject and js_PutBlockObject crashes in opt are occurring significantly often when running jsfunfuzz.
Comment 4•16 years ago
|
||
Okay. I'm actively working on this.
Reporter | ||
Comment 5•16 years ago
|
||
For automated testing, here's a testcase that's slightly different but has the same outcomes in shell:
({__proto__: #1=[#1#]})
for (var y = 0; y < 3; ++y) { for each (let z in ['', function(){}]) { let x = 1, c = []; } }
crashes opt TM at JS_GetGlobalForObject at 0x000000000000000c and also asserts at Assertion failure: StackBase(fp) + blockDepth == regs.sp, at ../jsinterp.cpp:6724
Reporter | ||
Comment 6•16 years ago
|
||
({__proto__: #1=[#1#]})
function f(){ eval("for (var y = 0; y < 1; ++y) { for each (let z in [null, function(){}, null, '', null, '', null]) { let x = 1, c = []; } }"); }
f();
crashes opt TM at js_Interpret at null and asserts identically: Assertion failure: StackBase(fp) + blockDepth == regs.sp, at ../jsinterp.cpp:6724
Summary: TM: Crash [@ JS_GetGlobalForObject] or [@ js_PutBlockObject] → TM: Crash [@ JS_GetGlobalForObject] or [@ js_PutBlockObject] or [@ js_Interpret]
Comment 7•16 years ago
|
||
jim, what's up here?
Comment 8•16 years ago
|
||
It seems like JITted code is not properly managing the scope chain. The code includes two nested lexical blocks. At some point after having left jitted code, JSOP_LEAVEBLOCK checks that the stack has the expected height, which it does (and the proper contents) --- but the scope chain contains only the outer block and the global block. The inner block that JSOP_LEAVEBLOCK expects to pop is already gone.
Comment 9•16 years ago
|
||
So, here's the disassembly:
// js> disfile('/home/jimb/mc/b/glob/t1.js')
// main:
// 00000: enterblock depth 0 {x: 0}
// 00003: anonfunobj (function () {})
// 00006: newinit 1 (JSProto_Object)
// 00008: endinit
// 00009: newinit 1 (JSProto_Object)
// 00011: endinit
// 00012: anonfunobj (function () {})
// 00015: anonfunobj (function () {})
// 00018: anonfunobj (function () {})
// 00021: newarray 6
// 00025: iter 3 (ENUMERATE|FOREACH)
// 00027: goto 67 (40)
// 00030: forlocal 0
// 00033: newinit 3 (JSProto_Array)
// 00035: enterblock depth 4 {y: 0}
// 00038: getlocal 0
// 00041: iter 1 (ENUMERATE)
// 00043: goto 57 (14)
// 00046: forlocal 4
// 00049: startxml
// 00050: string "<y/>"
// 00053: toxml
// 00054: arraypush 3
// 00057: nextiter
// 00058: ifne 46 (-12)
// 00061: enditer
// 00062: leaveblock 1
// 00065: endinit
// 00066: pop
// 00067: nextiter
// 00068: ifne 30 (-38)
// 00071: enditer
// 00072: leaveblock 1
// 00075: stop
The trace starts at 30, and we leave trace from a call to ObjectToIterator_tn, presumably from the JSOP_ITER at 41. However, although that iter instruction is in the scope of the inner lexical block (35--62), the VMSideExit record passed to LeaveTree has a null block, so LeaveTree doesn't reconstruct fp's block chain.
So, either we're not constructing the VMSideExit record properly, or we're being passed the wrong one.
Assignee | ||
Comment 11•16 years ago
|
||
Once js_GetScopeChain has been called, we can no longer properly restore the block chain.
This patch is slightly overzealous in that something like:
function f() { let (x) { (function(){}); } for (let i = 0; i < 5; ++i) print(i) } will not be traced anymore. If we did enough work to figure out that we could unset JSFRAME_POP_BLOCKS when the first let statment leaves, then we could avoid this. I doubt this is a huge problem, though.
Assignee: gal → mrbkap
Attachment #361844 -
Flags: review?(gal)
Assignee | ||
Comment 12•16 years ago
|
||
Sorry, that example should be:
var global;
function f() { let (x) { global = function(){}; } for (var i = 0; i < 5; ++i) { let k = i; print(k + '') } }
Updated•16 years ago
|
Attachment #361844 -
Flags: review?(gal) → review+
Comment 13•16 years ago
|
||
Comment on attachment 361844 [details] [diff] [review]
Fix
No slowdown on SS.
Assignee | ||
Comment 14•16 years ago
|
||
Whiteboard: fixed-in-tracemonkey
Comment 15•16 years ago
|
||
Is this fix enough? It seems possible with this patch to record a trace, then call js_GetScopeChain from the interpreter, then try to execute that trace (even though JSFRAME_POP_BLOCKS is set now).
Honestly I'm a little lost as to why we can't recover, but that's a separate issue.
Comment 16•16 years ago
|
||
(summarizing IRC conversations)
I'm not opposed to deciding we don't want to make this work properly right now. I think there are several bugs in the current code, and upvar might mess up some of the assumptions anyway.
Bug #1: JSFRAME_POP_BLOCKS might be unset at record time (when exit->block is captured) but set at trace-entry time. Fix: in snapshot, if JSFRAME_POP_BLOCKS is unset, go digging around in the script and find the appropriate Block object.
Bug #2: snapshot initializes exit->block using the current top frame. But for correctness, exit->block must refer to the entry frame. (Any frames on top of it can't have JSFRAME_POP_BLOCKS set, as that can't happen on trace--the frames simply don't exist to have js_GetScopeChain called on them.)
Bug #3: Likewise, LeaveTree needs to set entryFrame->blockChain, not cx->fp->blockChain after new frames are synthesized.
If there's a followup patch here, please mark all that code /*FIXME*/ or just delete it. It is (we hope) dead code at the moment.
Comment 17•16 years ago
|
||
OK, piling on, but Bug #4: I think neither lr nor innermost is the right VMSideExit to use to fix up entryFrame->blockChain in LeaveTree. Rather, we need to find the innermost VMSideExit that has the same entry frame as lr. So in code like this:
function f() {
while (loop1) {
let ...
while (loop2) {
let ...;
g();
}
}
}
function g() {
while (loop3) {
...
}
}
If we record all three loops, then start executing loop1 and bail out inside loop3, we have to use the exit->block from when we bailed out of loop2.
Assignee | ||
Updated•16 years ago
|
Assignee: mrbkap → jorendorff
Comment 18•16 years ago
|
||
Even with the patch pushed in comment #14 applied, the following will cause LeaveTree to set fp->blockChain to the compiler-allocated block object for the innermost block, even though fp->flags & JSFRAME_POP_BLOCKS is true and fp->scopeChain has mutable blocks on it. This leads eventually to:
Assertion failure: !js_IsActiveWithOrBlock(cx, fp->scopeChain, 0), at ../jsinterp.cpp:7150
for (let i = 1; i < 10; i++) {
let t=0;
for (let j = 1; j < 20; j++) {
t += j;
}
t = (function(){});
}
Assignee: jorendorff → mrbkap
Reporter | ||
Comment 19•16 years ago
|
||
(In reply to comment #18)
> Even with the patch pushed in comment #14 applied, the following will cause
> LeaveTree to set fp->blockChain to the compiler-allocated block object for the
> innermost block, even though fp->flags & JSFRAME_POP_BLOCKS is true and
> fp->scopeChain has mutable blocks on it. This leads eventually to:
>
> Assertion failure: !js_IsActiveWithOrBlock(cx, fp->scopeChain, 0), at
> ../jsinterp.cpp:7150
>
> for (let i = 1; i < 10; i++) {
> let t=0;
> for (let j = 1; j < 20; j++) {
> t += j;
> }
> t = (function(){});
> }
Is this testcase and assertion related to bug 472528, which Brendan indicates will be fixed by the heavyweight upvar patch?
Comment 20•16 years ago
|
||
Yes, it looks the same as that test case.
Comment 21•16 years ago
|
||
This crash is now being seen with today's Trunk build, when typing in the address bar (if JIT chrome is enabled). Crash signature is: [@ js_PutBlockObject ]: http://crash-stats.mozilla.com/report/index/eff0ea8e-5bdd-46fa-b465-b6de42090217
When will this be merged?
Comment 22•16 years ago
|
||
By the way, the crashes are happening on Windows.
Comment 24•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 26•16 years ago
|
||
Does this still have to land on 1.9.1? I just hit this on mozilla-1.9.1/20090218
Comment 29•16 years ago
|
||
Verified on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090218 Minefield/3.2a1pre. Awaiting fix to land on 1.9.1
Status: RESOLVED → VERIFIED
Comment 30•16 years ago
|
||
Can we get this in post-haste so we can respin the nightlies?
Comment 31•16 years ago
|
||
For the benefit of those having problems due to this bug: setting jit.chrome to false temporarily in about:config will workaround this problem.
One way to get to about:config without typing is to make a desktop shortcut to any website (just type one in), then change its URL in right-click/properties to about:config. You can then drag this onto your browser window. Another is to make and load a new bookmark for about:config with the bookmark organizer in the Tools menu.
Comment 32•16 years ago
|
||
typing about:config worked for me. only normal other urls caused the crash.
Comment 33•16 years ago
|
||
I am having this on OS X with Shiretoko nightly Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090218 Shiretoko/3.1b3pre Ubiquity/0.1.4
Turning off JIT for Chrome does indeed fix it for the time being. Will this fix be in the nightly for 1.9.1 released tomorrow? Interestingly enough, this bug didn't show up for me until the build I am running right now.
Reporter | ||
Updated•16 years ago
|
Flags: in-testsuite?
Updated•16 years ago
|
Target Milestone: --- → mozilla1.9.2a1
Comment 37•16 years ago
|
||
Keywords: fixed1.9.1
Reporter | ||
Comment 38•16 years ago
|
||
Prior to the checkin on 1.9.1, this also caused js_PutBlockObject crashes in TB:
http://crash-stats.mozilla.com/report/index/c9652198-2949-4dd9-93f1-d527e2090228
http://crash-stats.mozilla.com/report/index/f33ac5de-9bd0-4882-a8f8-7795a2090222
Comment 40•16 years ago
|
||
Verified fixed on 1.9.1 based on current crash reports. The latest reported crash happened with build 20090226034605.
Comment 41•16 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/1ab1141aa294
/cvsroot/mozilla/js/tests/js1_8/regress/regress-472450-01.js,v <-- regress-472450-01.js
initial revision: 1.1
/cvsroot/mozilla/js/tests/js1_8/regress/regress-472450-02.js,v <-- regress-472450-02.js
initial revision: 1.1
/cvsroot/mozilla/js/tests/js1_8/extensions/regress-472450-03.js,v <-- regress-472450-03.js
initial revision: 1.1
/cvsroot/mozilla/js/tests/js1_8/extensions/regress-472450-04.js,v <-- regress-472450-04.js
initial revision: 1.1
Flags: in-testsuite? → in-testsuite+
Updated•13 years ago
|
Crash Signature: [@ JS_GetGlobalForObject]
[@ js_PutBlockObject]
[@ js_Interpret]
You need to log in
before you can comment on or make changes to this bug.
Description
•