1370069 - Crash [@ js::GCMarker::eagerlyMarkChildren] and various other crashes and assertions involving gczeal(17)

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 130efc657df7 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

try {
    eval("}");
} catch (exc) {}
gczeal(17, 1);
6.900653737167637, (yield);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
js::GCMarker::eagerlyMarkChildren (this=0x7ffff695f180, rope=0x7ffff46a8f60) at js/src/gc/Marking.cpp:1194
#0  js::GCMarker::eagerlyMarkChildren (this=0x7ffff695f180, rope=0x7ffff46a8f60) at js/src/gc/Marking.cpp:1194
#1  0x0000000000bd32ef in js::SavedFrame::Lookup::trace (trc=0x7ffff695f180, this=<optimized out>) at js/src/vm/SavedStacks.cpp:199
#2  js::SavedFrame::AutoLookupVector::trace (this=0x7fffffffbe20, trc=0x7ffff695f180) at js/src/vm/SavedStacks.cpp:227
#3  0x0000000000e347e5 in JS::AutoGCRooter::trace (trc=0x7ffff695f180, this=0x7fffffffbe28) at js/src/gc/RootMarking.cpp:196
#4  JS::AutoGCRooter::traceAll (target=..., trc=trc@entry=0x7ffff695f180) at js/src/gc/RootMarking.cpp:209
#5  0x0000000000e3ee41 in js::gc::GCRuntime::traceRuntimeCommon (this=this@entry=0x7ffff695e5f8, trc=trc@entry=0x7ffff695f180, traceOrMark=traceOrMark@entry=js::gc::GCRuntime::MarkRuntime, lock=...) at js/src/gc/RootMarking.cpp:335
#6  0x0000000000e3f3ec in js::gc::GCRuntime::traceRuntimeForMajorGC (this=this@entry=0x7ffff695e5f8, trc=trc@entry=0x7ffff695f180, lock=...) at js/src/gc/RootMarking.cpp:266
#7  0x00000000009c20f4 in js::gc::GCRuntime::beginMarkPhase (this=this@entry=0x7ffff695e5f8, reason=reason@entry=JS::gcreason::DEBUG_GC, lock=...) at js/src/jsgc.cpp:3982
#8  0x00000000009d831e in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0x7ffff695e5f8, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC, lock=...) at js/src/jsgc.cpp:6183
#9  0x00000000009d9914 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff695e5f8, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6543
#10 0x00000000009da208 in js::gc::GCRuntime::collect (this=this@entry=0x7ffff695e5f8, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6692
#11 0x00000000009db24c in js::gc::GCRuntime::runDebugGC (this=this@entry=0x7ffff695e5f8) at js/src/jsgc.cpp:7229
#12 0x0000000000d2bc95 in js::gc::GCRuntime::gcIfNeededAtAllocation (this=0x7ffff695e5f8, cx=cx@entry=0x7ffff6924000) at js/src/gc/Allocator.cpp:230
#13 0x0000000000d39ae8 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=<optimized out>, cx=0x7ffff6924000, kind=js::gc::AllocKind::SHAPE) at js/src/gc/Allocator.cpp:191
#14 0x0000000000d3d11a in js::Allocate<js::Shape, (js::AllowGC)1> (cx=cx@entry=0x7ffff6924000) at js/src/gc/Allocator.cpp:142
#15 0x0000000000be3019 in js::EmptyShape::new_ (cx=cx@entry=0x7ffff6924000, base=..., base@entry=..., nfixed=nfixed@entry=7) at js/src/vm/Shape.cpp:1368
#16 0x0000000000c09a8e in js::EmptyShape::getInitialShape (cx=cx@entry=0x7ffff6924000, clasp=clasp@entry=0x1e9f720 <js::SavedFrame::class_>, proto=..., nfixed=<optimized out>, objectFlags=0) at js/src/vm/Shape.cpp:1473
#17 0x000000000099f354 in NewObject (cx=0x7ffff6924000, group=..., kind=js::gc::AllocKind::OBJECT8, newKind=js::TenuredObject, initialShapeFlags=<optimized out>) at js/src/jsobj.cpp:663
#18 0x000000000099f774 in js::NewObjectWithGivenTaggedProto (cx=cx@entry=0x7ffff6924000, clasp=clasp@entry=0x1e9f720 <js::SavedFrame::class_>, proto=..., allocKind=js::gc::AllocKind::OBJECT8, newKind=newKind@entry=js::TenuredObject, initialShapeFlags=initialShapeFlags@entry=0) at js/src/jsobj.cpp:733
#19 0x0000000000bbc785 in js::NewObjectWithGivenTaggedProto (initialShapeFlags=0, newKind=js::TenuredObject, proto=..., clasp=0x1e9f720 <js::SavedFrame::class_>, cx=0x7ffff6924000) at js/src/jsobjinlines.h:627
#20 js::NewObjectWithGivenProto (newKind=js::TenuredObject, proto=..., clasp=0x1e9f720 <js::SavedFrame::class_>, cx=0x7ffff6924000) at js/src/jsobjinlines.h:662
#21 js::SavedFrame::create (cx=cx@entry=0x7ffff6924000) at js/src/vm/SavedStacks.cpp:532
#22 0x0000000000bd044e in js::SavedStacks::createFrameFromLookup (this=this@entry=0x7ffff692a8f8, cx=cx@entry=0x7ffff6924000, lookup=..., lookup@entry=...) at js/src/vm/SavedStacks.cpp:1517
#23 0x0000000000bd05be in js::SavedStacks::getOrCreateSavedFrame (this=this@entry=0x7ffff692a8f8, cx=cx@entry=0x7ffff6924000, lookup=lookup@entry=...) at js/src/vm/SavedStacks.cpp:1504
#24 0x0000000000bd1ac7 in js::SavedStacks::insertFrames(JSContext*, js::FrameIter&, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=this@entry=0x7ffff692a8f8, cx=cx@entry=0x7ffff6924000, iter=..., frame=..., frame@entry=..., capture=capture@entry=<unknown type in /mnt/LangFuzz/work/builds/debug64/dist/bin/js, CU 0x4e14359, DIE 0x503529e>) at js/src/vm/SavedStacks.cpp:1410
#25 0x0000000000bd21d7 in js::SavedStacks::saveCurrentStack(JSContext*, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=0x7ffff692a8f8, cx=cx@entry=0x7ffff6924000, frame=frame@entry=..., capture=capture@entry=<unknown type in /mnt/LangFuzz/work/builds/debug64/dist/bin/js, CU 0x4e14359, DIE 0x503597c>) at js/src/vm/SavedStacks.cpp:1177
#26 0x00000000008e8468 in JS::CaptureCurrentStack(JSContext*, JS::MutableHandle<JSObject*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (cx=0x7ffff6924000, stackp=..., capture=capture@entry=<>) at js/src/jsapi.cpp:7192
#27 0x000000000094dbdb in CaptureStack (cx=<optimized out>, stack=...) at js/src/jsexn.cpp:377
#28 0x000000000095e82b in js::ErrorToException (cx=0x7ffff6924000, reportp=0x7fffffffc980, callback=<optimized out>, userRef=<optimized out>) at js/src/jsexn.cpp:695
#29 0x0000000000962473 in js::ReportErrorNumberVA (cx=cx@entry=0x7ffff6924000, flags=flags@entry=0, callback=callback@entry=0x94d890 <js::GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0, errorNumber=errorNumber@entry=1, argumentsType=argumentsType@entry=js::ArgumentsAreLatin1, ap=0x7fffffffca60) at js/src/jscntxt.cpp:905
#30 0x00000000008e4568 in JS_ReportErrorNumberLatin1VA (cx=0x7ffff6924000, errorCallback=0x94d890 <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=1, ap=ap@entry=0x7fffffffca60) at js/src/jsapi.cpp:5894
#31 0x00000000008e4618 in JS_ReportErrorNumberLatin1 (cx=cx@entry=0x7ffff6924000, errorCallback=errorCallback@entry=0x94d890 <js::GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0, errorNumber=errorNumber@entry=1) at js/src/jsapi.cpp:5883
#32 0x00000000009535e5 in js::ReportIsNotDefined (cx=cx@entry=0x7ffff6924000, id=id@entry=...) at js/src/jscntxt.cpp:956
#33 0x000000000095964e in js::ReportIsNotDefined (cx=0x7ffff6924000, name=..., name@entry=...) at js/src/jscntxt.cpp:965
#34 0x000000000053e760 in js::FetchName<(js::GetNameMode)0> (cx=0x7ffff6924000, receiver=..., holder=..., name=..., prop=..., vp=...) at js/src/vm/Interpreter-inl.h:187
#35 0x000000000052f9be in js::GetEnvironmentName<(js::GetNameMode)0> (vp=..., name=..., envChain=..., cx=<optimized out>) at js/src/vm/Interpreter-inl.h:257
#36 GetNameOperation (vp=..., pc=<optimized out>, fp=<optimized out>, cx=<optimized out>) at js/src/vm/Interpreter.cpp:218
[...]
#47 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8511
rax	0x1	1
rbx	0x7ffff46a8f60	140737294012256
rcx	0x0	0
rdx	0x100000000000	17592186044416
rsi	0x0	0
rdi	0x7ffff46fc0a0	140737294352544
rbp	0x7fffffffaf90	140737488334736
rsp	0x7fffffffac00	140737488333824
r8	0x7fffffffabc0	140737488333760
r9	0x7fffffffb008	140737488334856
r10	0x7ffff6925000	140737330171904
r11	0x246	582
r12	0x11e63e0	18768864
r13	0x1	1
r14	0x7ffff695f180	140737330409856
r15	0xfffe4b4b4b4b4b4b	-480163195565237
rip	0xe1b376 <js::GCMarker::eagerlyMarkChildren(JSRope*)+198>
=> 0xe1b376 <js::GCMarker::eagerlyMarkChildren(JSRope*)+198>:	mov    (%r15),%eax
   0xe1b379 <js::GCMarker::eagerlyMarkChildren(JSRope*)+201>:	and    $0x28,%eax



