649152 - TI: Crash [@ js::gc::MarkChildren]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

Attachment: shell testcase, unpack, chdir and run "js -n -m -a -f file1.js -f file2.js"

The attached testcase (unpack and run js -m -n -a -f file1.js -f file2.js) crashes on TI revision 265baede77e3, tested on 64 bit. It looks like a garbage collector problem and seems complex, the testcases are already minimized with various tools.

==7497== Invalid read of size 1
==7497==    at 0x4C35D6: js::gc::MarkChildren(JSTracer*, JSObject*) (jsgcinlines.h:393)
==7497==    by 0x4C3986: js::gc::TypedMarker(JSTracer*, JSObject*) (jsgcinlines.h:483)
==7497==    by 0x4C8A00: void js::gc::Mark<JSObject>(JSTracer*, JSObject*) (jsgcinlines.h:300)
==7497==    by 0x4C3E25: js::gc::MarkKind(JSTracer*, void*, unsigned int) (jsgcinlines.h:601)
==7497==    by 0x4C3EF8: js::gc::MarkValueRaw(JSTracer*, js::Value const&) (jsgcinlines.h:625)
==7497==    by 0x4C81E2: JSObject::markSlots(JSTracer*) (jsgc.cpp:2656)
==7497==    by 0x4C3718: js::gc::MarkChildren(JSTracer*, JSObject*) (jsgcinlines.h:418)
==7497==    by 0x4C3986: js::gc::TypedMarker(JSTracer*, JSObject*) (jsgcinlines.h:483)
==7497==    by 0x4CE937: void js::gc::Mark<JSObject_Slots8>(JSTracer*, JSObject_Slots8*) (jsgcinlines.h:300)
==7497==    by 0x4D51D0: js::gc::Arena<JSObject_Slots8>::mark(JSObject_Slots8*, JSTracer*) (jsgc.cpp:226)
==7497==    by 0x4C8FE3: js::gc::ConservativeGCTest js::MarkCell<JSObject_Slots8>(js::gc::Cell*, JSTracer*) (jsgc.cpp:592)
==7497==    by 0x4CFF44: js::MarkIfGCThingWord(JSTracer*, unsigned long, unsigned int&) (jsgc.cpp:664)
==7497==  Address 0xdadadadadadadaf7 is not stack'd, malloc'd or (recently) free'd
==7497== 
==7497== 
==7497== Process terminating with default action of signal 11 (SIGSEGV)

Comment 1

[Jeff Walden [:Waldo]]

Oops, TI-only.

Comment 2

[Brian Hackett [Laid off!]]

Hmm, I can't repro on the given changeset on OSX x64.  Since the conservative stack scanner is involved this bug will be extremely fragile and probably behave differently on different machines.  Can you paste a deeper call stack from valgrind?

Comment 3

[Christian Holler (:decoder)]

Full trace, now taken on revision a4131835b866, ran it on a Linux 64 bit:

