Last Comment Bug 700127 - Assertion failure: !fe->isNotType(JSVAL_TYPE_DOUBLE), at methodjit/FrameState.cpp:820
: Assertion failure: !fe->isNotType(JSVAL_TYPE_DOUBLE), at methodjit/FrameState...
Status: RESOLVED FIXED
[sg:nse][qa!] js-triage-needed
: assertion, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
: -- critical (vote)
: ---
Assigned To: Brian Hackett (:bhackett)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: langfuzz
  Show dependency treegraph
 
Reported: 2011-11-06 07:19 PST by Christian Holler (:decoder)
Modified: 2013-01-19 13:55 PST (History)
8 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
wontfix
-
wontfix
+
verified
11+
verified
unaffected


Attachments
patch (1.30 KB, patch)
2011-11-09 11:29 PST, Brian Hackett (:bhackett)
dvander: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2011-11-06 07:19:08 PST
The following test asserts on mozilla-central revision 921e1db5cf11 (options -m -n -a):


function addThis() {}
function Integer( value ) {
  try {
    checkValue( value )
  } catch (e) {  }
}
function checkValue( value ) {
  if ( addThis() != value || value ) 
        throw value='foo'; 
  return value;
}
Integer( 3 );
Integer( NaN );


Marking this s-s because this might be a type confusion/misinterpretation (stepping through the assert also yields an assertion "isDouble" failing). Not sure if this is critical in any way though.
Comment 1 Brian Hackett (:bhackett) 2011-11-09 11:29:54 PST
Created attachment 573264 [details] [diff] [review]
patch

The SSA analysis treats certain opcodes as having no fallthrough, and computes type information wrt the resulting CFG.  The compiler would still generate code around two of these ops (JSOP_THROW and JSOP_RETRVAL) as if they had a fallthrough, and tripped this assert when generating code to transition from the state after a THROW to the state at the start of the next opcode.  The fix just makes the compiler behave consistently with the SSA analysis here.

I don't think this is critical --- when merging the code the compiler can trip asserts but should not crash, and the code it is generating is dead.  At the start of the next opcode it will have updated its internal state to reflect the type state at that op and will generate correct code afterwards.
Comment 2 Daniel Veditz [:dveditz] 2011-11-09 16:43:25 PST
Since the patch was in the Compiler is this unrelated to TI? or is it a regression from that rewriting? In particular, do we need to worry about this affecting Firefox 8 (potential firedrill?) and need to get a patch into Firefox 9 ASAP?

Or is this simply not a security bug at all? That seems to be what you're saying in comment 1.
Comment 3 Brian Hackett (:bhackett) 2011-11-09 16:47:36 PST
This change came in with the modifications TI made to JM, so it will not affect Firefox 8 or earlier.  It does affect 9+.  I don't think this is a security bug, nor do I think it can manifest in a crash or incorrect behavior.  That said, it still might be a good idea to take it for Firefox 9 and 10, just to be sure.
Comment 4 Brian Hackett (:bhackett) 2011-11-09 19:09:57 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6378aa32385
Comment 5 Brian Hackett (:bhackett) 2011-11-10 07:21:43 PST
https://hg.mozilla.org/mozilla-central/rev/d6378aa32385
Comment 6 Alex Keybl [:akeybl] 2012-02-13 17:24:36 PST
Based upon the m-c check-in date, this should actually be resolved for FF11.
Comment 7 Lukas Blakk [:lsblakk] use ?needinfo 2012-02-23 16:46:21 PST
[Triage comment]

This bug is being tracked for landing on ESR branch.  Please land patches on http://hg.mozilla.org/releases/mozilla-esr10/by Thursday March 1, 2012 in order to be ready for go-to-build on Friday March 2, 2012.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more information.
Comment 8 Daniel Veditz [:dveditz] 2012-02-23 17:21:59 PST
Doesn't sound like a security bug per comment 3, but given the small safe patch we should take it on the ESR just to be sure.
Comment 9 Gary Kwong [:gkw] [:nth10sd] 2012-02-24 13:54:57 PST
Landed on ESR:

http://hg.mozilla.org/releases/mozilla-esr10/rev/019cdcf42157
Comment 10 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-05 10:13:23 PST
Verified fixed in Firefox 10.0.3esr.
Comment 11 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-07 14:15:53 PST
No assertion reproduce using test in comment 0 in js-shell built from today's mozilla-beta.
Comment 12 Christian Holler (:decoder) 2013-01-19 13:55:24 PST
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929

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