Closed
Bug 1103389
Opened 10 years ago
Closed 9 years ago
Assertion failure: from->toArgument()->index() % SimdStackAlignment == 0, at jit/LIR.cpp
Categories
(Core :: JavaScript Engine: JIT, defect)
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)
Attachments
(2 files)
10.62 KB,
text/plain
|
Details | |
3.22 KB,
patch
|
bbouvier
:
review+
|
Details | Diff | Splinter Review |
// Random chosen test: js/src/jit-test/tests/asm.js/simd-mandelbrot.js (function(global) { "use asm" var toF = global.Math.fround var f4 = global.SIMD.float32x4 function p(x, y, width, value, max_iterations) { x = x | 0 y = y | 0 width = width | 0 value = value | 0 max_iterations = max_iterations | 0 } function m(xf, yf, yd, max_iterations) { xf = toF(xf) yf = toF(yf) yd = toF(yd) max_iterations = max_iterations | 0 var _ = f4(0, 0, 0, 0), c_im4 = f4(0, 0, 0, 0) c_im4 = f4(yf, yd, yd, yf) d(c_im4) } })() asserts js debug shell on m-c changeset 7ab92d922d19 with --ion-eager --no-threads --ion-regalloc=lsra at Assertion failure: from->toArgument()->index() % SimdStackAlignment == 0, at jit/LIR.cpp. Debug configure options: LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -msse2 -mfpmath=sse -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -msse2 -mfpmath=sse" AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 HOST_CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse" sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --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(s) is/are: http://hg.mozilla.org/mozilla-central/file/7ab92d922d19/js/src/jit-test/tests/asm.js/simd-mandelbrot.js autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/4a1a4d119980 parent: 205834:ff2190c3dbfd user: Benjamin Bouvier date: Wed Sep 17 18:55:58 2014 +0200 summary: Bug 1068331: Fix register allocation of SimdValueX4; r=sunfish (Setting s-s because this involves LIR, but letting someone else set the sec-rating) Benjamin, is bug 1068331 a likely regressor?
Flags: needinfo?(benj)
Reporter | ||
Updated•10 years ago
|
status-firefox36:
--- → affected
Reporter | ||
Comment 1•10 years ago
|
||
(lldb) bt 5 * thread #1: tid = 0x34d75c, 0x0035548d js-dbg-opt-32-dm-nsprBuild-darwin-7ab92d922d19`js::jit::LMoveGroup::add(this=<unavailable>, from=<unavailable>, to=<unavailable>, type=<unavailable>) + 573 at LIR.cpp:538, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x0035548d js-dbg-opt-32-dm-nsprBuild-darwin-7ab92d922d19`js::jit::LMoveGroup::add(this=<unavailable>, from=<unavailable>, to=<unavailable>, type=<unavailable>) + 573 at LIR.cpp:538 frame #1: 0x0035b30e js-dbg-opt-32-dm-nsprBuild-darwin-7ab92d922d19`js::jit::LinearScanAllocator::reifyAllocations() [inlined] js::jit::VirtualRegister::ins(this=<unavailable>, this=0x028b52f8, this=0x003e5fde, ins=0x028b52f8, to=<unavailable>, type=FLOAT32X4) + 1182 at LinearScan.cpp:304 frame #2: 0x0035b2c1 js-dbg-opt-32-dm-nsprBuild-darwin-7ab92d922d19`js::jit::LinearScanAllocator::reifyAllocations(this=0x003e5fde) + 1105 at LinearScan.cpp:389 frame #3: 0x002edeb8 js-dbg-opt-32-dm-nsprBuild-darwin-7ab92d922d19`js::jit::LinearScanAllocator::go(this=0xbfffc168) + 392 at LinearScan.cpp:1316 frame #4: 0x002edb2c js-dbg-opt-32-dm-nsprBuild-darwin-7ab92d922d19`js::jit::GenerateLIR(mir=0x028b4688) + 1660 at Ion.cpp:1647 (lldb)
Comment 2•9 years ago
|
||
What versions have the SIMD code? trunk only? What happens if this value is unaligned?
Keywords: sec-high
Comment 3•9 years ago
|
||
(In reply to Daniel Veditz [:dveditz] from comment #2) > What versions have the SIMD code? trunk only? > What happens if this value is unaligned? Only Nightly has SIMD activated, other versions are unaffected. This bug shows up with --ion-regalloc=lsra (i.e. using the LSRA register allocator), which isn't used so far for SIMD code in the browser. In the browser, another register allocator (backtracking alloc) is used. If a value is unaligned, in an opt build, the browser will crash (with SIGBUS) when trying to read or write the value.
Comment 4•9 years ago
|
||
Dan, any interest to work on this? I could take it on, but my knowledge of our regalloc is fairly low...
Flags: needinfo?(sunfish)
Assignee | ||
Comment 5•9 years ago
|
||
I'm traveling so I can't get to this right away, but I will take a look at this. Fortunately, going by comment 3, it seems that the conditions needed to trigger the bug don't yet occur in the wild.
Assignee | ||
Comment 6•9 years ago
|
||
This is similar to bug 1099216, but this time it's LSimdValueFloat32x4.
Assignee: nobody → sunfish
Flags: needinfo?(sunfish)
Assignee | ||
Comment 7•9 years ago
|
||
This is a similar fix, of disabling defineReuseInput, plus the additional twist that LSRA doesn't seem to handle the tempCopy correctly. I decided to just work around it and use a plain temp and manual copy for now. This is another motivation for the project to enable backtracking by default.
Flags: needinfo?(benj)
Attachment #8535789 -
Flags: review?(benj)
Comment 8•9 years ago
|
||
Comment on attachment 8535789 [details] [diff] [review] simdvalue-reuse.patch Review of attachment 8535789 [details] [diff] [review]: ----------------------------------------------------------------- Can't wait for BT to be enabled by default. Can you add the test case please? ::: js/src/jit/shared/Lowering-x86-shared.cpp @@ +737,5 @@ > { > if (ins->type() == MIRType_Float32x4) { > + // Ideally, x would be used at start and reused for the output, however > + // register allocation currently doesn't permit us to tie together two > + // virtual registers with different types. Can you add a TODO and open up a bug, so that we don't forget to modify here (and the place in bug 1099216 as well) if/when BT gets enabled by default?
Attachment #8535789 -
Flags: review?(benj) → review+
Assignee | ||
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e6f63e57dc5
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3e6f63e57dc5
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox35:
--- → unaffected
status-firefox37:
--- → fixed
status-firefox-esr31:
--- → unaffected
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Updated•9 years ago
|
Group: core-security
Updated•9 years ago
|
Comment 11•9 years ago
|
||
JSBugMon: This bug has been automatically verified fixed. JSBugMon: This bug has been automatically verified fixed on Fx37
Comment 12•9 years ago
|
||
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)
Updated•9 years ago
|
Flags: needinfo?(sunfish)
Reporter | ||
Comment 13•9 years ago
|
||
(In reply to Christian Holler (:decoder) from comment #11) > 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 #12) > 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/3e6f63e57dc5 http://hg.mozilla.org/releases/mozilla-esr38/rev/3e6f63e57dc5
status-firefox38:
--- → fixed
status-firefox38.0.5:
--- → fixed
Flags: needinfo?(sunfish)
Flags: needinfo?(gary)
Comment 14•9 years ago
|
||
SIMD has been Nightly-only from the start.
You need to log in
before you can comment on or make changes to this bug.
Description
•