988791 - MOZ_CRASH() with Unaligned write, involving ARM constant pools

Status: RESOLVED DUPLICATE of bug 996715

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision df75761307b6 (x86 ARM simulator build, run with --fuzzing-safe --ion-regalloc=backtracking --ion-check-range-analysis):


var MyMath = {
  random: function() {
    this.seed = ((this.seed + 0x7ed55d16) + (this.seed << 12))  & 0xffffffff;
    this.seed = ((this.seed ^ 0xc761c23c) ^ (this.seed >>> 19)) & 0xffffffff;
    this.seed = ((this.seed + 0x165667b1) + (this.seed << 5))   & 0xffffffff;
    this.seed = ((3000 + 0xd3a2646c) ^ (this.seed << 9))   & 0xffffffff;
    this.seed = ((this.seed + 0xfd7046c5) + (this.seed << 3))   & 0xffffffff;
    this.seed = ((this.seed ^ 0xb55a4f09) ^ (this.seed >>> 16)) & 0xffffffff;
    return (this.seed & 0xfffffff) / 0x10000000;
  },
};
var kSplayTreeSize = 8000;
function GenerateKey() {
  return MyMath.random();
}
function InsertNewNode() {
  key = GenerateKey();
}
for (var i = 0; i < kSplayTreeSize; i++) InsertNewNode();

Comment 1

[Christian Holler (:decoder)]

Not sure if this can just happen with the backtracking allocator, but unaligned write could also mean we're writing somewhere where we shouldn't write.

Comment 2

[Daniel Veditz [:dveditz]]

The backtracking allocator is new, does that mean we can pin down a regression range?

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The backtracking allocator landed quite some time ago but was never turned on by default, until bug 983580 turned it on by default only for asm.js code, and only in nightly for now.

Comment 4

[Dan Gohman [:sunfish]]

This isn't an asm.js testcase though. It uses --ion-regalloc=backtracking, which has been in the tree for some time now, so it should be possible to bisect the regression (if it is a regression).

I did some basic debugging, but so far all I've learned is that the bug is very fragile. For example, I hacked codegen to emit an extra instruction in code which is never executed, and the symptoms disappeared. This suggests that something here is sensitive to code size or alignment.

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

> so it should be possible to bisect the regression (if it is a regression).

I have it on my goals to support bisecting ARM simulator builds.

> I did some basic debugging, but so far all I've learned is that the bug is
> very fragile.

I hope the testcase isn't the one that is fragile, otherwise bisection won't be reliable.

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Doug, Marty, any idea what's happening in this ARM issue?

Comment 7

[Douglas Crosher [:dougc]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #6)
> Doug, Marty, any idea what's happening in this ARM issue?

Bad branch offset.  Might be significant that it occurs directly after a constant pool.  It looks like a simple branch to OOL code, not a patched jump.  This is an Assembler buffer issue that should be addressed in the rewrite bug 972710 and it does not reproduce on the asmbuffer repo.

0xf7749950	vmov	r1, s30
0xf7749954	cmn	r1, #2147483649
0xf7749958	cmpne	r1, #2147483648
0xf774995c	b	#92                ; to 0xf77499c0 ok
<constant pool>
0xf77499c0	beq	#2344              ; from f774995c ok, to f774a2f0 X
0xf77499c4	str	r1, [r0, #24]
0xf77499c8	mvn	lr, #126
...
0xf774a2e8	push	{r0, r2, r3}
0xf774a2ec	vpush	{d0, d1, d2, d3, d4, d5, d6, d7}
0xf774a2f0	mov	r1, sp            ; from 0xf77499c0 XX
0xf774a2f4	bic	sp, sp, #7
0xf774a2f8	str	r1, [sp, #-4]!
0xf774a2fc	sub	sp, sp, #4
0xf774a300	vmov	r0, r1, d2

Comment 8

[Jan de Mooij [:jandem]]

(In reply to Douglas Crosher [:dougc] from comment #7)
> Bad branch offset.  Might be significant that it occurs directly after a
> constant pool.  It looks like a simple branch to OOL code, not a patched
> jump.  This is an Assembler buffer issue that should be addressed in the
> rewrite bug 972710 and it does not reproduce on the asmbuffer repo.

Thanks for looking into this. Do you have an ETA for bug 972710? We keep finding weird assembler buffer bugs and they all look pretty scary and potential topcrash material. Is there a simple fix for this issue we can use for current branches?

Comment 9

[Douglas Crosher [:dougc]]

(In reply to Jan de Mooij [:jandem] from comment #8)
> (In reply to Douglas Crosher [:dougc] from comment #7)
> > Bad branch offset.  Might be significant that it occurs directly after a
> > constant pool.  It looks like a simple branch to OOL code, not a patched
> > jump.  This is an Assembler buffer issue that should be addressed in the
> > rewrite bug 972710 and it does not reproduce on the asmbuffer repo.
> 
> Thanks for looking into this. Do you have an ETA for bug 972710? We keep
> finding weird assembler buffer bugs and they all look pretty scary and
> potential topcrash material.

A lot more work is needed. My hope is to have something to review in two months.

> Is there a simple fix for this issue we can use for current branches?

Not clear yet.

Comment 10

[Jan de Mooij [:jandem]]

ARM constant pool issue, assigning to Marty. Douglas, thanks for the analysis in comment 7.

Comment 11

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 951e3a671279).

Comment 12

[Christian Holler (:decoder)]

JSBugMon: Fix Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/9d8a5c8d8328
user:        Marty Rosenberg
date:        Tue May 27 09:40:35 2014 -0400
summary:     bug 996715: Remove the code that bails when determining if the second instruction in a chunk is a branch. (r=dougc)

This iteration took 523.587 seconds to run.

Comment 13

[Christian Holler (:decoder)]

Marty, does comment 12 make any sense for fixing this?

Comment 14

[Douglas Crosher [:dougc]]

(In reply to Christian Holler (:decoder) from comment #13)
> Marty, does comment 12 make any sense for fixing this?

This bug should have been fixed by the patch in bug 996715.

Comment 15

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Duping per comment 14.