1236098 - Assertion failure: hasScript(), at js/src/jsfun.h:432

Status: RESOLVED DUPLICATE of bug 1312491

(Core :: JavaScript Engine, defect)

Description

[Bob Clary [:bc] (inactive)]

Attachment: stack

1. http://online.auctionnation.com/auction/591
2. Assertion failure: hasScript(), at /builds/slave/m-cen-lx-d-0000000000000000000/build/src/js/src/jsfun.h:432

Beta/44, Aurora/45, Nightly/46 Linux, OSX, Windows

per bug 1233343 comment 9, this is a distinct bug.

Doesn't reproduce with a saved version.

Comment 1

[Bob Clary [:bc] (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=62efa6d695d0f7e7e862035ce8c0798faa45ae50&tochange=3a05ea83f1dd7994bd88b8551c50aa92864b051d

=> bug 1199143

Comment 2

[Bob Clary [:bc] (inactive)]

I can not reproduce this with the url on today's Nightly build on Linux x86_64 nor with a build from 2015-12-30. There were 14 other urls where this assertion was seen with the latest instance being on 2016-02-22. While retesting these urls with today's builds automation reproduced the assertion. In particular, http://www.thebodyshop.fr/best-sellers/top-best-sellers.aspx#/top-best-sellers.aspx was reproducible and I can reproduce using a saved version though it may still have network accesses embedded. I'm attempting to reduce it a bit.

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Bob Clary [:bc:] from comment #2)
> I'm attempting to reduce it a bit.

Excellent, thanks. I'll wait for the reduced test case before investigating this.

Comment 4

[Bob Clary [:bc] (inactive)]

jandem: I've been trying to get it reduced, but it appears that after a point the assertion is either mixed up with a crash or becomes random. I've been unsuccessful in reducing the main html page to less than a 1000 lines so far. I wouldn't wait for me to get this reduced as it may take some time. Running lithium locally to reduce the page means I can't effectively use my workstation and I'll have to stop for a bit.

Comment 5

[Jan de Mooij [:jandem]]

I think I know what's going on. Let me try to write a reduced testcase...

Comment 6

[Jan de Mooij [:jandem]]

Below is a testcase that fails in the JS shell with --no-threads

When Ion inlines a lambda, it can end up creating a CallObject with a lazy JSFunction as callee. The code in AssertDynamicScopeMatchesStaticScope, though, assumes a CallObject has a non-lazy callee...

It's tricky. We should probably audit all callers of callObj->callee() to make sure they handle the lazy callee case correctly. I can't think of another fix.

function f() {
    var arr = [];
    for (var i=0; i<1600; i++) {
        arr.push(function() {
            var x = 0;
            if (j === 1500)
                (function() { assertEq(x, 0); })();
        });
    }
    for (var j=0; j<arr.length; j++)
        arr[j]();
}
f();

Comment 7

[Jan de Mooij [:jandem]]

The right thing to do here is probably to rename CallObject::callee() to calleeMaybeLazy(), and then audit all callers.

I'll try to get to this either this week or next week.

Comment 8

[Jan de Mooij [:jandem]]

Shu, do you have any thoughts on comment 6 and comment 7? I'm not really happy with it but I can't think of a simpler fix.

Comment 9

[Shu-yu Guo [:shu]]

(In reply to Jan de Mooij [:jandem] from comment #8)
> Shu, do you have any thoughts on comment 6 and comment 7? I'm not really
> happy with it but I can't think of a simpler fix.

In the case of AssertDynamicScopeMatchesStaticScope, the laziness of the callee doesn't *actually* matter. It was the easiest way to get at a JSScript. Even in the crashing test case from comment 6, note that the script is actually already delazified: i.funScript() is the JSScript* we want, gotten from the canonical JSFunction. The crash is because the closure JSFunction Ion put into the CallObject hasn't tried to delazify yet. If it did it'd hit the cache on the LazyScript:

(gdb) p scope->as<js::CallObject>().callee().lazyScript()->maybeScript()
$12 = (JSScript *) 0x7ffff506c300
(gdb) p i.funScript()
$13 = (JSScript *) 0x7ffff506c300

The right fix in this case is perhaps adding a hasSameScript() predicate that can see through the "LazyScript cache" case, or perhaps have nonLazyScript() automatically resolve if a JSScript is already cached.

Comment 10

[Carsten Book [:Tomcat]]

Attachment: new stack from bughunter

new url to reproduce is also http://www.thebodyshop.fr/maquillage/bronzant-honey-bronze.aspx#/bronzant-honey-bronze.aspx that show the same assertion failure

Comment 12

[Jan de Mooij [:jandem]]

This was fixed in bug 1312491.