Bug 206166 – scanSideState scans too much side state

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Samuel Groß]

The following sample sometimes crashes JSC built from current HEAD:

    var v2 = 2190736854 + 1;
    var v3 = v2 + Object;
    var v4 = v3;
    var v7 = 0;
    do {
        print(v7);
        function v8(v9,v10,v11,v12,v13) {
            try {
                var v14 = v13();
            } catch(v16) {
            }
            var v18 = gc();
            if (v18) {
            } else {
            }
            var v20 = v9;
            do {
                var v21 = v20 + 1;
                v20 = v21;
            } while (v20 < 4);
        }
        var v24 = new Int32Array();
        var v25 = v8(...v4);
        for (var v29 = -1024; v29 < 100; v29++) {
        }
        var v30 = v7 + 1;
        var v32 = 4294967295;
        v7 = v30;
    } while (v7 < 10000);
    var v36 = 0;
    var v38 = 16;
    gc();
    print("Try again...");

    /*
    > lldb -- ./JSCBuild/Debug/bin/jsc --validateOptions=true --useConcurrentJIT=false --useConcurrentGC=false --thresholdForJITSoon=10 --thresholdForJITAfterWarmUp=10 --thresholdForOptimizeAfterWarmUp=100 --thresholdForOptimizeAfterLongWarmUp=100 --thresholdForOptimizeAfterLongWarmUp=100 --thresholdForFTLOptimizeAfterWarmUp=1000 --thresholdForFTLOptimizeSoon=1000 --gcAtEnd=true crash.js
    ...
    thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10a700000)
        frame #0: 0x000000010115d996 JavaScriptCore`void JSC::ConservativeRoots::genericAddSpan<JSC::DummyMarkHook>(this=0x00007ffeefbfc040, begin=0x000000010a6fff08, end=0x000000010a700008, markHook=0x00007ffeefbfbef0) at ConservativeRoots.c
    pp:104:27
       101      HeapVersion markingVersion = m_heap.objectSpace().markingVersion();
       102      HeapVersion newlyAllocatedVersion = m_heap.objectSpace().newlyAllocatedVersion();
       103      for (char** it = static_cast<char**>(begin); it != static_cast<char**>(end); ++it)
    -> 104          genericAddPointer(*it, markingVersion, newlyAllocatedVersion, filter, markHook);
       105  }
       106
       107  class DummyMarkHook {
    Target 0: (jsc) stopped.
    (lldb) bt
    * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10a700000)
      * frame #0: 0x000000010115d996 JavaScriptCore`void JSC::ConservativeRoots::genericAddSpan<JSC::DummyMarkHook>(this=0x00007ffeefbfc040, begin=0x000000010a6fff08, end=0x000000010a700008, markHook=0x00007ffeefbfbef0) at ConservativeRoots.cpp:104:27
        frame #1: 0x000000010115d819 JavaScriptCore`JSC::ConservativeRoots::add(this=0x00007ffeefbfc040, begin=0x000000010a6fff08, end=0x000000010a700008) at ConservativeRoots.cpp:116:5
        frame #2: 0x00000001019fe2de JavaScriptCore`JSC::VM::scanSideState(this=0x000000010a000000, roots=0x00007ffeefbfc040) const at VM.cpp:1068:15
        frame #3: 0x000000010116c202 JavaScriptCore`JSC::Heap::gatherScratchBufferRoots(this=0x000000010a000040, roots=0x00007ffeefbfc040) at Heap.cpp:724:10
        frame #4: 0x00000001011a2810 JavaScriptCore`JSC::Heap::addCoreConstraints(this=0x0000000107ec02c0, slotVisitor=0x0000000107ef2000)::$_30::operator()(JSC::SlotVisitor&) at Heap.cpp:2736:17
        frame #5: 0x00000001011a26f1 JavaScriptCore`WTF::Detail::CallableWrapper<JSC::Heap::addCoreConstraints()::$_30, void, JSC::SlotVisitor&>::call(this=0x0000000107ec02b8, in=0x0000000107ef2000) at Function.h:52:39
        frame #6: 0x00000001012007d7 JavaScriptCore`WTF::Function<void (JSC::SlotVisitor&)>::operator(this=0x0000000107ec1568, in=0x0000000107ef2000)(JSC::SlotVisitor&) const at Function.h:84:35
        frame #7: 0x000000010120072c JavaScriptCore`JSC::SimpleMarkingConstraint::executeImpl(this=0x0000000107ec1540, visitor=0x0000000107ef2000) at SimpleMarkingConstraint.cpp:47:5
        frame #8: 0x00000001011ea91c JavaScriptCore`JSC::MarkingConstraint::execute(this=0x0000000107ec1540, visitor=0x0000000107ef2000) at MarkingConstraint.cpp:57:5
        frame #9: 0x00000001011ec2a0 JavaScriptCore`JSC::MarkingConstraintSolver::runExecutionThread(this=0x00007ffeefbfc8e0, visitor=0x0000000107ef2000, preference=NextConstraintFirst, pickNext=ScopedLambda<WTF::Optional<unsigned int> ()> @ 0x00007ffeefbfc670)>) at MarkingConstraintSolver.cpp:239:25
        frame #10: 0x00000001011fd474 JavaScriptCore`JSC::MarkingConstraintSolver::execute(this=0x000000010a6fffb0, visitor=0x0000000107ef2000)>)::$_31::operator()(JSC::SlotVisitor&) const at MarkingConstraintSolver.cpp:68:42
        frame #11: 0x00000001011fd401 JavaScriptCore`WTF::SharedTaskFunctor<void (JSC::SlotVisitor&), JSC::MarkingConstraintSolver::execute(JSC::MarkingConstraintSolver::SchedulerPreference, WTF::ScopedLambda<WTF::Optional<unsigned int> ()>)::$_31>::run(this=0x000000010a6fffa0, arguments=0x0000000107ef2000) at SharedTask.h:91:16
        frame #12: 0x0000000101177bea JavaScriptCore`JSC::Heap::runTaskInParallel(this=0x000000010a000040, task=RefPtr<WTF::SharedTask<void (JSC::SlotVisitor &)>, WTF::DumbPtrTraits<WTF::SharedTask<void (JSC::SlotVisitor &)> > > @ 0x00007ffeefbfc768)>, WTF::DumbPtrTraits<WTF::SharedTask<void (JSC::SlotVisitor&)> > >) at Heap.cpp:3023:11
        frame #13: 0x00000001011ec052 JavaScriptCore`void JSC::Heap::runFunctionInParallel<JSC::MarkingConstraintSolver::execute(JSC::MarkingConstraintSolver::SchedulerPreference, WTF::ScopedLambda<WTF::Optional<unsigned int> ()>)::$_31>(this=0x000000010a000040, func=0x00007ffeefbfc7d8)>)::$_31 const&) at Heap.h:390:9
        frame #14: 0x00000001011ebdc2 JavaScriptCore`JSC::MarkingConstraintSolver::execute(this=0x00007ffeefbfc8e0, preference=NextConstraintFirst, pickNext=ScopedLambda<WTF::Optional<unsigned int> ()> @ 0x00007ffeefbfc800)>) at MarkingConstraintSolver.cpp:67:16
        frame #15: 0x00000001011eb6b2 JavaScriptCore`JSC::MarkingConstraintSolver::drain(this=0x00007ffeefbfc8e0, unexecuted=0x0000000107ef6058) at MarkingConstraintSolver.cpp:99:5
        frame #16: 0x00000001011eb375 JavaScriptCore`JSC::MarkingConstraintSet::executeConvergenceImpl(this=0x0000000107ef6050, visitor=0x0000000107ef2000) at MarkingConstraintSet.cpp:113:16
        frame #17: 0x00000001011eb29d JavaScriptCore`JSC::MarkingConstraintSet::executeConvergence(this=0x0000000107ef6050, visitor=0x0000000107ef2000) at MarkingConstraintSet.cpp:85:19
        frame #18: 0x0000000101170003 JavaScriptCore`JSC::Heap::runFixpointPhase(this=0x000000010a000040, conn=Mutator) at Heap.cpp:1398:43
        frame #19: 0x000000010116ee2b JavaScriptCore`JSC::Heap::runCurrentPhase(this=0x000000010a000040, conn=Mutator, currentThreadState=0x00007ffeefbfcf70) at Heap.cpp:1227:18
        frame #20: 0x000000010119dd6a JavaScriptCore`JSC::Heap::collectInMutatorThread(this=0x00007ffeefbfcfd0, state=0x00007ffeefbfcf70)::$_0::operator()(JSC::CurrentThreadState&) const at Heap.cpp:1850:52
        frame #21: 0x000000010119dd1c JavaScriptCore`WTF::ScopedLambdaFunctor<void (JSC::CurrentThreadState&), JSC::Heap::collectInMutatorThread()::$_0>::implFunction(argument=0x00007ffeefbfcfc0, arguments=0x00007ffeefbfcf70) at ScopedLambda.h:106:16
        frame #22: 0x00000001011d0f8c JavaScriptCore`void WTF::ScopedLambda<void (JSC::CurrentThreadState&)>::operator(this=0x00007ffeefbfcfc0, arguments=0x00007ffeefbfcf70)<JSC::CurrentThreadState&>(JSC::CurrentThreadState&) const at ScopedL
    ambda.h:58:16
        frame #23: 0x00000001011d0ebe JavaScriptCore`JSC::callWithCurrentThreadState(lambda=0x00007ffeefbfcfc0)> const&) at MachineStackMarker.cpp:223:5
        frame #24: 0x0000000101173c6b JavaScriptCore`JSC::Heap::collectInMutatorThread(this=0x000000010a000040) at Heap.cpp:1862:13
        frame #25: 0x00000001011739fb JavaScriptCore`JSC::Heap::stopIfNecessarySlow(this=0x000000010a000040, oldState=37) at Heap.cpp:1831:9
        frame #26: 0x0000000101174b2b JavaScriptCore`void JSC::Heap::waitForCollector<JSC::Heap::waitForCollection(unsigned long long)::$_27>(this=0x000000010a000040, func=0x00007ffeefbfd090)::$_27 const&) at Heap.cpp:1888:13
        frame #27: 0x000000010116eadc JavaScriptCore`JSC::Heap::waitForCollection(this=0x000000010a000040, ticket=141) at Heap.cpp:2150:5
        frame #28: 0x000000010116e3b0 JavaScriptCore`JSC::Heap::collectSync(this=0x000000010a000040, request=GCRequest @ 0x00007ffeefbfd130) at Heap.cpp:1146:5
        frame #29: 0x000000010116e4a9 JavaScriptCore`JSC::Heap::collectNow(this=0x000000010a000040, synchronousness=Sync, request=GCRequest @ 0x00007ffeefbfd190) at Heap.cpp:1092:9
        frame #30: 0x000000010002cda6 jsc`functionGCAndSweep(globalObject=0x000000010a1fc068, (null)=0x00007ffeefbfd1d0) at jsc.cpp:1490:13
        frame #31: 0x00003dfc64601178
        frame #32: 0x00003dfc64602da8
        frame #33: 0x000000010145a517 JavaScriptCore`llint_entry + 128442
    */

