Changeset 241927 in webkit


Ignore:
Timestamp:
Feb 21, 2019 6:02:32 PM (5 years ago)
Author:
mark.lam@apple.com
Message:

Add more doesGC() assertions.
​https://bugs.webkit.org/show_bug.cgi?id=194911
<rdar://problem/48285723>

Reviewed by Saam Barati and Yusuke Suzuki.

  • dfg/DFGOSRExit.cpp:

(JSC::DFG::OSRExit::compileOSRExit):

  • Set expectDoesGC here because we no longer have to worry about missing store barriers in optimized code after this point. This will prevent false positive assertion failures arising from functions called beneath compileOSRExit().

(JSC::DFG::OSRExit::compileExit):

  • Add a comment to explain why the generated ramp needs to set expectDoesGC even though compileOSRExit() also sets it. Reason: compileOSRExit() is only called for the first OSR from this code origin, the generated ramp is called for many subsequents OSR exits from this code origin.
  • ftl/FTLOSRExitCompiler.cpp:

(JSC::FTL::compileStub):

  • Added a comment for the equivalent reason to the one above.

(JSC::FTL::compileFTLOSRExit):

  • Set expectDoesGC here because we no longer have to worry about missing store barriers in optimized code after this point. This will prevent false positive assertion failures arising from functions called beneath compileFTLOSRExit().
  • heap/CompleteSubspace.cpp:

(JSC::CompleteSubspace::tryAllocateSlow):

  • heap/CompleteSubspaceInlines.h:

(JSC::CompleteSubspace::allocateNonVirtual):

  • assert expectDoesGC.
  • heap/DeferGC.h:

(JSC::DeferGC::~DeferGC):

  • assert expectDoesGC.
  • Also added WTF_FORBID_HEAP_ALLOCATION to DeferGC, DeferGCForAWhile, and DisallowGC because all 3 should be stack allocated RAII objects.
  • heap/GCDeferralContext.h:
  • heap/GCDeferralContextInlines.h:

(JSC::GCDeferralContext::~GCDeferralContext):

  • Added WTF_FORBID_HEAP_ALLOCATION.
  • assert expectDoesGC.
  • heap/Heap.cpp:

(JSC::Heap::collectNow):
(JSC::Heap::collectAsync):
(JSC::Heap::collectSync):
(JSC::Heap::stopIfNecessarySlow):
(JSC::Heap::collectIfNecessaryOrDefer):

  • heap/HeapInlines.h:

(JSC::Heap::acquireAccess):
(JSC::Heap::stopIfNecessary):

  • heap/LargeAllocation.cpp:

(JSC::LargeAllocation::tryCreate):

  • heap/LocalAllocatorInlines.h:

(JSC::LocalAllocator::allocate):

  • conservatively assert expectDoesGC on these functions that may trigger a GC though they don't always do.
  • runtime/DisallowScope.h:
  • DisallowScope should be stack allocated because it's an RAII object.
  • runtime/JSCellInlines.h:

(JSC::tryAllocateCellHelper):

  • Remove the expectDoesGC assertion because it is now covered by assertions in CompleteSubspace, LargeAllocation, and LocalAllocator.
  • runtime/RegExpMatchesArray.h:

(JSC::createRegExpMatchesArray):

  • assert expectDoesGC.
