Closed Bug 1350552 Opened 3 years ago Closed 3 years ago

Hit MOZ_CRASH(ARM simulator breakpoint) at js/src/jit/arm/Simulator-arm.cpp:3185 with asm.js

Categories

(Core :: JavaScript Engine, defect, critical)

55 Branch
ARM
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox52 --- wontfix
firefox-esr52 --- wontfix
firefox53 --- fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: decoder, Assigned: bbouvier)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [jsbugmon:update,bisect])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 65b0ac174753 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu --enable-simulator=arm, run with --fuzzing-safe --arm-hwcap=vfp --ion-offthread-compile=off):

function a() {
    "use asm";
    function f() {
        return (((((-1) >>> (0 + 0)) | 0) % 10000) >> (0 + 0)) | 0;
    }
    return f;
}
a()()



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0850f978 in js::jit::Simulator::decodeType01 (this=0xf7942000, instr=0x43d0c050) at js/src/jit/arm/Simulator-arm.cpp:3185
#0  0x0850f978 in js::jit::Simulator::decodeType01 (this=0xf7942000, instr=0x43d0c050) at js/src/jit/arm/Simulator-arm.cpp:3185
#1  0x0850c4aa in js::jit::Simulator::instructionDecode (this=0xf7942000, instr=0x43d0c050) at js/src/jit/arm/Simulator-arm.cpp:4687
#2  0x0850fffa in js::jit::Simulator::execute<false> (this=0xf7942000) at js/src/jit/arm/Simulator-arm.cpp:4760
#3  js::jit::Simulator::callInternal (this=0xf7942000, entry=0x43d0c098 "\004\340-\345\360\037-\351\020\212", <incomplete sequence \355>) at js/src/jit/arm/Simulator-arm.cpp:4848
#4  0x085102e1 in js::jit::Simulator::call (this=<optimized out>, entry=0x43d0c098 "\004\340-\345\360\037-\351\020\212", <incomplete sequence \355>, argument_count=<optimized out>) at js/src/jit/arm/Simulator-arm.cpp:4931
#5  0x08934900 in js::wasm::Instance::callExport (this=0xf5174250, cx=0xf791d000, funcIndex=4097, args=...) at js/src/wasm/WasmInstance.cpp:655
#6  0x0893535c in WasmCall (cx=0xf791d000, argc=0, vp=0xf5055058) at js/src/wasm/WasmJS.cpp:1114
#7  0x08179287 in js::CallJSNative (cx=0xf791d000, native=0x89352b0 <WasmCall(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:282
[...]
#21 main (argc=5, argv=0xffffcdb4, envp=0xffffcdcc) at js/src/shell/js.cpp:8461
eax	0x0	0
ebx	0x8cd4ff4	147673076
ecx	0xf7da4864	-136689564
edx	0x0	0
esi	0x8cd4ff4	147673076
edi	0xf7da3df8	-136692232
ebp	0xffffbd88	4294950280
esp	0xffffbd30	4294950192
eip	0x850f978 <js::jit::Simulator::decodeType01(js::jit::SimInstruction*)+5112>
=> 0x850f978 <js::jit::Simulator::decodeType01(js::jit::SimInstruction*)+5112>:	movl   $0x0,0x0
   0x850f982 <js::jit::Simulator::decodeType01(js::jit::SimInstruction*)+5122>:	ud2
wasm is also on the stack, might this be related to bug 1350550? Setting needinfo? from Benjamin as a start.
Flags: needinfo?(bbouvier)
This looks like it could be related to a change I landed recently to disambiguate signed and unsigned modulo.
Unaligned stack pointer (not aligned to 8 bytes) in call to SoftModI.  Most of the flags above are not needed, a regular debug build will do, if run with --arm-hwcap=vfp.  Probably just an untested configuration.

Note that --arm-hwcap=vfp does *not* simply enable VFP with the other CPU flags being the same, it resets all other flags.  So this is basically an ARMv6 without integer div/mod, which is why we're seeing this here.  Note ARMv6 is Tier-3 at best.

For reference, default flags are (js/src/arm/Architecture-arm.cpp:209):

    flags = HWCAP_ARMv7 | HWCAP_VFP | HWCAP_VFPv3 | HWCAP_VFPv4 | HWCAP_NEON | HWCAP_IDIVA | HWCAP_FIXUP_FAULT;

Gary, asm.js is translated internally to wasm, so every asm.js program will have wasm on the stack.

