Assertion failure: ((offset) % 4) == 0 (../nanojit/NativeARM.cpp:1628)

VERIFIED FIXED in Q2 12 - Cyril

Status

Tamarin
Virtual Machine
P3
major
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: Brent Baker, Assigned: William Maddox)

Tracking

(Blocks: 1 bug)

unspecified
Q2 12 - Cyril
ARM
All
Bug Flags:
in-testsuite +
flashplayer-qrb +
flashplayer-bug -
flashplayer-triage +

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
Assertion failure: ((offset) % 4) == 0 (../nanojit/NativeARM.cpp:1628) when running ecma3/TypeConversion/e9_3_1_3_rt.as

This was introduced in:

changeset:   6939:ab76e2ab1678
user:        Lars T Hansen <lhansen@adobe.com>
date:        Mon Nov 21 13:28:26 2011 +0100
summary:     Minor adjustments for issues uncovered in the review
Flags: in-testsuite+
Flags: flashplayer-triage+
Flags: flashplayer-qrb+
Flags: flashplayer-bug-
(Reporter)

Comment 1

7 years ago
This actually asserts running anything that is jitted and NOT just TypeConversion testcase when running a debug build
(Assignee)

Comment 2

7 years ago
The bug is due to this change in CodegenLIR.cpp:

- emitIns(BUILTIN_number,  ldd(subp(val, AtomConstants::kDoubleType), 0, ACCSET_OTHER) )
+            emitIns(BUILTIN_number,  ldd(val, -AtomConstants::kDoubleType, ACCSET_OTHER) )

The floating-point load and store instructions on ARM assume that the displacement is a multiple of four, and cannot encode other values.  The ARM code generator just passes the restriction through to LIR_ldd.  Since we expect LIR_ldd to respect natural alignment, this usually works, but the code is being clever here, using the displacement to subtract off the tag.
(Assignee)

Comment 3

7 years ago
Created attachment 576411 [details] [diff] [review]
Allow unaligned displacements to LIR_ldd/ldf/std/stf etc.

This patch relaxes the restriction on the displacement, at the expense of a potentially useful heuristic for detecting unaligned accesses.  It effectively undoes the clever tag-removal trick on ARM, but allows it to operate on other platforms such as x86 without cluttering the front-end or LIR semantics.
Attachment #576411 - Flags: feedback?(lhansen)

Comment 4

7 years ago
(In reply to William Maddox from comment #2)

> ... but the
> code is being clever here, using the displacement to subtract off the tag.

You flatter - all I did was change the code to its most natural form.  Remarkable that we haven't seen this problem before.

(I wonder idly whether the ARM assembler has peephole optimization to fuse the two instructions that must have been generated by the original LIR into one, as it should have.)
(Assignee)

Comment 5

7 years ago
(In reply to Lars T Hansen from comment #4)

> (I wonder idly whether the ARM assembler has peephole optimization to fuse
> the two instructions that must have been generated by the original LIR into
> one, as it should have.)

It cannot do so in general, because the load and store instructions are encoded
with two implicit zero low-order bits.

Comment 6

7 years ago
Comment on attachment 576411 [details] [diff] [review]
Allow unaligned displacements to LIR_ldd/ldf/std/stf etc.

To my eye the logic looks exactly right (rematerialize the explicit offset calculation when needed). I'm not all that familiar with ARM or the ARM assembler's conventions though, so issues on that level could have eluded me.
Attachment #576411 - Flags: feedback?(lhansen) → feedback+

Updated

7 years ago
Assignee: nobody → wmaddox
Severity: normal → major
Priority: -- → P3
Target Milestone: --- → Q2 12 - Cyril

Comment 7

7 years ago
Comment on attachment 576411 [details] [diff] [review]
Allow unaligned displacements to LIR_ldd/ldf/std/stf etc.

If there's any chance IP can be clobberred by underrunProtect generating a jump between code hunks, then we will need underrunProtect before sequences that use IP as scratch register, e.g. here:

+                FLDD(r, IP, 0);
+                asm_add_imm(IP, FP, d);

If underrunProtect cannot clobber IP then we're ok.  (and in the future could make IP available to the register allocator).
(Assignee)

Comment 8

7 years ago
http://asteam.macromedia.com/hg/tr-float/rev/232377fad4ab
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Reporter)

Updated

7 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.