648773 - TI+JM: crash [@js::gc::MarkChildren]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

There's a new GC-related crash on the JM branch. Bisecting points to this revision: http://hg.mozilla.org/projects/jaegermonkey/rev/d3215d1e985a

It crashes because there's an array with invalid obj->newType field:
--
(gdb) p obj->newType
$2 = (class js::types::TypeObject *) 0x4

(gdb) p js_DumpObject(obj)
object 0xd219d8
class 0x3f76e0 Array
flags: indexed
proto <Array object at 0xd040f0>
parent <global object at 0xd020a8>
private 0x4
properties:
    ((Shape *) 0x79d4f50) permanent shared getterOp=0x4362d setterOp=0x49a85 "length": slot -1
    ((Shape *) 0x79d4f78) enumerate 0: slot 0 = <Array object at 0xd2a750>
    ((Shape *) 0x79d4fa0) enumerate 1: slot 1 = <Array object at 0xd2a7b8>
    ((Shape *) 0x79d4fc8) enumerate 2: slot 2 = <Array object at 0xd2a820>
--
It's not always 0x4, I will attach a Valgrind log where it's 0x15.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Valgrind and GDB output

Comment 2

[Jan de Mooij [:jandem]]

(In reply to comment #0)
> It's not always 0x4, I will attach a Valgrind log where it's 0x15.

Never mind, obj->newType is 0x4 and obj->newType->marked is 0x15.

Testcase is complex, deeply nested objects/arrays. decoder will use his grammar-based reducer to minimize it further and attach it here.

Comment 3

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

(In reply to comment #2)
> Testcase is complex, deeply nested objects/arrays. decoder will use his
> grammar-based reducer to minimize it further and attach it here.

I might have seen this crash lying around though it seems to be mostly unreproducible on my side.

Comment 4

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

See bug 643839 which may or may not be related.

Comment 5

[Christian Holler (:decoder)]

I'm working on the minimization but because the test is long running, it's going to take a while :)

Comment 6

[Christian Holler (:decoder)]

By adding gczeal(2) to the test, I was able to reduce the runtime of the test from 8 seconds to 1 which greatly helped the reducing. Line based reduction got it down from initially over 200 kb to 11 kb of code, LangDDMin afterwards reduced it to these lovely 187 bytes:

gczeal(2);
for (var loopa2 = 0; loopa2 < 13; ) 
        [, , , , , , ][Float64Array()] = 72413.8139177333;
function a24(a25) {}
if ([, , , , , , , ]) {} else;
function a124(a125, a126, a127) {}

Comment 7

[Christian Holler (:decoder)]

Okay, the last three lines are not even relevant after taking a closer look. I will check why they were not removed by the algorithm, but this is sufficient:

gczeal(2);
for (var loopa2 = 0; loopa2 < 13; ) 
        [, , , , , , ][Float64Array()] = 72413.8139177333;

Comment 8

[Brian Hackett [Laid off!]]

Great reduction!

This bug was introduced when applying the patch in bug 648321 to JM.  When slowifying dense arrays, we cleared the initialized length well after changing the object's class, violating the invariant that the initialized length is used iff an object is a dense array.  Since newType and initializedLength are in a union, this broke a GC occurring during slowification as it interpreted the mangled newType as a pointer.

http://hg.mozilla.org/projects/jaegermonkey/rev/b6d65a4eb2b3

Comment 9

[Sheila Mooney]

Did this actually get put in the trunk? If so, when?

Comment 10

[Brian Hackett [Laid off!]]

(In reply to comment #9)
> Did this actually get put in the trunk? If so, when?

This bug (and all others blocking bug 619415) are in the jaegermonkey branch, which has not merged into the tracemonkey or mozilla-central branches yet.

Comment 11

[Sheila Mooney]

Ok, thanks. We are seeing an increase in this signature since 4.0 that we can't explain. It's been around for a while but something caused it to regress.