748613 - (CVE-2012-1939) [ESR] Assertion failure: [infer failure] Missing type pushed 0: float, at jsinfer.cpp:348

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following test asserts on mozilla-esr10 revision 65efc6ce102f (options -m -n -a):


try {
Object.prototype.nameSETS = 0;
function ownProperties() {
    var props = {};
    var r = function () {};
    for (var a in r) {
        let (a = function() { for (var r=0;r<6;++r) ++a; }) {
            a();
        }
        props[a] = true;
    }
}
ownProperties();
} catch(exc1) {}



According to bisection, the issue might have been fixed on mozilla-central by this revision:

The first good revision is:
changeset:   83258:9272bb82eeba
user:        Luke Wagner
date:        Fri Oct 07 12:02:50 2011 -0700
summary:     Bug 692274, part 3 - Remove JSOP_BLOCKCHAIN and JSOP_NULLBLOCKCHAIN, which produces incorrect let scoping until the next patch (r=jorendorff)


Marking s-s because infer failures can be security related.

Comment 1

[Luke Wagner [:luke]]

Bug 692274 is not a bug you want to backport.

Comment 2

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

See bug 703857 and bug 709633.

Comment 3

[Christian Holler (:decoder)]

If this is bug 703857, then it is sg:critical and we must fix it somehow on ESR.

Comment 4

[Luke Wagner [:luke]]

I don't know what the actual error is.  Bug 703857 comment 1 seems to.  Brian: is there some coarse-grained de-optimization we can apply here like "assume all let vars are closed" or something?

Comment 5

[Brian Hackett [Laid off!]]

This is the same problem (and testcase) as bug 703857, the upvar analysis around let variables is broken pre-692274.  The problem is with the 'var a' in the outer function being mislabelled by the frontend as not closed.  TI already treats let variables as totally unknown, so there isn't an easy deoptimization I can see and an actual fix (or 692274 backport) is needed.

Comment 6

[Daniel Veditz [:dveditz]]

I agree w/Luke's comment 1. -- any fixes short of backporting the whole 300K patch?

At first I was hopeful given the Oct date of the bisect in comment 0 since that would be in Firefox 10/ESR. But that's the changeset date, the m-c push date was actually late December (Firefox 12).

Comment 7

[Luke Wagner [:luke]]

Attachment: fix

Alright, I found the underlying bug that got swept away by bug 692274.  Pretty straight-forward.

Comment 8

[Jason Orendorff [:jorendorff]]

Comment on attachment 618388 [details] [diff] [review]
fix

Nice work. Want to add the test? Slightly more minimal:

(function () {
    var a = "0";
    let (a = function() { ++a; }) {
        a();
    }
    "" + a;
})();

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking for ESR shipping with Firefox 13, we won't chemspill the current ESR release already out there, which is affected, so this is the best we can do.

Comment 10

[Lukas Blakk [:lsblakk] use ?needinfo]

Please nominate for approval-mozilla-esr and we can approve/land closer to the release date.

Comment 11

[Luke Wagner [:luke]]

Comment on attachment 618388 [details] [diff] [review]
fix

[Approval Request Comment]
Fix Landed on Version: not on trunk
Risk to taking this patch (and alternatives if risky): less than backporting the giant patch, but not zero

Comment 12

[Luke Wagner [:luke]]

Since this is an ESR-only landing, should I be including a test-case that points directly to the bug (as suggested in comment 8)?

Comment 13

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 618388 [details] [diff] [review]
fix

approved for landing to esr10, if it's not too much to also land the test case, I'm always a fan of tests :)

Comment 14

[Luke Wagner [:luke]]

https://hg.mozilla.org/releases/mozilla-esr10/rev/de7fee495798

Comment 15

[u279076]

Verified fixed in Firefox 10.0.5esrpre 2012-05-31.