Closed Bug 472450 Opened 11 years ago Closed 11 years ago

TM: Crash [@ JS_GetGlobalForObject] or [@ js_PutBlockObject] or [@ js_Interpret]

Categories

(Core :: JavaScript Engine, defect, P2, critical)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: gkw, Assigned: mrbkap)

References

Details

(4 keywords, Whiteboard: fixed-in-tracemonkey)

Crash Data

Attachments

(1 file)

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?
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
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
Depends on: upvar2
Assignee: general → jim
Status: NEW → ASSIGNED
These JS_GetGlobalForObject and js_PutBlockObject crashes in opt are occurring significantly often when running jsfunfuzz.
Okay.  I'm actively working on this.
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
({__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]
jim, what's up here?
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.
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.
I have an idea. Let me try something.
Assignee: jim → gal
Attached patch FixSplinter Review
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)
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 + '') } }
Attachment #361844 - Flags: review?(gal) → review+
Comment on attachment 361844 [details] [diff] [review]
Fix

No slowdown on SS.
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.
(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.
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: mrbkap → jorendorff
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
(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?
Yes, it looks the same as that test case.
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?
By the way, the crashes are happening on Windows.
Duplicate of this bug: 478886
http://hg.mozilla.org/mozilla-central/rev/592836729d33
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Duplicate of this bug: 478983
Does this still have to land on 1.9.1? I just hit this on mozilla-1.9.1/20090218
Duplicate of this bug: 479049
Duplicate of this bug: 479053
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
Can we get this in post-haste so we can respin the nightlies?
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.
typing about:config worked for me. only normal other urls caused the crash.
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.
Duplicate of this bug: 479247
Duplicate of this bug: 479157
Flags: in-testsuite?
Target Milestone: --- → mozilla1.9.2a1
Duplicate of this bug: 480278
Duplicate of this bug: 481404
Verified fixed on 1.9.1 based on current crash reports. The latest reported crash happened with build 20090226034605.
OS: Mac OS X → All
Hardware: x86 → All
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+
Crash Signature: [@ JS_GetGlobalForObject] [@ js_PutBlockObject] [@ js_Interpret]
You need to log in before you can comment on or make changes to this bug.