Closed Bug 700127 Opened 8 years ago Closed 8 years ago

Assertion failure: !fe->isNotType(JSVAL_TYPE_DOUBLE), at methodjit/FrameState.cpp:820

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
firefox9 - wontfix
firefox10 - wontfix
firefox11 + verified
firefox-esr10 11+ verified
status1.9.2 --- unaffected

People

(Reporter: decoder, Assigned: bhackett1024)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [sg:nse][qa!] js-triage-needed)

Attachments

(1 file)

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.
Attached patch patchSplinter Review
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.
Assignee: general → bhackett1024
Attachment #573264 - Flags: review?(dvander)
Attachment #573264 - Flags: review?(dvander) → review+
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.
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.
https://hg.mozilla.org/mozilla-central/rev/d6378aa32385
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: js-triage-needed → js-triage-needed [qa+]
Based upon the m-c check-in date, this should actually be resolved for FF11.
[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.
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.
Whiteboard: js-triage-needed [qa+] → [sg:nse]js-triage-needed [qa+]
Verified fixed in Firefox 10.0.3esr.
No assertion reproduce using test in comment 0 in js-shell built from today's mozilla-beta.
Whiteboard: [sg:nse]js-triage-needed [qa+] → [sg:nse][qa!] js-triage-needed
Group: core-security
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.