Last Comment Bug 748613 - (CVE-2012-1939) [ESR] Assertion failure: [infer failure] Missing type pushed 0: float, at jsinfer.cpp:348
: [ESR] Assertion failure: [infer failure] Missing type pushed 0: float, at jsi...
: assertion, sec-critical, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 10 Branch
: x86_64 Linux
-- critical (vote)
: ---
Assigned To: Luke Wagner [:luke]
: Jason Orendorff [:jorendorff]
Depends on: 692274
Blocks: langfuzz
  Show dependency treegraph
Reported: 2012-04-24 17:32 PDT by Christian Holler (:decoder)
Modified: 2013-11-07 11:29 PST (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fix (1.92 KB, patch)
2012-04-25 12:21 PDT, Luke Wagner [:luke]
jorendorff: review+
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description User image Christian Holler (:decoder) 2012-04-24 17:32:49 PDT
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; }) {
        props[a] = true;
} 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 User image Luke Wagner [:luke] 2012-04-24 17:59:20 PDT
Bug 692274 is not a bug you want to backport.
Comment 2 User image Gary Kwong [:gkw] [:nth10sd] 2012-04-24 18:02:51 PDT
See bug 703857 and bug 709633.
Comment 3 User image Christian Holler (:decoder) 2012-04-24 18:16:32 PDT
If this is bug 703857, then it is sg:critical and we must fix it somehow on ESR.
Comment 4 User image Luke Wagner [:luke] 2012-04-24 18:22:34 PDT
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 User image Brian Hackett (:bhackett) 2012-04-25 06:12:56 PDT
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 User image Daniel Veditz [:dveditz] 2012-04-25 10:24:10 PDT
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 User image Luke Wagner [:luke] 2012-04-25 12:21:49 PDT
Created attachment 618388 [details] [diff] [review]

Alright, I found the underlying bug that got swept away by bug 692274.  Pretty straight-forward.
Comment 8 User image Jason Orendorff [:jorendorff] 2012-05-03 08:22:34 PDT
Comment on attachment 618388 [details] [diff] [review]

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

(function () {
    var a = "0";
    let (a = function() { ++a; }) {
    "" + a;
Comment 9 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-05-03 12:05:11 PDT
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 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-05-03 12:05:39 PDT
Please nominate for approval-mozilla-esr and we can approve/land closer to the release date.
Comment 11 User image Luke Wagner [:luke] 2012-05-03 12:50:42 PDT
Comment on attachment 618388 [details] [diff] [review]

[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 User image Luke Wagner [:luke] 2012-05-07 08:42:54 PDT
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 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-05-14 12:28:08 PDT
Comment on attachment 618388 [details] [diff] [review]

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 15 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-31 16:06:25 PDT
Verified fixed in Firefox 10.0.5esrpre 2012-05-31.

Note You need to log in before you can comment on or make changes to this bug.