936966 - Assertion failure: !aheader->hasFreeThings(), at jsgc.h:528 Assertion failure: !(addr & ArenaMask), at ../gc/Heap.h:847 or Crash [@ GetGCThingMarkBitmap]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 16949049f03d (run with --fuzzing-safe):


function foo() {
    bar(1,2,3,4,5,6,7,8,9);
}
function bar(... name) {
    foo();
}
foo();

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Andrew McCreight [:mccr8]]

Some of those sound memory corruption-y.

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, failed due to error (try manually).

Comment 4

[Terrence Cole [:terrence]]

(In reply to Andrew McCreight [:mccr8] from comment #2)
> Some of those sound memory corruption-y.

You have the right of it. Here is the backtrace where it's getting stomped on:

Old value = 0xf5956000
New value = 0x8
0x00007ffff7fd9150 in ?? ()
(gdb) bt
#0  0x00007ffff7fd9150 in ?? ()
#1  0x00000000010cb930 in ?? ()
#2  0x00007ffff7fd91a8 in ?? ()
#3  0x0000000000000080 in ?? ()
#4  0x00007ffff5954b00 in ?? ()
#5  0x0000000000000000 in ?? ()

The jitcode here is:

   0x7ffff7fd9000:	adc    %al,%bl
   0x7ffff7fd9002:	xchg   %eax,%ebp
   0x7ffff7fd9003:	cmc    
   0x7ffff7fd9004:	(bad)  
   0x7ffff7fd9005:	jg     0x7ffff7fd9007
   0x7ffff7fd9007:	add    %cl,-0x7d(%rax)
   0x7ffff7fd900a:	in     (%dx),%al
   0x7ffff7fd900b:	or     %cl,-0x75(%rax)
   0x7ffff7fd900e:	rex.R and $0x28,%al
   0x7ffff7fd9011:	shr    $0x2f,%rax
   0x7ffff7fd9015:	cmp    $0x1fff2,%eax
   0x7ffff7fd901b:	jne    0x7ffff7fd91eb
   0x7ffff7fd9021:	add    $0x8,%rsp
   0x7ffff7fd9025:	sub    $0x8,%rsp
   0x7ffff7fd9029:	mov    0x28(%rsp),%rax
   0x7ffff7fd902e:	shr    $0x2f,%rax
   0x7ffff7fd9032:	cmp    $0x1fff2,%eax
   0x7ffff7fd9038:	jne    0x7ffff7fd9043
   0x7ffff7fd903e:	jmpq   0x7ffff7fd9044
   0x7ffff7fd9043:	int3   
   0x7ffff7fd9044:	add    $0x8,%rsp
   0x7ffff7fd9048:	sub    $0x8,%rsp
   0x7ffff7fd904c:	cmp    %rsp,0xfe0838
   0x7ffff7fd9054:	jae    0x7ffff7fd91f5
   0x7ffff7fd905a:	cmpl   $0x0,0xfe14a0
   0x7ffff7fd9062:	jne    0x7ffff7fd9204
   0x7ffff7fd9068:	movabs 0x10995c8,%rax
   0x7ffff7fd9072:	cmp    %rax,0x10995d0
   0x7ffff7fd907a:	jbe    0x7ffff7fd9204
   0x7ffff7fd9080:	add    $0x60,%rax
   0x7ffff7fd9084:	movabs %rax,0x10995c8
   0x7ffff7fd908e:	sub    $0x60,%rax
   0x7ffff7fd9092:	movabs $0x7ffff5959178,%r11
   0x7ffff7fd909c:	mov    %r11,(%rax)
   0x7ffff7fd909f:	movabs $0x7ffff593ab30,%r11
   0x7ffff7fd90a9:	mov    %r11,0x8(%rax)
   0x7ffff7fd90ad:	movq   $0x0,0x10(%rax)
   0x7ffff7fd90b5:	add    $0x30,%rax
   0x7ffff7fd90b9:	mov    %rax,-0x18(%rax)
   0x7ffff7fd90bd:	add    $0xffffffffffffffd0,%rax
   0x7ffff7fd90c1:	movl   $0x6,0x28(%rax)
   0x7ffff7fd90c8:	movl   $0x0,0x24(%rax)
   0x7ffff7fd90cf:	movl   $0x0,0x2c(%rax)
   0x7ffff7fd90d6:	movl   $0x0,0x20(%rax)
   0x7ffff7fd90dd:	mov    0x18(%rax),%rax
   0x7ffff7fd90e1:	movabs $0xfff8800000000001,%r11
   0x7ffff7fd90eb:	mov    %r11,(%rax)
   0x7ffff7fd90ee:	movabs $0xfff8800000000002,%r11
   0x7ffff7fd90f8:	mov    %r11,0x8(%rax)
   0x7ffff7fd90fc:	movabs $0xfff8800000000003,%r11
   0x7ffff7fd9106:	mov    %r11,0x10(%rax)
   0x7ffff7fd910a:	movabs $0xfff8800000000004,%r11
   0x7ffff7fd9114:	mov    %r11,0x18(%rax)
   0x7ffff7fd9118:	movabs $0xfff8800000000005,%r11
   0x7ffff7fd9122:	mov    %r11,0x20(%rax)
   0x7ffff7fd9126:	movabs $0xfff8800000000006,%r11
   0x7ffff7fd9130:	mov    %r11,0x28(%rax)
   0x7ffff7fd9134:	movabs $0xfff8800000000007,%r11
   0x7ffff7fd913e:	mov    %r11,0x30(%rax)
   0x7ffff7fd9142:	movabs $0xfff8800000000008,%r11
   0x7ffff7fd914c:	mov    %r11,0x38(%rax)
=> 0x7ffff7fd9150:	movabs $0xfff8800000000009,%r11
   0x7ffff7fd915a:	mov    %r11,0x40(%rax)
   0x7ffff7fd915e:	movl   $0x9,-0xc(%rax)
   0x7ffff7fd9165:	movabs $0xfff9000000000000,%r11
   0x7ffff7fd916f:	mov    %r11,(%rsp)
   0x7ffff7fd9173:	movabs $0x7ffff5954b00,%rax
   0x7ffff7fd917d:	testl  $0x10000,0x20(%rax)
   0x7ffff7fd9184:	je     0x7ffff7fd91b1
   0x7ffff7fd918a:	mov    0x28(%rax),%rbx
   0x7ffff7fd918e:	mov    0x78(%rbx),%rbx
   0x7ffff7fd9192:	test   %rbx,%rbx
   0x7ffff7fd9195:	je     0x7ffff7fd91b1
   0x7ffff7fd919b:	pushq  $0x0
   0x7ffff7fd91a0:	push   %rax
   0x7ffff7fd91a1:	pushq  $0x80
   0x7ffff7fd91a6:	callq  *%rbx
   0x7ffff7fd91a8:	add    $0x18,%rsp
   0x7ffff7fd91ac:	jmpq   0x7ffff7fd91c2
   0x7ffff7fd91b1:	push   %rsp
   0x7ffff7fd91b2:	pushq  $0x0
   0x7ffff7fd91b7:	push   %rax
   0x7ffff7fd91b8:	pushq  $0x200
   0x7ffff7fd91bd:	callq  0x7ffff7ff0b98
   0x7ffff7fd91c2:	movabs $0xfff9000000000000,%rcx
   0x7ffff7fd91cc:	add    $0x8,%rsp
   0x7ffff7fd91d0:	retq   
   0x7ffff7fd91d1:	nop
   0x7ffff7fd91d2:	nop
   0x7ffff7fd91d3:	nop
   0x7ffff7fd91d4:	nop
   0x7ffff7fd91d5:	nop
   0x7ffff7fd91d6:	nop
   0x7ffff7fd91d7:	nop
   0x7ffff7fd91d8:	nop

Comment 5

[Terrence Cole [:terrence]]

"It" in this case is |ArenaHeader::next|.

Comment 6

[Jan de Mooij [:jandem]]

Brian, could this be more JSOP_REST fallout?

Comment 7

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

Likely, as autoBisect points out:

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/3f88f1e41372
user:        Brian Hackett
date:        Tue Nov 05 17:54:29 2013 -0800
summary:     Bug 935027 - Don't create 'rest' template objects in IonBuilder, r=jandem.

Comment 8

[Brian Hackett [Laid off!]]

Attachment: patch

This is a preexisting issue exposed by bug 935027.  MNewArray's code generation couldn't cope with the template object not having the optimal amount of inline space for the number of elements in the array.

Comment 9

[Christian Holler (:decoder)]

Can we land this? It's causing lots of trouble during fuzzing.

Comment 10

[Brian Hackett [Laid off!]]

The tree has been closed pretty much continuously since I wrote this patch:

https://hg.mozilla.org/integration/mozilla-inbound/rev/f2c02e984744

Comment 11

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

The tracking-firefox28+ flag was probably accidentally cleared.

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f2c02e984744

Comment 13

[Christian Holler (:decoder)]

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

Comment 14

[Christian Holler (:decoder)]

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

Comment 15

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

Oh, I know. Ever since comment 12, this should be marked FIXED, then JSBugMon can work its magic...

Comment 16

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 17

[Jeff Walden [:Waldo]]

I think this should be backported.  The testcase here only involves rest arrays, which landed same cycle as this fix.  But the underlying issue affects MNewArray more generally -- and it's existed since bug 922270 landed, which occurred during what's currently beta.

Bug 937793 is one instance where lack of this patch converts a mistake into something probably exploitable.  Could we get this backported in case there are any other lurking issues like that?  (I plan to backport bug 937793 in any case, but I'm not really comfortable assuming that's the only instance.)

Comment 18

[Jeff Walden [:Waldo]]

Comment on attachment 831676 [details] [diff] [review]
patch

Generally see comment 17.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 922270 for how I observed it (the code dates to bug 807853, landed unused -- not clear when it first became used)
User impact if declined: ongoing existence of dodgy math that, if invoked, can cause things to go awry in rather-unclear ways
Testing completed (on m-c, etc.): already on trunk, already on aurora from an earlier uplift -- just needs to land on beta for completeness
Risk to taking this patch (and alternatives if risky): pretty low, this code's obviously more self-consistent than it was before, which just *makes sense*
String or IDL/UUID changes made by this patch: N/A

Comment 19

[Lukas Blakk [:lsblakk] use ?needinfo]

Updating status for 27 then, if it's affected until this lands.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/77e29fc57c6b