Closed Bug 915171 Opened 6 years ago Closed 6 years ago

Assertion failure: barrier->type() == MIRType_Object || barrier->type() == MIRType_Value, at jit/IonBuilder.cpp:3762

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: decoder, Assigned: h4writer)

References

(Blocks 2 open bugs)

Details

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

Attachments

(2 files, 2 obsolete files)

The following testcase asserts on mozilla-central revision 9e9f74116749 (threadsafe build, run with --fuzzing-safe --thread-count=2 --ion-parallel-compile=on --ion-eager --ion-regalloc=backtracking --ion-compile-try-catch):


function exitFunc() {}
  exitFunc();
function C() {
  new exitFunc();
}
new C;
Forgot to strip the options variable. This also reproduces just with --ion-eager and it probably also doesn't require a threadsafe build. Marked this s-s because that kind of assertion has been s-s previously.
Whiteboard: [jsbugmon:update,bisect]
IonBuilder type asserts generally seem bad, so I'm going to mark it sec-high.  Adjust as appropriate.
Keywords: sec-high
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/94d54fe84c77
user:        Hannes Verschore
date:        Fri Sep 06 15:10:54 2013 +0200
summary:     Bug 909717: IonBuilder: Introduce typed typebarriers, r=jandem

This iteration took 364.332 seconds to run.
Hannes, is bug 909717 likely related?
Attachment #803021 - Attachment is obsolete: true
Yes, it is a assertion I added. I guessed it before I saw the bisect ;).
Assignee: general → hv1989
Attached patch bug915171 (obsolete) — Splinter Review
Thistype can also return Undefined. So that's why we are failing here. I even think the assert can get removed, since it doesn't check something actually. 

Pick a random type and there shouldn't be a problem if it flows there. It's just asserting that ThisType only gives these types.

Actually looking back, there was a fault in the original code :0
Attachment #803705 - Flags: review?(jdemooij)
Bug may get opened. Not security related. Just bogus assert.
Keywords: sec-high
Thanks Hannes.
Group: core-security
Yeah we should probably just remove the assert. It's also possible for ThisTypes to be int32 for instance:

---
function f() {}

for (var i=0; i<100; i++)
    f.call(3);
---

Function #2 test.js:2 "f"
    return: missing
    this: int
#2:00000:   2  stop
Attached patch bug915171Splinter Review
Oh right. Didn't think about that case.
Attachment #803705 - Attachment is obsolete: true
Attachment #803705 - Flags: review?(jdemooij)
Attachment #803886 - Flags: review?(jdemooij)
Attachment #803886 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/dcede2379902
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Blocks: 909717
You need to log in before you can comment on or make changes to this bug.