863755 - IonMonkey: Assertion failure: a.isGeneralReg(), at ../ion/shared/CodeGenerator-shared-inl.h:32

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 64d6d002e888 (run with --ion-eager):


function TestCase( e, a) {
  getTestCaseResult(e, a);
}
function getTestCaseResult(expected, actual) {}
var msPerSecond =  1000;
var TIME_0000  = (function () {  })();
var now = new Date();
var TIME_NOW = now.valueOf();
function msFromTime(t) {
  var ms = t % msPerSecond;
  return ((ms < 0) ? msPerSecond + ms : ms );
}
new TestCase(false,  eval("true, false"));
addTestCase( TIME_NOW );
addTestCase( TIME_0000 );
function addTestCase( t ) {
  for ( m = 0; m <= 1000; m+=100 ) {
    new TestCase(msFromTime(t), (new Date(t)).getMilliseconds());
  }
}

Comment 1

[Christian Holler (:decoder)]

Backtrace for assertion:


Program received signal SIGSEGV, Segmentation fault.
ToRegister (a=...) at ../ion/shared/CodeGenerator-shared-inl.h:32
32          JS_ASSERT(a.isGeneralReg());
(gdb) bt
#0  ToRegister (a=...) at ../ion/shared/CodeGenerator-shared-inl.h:32
#1  js::ion::ToRegister (a=...) at ../ion/shared/CodeGenerator-shared-inl.h:30
#2  0x0000000000a184c8 in ToRegister (a=<optimized out>) at ../ion/shared/CodeGenerator-shared-inl.h:39
#3  js::ion::CodeGeneratorX64::ToValue (this=<optimized out>, ins=<optimized out>, pos=<optimized out>) at /srv/repos/mozilla-central/js/src/ion/x64/CodeGenerator-x64.cpp:28
#4  0x0000000000b1d85c in js::ion::CodeGenerator::visitUnboxDouble (this=0xffe300, lir=0x1010d80) at /srv/repos/mozilla-central/js/src/ion/CodeGenerator.cpp:4683
#5  0x0000000000b2571f in js::ion::CodeGenerator::generateBody (this=0xffe300) at /srv/repos/mozilla-central/js/src/ion/CodeGenerator.cpp:2237
#6  0x0000000000b26c84 in js::ion::CodeGenerator::generate (this=0xffe300) at /srv/repos/mozilla-central/js/src/ion/CodeGenerator.cpp:4558
#7  0x000000000089a67b in js::ion::GenerateCode (mir=0xffb2c0, lir=0x100eef0, maybeMasm=0x0) at /srv/repos/mozilla-central/js/src/ion/Ion.cpp:1194
#8  0x000000000089a752 in CompileBackEnd (mir=0xffb2c0, maybeMasm=<optimized out>) at /srv/repos/mozilla-central/js/src/ion/Ion.cpp:1213
#9  js::ion::SequentialCompileContext::compile (this=<optimized out>, builder=0xffb2c0, graph=<optimized out>, autoDelete=...) at /srv/repos/mozilla-central/js/src/ion/Ion.cpp:1402
#10 0x000000000089b353 in js::ion::IonCompile<js::ion::SequentialCompileContext> (cx=0xf517f0, script=0x7ffff6038570, fun=(JSFunction *) 0x7ffff6045340 [object Function "addTestCase"], osrPc=0x0, constructing=false, 
    compileContext=...) at /srv/repos/mozilla-central/js/src/ion/Ion.cpp:1342
#11 0x000000000089bace in js::ion::Compile<js::ion::SequentialCompileContext> (cx=<optimized out>, script=0x7ffff6038570, fun=..., osrPc=<optimized out>, constructing=<optimized out>, compileContext=...)
    at /srv/repos/mozilla-central/js/src/ion/Ion.cpp:1573