Marking s-s due to GC issue. This is a frequent issue, triggering tons of different crashes, therefore marking as fuzzblocker. Needs quick fixing to get fuzzing running again.

Comment 1

[Fuzzing Team]

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

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/98b894115e89
user:        Jon Coppeard
date:        Fri Jun 02 10:32:37 2017 +0100
summary:     Bug 1369444 - Sweep the atoms table incrementally r=sfink

This iteration took 272.193 seconds to run.

Comment 2

[Christian Holler (:decoder)]

Putting a needinfo on :jonco and :sfink so we can get this fixed asap. Thanks!

Comment 3

[Jon Coppeard (:jonco)]

This has turned up a few problems in incremental atom sweeping.  Patch to follow.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: bug1370069-atom-sweeping

There were a whole bunch of things wrong with this.  Here's a patch to:

1. Create the extra atoms table before we return to the mutator, rather than when we start sweeping.  This was the main problem and meant we didn't check for dying atoms when reading the table during sweeping.

2. Call budget.step() while sweeping, which out which we don't actually sweep incrementally.

3. Fix a bunch of places where we need to check the extra atoms table if it exists.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6bb91fea7dba85dbe9f18efac690c1c0e700374&group_state=expanded&selectedJob=104565995

Comment 5

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8874474 [details] [diff] [review]
bug1370069-atom-sweeping