All that said there's a real bug here.  Running with my standard config string: ARMHWCAP=armv7,vfp,vfpv3,vfpv4,neon,idiva,hardfp but removing idiva, I still get a crash.
This is the most recent function entry in that trace and explains the unaligned stack pointer:

  0x5657c000  e52de004       str lr, [sp, #-4]!    ; return address
  0x5657c004  e52d9004       str r9, [sp, #-4]!    ; tls
  0x5657c008  e52db004       str fp, [sp, #-4]!    ; frame pointer
Instruction trace, for safekeeping:

  0x31677098  e52de004       str lr, [sp, #-4]!
  0x3167709c  e92d1ff0       stmdb sp!, {r4, r5, r6, r7, r8, r9, r10, fp, ip}
  0x316770a0  ed2d8a10       vstmdb sp!, {s16-s31}
  0x316770a4  e1a04000       mov r4, r0
  0x316770a8  e1a09001       mov r9, r1
  0x316770ac  e599a00c       ldr r10, [r9, #+12]
  0x316770b0  e52d4004       str r4, [sp, #-4]!
  0x316770b4  e5995000       ldr r5, [r9, #+0]
  0x316770b8  e59550cc       ldr r5, [r5, #+204]
  0x316770bc  e585d04c       str sp, [r5, #+76]
  0x316770c0  e3cdd007       bic sp, sp, #7
  0x316770c4  e3a0b000       mov fp, #0
  0x316770c8  e31d0007       tst sp, #7
  0x316770cc  0a000000       beq +8 -> 0x316770d4
  0x316770d4  ebffffc9       bl -212 -> 0x31677000
  0x31677000  e52de004       str lr, [sp, #-4]!        ; function
  0x31677004  e52d9004       str r9, [sp, #-4]!        ;   entry
  0x31677008  e52db004       str fp, [sp, #-4]!        ;    here
  0x3167700c  e1a0b00d       mov fp, sp
  0x31677010  e3e01000       mvn r1, #0
  0x31677014  e1a00001       mov r0, r1
  0x31677018  e59f106c       ldr r1, [pc, #+108]
  0x3167701c  e1a04000       mov r4, r0
  0x31677020  e3500102       cmp r0, #-2147483648      ; mod starts here
  0x31677024  03710001       cmneq r1, #1
  0x31677028  1a000001       bne +12 -> 0x31677034
  0x31677034  e3510000       cmp r1, #0
  0x31677038  b3500000       cmplt r0, #0              
  0x3167703c  1a000001       bne +12 -> 0x31677048
  0x31677048  e31d0007       tst sp, #7                ; this alignment check fails
  0x3167704c  0a000000       beq +8 -> 0x31677054
  0x31677050  e120107e       bkpt 270
After the 3 pushes to the stack, there should be a final sp adjustment that leaves sp aligned.  However, this only happens (in the CodeGeneratorShared() ctor) if gen->usesSimd() || gen->performsCall(), so my guess here is that these soft calls are not setting gen->performsCall().
Version: Trunk → 55 Branch
Attached patch 1350552.patchSplinter Review
Indeed, this fixes the issue.
Assignee: nobody → bbouvier
Flags: needinfo?(bbouvier)
Attachment #8851681 - Flags: review?(luke)
Comment on attachment 8851681 [details] [diff] [review]
1350552.patch

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

Thanks!
Attachment #8851681 - Flags: review?(luke) → review+
Pushed by bbouvier@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d31105b29da
Set performCall for soft calls on ARM; r=luke
https://hg.mozilla.org/mozilla-central/rev/7d31105b29da
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment on attachment 8851681 [details] [diff] [review]
1350552.patch

This is in all the versions that have asm.js/wasm, but for ARMv6; it falls in the category of nice to have for support of old tier-3 platforms, but not mandatory fixes.

Approval Request Comment
[Feature/Bug causing the regression]: asm.js probably
[User impact if declined]: crash on ARMv6
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not much
[Why is the change risky/not risky?]: conservative change, a few lines of code only
[String changes made/needed]: n/a
Attachment #8851681 - Flags: approval-mozilla-beta?
Attachment #8851681 - Flags: approval-mozilla-aurora?
Comment on attachment 8851681 [details] [diff] [review]
1350552.patch

Fix a crash related to wasm. Aurora54+ & Beta53+.
Attachment #8851681 - Flags: approval-mozilla-beta?
Attachment #8851681 - Flags: approval-mozilla-beta+
Attachment #8851681 - Flags: approval-mozilla-aurora?
Attachment #8851681 - Flags: approval-mozilla-aurora+
Setting qe-verify- based on Benjamin's assessment on manual testing needs (see Comment 11) and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.