#12 0x000000000089bfcb in js::ion::CanEnter (cx=0xf517f0, script=0x7ffff6038570, fp=..., isConstructing=false) at /srv/repos/mozilla-central/js/src/ion/Ion.cpp:1683
#13 0x000000000054601a in js::RunScript (cx=0xf517f0, fp=0x7ffff62f20c0) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:320
#14 0x000000000054667f in js::InvokeKernel (cx=0xf517f0, args=..., construct=js::NO_CONSTRUCT) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:421
#15 0x000000000054746e in Invoke (args=..., cx=0xf517f0, construct=<optimized out>) at /srv/repos/mozilla-central/js/src/jsinterp.h:134
#16 js::Invoke (cx=0xf517f0, thisv=..., fval=..., argc=1, argv=<optimized out>, rval=0x7fffffffd2f0) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:454
#17 0x0000000000ae467c in js::ion::DoCallFallback (cx=0xf517f0, frame=0x7fffffffd388, stub=0xf4d480, argc=1, vp=0x7fffffffd338, res=JSVAL_VOID) at /srv/repos/mozilla-central/js/src/ion/BaselineIC.cpp:6319
#18 0x00007ffff7f95aa6 in ?? ()


djvj mentioned on IRC that this could be harmful, so marking s-s for now.

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   129048:61e661489460
user:        Hannes Verschore
date:        Wed Apr 17 11:31:50 2013 +0200
summary:     Bug 862100: IonMonkey: AddTypeBarrier shouldn't unbox, when type is already double, r=nbp

This iteration took 155.055 seconds to run.

Comment 3

[Christian Holler (:decoder)]

Needinfo from Hannes based on comment 2 :)

Comment 4

[Hannes Verschore [:h4writer]]

From a quick glance at this we need to add BoxPolicy to MUnbox. This is already planned for the future with the type inference changes planned for IM. I'll have a better look and create the patch tonight/tomorrow morning

Comment 5

[Hannes Verschore [:h4writer]]

Attachment: Box input to MUnbox

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 740716 [details] [diff] [review]
Box input to MUnbox

Review of attachment 740716 [details] [diff] [review]:
-----------------------------------------------------------------

Please add the testcase in comment 0, r=me with that.

Comment 7

[Hannes Verschore [:h4writer]]

Comment on attachment 740716 [details] [diff] [review]
Box input to MUnbox

@Jan, I'll land the testcase after I landed on mozilla-aurora

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Very hard. It doesn't point to were the issue is :D

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
- Testcase will get landed after aurora landing.
- check-in comment is: Bug 863755: IonMonkey: Use BoxPolicy on MUnbox, r=jandem

Which older supported branches are affected by this flaw?
Aurora

If not all supported branches, which bug introduced the flaw?
Original bug: Bug 796114

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Yes, should apply cleanly on aurora

How likely is this patch to cause regressions; how much testing does it need?
I would almost say none, but that always pans out wrong. But not much at all. Very straightforward patch.

Comment 8

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 740716 [details] [diff] [review]
Box input to MUnbox

sec-approval+ (which applies to M-C). Please nominate patch for Aurora. Release Management team may want a security rating to determine need but I expect that we'll probably just take it.

Comment 9

[Hannes Verschore [:h4writer]]

Comment on attachment 740716 [details] [diff] [review]
Box input to MUnbox

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
Bug 862100 (that depends on bug 796114)

User impact if declined:
Possible crashes during compilation on special crafted scripts.

Testing completed (on m-c, etc.): 
Hasn't landed on m-c since a big improvement removed all this code on m-c. Note that this big improvement does contain this small fix. So if I can count that: half a day on m-i. But this has also landed on ionmonkey branch (that has tbpl builds etc). There it is checked in for multiple days, up to a week.

Risk to taking this patch (and alternatives if risky): 
Very low.

String or IDL/UUID changes made by this patch:
/

Comment 10

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 8b1a7228674a).

Comment 11

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6ed230e228f5

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Why did this land on aurora before hitting inbound/m-c?

Comment 13

[Christian Holler (:decoder)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #12)
> Why did this land on aurora before hitting inbound/m-c?

Because comment 9 says that this bug got already fixed on m-c by another large change, that (I assume) cannot be backported to aurora.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

aha, gotcha. Thanks :)

Comment 15

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 16

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9063e9ebc320

Comment 17

[u279076]

Marking status-firefox23:verified based on comment 15.