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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: decoder, Assigned: bhackett)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
x86_64
Linux
assertion, regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox9- wontfix, firefox10- wontfix, firefox11+ verified, firefox-esr1011+ verified, status1.9.2 unaffected)

Details

(Whiteboard: [sg:nse][qa!] js-triage-needed)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
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.
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.
status-firefox10: --- → affected
status-firefox11: --- → affected
status-firefox9: --- → affected
tracking-firefox10: --- → +
tracking-firefox11: --- → +
tracking-firefox9: --- → +
Keywords: regression
(Assignee)

Comment 3

6 years ago
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.
(Assignee)

Comment 4

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6378aa32385
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/mozilla-central/rev/d6378aa32385
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: js-triage-needed → js-triage-needed [qa+]

Comment 6

5 years ago
Based upon the m-c check-in date, this should actually be resolved for FF11.
status-firefox11: affected → fixed
[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.
tracking-firefox-esr10: --- → 11+
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.
status1.9.2: --- → unaffected
status-firefox-esr10: --- → affected
status-firefox10: affected → wontfix
status-firefox9: affected → wontfix
tracking-firefox10: + → -
tracking-firefox9: + → -
Whiteboard: js-triage-needed [qa+] → [sg:nse]js-triage-needed [qa+]
Landed on ESR:

http://hg.mozilla.org/releases/mozilla-esr10/rev/019cdcf42157
status-firefox-esr10: affected → fixed
Verified fixed in Firefox 10.0.3esr.
status-firefox-esr10: fixed → verified
No assertion reproduce using test in comment 0 in js-shell built from today's mozilla-beta.
status-firefox11: fixed → verified
Whiteboard: [sg:nse]js-triage-needed [qa+] → [sg:nse][qa!] js-triage-needed
Group: core-security
(Reporter)

Comment 12

4 years ago
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.