After updating my local debug build around noon pacific time today from mozilla-central, I crashed twice today with: Assertion failure: hasScript(), at /home/dbaron/builds/ssd/mozilla-central/mozilla/js/src/jsfun.h:337 The second time was on Google Maps (classic). I forgot what the first was on; it was during a meeting. I presume this is a regression from bug 886193.

(crashed a third time while restoring my session, too)

Looks like I misunderstood the circumstances under which a function can be the scope object for another function. I thought that could only happen if the outer function contained the inner one as a child - in which case numInnerFunctions should be > 0. Anyway, since we have a context just slightly higher up the stack here, this patch funnels it through to GenerateScopeChainGuard and makes that use fun->getOrCreateScript instead of nonLazyScript. r? instead of just landing because of the error handling stuff.

Comment on attachment 8358788 [details] [diff] [review] Make sure functions are non-lazy when generating Ion scope chain guards Review of attachment 8358788 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/IonCaches.cpp @@ +3987,5 @@ > // variables). > CallObject *callObj = &scopeObj->as<CallObject>(); > if (!callObj->isForEval()) { > JSFunction *fun = &callObj->callee(); > + JSScript *script = fun->getOrCreateScript(cx); getOrCreateScript can GC I think, so you'd have to root scopeObj and probably other pointers on the stack. However, we're only interested in script->funHasExtensibleScope() for a minor optimization (avoid a shape guard), so I think delazifying the whole script is probably not worth it and we can just check hasScript() before calling nonLazyScript(). It seems an edge case because: (1) we don't relazify if the compartment is active, so it's unlikely to hit any of our major benchmarks (2) apparently none of our tests hit this case (3) *NAME (not *GNAME) ops/ICs aren't used much since the aliased var ops (think slow stuff like eval, with etc).

Good points!

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/b3ccaf008518

With the above patch, I crashed again on Google Maps.

I don't understand this crash at all. We're executing in the JIT, and have a JSContext in NameIC::attachReadSlot, so we're on the main thread. According to the documentation for compileLock[1], this means we can always read compilation data. Jandem, can you shed some light on this and tell me how to fix it? [1]: http://mxr.mozilla.org/mozilla-central/source/js/src/vm/Runtime.h#816

(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #6) > With the above patch, I crashed again on Google Maps. Jandem pointed out that, from the gdb output, it very much looks like the patch isn't applied. With the patch, the line number in frame #7 would have to be different, at the very least. dbaron, can you check whether something went wrong with applying the patch (or updating from m-i, if that's what you did)?

(In reply to Till Schneidereit [:till] from comment #8) > (In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #6) > > With the above patch, I crashed again on Google Maps. > > Jandem pointed out that, from the gdb output, it very much looks like the > patch isn't applied. With the patch, the line number in frame #7 would have > to be different, at the very least. > > dbaron, can you check whether something went wrong with applying the patch > (or updating from m-i, if that's what you did)? Ah, I did a one-off import of the patch, but I think I might have then pulled and built again, and forgotten that I'd done that. I'll update again now that it's on m-c.

Yeah, sorry, seems to be fixed now.