951988 - Assertion failure: mir->fallible(), at jit/arm/CodeGenerator-arm.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

Attachment: stack

d((!([]) | 0) % ~1)

asserts js debug shell on m-c changeset 7d120481a6ae with --ion-eager --ion-parallel-compile=off at Assertion failure: mir->fallible(), at jit/arm/CodeGenerator-arm.cpp

My configure flags are:

CC="gcc -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" AR=ar CXX="g++ -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" sh ./configure --target=arm-linux-gnueabi --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

Comment 1

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/6787bcb8ea7e
user:        Dan Gohman
date:        Mon Dec 02 13:41:54 2013 -0800
summary:     Bug 944963 - IonMonkey: Add a ModSelf operator to fix an x86 constraint problem with x%x. r=bhackett

Dan, is bug 944963 a likely regressor?

Comment 2

[Dan Gohman [:sunfish]]

Attachment: mmod-negative-dividend-checks.patch

Confirmed; it's bug 944963. I unfortunately don't have time to look at this properly right now, but attached is a completely untested and slightly rough patch which has a good chance of fixing it, or is close at least. Anyone can pick this up if they want before I can get to it.

Comment 3

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

Comment on attachment 8350359 [details] [diff] [review]
mmod-negative-dividend-checks.patch

This fails to compile. Build failure log coming up.

Comment 4

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

Attachment: build failure log with Dan's untested patch

Comment 5

[Douglas Crosher [:dougc]]

Attachment: Use canBeNegativeDividend() in the ARM backend.

The logic looks sounds, thank you for the fix.  This just corrects some minor typos.

Comment 6

[Jan de Mooij [:jandem]]

sunfish, dougc: can one of you make sure this gets fixed? Regressing changeset is on Aurora; let's fix this before it hits beta.

Comment 7

[Dan Gohman [:sunfish]]

Comment on attachment 8350927 [details] [diff] [review]
Use canBeNegativeDividend() in the ARM backend.

Doug's fixes look good to me. Requesting an independent review.

The tl;dr here is that range analysis is now determining that MMod operators are infallible in more cases. This patch updates the ARM codegen to avoid emitting bailout code for such operators.

Comment 8

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a992353f9c7

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7a992353f9c7

Comment 10

[Douglas Crosher [:dougc]]

Comment on attachment 8350927 [details] [diff] [review]
Use canBeNegativeDividend() in the ARM backend.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 944963
User impact if declined: assertion failures compiling JS on ARM devices
Testing completed (on m-c, etc.): tested on m-c
Risk to taking this patch (and alternatives if risky): expected low risk of introducing more bugs.  The alternative would be to backout bug 944963 changes.
String or IDL/UUID changes made by this patch: n/a

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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