Closed Bug 1495731 Opened 6 years ago Closed 6 years ago

Remove JS_VOLATILE_ARM gcc-4 bug workaround

Categories

(Core :: JavaScript: WebAssembly, enhancement, P1)

ARM
All
enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: lth, Assigned: lth)

References

Details

Attachments

(1 file)

Four years ago, in https://bugzilla.mozilla.org/show_bug.cgi?id=1097253, a gcc-4.7-on-b2g compiler bug was quashed by making some pointers in the typedarray code volatile, this workaround is now located at https://searchfox.org/mozilla-central/source/js/src/vm/TypedArrayObject-inl.h#283.  The bug in question made the compiler emit word-width loads and stores for sequences of byte loads and stores, with the result that some accesses were unaligned; these were not supported always and would cause crashes.

For bug 1394420 I'm running into this again because the 'volatile' qualifier is needed on some low-level APIs on ARM if the TypedArray level is going to use them; there's a very brief discussion at https://bugzilla.mozilla.org/show_bug.cgi?id=1394420#c7 but effectively the types just have to work out.

I've talked to waldo, erahm and froydnj about this.  Our minimum compiler version on ARM is now gcc 6.1.  Additionally, current consumer ARM hardware does generally support unaligned accesses.

The conclusion is that I should do a little light testing just to be safe, but we should probably remove JS_VOLATILE_ARM from the code.
So here's a test:

Compiler is "arm-linux-gnueabihf-gcc (Ubuntu/Linaro 7.3.0-27ubuntu1~18.04) 7.3.0".  Use this to cross-compile JS shell, and run on an NVidia Tegra dev board (mine is running a franken-Ubuntu install in the fractal space between 14 and 16 following a botched upgrade that left the system mostly operational).  Run the test case attached on bug 1097253, suitably modified.  It runs fine.  Now the fun part.  /proc/cpu/alignment says this:

User:		0
System:		1549
Skipped:	0
Half:		0
Word:		0
DWord:		0
Multi:		1549
User faults:	2 (fixup)

The last line is the interesting one, it says that the chip may fault on unaligned accesses but the kernel will fixup.  The other lines are counters for such fixups.  These counters have been stable after several test runs; they increase occasionally but not while I'm running tests.  This suggests somewhat that (a) if the compiler did emit unaligned accesses we should see evidence of them and (b) the compiler does not generate unaligned accesses.

As evidence goes this is pretty weak, but it's the best I can do at the moment, and it seems OK.

I've seen evidence elsewhere (https://stackoverflow.com/questions/42295951/unaligned-accesses-are-not-detected-by-raspberry-pi-version-1) that older GCC were found by others to have the bug we encountered.
I think I got the nod for this on IRC but let's do a review anyway, if for no other reason than to check my reasoning in the previous comments.
Attachment #9013722 - Flags: review?(jwalden+bmo)
Attachment #9013722 - Flags: review?(jwalden+bmo) → review+
Lars, do you need extra modification, or just settings checkin-needed would do it?
Flags: needinfo?(lhansen)
Landing it today, I hope...
Flags: needinfo?(lhansen)
Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb430eaf5521
remove JS_VOLATILE_ARM, it is no longer relevant.  r=waldo
https://hg.mozilla.org/mozilla-central/rev/bb430eaf5521
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: