Closed Bug 1006885 Opened 7 years ago Closed 7 years ago

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


(Core :: JavaScript Engine: JIT, defect)

Not set



Tracking Status
firefox31 + fixed
firefox32 + fixed


(Reporter: decoder, Assigned: h4writer)



(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])


(2 files)

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);
Whiteboard: [jsbugmon:update,bisect]
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, failed due to error (try manually).
Need to check out the bisection issue here.
Flags: needinfo?(gary)
Turns out this is intermittent, and I can reproduce after retrying multiple times. Bisection underway. (hint: use the "range" interestingness test)
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
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?
Blocks: 1001850
Flags: needinfo?(gary) → needinfo?(hv1989)
Keywords: regression
Attached patch PatchSplinter Review
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)!
Assignee: nobody → hv1989
Attachment #8419977 - Flags: review?(jdemooij)
Flags: needinfo?(hv1989)
(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!
(and I take it that you mean "intermittent" instead of "intermediate")
Comment on attachment 8419977 [details] [diff] [review]

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

Please also add a testcase. r=me with that.
Attachment #8419977 - Flags: review?(jdemooij) → review+

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 ;)
Tracking because it is a critical bug
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment on attachment 8419977 [details] [diff] [review]

[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: /
Attachment #8419977 - Flags: approval-mozilla-aurora?
Attachment #8419977 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.