==5165== Invalid read of size 1
==5165==    at 0x4C39EE: js::gc::MarkChildren(JSTracer*, JSObject*) (jsgcinlines.h:393)
==5165==    by 0x4C3D9E: js::gc::TypedMarker(JSTracer*, JSObject*) (jsgcinlines.h:483)
==5165==    by 0x4C8E18: void js::gc::Mark<JSObject>(JSTracer*, JSObject*) (jsgcinlines.h:300)
==5165==    by 0x4C423D: js::gc::MarkKind(JSTracer*, void*, unsigned int) (jsgcinlines.h:601)
==5165==    by 0x4C4310: js::gc::MarkValueRaw(JSTracer*, js::Value const&) (jsgcinlines.h:625)
==5165==    by 0x4C85FA: JSObject::markSlots(JSTracer*) (jsgc.cpp:2656)
==5165==    by 0x4C3B30: js::gc::MarkChildren(JSTracer*, JSObject*) (jsgcinlines.h:418)
==5165==    by 0x4C3D9E: js::gc::TypedMarker(JSTracer*, JSObject*) (jsgcinlines.h:483)
==5165==    by 0x4CED4F: void js::gc::Mark<JSObject_Slots8>(JSTracer*, JSObject_Slots8*) (jsgcinlines.h:300)
==5165==    by 0x4D55E8: js::gc::Arena<JSObject_Slots8>::mark(JSObject_Slots8*, JSTracer*) (jsgc.cpp:226)
==5165==    by 0x4C93FB: js::gc::ConservativeGCTest js::MarkCell<JSObject_Slots8>(js::gc::Cell*, JSTracer*) (jsgc.cpp:592)
==5165==    by 0x4D035C: js::MarkIfGCThingWord(JSTracer*, unsigned long, unsigned int&) (jsgc.cpp:664)
==5165==    by 0x4C5145: js::MarkWordConservatively(JSTracer*, unsigned long) (jsgc.cpp:724)
==5165==    by 0x4C5273: js::MarkRangeConservatively(JSTracer*, unsigned long const*, unsigned long const*) (jsgc.cpp:755)
==5165==    by 0x4C5326: js::MarkThreadDataConservatively(JSTracer*, js::ThreadData*) (jsgc.cpp:772)
==5165==    by 0x4C53C8: js::MarkConservativeStackRoots(JSTracer*) (jsgc.cpp:811)
==5165==    by 0x4C6DF7: js::MarkRuntime(JSTracer*) (jsgc.cpp:1648)
==5165==    by 0x4C7C99: MarkAndSweep(JSContext*, JSCompartment*, JSGCInvocationKind) (jsgc.cpp:2207)
==5165==    by 0x4C82AB: GCUntilDone(JSContext*, JSCompartment*, JSGCInvocationKind) (jsgc.cpp:2558)
==5165==    by 0x4C848F: js_GC(JSContext*, JSCompartment*, JSGCInvocationKind) (jsgc.cpp:2627)
==5165==    by 0x4C5F6E: RunLastDitchGC(JSContext*) (jsgc.cpp:1124)
==5165==    by 0x4D38BF: bool RefillTypedFreeList<js::Shape>(JSContext*, unsigned int) (jsgc.cpp:1144)
==5165==    by 0x4C60BE: RefillFinalizableFreeList(JSContext*, unsigned int) (jsgc.cpp:1209)
==5165==    by 0x58823B: js::Shape* NewFinalizableGCThing<js::Shape>(JSContext*, unsigned int) (jsgcinlines.h:190)
==5165==    by 0x587CCE: js_NewGCShape(JSContext*) (jsgcinlines.h:245)
==5165==    by 0x585777: js::PropertyTree::newShape(JSContext*) (jspropertytree.cpp:70)
==5165==    by 0x585D47: js::PropertyTree::getChild(JSContext*, js::Shape*, js::Shape const&) (jspropertytree.cpp:182)
==5165==    by 0x5A66D5: JSObject::getChildProperty(JSContext*, js::Shape*, js::Shape&) (jsscope.cpp:495)
==5165==    by 0x5A753B: JSObject::addPropertyInternal(JSContext*, jsid, int (*)(JSContext*, JSObject*, jsid, js::Value*), int (*)(JSContext*, JSObject*, jsid, int, js::Value*), un
signed int, unsigned int, unsigned int, int, js::Shape**) (jsscope.cpp:750)
==5165==    by 0x5A784D: JSObject::putProperty(JSContext*, jsid, int (*)(JSContext*, JSObject*, jsid, js::Value*), int (*)(JSContext*, JSObject*, jsid, int, js::Value*), unsigned i
nt, unsigned int, unsigned int, int) (jsscope.cpp:837)
==5165==    by 0x52839E: js_SetPropertyHelper(JSContext*, JSObject*, jsid, unsigned int, js::Value*, int) (jsobj.cpp:5961)
==5165==    by 0x77DC2F: js::Interpret(JSContext*, JSStackFrame*, unsigned int, JSInterpMode) (jsinterp.cpp:4580)
==5165==    by 0x4FB28C: js::RunScript(JSContext*, JSScript*, JSStackFrame*) (jsinterp.cpp:681)
==5165==    by 0x4FC86F: js::Execute(JSContext*, JSObject*, JSScript*, JSStackFrame*, unsigned int, js::Value*) (jsinterp.cpp:1071)
==5165==    by 0x435585: JS_ExecuteScript (jsapi.cpp:5226)
==5165==    by 0x405A15: Process(JSContext*, JSObject*, char*, int, int) (js.cpp:451)
==5165==    by 0x406840: ProcessArgs(JSContext*, JSObject*, char**, int) (js.cpp:889)
==5165==    by 0x4119AC: Shell(JSContext*, int, char**, char**) (js.cpp:5858)
==5165==    by 0x411C26: main (js.cpp:5980)
==5165==  Address 0xdadadadadadadaf7 is not stack'd, malloc'd or (recently) free'd

Comment 4

[Christian Holler (:decoder)]

Seems unrelated to the similar TM bug 643839, as that bug was opt-only, this one is in debug mode. But I can still reproduce this on dca50d9a5047.

Comment 5

[Christian Holler (:decoder)]

Brian, can you reproduce this by now, or do you need more information?

Comment 6

[Brian Hackett [Laid off!]]

Ooooh, just tried again and I can repro now.  Hopefully can fix this today.

Comment 7

[Brian Hackett [Laid off!]]

This is a GC hazard introduced by the patch in bug 648321 (up for review on TM, but not pushed there yet).  During dense array slowification, that patch changes the array to a slow array before making the data properties, rather than after (necessary because slot organization is different between dense arrays and other objects).  The slow array only roots the values in slots up to its slotSpan though, so if a GC happens during slowification then dense elements which properties haven't been made for yet are at risk of collection.

This fix just roots the whole dense slot array during makeDenseArraySlow.

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

Comment 8

[Sheila Mooney]

Did this get ever get checked into the trunk? Can you confirm?

Comment 9

[Josh Matthews [:jdm]]

This is a fix for a feature that has not merged to trunk yet.