1054512 - Crash [@ js::jit::LIRGenerator::visitToInt32]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

function f(x) {
    x((x | 0) + x);
};
try {
    f(1);
} catch (e) {}
for (var k = 0; k < 1; ++k) {
    try {
        f(Symbol());
    } catch (e) {}
}

crashes js debug and opt shell on m-c changeset d7e78f0c1465 with --no-threads --ion-eager --ion-offthread-compile=off at js::jit::LIRGenerator::visitToInt32.

My configure flags are: (opt)

MAKE=mozmake AR=ar sh c://Users//mozillaadmin//trees//mozilla-central//js//src//configure --disable-debug --enable-optimize --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --enable-nspr-build

(debug)

MAKE=mozmake AR=ar sh c://Users//mozillaadmin//trees//mozilla-central//js//src//configure --enable-debug --enable-optimize --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --enable-nspr-build

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/8e4e04daf2a6
user:        [e-mail], Jason Orendorff
date:        Thu Jul 31 09:05:18 2014 -0500
summary:     Bug 1042602 - Symbol behavior changes in ES6 draft rev 26. r=h4writer.

Jason, is bug 1042602 a likely regressor?

Setting s-s and assuming worse-case sec-critical (because of LIR) as a start. This seems Win-only, but I'm not sure.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: debug and opt stacks

Comment 2

[Hannes Verschore [:h4writer]]

NeedInfo myself. Since I know more about the jit part, which is causing the crash.

Comment 3

[Hannes Verschore [:h4writer]]

Attachment: Fix

This is caused by a difference in what ToInt32Policy does and the rules we need to add when adding a MToInt32::New during Type Analysis.

So to fix this once and for all. Remove all these extra rules and just call the typePolicy of that instruction.

Comment 4

[Hannes Verschore [:h4writer]]

Comment on attachment 8474487 [details] [diff] [review]
Fix

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

::: js/src/jit/MIR.h
@@ +3129,5 @@
>      MUnbox(MDefinition *ins, MIRType type, Mode mode, BailoutKind kind)
>        : MUnaryInstruction(ins),
>          mode_(mode)
>      {
> +        // Only allow unboxing to another type if input isn't a Value.

Rephrase: Only allow unboxing a non MIRType_Value, when input and output types don't match.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8474487 [details] [diff] [review]
Fix

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

This eliminates a lot of code duplication, nice!

Comment 6

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43494708df76

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/43494708df76

Comment 8

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.