1006885 - Assertion failure: target->isInterpretedConstructor() || target->isNativeConstructor(), at jit/IonBuilder.cpp:5370

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 87c8f870e2b9 (run with --fuzzing-safe --ion-eager):


function checkConstruct(thing, buggy) {
    try {
        new thing();
    } catch (e) {}
}
var boundFunctionPrototype = Function.prototype.bind();
checkConstruct(boundFunctionPrototype, true);
var boundBuiltin = Math.sin.bind();
checkConstruct(boundBuiltin, true);

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, failed due to error (try manually).

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Need to check out the bisection issue here.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Turns out this is intermittent, and I can reproduce after retrying multiple times. Bisection underway. (hint: use the "range" interestingness test)

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/53649d31c8b4
user:        Hannes Verschore
date:        Mon Apr 28 13:44:13 2014 +0200
summary:     Bug 1001850 - IonMonkey: Remove the intermediate native call when calling a bound function, r=jandem

Hannes, is bug 1001850 a likely regressor?

Comment 6

[Hannes Verschore [:h4writer]]

Attachment: Patch

MCallKnown doesn't handle a constructing call to non native/scripted constructing call. (Which just throws.)

(We have the same code in getPolyCallTargets to handle this. This was just oversight.)

Note: this should normally not be intermediate (and also wasn't intermediate on my computer when using --ion-parallel-compile=off)!

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Hannes Verschore [:h4writer] from comment #6)
> Note: this should normally not be intermediate (and also wasn't intermediate
> on my computer when using --ion-parallel-compile=off)!

Darn, you're right!

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(and I take it that you mean "intermittent" instead of "intermediate")

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 8419977 [details] [diff] [review]
Patch

Review of attachment 8419977 [details] [diff] [review]:
-----------------------------------------------------------------

Please also add a testcase. r=me with that.

Comment 10

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e0cd88e8e636

I'll request uplift Monday.

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #8)
> (and I take it that you mean "intermittent" instead of "intermediate")
Yeah ;). Mijn engels is niet zo goed ;)

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Tracking because it is a critical bug

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e0cd88e8e636

Comment 13

[Hannes Verschore [:h4writer]]

Comment on attachment 8419977 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 1001850

User impact if declined: Wrong behaviour and possibly crashes?

Testing completed (on m-c, etc.): m-i for 2-3 days

Risk to taking this patch (and alternatives if risky): This reverts an edge-case to use the old behaviour again. That path has been tested elaborate. Not really risky.


String or IDL/UUID changes made by this patch: /

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/095e3b721d30