901391 - Assertion failure: ins->type() == MIRType_Value, at ../ion/MIR.h:2156

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 0a63cd911b4f (run with --fuzzing-safe --ion-eager):


function testPartition() {
  if( "null" || new testPartition()) {}
}
testPartition();

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Christian Holler (:decoder)]

S-s because these type failures can be security-related.

Comment 3

[Christian Holler (:decoder)]

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

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/471dcc138dfe
user:        Shu-yu Guo
date:        Fri Aug 02 08:24:56 2013 -0700
summary:     Bug 898576 - Ignore cold phi inputs when specializing phi types. (r=jandem)

This iteration took 344.603 seconds to run.

Comment 4

[Christian Holler (:decoder)]

Needinfo from Shu based on comment 3 :)

Comment 5

[Shu-yu Guo [:shu]]

Attachment: fix + testcase

Jan, I wasn't aware that MIR with empty observed type sets could be anything but Value (but it makes sense of course, constants and whatnot).

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 785841 [details] [diff] [review]
fix + testcase

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

::: js/src/ion/IonAnalysis.cpp
@@ +556,5 @@
> +                // Otherwise we need to bail out, since we know this branch
> +                // will fail to convert to phiType.
> +                MBail *bail = MBail::New();
> +                in->block()->insertBefore(in->block()->lastIns(), bail);
> +                phi->replaceOperand(i, bail);

MBail doesn't really return anything (it has MIRType_None). I think it's simpler to keep the old code and insert an MBox right before the MUnbox if in->type() != MIRType_Value. It's a bit lame but since we will invalidate before we reach this code it doesn't really matter.

::: js/src/jit-test/tests/ion/bug901391.patch
@@ +1,1 @@
> +function testPartition() {

bug901391.patch -> bug901391.js

Comment 7

[Shu-yu Guo [:shu]]

Attachment: Part 1: Add ThreadSafeContext v2

Good catches, thanks.

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 786586 [details] [diff] [review]
Part 1: Add ThreadSafeContext v2

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

Nice, thanks.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/394ec22dc98c

Comment 10

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.