Closed Bug 1337967 Opened 7 years ago Closed 7 years ago

Assertion failure: !cycleEnd_, at js/src/jit/MoveResolver.h:278

Categories

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

x86
macOS
defect

Tracking

()

RESOLVED DUPLICATE of bug 1299147

People

(Reporter: gkw, Unassigned)

References

Details

(Keywords: assertion, bugmon, testcase, Whiteboard: [fuzzblocker])

Attachments

(2 files, 2 obsolete files)

The following testcase crashes on mozilla-central revision 5e17f9181c6c (build with --enable-debug --enable-more-deterministic --32 --enable-simulator=arm, run with --fuzzing-safe --no-threads):

See attachment.

Backtrace:

#0  js::jit::MoveResolver::PendingMove::setCycleEnd (this=<optimized out>, this=<optimized out>, cycleSlot=<optimized out>) at js/src/jit/MoveResolver.h:278
#1  js::jit::MoveResolver::resolve (this=0xf71c9404) at js/src/jit/MoveResolver.cpp:167
#2  0x0824debb in js::jit::CodeGenerator::visitMoveGroup (this=0xf71c9000, group=0xf0a3ae18) at js/src/jit/CodeGenerator.cpp:3048
#3  0x0836149a in js::jit::LMoveGroup::accept (this=0xf0a3ae18, visitor=0xf71c9000) at js/src/jit/shared/LIR-shared.h:116
#4  0x08299ab7 in js::jit::CodeGenerator::generateBody (this=0xf71c9000) at js/src/jit/CodeGenerator.cpp:5314
/snip

For detailed crash information, see attachment.
Attached file Testcase (obsolete) —
function mathy4(x) {
    mathy0(Math.fround(Math.cos())) ? -x : Math.asin
}
function mathy0(x) Math.fround(~Math.fround(0x100000001)) || 0x100000000() ? x : Math.fround(0x0ffffffff)
for (var j = 0; j < 99; ++j) {
    for (var k = 0; k < 99; ++k) try {
        mathy4();
    } catch (e) {}
}

is probably a better testcase:

$ ./js-dbg-32-dm-clang-armSim-darwin-5e17f9181c6c --fuzzing-safe --no-threads testcase.js
Assertion failure: !cycleEnd_, at js/src/jit/MoveResolver.h:278
Segmentation fault: 11
Whiteboard: [jsbugmon:update] → [jsbugmon:ignore]
Attachment #8835160 - Attachment is obsolete: true
Attachment #8835161 - Attachment is obsolete: true
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/b7adf3986079
user:        Sander Mathijs van Veen
date:        Tue Feb 07 07:18:00 2017 +0100
summary:     Bug 1337367 - Postpone spilling bundles till after regalloc main loop r=bhackett

Sander, is bug 1337367 a likely regressor?

This is also fairly common with several different symptoms, setting [fuzzblocker].
Blocks: 1337367
Flags: needinfo?(sandervv)
Whiteboard: [jsbugmon:ignore] → [fuzzblocker]
$ ./js-32-dm-clang-armSim-darwin-5e17f9181c6c --fuzzing-safe --no-threads --no-baseline --no-ion testcase.js
117403643
-28339712

$ ./js-32-dm-clang-armSim-darwin-5e17f9181c6c --fuzzing-safe --no-threads testcase.js
117403643
966826880

$ ./js-32-dm-clang-armSim-darwin-5e17f9181c6c --fuzzing-safe --no-threads --ion-eager testcase.js
117403643
-28339712


# Full configuration command with needed environment variables is:
# 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 ./configure --target=i386-apple-darwin14.5.0 --enable-simulator=arm --disable-jemalloc --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests
For testcase in comment 3:

# Full configuration command with needed environment variables is:
# 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 ./configure --target=i386-apple-darwin14.5.0 --enable-simulator=arm --disable-jemalloc --enable-debug --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests
Priority: -- → P1
Component: JavaScript Engine → JavaScript Engine: JIT
Group: javascript-core-security
Flags: needinfo?(sandervv)
There is a structural problem in the ARM move resolver. I'm going to take a stab at it in bug 1299147 and will check it also solves the issues found here.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
The patch did not introduce the problem, but showed that resolving the moves is sometimes incorrect. Without the patch, the bundles were spilled and the problem was not visible.

The MoveGroup that causes the assertion failure is:

> [MoveGroup] [d0 -> d1] [r3 -> r0] [s3 -> s1] [s2 -> s0] [r4 -> r1]

On arm, floating point register d(n) overlaps with register s(2n) and s(2n+1).

The following floating point registers need renaming:
> move d0 -> d1
> move s3 -> s1
> move s2 -> s0
When moving d0 -> d1, the values of registers s2 and s3 are overwritten.

From architecture-arm.h:
> d15 is the ARM scratch float register.

The solution would be to have 1) a floating point scratch register, or 2) spill the register to the stack and load at the end. With a scratch register (called "ds"), the solution would be:
> move d0 -> ds
> move s3 -> s1
> move s2 -> s0
> move ds -> d1

(Although it's marked as a duplicate, I submit this comment to document what I found while debugging the issue.)
Depends on: 1368577
Group: javascript-core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: