942604 - Assertion failure: consumer->isConsistentFloat32Use(), at jit/IonAnalysis.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

Attachment: stack

for (var j = 0; j < 999; ++j) {
    try {
        (function() {
            function f(x) {
                x = (new Float32Array)[i >> 2];
                {}(w) = arguments[0]
            }
            return f
        })()();
    } catch (e) {}
}

asserts js debug shell on m-c changeset 74ab61b8d0f0 without any CLI arguments at Assertion failure: consumer->isConsistentFloat32Use(), at jit/IonAnalysis.cpp

My configure flags are:

CC="clang -Qunused-arguments" AR=ar CXX="clang++ -Qunused-arguments" sh ./configure --target=x86_64-apple-darwin12.5.0 --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/2963a336e7ec
user:        Kannan Vijayan
date:        Mon Sep 30 10:24:30 2013 -0400
summary:     Bug 921120 - Enable Ion-compilation of JSOP_SETARG for functions which use magic arguments. r=nbp

Kannan, is bug 921120 a likely regressor?

Comment 2

[Kannan Vijayan [:djvj]]

Yeah.  MSetFrameArgument doesn't handle Float32 incomings.  Taking bug.

Comment 3

[Kannan Vijayan [:djvj]]

Attachment: bug-942604.patch

Fixes this by adding a ToDouble prior to to MSetFrameArgument if the incoming value is typed Float32.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8338003 [details] [diff] [review]
bug-942604.patch

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

Good news! You can do simpler, just by making MSetFrameArgument a NoFloatPolicy for the value argument (see also MArrayPush for instance). Then you don't have to worry about adding yourself the ToDouble before the operation.

Comment 5

[Kannan Vijayan [:djvj]]

Attachment: Updated patch.

Fixed patch as per recommendation.

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8338545 [details] [diff] [review]
Updated patch.

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

Cool, thanks!

Comment 7

[Kannan Vijayan [:djvj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce322969a746

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ce322969a746

Comment 9

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8338545 [details] [diff] [review]
Updated patch.

decoder said he hits this one on beta and uplifting should be trivial.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 888109
User impact if declined: correctness issues
Testing completed (on m-c, etc.): testing on m-i and m-c completed
Risk to taking this patch (and alternatives if risky): very low, if not no risk at all
String or IDL/UUID changes made by this patch: N/A

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1a7dbb4d887e