I haven't had a chance to analyze this crash properly, but it seems to be related to this commit: https://github.com/WebKit/webkit/commit/39080824d26a2246b742cf7d4ebc50fe36b515c4 and so I assume that no current release of WebKit is affected by this. The sample is unfortunately quite unreliably, only crashing for me roughly once in ten attempts.

[Radar WebKit Bug Importer]

<rdar://problem/58524626>

[Keith Miller]

Thanks for the report. I'll take a look.

[Keith Miller]

Created attachment 387551 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 387551 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387551&action=review

r=me too with comment.
Who is initializing CheckpointOSRExitSideState tmps? Are all elements cleared (JSEmpty) when declaring it? Can we add default initializer to `tmps` field as,

JSValue tmps[maxNumCheckpointTmps] { };

> Source/JavaScriptCore/ChangeLog:3
> +        JSC: Crash during GC

Let's rename the title.

[Keith Miller]

(In reply to Yusuke Suzuki from comment #4)
> Comment on attachment 387551 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=387551&action=review
> 
> r=me too with comment.
> Who is initializing CheckpointOSRExitSideState tmps? Are all elements
> cleared (JSEmpty) when declaring it? Can we add default initializer to
> `tmps` field as,
> 
> JSValue tmps[maxNumCheckpointTmps] { };

Currently, no one. I can do that though.

> 
> > Source/JavaScriptCore/ChangeLog:3
> > +        JSC: Crash during GC
> 
> Let's rename the title.

Done.

[Keith Miller]

Created attachment 387553 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 387553 [details]
Patch for landing

Clearing flags on attachment: 387553

Committed r254491: <https://trac.webkit.org/changeset/254491>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

This change broke many tests on Debug with the new assertion.

Tracked in https://bugs.webkit.org/show_bug.cgi?id=206229