Closed Bug 1099216 Opened 7 years ago Closed 7 years ago

Assertion failure: from->toStackSlot()->slot() % SimdStackAlignment == 0, at jit/LIR.cpp

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla37
Tracking Status
firefox35 --- unaffected
firefox36 --- unaffected
firefox37 --- verified
firefox38 --- fixed
firefox38.0.5 --- fixed
firefox39 --- verified
firefox-esr31 --- unaffected

People

(Reporter: gkw, Assigned: sunfish)

References

Details

(4 keywords, Whiteboard: [jsbugmon:])

Attachments

(2 files)

function g() {
    Function.apply(null, arguments)
}
function f() {}
f(g("global", "\
    \"use asm\";\
    var frd = global.Math.fround;\
    var fx4 = global.SIMD.float32x4;\
    var fsp = fx4.splat;\
    function e(y) {\
        y = frd(y);\
        var x = frd(0);\
        var c = fx4(0, 0, 0, 0);\
        x = frd(x / x);\
        s();\
        d(fsp(x));\
    }\
"))

asserts js debug shell on m-c changeset 7f0d92595432 with --no-ion --no-baseline --no-threads --ion-regalloc=lsra at Assertion failure: from->toStackSlot()->slot() % SimdStackAlignment == 0, at jit/LIR.cpp.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

This was found by combining random jit-tests together with jsfunfuzz. The specific file, which was run with random flag combinations, is:

http://hg.mozilla.org/mozilla-central/file/7f0d92595432/js/src/jit-test/tests/asm.js/simd-fbirds.js

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/76009dc6ed72
user:        Benjamin Bouvier
date:        Tue Sep 30 11:33:11 2014 +0200
summary:     Bug 1068725: More debugging and assertions for MoveGroups; r=sunfish

Benjamin, is bug 1068725 a possible regressor?

Setting s-s and assuming sec-critical because this seems to involve LIR and SIMD.
Flags: needinfo?(benj)
Attached file stack
(lldb) bt 5
* thread #1: tid = 0x26d665, 0x000000010031421d js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::jit::LMoveGroup::add(this=<unavailable>, from=<unavailable>, to=<unavailable>, type=<unavailable>) + 509 at LIR.cpp:538, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010031421d js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::jit::LMoveGroup::add(this=<unavailable>, from=<unavailable>, to=<unavailable>, type=<unavailable>) + 509 at LIR.cpp:538
    frame #1: 0x0000000100319ab2 js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::jit::LinearScanAllocator::reifyAllocations() [inlined] js::jit::VirtualRegister::ins(this=<unavailable>, this=0x0000000102907e98, this=0x00007fff5fbf98f8, ins=0x0000000102907e98, to=<unavailable>, type=<unavailable>) + 1362 at LinearScan.cpp:304
    frame #2: 0x0000000100319a67 js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::jit::LinearScanAllocator::reifyAllocations(this=0x00007fff5fbf98f8) + 1287 at LinearScan.cpp:389
    frame #3: 0x00000001002b6bf1 js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::jit::LinearScanAllocator::go(this=0x00007fff5fbf98f8) + 385 at LinearScan.cpp:1316
    frame #4: 0x00000001002b680a js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::jit::GenerateLIR(mir=0x0000000102906cd8) + 1882 at Ion.cpp:1644
(lldb)
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
(In reply to Christian Holler (:decoder) from comment #2)
> JSBugMon: Cannot process bug: Unable to automatically reproduce, please
> track manually.

Again, this needs --ion-regalloc=lsra.
This test case shows the same issue and exposes what's the worse that could happen in an opt build: a crash.

(function(global) {
    "use asm";
    var frd = global.Math.fround;
    var fx4 = global.SIMD.float32x4;
    var fsp = fx4.splat;
    function s(){}
    function d(x){x=fx4(x);}
    function e() {
        var x = frd(0);
        x = frd(x / x);
        s();
        d(fsp(x));
    }
    return e;
})(this)();
Unclear if this is sec-critical, leaving someone else to set the rating.
Keywords: sec-critical
Hoping Dan (:sunfish) might be able to help out here...
Flags: needinfo?(sunfish)
Lowering is using defineReusedInput with a float32x4 LSimdSplatx4 because the codegen wants the input and the output to be in the same register. However, the input is a scalar and the output is a SIMD value, and in LSRA, what happens is: the scalar is spilled for part of its lifetime, so it is assigned a scalar stack slot, and then the SIMD value is tied to it, so it gets the same stack slot, but it's not aligned properly for SIMD.
Assignee: nobody → sunfish
Flags: needinfo?(sunfish)
Flags: needinfo?(benj)
Attachment #8535754 - Flags: review?(benj)
Also, since this is SIMD-specific, only Nightly is affected.
Comment on attachment 8535754 [details] [diff] [review]
splat-reuse.patch

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

Thanks! Somehow BT doesn't have this limitation, could LSRA be extended to handle this as well?
Attachment #8535754 - Flags: review?(benj) → review+
And also, could you add the test case please? (nightly only issue, so i assume it's fine)
https://hg.mozilla.org/integration/mozilla-inbound/rev/4dc237fb2fba

I think it's better to get BT on by default for Ion than extend LSRA at this point. Also, I added a testcase.
https://hg.mozilla.org/mozilla-central/rev/4dc237fb2fba
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Flags: in-testsuite?
Test case added in bug 1103389.
Flags: in-testsuite? → in-testsuite+
Group: javascript-core-security
Depends on: 1115217
Group: core-security
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed on Fx37
Is there a reason why this wasn't marked with any status (fixed or otherwise) for 38? I assume we fixed it there when it was fixed on 37 and 39?
Flags: needinfo?(gary)
Flags: needinfo?(sunfish)
(In reply to Christian Holler (:decoder) from comment #14)
> JSBugMon: This bug has been automatically verified fixed.
> JSBugMon: This bug has been automatically verified fixed on Fx37

(In reply to Al Billings [:abillings] from comment #15)
> Is there a reason why this wasn't marked with any status (fixed or
> otherwise) for 38? I assume we fixed it there when it was fixed on 37 and 39?

The versions were marked as above, perhaps one was missed.

Anyway, I checked that it was fixed for 38:

http://hg.mozilla.org/releases/mozilla-release/rev/4dc237fb2fba
Flags: needinfo?(sunfish)
Flags: needinfo?(gary)
SIMD has been Nightly-only from the start.
You need to log in before you can comment on or make changes to this bug.