Closed Bug 808481 Opened 7 years ago Closed 7 years ago

"Assertion failure: lifetime->entry == uint32_t(entryTarget - outerScript->code),"

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla20
Tracking Status
firefox16 --- wontfix
firefox17 + wontfix
firefox18 + fixed
firefox19 + fixed
firefox20 + fixed
firefox-esr10 --- unaffected
firefox-esr17 18+ fixed

People

(Reporter: gkw, Assigned: bhackett)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [jsbugmon:update][adv-main18+][adv-esr17+])

Attachments

(2 files)

Attached file stack
mjitChunkLimit(35)
eval("\
    0;\
    L: switch(5) {\
        case 1: c:break\
    }\
    while(a);\
")

asserts js debug shell on m-c changeset 2937fd8e35a1 with -a at Assertion failure: lifetime->entry == uint32_t(entryTarget - outerScript->code),

Assuming related to bug 747226. s-s and assuming sec-critical because similar previously-found assertions were also s-s.


===

Due to skipped revisions, the first bad revision could be any of:
changeset:   106120:300ac3d58291
user:        David Anderson
date:        Thu Apr 19 15:02:11 2012 -0700
summary:     Enable JITs and type inference by default, and give Ion first chance (bug 724751, r=jandem).

changeset:   106121:de015aff650d
user:        David Anderson
date:        Thu Apr 19 15:02:47 2012 -0700
summary:     Enable IonMonkey in the browser, and introduce an about:config option to pref it off (bug 745390, r=dmandelin).

changeset:   106122:9e64f779b611
user:        David Anderson
date:        Thu Apr 19 16:57:58 2012 -0700
summary:     Disable TypeScript::SetScope (bug 747226, r=bhackett).

changeset:   106123:bc1833f2111e
user:        Christian Holler
date:        Mon Apr 23 17:32:32 2012 +0200
summary:     Bug 747902 - Add --ion, -n and -m flags back for compatibility
Likely a WONTFIX for 16 since it's too late, nominating for the others.
David - assigning to you since you're likely the culprit if law of averages are to be believed.  Is there an obvious backout here that could return us to a known state? We're going to build on beta 5 tomorrow so the clock's ticking the 17 release.
Assignee: general → dvander
jandem / bhackett were also working on similar assertions before - see bug 781859 and its various dupes.
Wontfixing for 17, if someone has a low-risk fix that can be verified and uplifted before Monday's last beta going to build, please nominate.
(In reply to Lukas Blakk [:lsblakk] from comment #4)
> Wontfixing for 17, if someone has a low-risk fix that can be verified and
> uplifted before Monday's last beta going to build, please nominate.

If it's wontfix for 17, it should at least be tracking for 17 esr.
Just found this as well, assuming it's the same thing (32 bit debug build, also only reproduces with -a):

mjitChunkLimit(10);
for (var i = 0; i < 20; i++)
function f0(p0) {
    var v0;
    if (i)
        'bar';
    while (v0 & v0)
        break;
}
f0(0);
David - thoughts on this bug?
Sorry, I missed the earlier ping on this bug. Law of average blaming me is definitely fair :) but this one is deep in JM+TI territory. There's no obvious backout. 

Brian or Jan, could you take a look?
Assignee: dvander → general
Assigning to Jan, who said he'd talk to Brian about this as a next step.
Assignee: general → jdemooij
Attached patch patchSplinter Review
Bug in the state machine used for picking chunk boundaries.  This code doesn't want to start new chunks after jumps which target their fallthrough opcode, but it also wants to start new chunks at the goto before a for or while loop.  When these goals conflict it ends up using a chunk boundary at the first opcode in the loop, effectively treating it like a do-while loop during compilation.  This might cause problems with the LoopState analysis, not sure.  The attached patch fixes this situation by making the state machine even more horrible.
Attachment #689200 - Flags: review?(dvander)
Attachment #689200 - Flags: review?(dvander) → review+
(just a drive-by comment that the patch will likely need sec-approval since the issue is present on the branches)
Flags: needinfo?(bhackett1024)
It will need sec-approval because it is a sec-critical bug, per policy.
Attachment #689200 - Flags: sec-approval?
There's a questionaire in the comments which appears, that one needs to fill out when nominating for sec-approval:

[Security approval request comment]
How easily can the security issue be deduced from the patch?

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Which older supported branches are affected by this flaw?

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

How likely is this patch to cause regressions; how much testing does it need?
Gary, when they set sec-approval to ?, the comment field gets auto-populated with that.

To be clear, if this *didn't* affect branches (We'd never shipped it to anything outside trunk), this could just be checked in without approval.
[Security approval request comment]
How easily can the security issue be deduced from the patch?

Not at all.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No.

Which older supported branches are affected by this flaw?

Back to Fx 12

If not all supported branches, which bug introduced the flaw?

bug 706914

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Backporting is trivial.

How likely is this patch to cause regressions; how much testing does it need?

Not likely at all.
Flags: needinfo?(bhackett1024)
Comment on attachment 689200 [details] [diff] [review]
patch

Looks good. Sec-approval+. We should have branch patches made.
Attachment #689200 - Flags: sec-approval? → sec-approval+
Please nominate for branch approval as soon as possible (aurora/beta/esr17). Thanks!
Assignee: jdemooij → bhackett1024
Comment on attachment 689200 [details] [diff] [review]
patch

[Approval Request Comment]
See comment 15.
Attachment #689200 - Flags: approval-mozilla-esr17?
Attachment #689200 - Flags: approval-mozilla-beta?
Attachment #689200 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/ffcf9d812533
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Attachment #689200 - Flags: approval-mozilla-esr17?
Attachment #689200 - Flags: approval-mozilla-esr17+
Attachment #689200 - Flags: approval-mozilla-beta?
Attachment #689200 - Flags: approval-mozilla-beta+
Attachment #689200 - Flags: approval-mozilla-aurora?
Attachment #689200 - Flags: approval-mozilla-aurora+
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main18+][adv-esr17+]
Group: core-security
You need to log in before you can comment on or make changes to this bug.