Location:
trunk/Source/JavaScriptCore
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r241923 r241927  
     12019-02-21  Mark Lam  <mark.lam@apple.com>
     2
     3        Add more doesGC() assertions.
     4        https://bugs.webkit.org/show_bug.cgi?id=194911
     5        <rdar://problem/48285723>
     6
     7        Reviewed by Saam Barati and Yusuke Suzuki.
     8
     9        * dfg/DFGOSRExit.cpp:
     10        (JSC::DFG::OSRExit::compileOSRExit):
     11        - Set expectDoesGC here because we no longer have to worry about missing store
     12          barriers in optimized code after this point.  This will prevent false positive
     13          assertion failures arising from functions called beneath compileOSRExit().
     14
     15        (JSC::DFG::OSRExit::compileExit):
     16        - Add a comment to explain why the generated ramp needs to set expectDoesGC even
     17          though compileOSRExit() also sets it.  Reason: compileOSRExit() is only called
     18          for the first OSR from this code origin, the generated ramp is called for many
     19          subsequents OSR exits from this code origin.
     20
     21        * ftl/FTLOSRExitCompiler.cpp:
     22        (JSC::FTL::compileStub):
     23        - Added a comment for the equivalent reason to the one above.
     24
     25        (JSC::FTL::compileFTLOSRExit):
     26        - Set expectDoesGC here because we no longer have to worry about missing store
     27          barriers in optimized code after this point.  This will prevent false positive
     28          assertion failures arising from functions called beneath compileFTLOSRExit().
     29
     30        * heap/CompleteSubspace.cpp:
     31        (JSC::CompleteSubspace::tryAllocateSlow):
     32        * heap/CompleteSubspaceInlines.h:
     33        (JSC::CompleteSubspace::allocateNonVirtual):
     34        - assert expectDoesGC.
     35
     36        * heap/DeferGC.h:
     37        (JSC::DeferGC::~DeferGC):
     38        - assert expectDoesGC.
     39        - Also added WTF_FORBID_HEAP_ALLOCATION to DeferGC, DeferGCForAWhile, and DisallowGC
     40          because all 3 should be stack allocated RAII objects.
     41
     42        * heap/GCDeferralContext.h:
     43        * heap/GCDeferralContextInlines.h:
     44        (JSC::GCDeferralContext::~GCDeferralContext):
     45        - Added WTF_FORBID_HEAP_ALLOCATION.
     46        - assert expectDoesGC.
     47
     48        * heap/Heap.cpp:
     49        (JSC::Heap::collectNow):
     50        (JSC::Heap::collectAsync):
     51        (JSC::Heap::collectSync):
     52        (JSC::Heap::stopIfNecessarySlow):
     53        (JSC::Heap::collectIfNecessaryOrDefer):
     54        * heap/HeapInlines.h:
     55        (JSC::Heap::acquireAccess):
     56        (JSC::Heap::stopIfNecessary):
     57        * heap/LargeAllocation.cpp:
     58        (JSC::LargeAllocation::tryCreate):
     59        * heap/LocalAllocatorInlines.h:
     60        (JSC::LocalAllocator::allocate):
     61        - conservatively assert expectDoesGC on these functions that may trigger a GC
     62          though they don't always do.
     63
     64        * runtime/DisallowScope.h:
     65        - DisallowScope should be stack allocated because it's an RAII object.
     66
     67        * runtime/JSCellInlines.h:
     68        (JSC::tryAllocateCellHelper):
     69        - Remove the expectDoesGC assertion because it is now covered by assertions in
     70          CompleteSubspace, LargeAllocation, and LocalAllocator.
     71
     72        * runtime/RegExpMatchesArray.h:
     73        (JSC::createRegExpMatchesArray):
     74        - assert expectDoesGC.
     75
    1762019-02-21  Yusuke Suzuki  <ysuzuki@apple.com>
    277

  • trunk/Source/JavaScriptCore/dfg/DFGOSRExit.cpp

    r241849 r241927  
    10161016    auto scope = DECLARE_THROW_SCOPE(*vm);
    10171017
     1018    if (validateDFGDoesGC) {
     1019        // We're about to exit optimized code. So, there's no longer any optimized
     1020        // code running that expects no GC.
     1021        vm->heap.setExpectDoesGC(true);
     1022    }
     1023
    10181024    if (vm->callFrameForCatch)
    10191025        RELEASE_ASSERT(vm->callFrameForCatch == exec);
    … …  
    14001406        // code running that expects no GC. We need to set this before arguments
    14011407        // materialization below (see emitRestoreArguments()).
     1408
     1409        // Even though we set Heap::m_expectDoesGC in compileOSRExit(), we also need
     1410        // to set it here because compileOSRExit() is only called on the first time
     1411        // we exit from this site, but all subsequent exits will take this compiled
     1412        // ramp without calling compileOSRExit() first.
    14021413        jit.store8(CCallHelpers::TrustedImm32(true), vm.heap.addressOfExpectDoesGC());
    14031414    }

  • trunk/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp

    r241849 r241927  
    249249        // code running that expects no GC. We need to set this before object
    250250        // materialization below.
     251
     252        // Even though we set Heap::m_expectDoesGC in compileFTLOSRExit(), we also need
     253        // to set it here because compileFTLOSRExit() is only called on the first time
     254        // we exit from this site, but all subsequent exits will take this compiled
     255        // ramp without calling compileFTLOSRExit() first.
    251256        jit.store8(CCallHelpers::TrustedImm32(true), vm->heap.addressOfExpectDoesGC());
    252257    }
    … …  
    527532
    528533    VM& vm = exec->vm();
     534
     535    if (validateDFGDoesGC) {
     536        // We're about to exit optimized code. So, there's no longer any optimized
     537        // code running that expects no GC.
     538        vm.heap.setExpectDoesGC(true);
     539    }
     540
    529541    if (vm.callFrameForCatch)
    530542        RELEASE_ASSERT(vm.callFrameForCatch == exec);

  • trunk/Source/JavaScriptCore/heap/CompleteSubspace.cpp

    r240216 r241927  
    11/*
    2  * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    123123void* CompleteSubspace::tryAllocateSlow(VM& vm, size_t size, GCDeferralContext* deferralContext)
    124124{
     125    if (validateDFGDoesGC)
     126        RELEASE_ASSERT(vm.heap.expectDoesGC());
     127
    125128    sanitizeStackForVM(&vm);
    126129   

  • trunk/Source/JavaScriptCore/heap/CompleteSubspaceInlines.h

    r232132 r241927  
    11/*
    2  * Copyright (C) 2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2018-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3030ALWAYS_INLINE void* CompleteSubspace::allocateNonVirtual(VM& vm, size_t size, GCDeferralContext* deferralContext, AllocationFailureMode failureMode)
    3131{
     32    if (validateDFGDoesGC)
     33        RELEASE_ASSERT(vm.heap.expectDoesGC());
     34
    3235    if (Allocator allocator = allocatorForNonVirtual(size, AllocatorForMode::AllocatorIfExists))
    3336        return allocator.allocate(deferralContext, failureMode);

  • trunk/Source/JavaScriptCore/heap/DeferGC.h

    r218794 r241927  
    11/*
    2  * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3434class DeferGC {
    3535    WTF_MAKE_NONCOPYABLE(DeferGC);
     36    WTF_FORBID_HEAP_ALLOCATION;
    3637public:
    3738    DeferGC(Heap& heap)
    … …  
    4344    ~DeferGC()
    4445    {
     46        if (validateDFGDoesGC)
     47            RELEASE_ASSERT(m_heap.expectDoesGC());
    4548        m_heap.decrementDeferralDepthAndGCIfNeeded();
    4649    }
    … …  
    5255class DeferGCForAWhile {
    5356    WTF_MAKE_NONCOPYABLE(DeferGCForAWhile);
     57    WTF_FORBID_HEAP_ALLOCATION;
    5458public:
    5559    DeferGCForAWhile(Heap& heap)
    … …  
    7074class DisallowGC : public DisallowScope<DisallowGC> {
    7175    WTF_MAKE_NONCOPYABLE(DisallowGC);
     76    WTF_FORBID_HEAP_ALLOCATION;
    7277    typedef DisallowScope<DisallowGC> Base;
    7378public:

  • trunk/Source/JavaScriptCore/heap/GCDeferralContext.h

    r227617 r241927  
    11/*
    2  * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2626#pragma once
    2727
     28#include <wtf/ForbidHeapAllocation.h>
     29
    2830namespace JSC {
    2931
    … …  
    3335
    3436class GCDeferralContext {
     37    WTF_FORBID_HEAP_ALLOCATION;
     38
    3539    friend class Heap;
    3640    friend class BlockDirectory;

  • trunk/Source/JavaScriptCore/heap/GCDeferralContextInlines.h

    r225725 r241927  
    11/*
    2  * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
     2 * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3838ALWAYS_INLINE GCDeferralContext::~GCDeferralContext()
    3939{
     40    if (validateDFGDoesGC)
     41        RELEASE_ASSERT(m_heap.expectDoesGC());
     42
    4043    if (UNLIKELY(m_shouldGC))
    4144        m_heap.collectIfNecessaryOrDefer();

  • trunk/Source/JavaScriptCore/heap/Heap.cpp

    r241655 r241927  
    11/*
    2  *  Copyright (C) 2003-2018 Apple Inc. All rights reserved.
     2 *  Copyright (C) 2003-2019 Apple Inc. All rights reserved.
    33 *  Copyright (C) 2007 Eric Seidel <eric@webkit.org>
    44 *
    … …  
    10301030void Heap::collectNow(Synchronousness synchronousness, GCRequest request)
    10311031{
     1032    if (validateDFGDoesGC)
     1033        RELEASE_ASSERT(expectDoesGC());
     1034
    10321035    switch (synchronousness) {
    10331036    case Async: {
    … …  
    10621065void Heap::collectAsync(GCRequest request)
    10631066{
     1067    if (validateDFGDoesGC)
     1068        RELEASE_ASSERT(expectDoesGC());
     1069
    10641070    if (!m_isSafeToCollect)
    10651071        return;
    … …  
    10831089void Heap::collectSync(GCRequest request)
    10841090{
     1091    if (validateDFGDoesGC)
     1092        RELEASE_ASSERT(expectDoesGC());
     1093
    10851094    if (!m_isSafeToCollect)
    10861095        return;
    1087    
     1096
    10881097    waitForCollection(requestCollection(request));
    10891098}
    … …  
    17381747void Heap::stopIfNecessarySlow()
    17391748{
     1749    if (validateDFGDoesGC)
     1750        RELEASE_ASSERT(expectDoesGC());
     1751
    17401752    while (stopIfNecessarySlow(m_worldState.load())) { }
    17411753   
    … …  
    17501762bool Heap::stopIfNecessarySlow(unsigned oldState)
    17511763{
     1764    if (validateDFGDoesGC)
     1765        RELEASE_ASSERT(expectDoesGC());
     1766
    17521767    RELEASE_ASSERT(oldState & hasAccessBit);
    17531768    RELEASE_ASSERT(!(oldState & stoppedBit));
    … …  
    25392554{
    25402555    ASSERT(deferralContext || isDeferred() || !DisallowGC::isInEffectOnCurrentThread());
     2556    if (validateDFGDoesGC)
     2557        RELEASE_ASSERT(expectDoesGC());
    25412558
    25422559    if (!m_isSafeToCollect)
    25432560        return;
     2561
    25442562    switch (mutatorState()) {
    25452563    case MutatorState::Running:

  • trunk/Source/JavaScriptCore/heap/HeapInlines.h

    r229798 r241927  
    239239inline void Heap::acquireAccess()
    240240{
     241    if (validateDFGDoesGC)
     242        RELEASE_ASSERT(expectDoesGC());
     243
    241244    if (m_worldState.compareExchangeWeak(0, hasAccessBit))
    242245        return;
    … …  
    263266inline void Heap::stopIfNecessary()
    264267{
     268    if (validateDFGDoesGC)
     269        RELEASE_ASSERT(expectDoesGC());
     270
    265271    if (mayNeedToStop())
    266272        stopIfNecessarySlow();

  • trunk/Source/JavaScriptCore/heap/LargeAllocation.cpp

    r240216 r241927  
    3737LargeAllocation* LargeAllocation::tryCreate(Heap& heap, size_t size, Subspace* subspace)
    3838{
     39    if (validateDFGDoesGC)
     40        RELEASE_ASSERT(heap.expectDoesGC());
     41
    3942    // This includes padding at the end of the allocation to maintain the distancing constraint.
    4043    constexpr size_t distancing = minimumDistanceBetweenCellsFromDifferentOrigins;

  • trunk/Source/JavaScriptCore/heap/LocalAllocatorInlines.h

    r227617 r241927  
    11/*
    2  * Copyright (C) 2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2018-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3232ALWAYS_INLINE void* LocalAllocator::allocate(GCDeferralContext* deferralContext, AllocationFailureMode failureMode)
    3333{
     34    if (validateDFGDoesGC)
     35        RELEASE_ASSERT(m_directory->heap()->expectDoesGC());
    3436    return m_freeList.allocate(
    3537        [&] () -> HeapCell* {

  • trunk/Source/JavaScriptCore/runtime/DisallowScope.h

    r233722 r241927  
    11/*
    2  * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2626#pragma once
    2727
     28#include <wtf/ForbidHeapAllocation.h>
    2829#include <wtf/Noncopyable.h>
    2930
    … …  
    3334class DisallowScope {
    3435    WTF_MAKE_NONCOPYABLE(DisallowScope);
     36    WTF_FORBID_HEAP_ALLOCATION;
    3537public:
    3638#ifdef NDEBUG

  • trunk/Source/JavaScriptCore/runtime/JSCellInlines.h

    r241849 r241927  
    167167{
    168168    VM& vm = *heap.vm();
    169     if (validateDFGDoesGC)
    170         RELEASE_ASSERT(heap.expectDoesGC());
    171 
    172169    ASSERT(deferralContext || !DisallowGC::isInEffectOnCurrentThread());
    173170    ASSERT(size >= sizeof(T));

  • trunk/Source/JavaScriptCore/runtime/RegExpMatchesArray.h

    r232951 r241927  
    6363    RegExp* regExp, unsigned startOffset, MatchResult& result)
    6464{
     65    if (validateDFGDoesGC)
     66        RELEASE_ASSERT(vm.heap.expectDoesGC());
     67
    6568    Vector<int, 32> subpatternResults;
    6669    int position = regExp->matchInline(vm, inputValue, startOffset, subpatternResults);
Note: See TracChangeset for help on using the changeset viewer.