866042 - IonMonkey: Assertion failure: ins->type() == MIRType_Double || ins->type() == MIRType_Value, at ion/IonBuilder.cpp:3098

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-aurora revision 4cf75b25cdc3 (run with --ion-eager):


var msPerDay =  86400000;
function UTC(t) {
  return 0;
}
function MakeDay( year, month, date ) {}
function MakeDate( day, time ) {
  return ( day * msPerDay ) + time;
}
function TimeClip( t ) {
  if ( isNaN( t ) ) {
    return ( Number.NaN );
  }
  return ( ToInteger( t ) );
}
function ToInteger( t ) {}
d = new Date(0);
addNewTestCase(
  UTCDateFromTime(SetMonth(0,1,1)),
  LocalDateFromTime(SetMonth(0,1,1)) );
function addNewTestCase( DateString, UTCDate, LocalDate) {}
function LocalDateFromTime(t) {}
function UTCDateFromTime(t) {
  d.value = TimeClip( MakeDate( MakeDay( d.year, d.month, d.date ), d.time ) );
}
function SetMonth( t, mon, date ) {
  return ( TimeClip (UTC(1899)) );
}

Comment 1

[Christian Holler (:decoder)]

Found during aurora fuzzing as requested in bug 865339. h4writer will take care of it :)

Comment 2

[Hannes Verschore [:h4writer]]

Attachment: Possible patch

For now this looks like an over-restrictive assert. This testcase won't do something wrong when removing that assert. I'm gonna investigate a little bit further to be sure it can't be abused otherwise.

Comment 3

[Hannes Verschore [:h4writer]]

Comment on attachment 742291 [details] [diff] [review]
Possible patch

Yes, an over restrictive assert. Type could be MIRType_Int32. (Actually we could remove the MToInt32 in that case, but opted against, since we are working on mozilla-aurora. Less change is better).

Comment 4

[Christian Holler (:decoder)]

Not security-sensitive based on comment 3. Unhiding :)

Comment 5

[Hannes Verschore [:h4writer]]

Comment on attachment 742291 [details] [diff] [review]
Possible patch

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

User impact if declined: Nothing on release builds, asserts on debug build

Testing completed (on m-c, etc.): passes jit-tests, can't land on m-c/m-i, since that particular code is gone.

Risk to taking this patch (and alternatives if risky): 
Since this is on aurora, I already went for the easiest fix that touches the least code. This has almost no risk, since the code has been running for weeks in normal builds (where there are no asserts). 

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

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Is this about FF22 code or about aurora tests?  If we don't take this on aurora what happens when m-c (FF23) moves to aurora?

Comment 7

[Hannes Verschore [:h4writer]]

This is a change in FF22 only code. On FF23 this code is totally rewritten and doesnt have this problem. If needed I can givethe bugnumber where this has been done. Ergo upon merge from m-c the issue will just dissappear.
Note that this patch also doesn't change behaviour on FF22, but just adjust a signalition that was giving false positives.

Comment 8

[Alex Keybl [:akeybl]]

Comment on attachment 742291 [details] [diff] [review]
Possible patch

Approving for correctness and based upon low risk evaluation, even given no known user impact.

Comment 9

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b0765f8b287e