OdinMonkey: Crash [@ js::jit::SplitCriticalEdges] or Assertion failure: GetARMFlags() <= ((4294967295U) >> ARCH_BITS), at jit/AsmJSModule.cpp

RESOLVED FIXED in mozilla28

Status

()

Core
JavaScript Engine: JIT
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gkw, Assigned: dougc)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla28
ARM
Linux
assertion, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fuzzblocker], crash signature)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 8336399 [details]
stack

(function () { "use asm"; })

asserts js debug shell on m-c changeset c7cbfa315d46 without any CLI arguments at Assertion failure: GetARMFlags() <= ((4294967295U) >> ARCH_BITS), at jit/AsmJSModule.cpp on my ARM pandaboard.

My configure flags are:

CC="gcc -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" AR=ar CXX="g++ -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" sh ./configure --target=arm-linux-gnueabi --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

After looking at this with Jesse, this assertion was added in bug 900669. Luke, is bug 900669 likely to have caused this? This blocks fuzzing on ARM due to its simplicity.
Flags: needinfo?(luke)
(Reporter)

Comment 1

5 years ago
I will have to temporarily pass in --no-asmjs to bypass this issue. Moreover this happened on soft floating point ARM Ubuntu Linux, not sure if it occurs in hard floating point systems.
Given that all the HWCAP_* flags are in bits 0-7, this must be the result of the line
  flags = aux.a_un.a_val;
in GetARMFlags().  Anyone know what this means?  Are HWCAP_* specifically defined to match whatever is in aux.a_un.a_val?  If so, then I guess this assertion just means there are other flags than the 0-7 we are looking at?  In that case, the assertion is innocuous, but it would be good to mask them off.
Flags: needinfo?(luke)
(Reporter)

Updated

5 years ago
Summary: Assertion failure: GetARMFlags() <= ((4294967295U) >> ARCH_BITS), at jit/AsmJSModule.cpp → OdinMonkey: Assertion failure: GetARMFlags() <= ((4294967295U) >> ARCH_BITS), at jit/AsmJSModule.cpp
(Assignee)

Comment 3

5 years ago
Created attachment 8336421 [details] [diff] [review]
Move the HWCAP_ARMv7 flag down to bit 29 to free up the upper two bit for use as an arch id in the asm.js code cache
(Reporter)

Comment 4

5 years ago
I get a large testcase that crashes at js::jit::SplitCriticalEdges, that eventually reduces to this assertion.
Crash Signature: [@ js::jit::SplitCriticalEdges]
Summary: OdinMonkey: Assertion failure: GetARMFlags() <= ((4294967295U) >> ARCH_BITS), at jit/AsmJSModule.cpp → OdinMonkey: Crash [@ js::jit::SplitCriticalEdges] or Assertion failure: GetARMFlags() <= ((4294967295U) >> ARCH_BITS), at jit/AsmJSModule.cpp
(Assignee)

Comment 5

5 years ago
(In reply to Gary Kwong [:gkw] [:nth10sd] (yes, still catching up on bugmail) from comment #4)
> I get a large testcase that crashes at js::jit::SplitCriticalEdges, that
> eventually reduces to this assertion.

Could I ask if this occurred with the above patch applied?

If not then does anyone concur that the above patch would address the issue?

It might also be prudent to check that the upper unused bits are zero in GetARMFlags().
(Reporter)

Comment 6

5 years ago
> Could I ask if this occurred with the above patch applied?

This patch does fix the assert. I don't have a nice reduced testcase for the crash, and it is no longer around anymore.

Perhaps we could ask for review for the patch and land it first?
Flags: needinfo?(dtc-moz)
(Assignee)

Comment 7

5 years ago
Comment on attachment 8336421 [details] [diff] [review]
Move the HWCAP_ARMv7 flag down to bit 29 to free up the upper two bit for use as an arch id in the asm.js code cache

This flags appears to be defined here only in case the system header file asm/hwcap.h does not define it.  The high bit was probably chosen to avoid conflict with system defined flags.  It does not appear to cause any conflict to move it down to bit 29, and this makes room for the asm.js cache arch id.

Perhaps it would also be prudent to assert that the upper bits returned by aux.a_un.a_val are zero?
Attachment #8336421 - Flags: review?(mrosenberg)
(Assignee)

Updated

5 years ago
Flags: needinfo?(dtc-moz)
Attachment #8336421 - Flags: review?(mrosenberg) → review+
(Reporter)

Comment 8

5 years ago
Helping to land here, to unblock fuzzing asm.js on ARM platforms:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e96d70a3f9b

Thanks for fixing this, Douglas!
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla28
This was backed out and re-landed while investigating frequent B2G reftest timeouts.
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec574743984d
https://hg.mozilla.org/mozilla-central/rev/ec574743984d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Assignee: general → dtc-moz
You need to log in before you can comment on or make changes to this bug.