Closed
Bug 759209
Opened 13 years ago
Closed 12 years ago
IonMonkey: (ARM) Assertion failure: data == imm, at ion/arm/Assembler-arm.h:540
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: decoder, Unassigned)
References
Details
(Keywords: assertion, testcase)
Attachments
(2 files)
|
(deleted),
text/plain
|
Details | |
|
(deleted),
patch
|
jbramley
:
review+
|
Details | Diff | Splinter Review |
The attached testcase asserts on ionmonkey-arm (private branch) revision (run with --ion -n -m --ion-eager).
|
Reporter | |
Updated•12 years ago
|
Summary: IonMonkey: Assertion failure: data == imm, at ion/arm/Assembler-arm.h:540 → IonMonkey: (ARM) Assertion failure: data == imm, at ion/arm/Assembler-arm.h:540
|
||
Comment 1•12 years ago
|
||
Haven't quite finished looking into this, but it seems *quite* impressive.
Currently, this looks like a very silly off by one error that should have existed since the pool code was written. I'd thought I had caught all of those at this point.
|
|
||
Comment 2•12 years ago
|
||
Well, I was mostly right. There were *two* off by one errors, in the same direction!
Guess that code path wasn't tested much at all.
|
|
||
Comment 3•12 years ago
|
||
I think there are still some off by one issues, but this works, so I'm tempted to not go hunting. If there are off by one errors, they are in opposite directions, so they cancel out.
Attachment #629026 -
Flags: review?(Jacob.Bramley)
|
||
Comment 4•12 years ago
|
||
Comment on attachment 629026 [details] [diff] [review]
/home/mrosenberg/patches/fixPoolOffByOne-r0.patch
Review of attachment 629026 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/ion/arm/Assembler-arm.h
@@ +1240,5 @@
> {
> // Set up the backwards double region
> new (&pools_[2]) Pool (1024, 8, 4, 8, 8, true);
> // Set up the backwards 32 bit region
> + new (&pools_[3]) Pool (4096, 4, 4, 8, 4, true, true);
Is the 'bias' offset the PC+8 thing? If so, this change is fine.
Attachment #629026 -
Flags: review?(Jacob.Bramley) → review+
|
|
||
Comment 5•12 years ago
|
||
(In reply to Jacob Bramley [:jbramley] from comment #4)
> Comment on attachment 629026 [details] [diff] [review]
> /home/mrosenberg/patches/fixPoolOffByOne-r0.patch
>
> Review of attachment 629026 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: js/src/ion/arm/Assembler-arm.h
> @@ +1240,5 @@
> > {
> > // Set up the backwards double region
> > new (&pools_[2]) Pool (1024, 8, 4, 8, 8, true);
> > // Set up the backwards 32 bit region
> > + new (&pools_[3]) Pool (4096, 4, 4, 8, 4, true, true);
>
> Is the 'bias' offset the PC+8 thing? If so, this change is fine.
Yeah, I'm not entirely sure what I was thinking when I wrote this code. This is definitely one case where named arguments would make me so happy, because seeing "bais:=4" would have been a red flag from the beginning.
|
|
||
Comment 6•12 years ago
|
||
landed, seems stable: http://hg.mozilla.org/projects/ionmonkey/rev/2062cc1c4b06
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•