Review of attachment 8874474 [details] [diff] [review]:
-----------------------------------------------------------------

Ouch. Sorry, I should have caught at least some of this.

::: js/src/jsatom.cpp
@@ +261,5 @@
> +LookupAtomState(JSRuntime* rt, const AtomHasher::Lookup& lookup)
> +{
> +    MOZ_ASSERT(rt->currentThreadHasExclusiveAccess());
> +
> +    AtomSet::Ptr p = rt->unsafeAtoms().lookup(lookup); // Safe because we hold the lock.

You don't want to pass in a const AutoLockForExclusiveAccess& here? It looks like it's always available.

Comment 6

[Jon Coppeard (:jonco)]

(In reply to Steve Fink [:sfink] [:s:] from comment #5)
> Sorry, I should have caught at least some of this.

Yeah I feel like I should have tested this more thoroughly too.

> You don't want to pass in a const AutoLockForExclusiveAccess& here?

It's not available in AtomIsPinnedInRuntime.

Comment 7

[Jon Coppeard (:jonco)]

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

Comment 8

[Steve Fink [:sfink] [:s:]]

(In reply to Jon Coppeard (:jonco) from comment #6)
> (In reply to Steve Fink [:sfink] [:s:] from comment #5)
> > Sorry, I should have caught at least some of this.
> 
> Yeah I feel like I should have tested this more thoroughly too.
> 
> > You don't want to pass in a const AutoLockForExclusiveAccess& here?
> 
> It's not available in AtomIsPinnedInRuntime.

Oh! You're right, I misread that. You could almost get it by conditionally using rt->exclusiveAccessLock, but even that might not be held if (!hasHelperThreadZones() && activeThreadHasExclusiveAccess), I guess.

Never mind, then.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 10

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.