999759 - Crash [@ js::jit::IonBailoutIterator::IonBailoutIterator]

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

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

Attachment: stack

function f(f, inputs) {
    for (var j = 0; j < inputs.length; ++j) {
        for (var k = 0; k < inputs.length; ++k) {
            try {
                f(inputs[j], inputs[k]);
            } catch (e) {}
        }
    }
}
s = newGlobal()
s["f"] = this["f"].bind(this)
evalcx("\
    function f1() {\
        __defineSetter__(\"e\", function(x) {\
            this[\"__proto__\"] = x\
        })\
    }\
    f(f1, [0]);\
    e = Proxy.createFunction((function() {\
        return {\
            has: function() {}\
        }\
    })()\
    , function(){}\
    );\
    f2 = (function() {});\
    setJitCompilerOption('ion.usecount.trigger', 0);\
    f3 = (function(x, y) {\
        return ((-y * ((((!(x >>> 0)) >>> 0) + \
            (((Math.fround(Math.fround(Math.fround(x))) >>> 0) << \
            (y | 0)) | 0)) | 0)) >>> 0) * \
            Math.pow(((x | 0) | 0), f2(x, +(d(l(0xf, -E)))))\
    });\
    f(f3, [-0, Math.PI, -Number])\
", s)

crashes js opt shell on m-c changeset c962bde5ac0b with --baseline-eager --ion-parallel-compile=off --ion-gvn=pessimistic --ion-check-range-analysis at js::jit::IonBailoutIterator::IonBailoutIterator on an ARM ODROID board with Ubuntu 14.04.

My configure flags are:

AR=ar sh /home/odroid/trees/mozilla-central/js/src/configure --enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

Valgrind shows an invalid read of size 4:

==8340== Invalid read of size 4
==8340==    at 0x1699E4: js::jit::IonBailoutIterator::IonBailoutIterator(js::jit::JitActivationIterator const&, js::jit::BailoutStack*) (Bailouts-arm.cpp:82)
==8340==    by 0xCF5EB: js::jit::Bailout(js::jit::BailoutStack*, js::jit::BaselineBailoutInfo**) (Bailouts.cpp:79)
==8340==    by 0x4841537: ???
==8340==  Address 0xffffffb0 is not stack'd, malloc'd or (recently) free'd
==8340==
==8340==
==8340== Process terminating with default action of signal 11 (SIGSEGV)
==8340==  Access not within mapped region at address 0xFFFFFFB0
==8340==    at 0x1699E4: js::jit::IonBailoutIterator::IonBailoutIterator(js::jit::JitActivationIterator const&, js::jit::BailoutStack*) (Bailouts-arm.cpp:82)
==8340==    by 0xCF5EB: js::jit::Bailout(js::jit::BailoutStack*, js::jit::BaselineBailoutInfo**) (Bailouts.cpp:79)
==8340==    by 0x4841537: ???

s-s and setting sec-critical initially because this looks like a scary ARM crash (plus an invalid read from Valgrind). Setting needinfo? from our ARM gurus.

Comment 1

[Marty Rosenberg [:mjrosenb]]

ok, I was able o reproduce this, looking into it.

Comment 2

[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/21879e298728
user:        Marty Rosenberg
date:        Fri Mar 07 11:25:38 2014 -0500
summary:     Bug 973874: Ensure that all offsets that we take into the assembly buffer are created by the instruction we want to branch to (r=dougc)

Comment 3

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

Setting flags according to landing of bug 973874.

Comment 4

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

mjrosenb requests a retest.

Comment 5

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

I can still reproduce on m-c rev b66e279688a1.

Comment 6

[Marty Rosenberg [:mjrosenb]]

Looks like one of the patches in my queue fixes this. my guess would be 996715.

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Any further determination of which patch fixes this so we can get uplift and verification?

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

We've run out of FF30 betas to try this out on, have to carry forward to target FF31 unless there is a clean, low-risk backout available to land before Monday's RC.

Comment 9

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 10

[Christian Holler (:decoder)]

Retrying this with the gvn option added to JSBugMon :)

Comment 11

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 12

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

Critsmash check: I need to retest.

Comment 13

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

autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/9d8a5c8d8328
user:        Marty Rosenberg
date:        Tue May 27 09:40:35 2014 -0400
summary:     bug 996715: Remove the code that bails when determining if the second instruction in a chunk is a branch. (r=dougc)

Indeed, Marty is right as per comment 6, fixed by bug 996715.

Comment 14

[Justin Wood (:Callek)]

trying to apply this to SeaMonkey 2.26.1 (Gecko 29) resulted in patch conflicts, and due to the nature of this patchset it seems like I won't be able to take it.

Comment 15

[Kamil Jozwiak [:kjozwiak]]

Reproduced the original issue using revision c962bde5ac0b. Used the poc code and the flags from comment #0:

Runtime call with unaligned stack!
==26297== Invalid write of size 4
==26297==    at 0x8294E19: js::jit::Simulator::softwareInterrupt(js::jit::SimInstruction*) (Simulator-arm.cpp:2125)
==26297==    by 0x82932BB: js::jit::Simulator::instructionDecode(js::jit::SimInstruction*) (Simulator-arm.cpp:4036)

Verified using the following builds:

- firefox33 using revision 7883d8e9f210 [Passed with no crashes, Valgrind didn't report any issues]
- firefox32 using revision 0874ec920408 [Passed with no crashes, Valgrind didn't report any issues]
- firefox31 using revision 6befadcaa685 [Passed with no crashes, Valgrind didn't report any issues]
- firefox31 using revision e6aee77687d3 [Passed with no crashes, Valgrind didn't report any issues]
- firefox-esr24 using revision 31b0c1ff3c0b [Passed with no crashes, Valgrind didn't report any issues]