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.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@241927 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,78 @@
+2019-02-21  Mark Lam  <[email protected]>
+
+        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.
+
 2019-02-21  Yusuke Suzuki  <[email protected]>
 
         [JSC] Use Fast Malloc as much as possible

Source/JavaScriptCore/dfg/DFGOSRExit.cpp

@@ -1015,6 +1015,12 @@ void JIT_OPERATION OSRExit::compileOSRExit(ExecState* exec)
     VM* vm = &exec->vm();
     auto scope = DECLARE_THROW_SCOPE(*vm);
 
+    if (validateDFGDoesGC) {
+        // We're about to exit optimized code. So, there's no longer any optimized
+        // code running that expects no GC.
+        vm->heap.setExpectDoesGC(true);
+    }
+
     if (vm->callFrameForCatch)
         RELEASE_ASSERT(vm->callFrameForCatch == exec);
 
@@ -1399,6 +1405,11 @@ void OSRExit::compileExit(CCallHelpers& jit, VM& vm, const OSRExit& exit, const
         // We're about to exit optimized code. So, there's no longer any optimized
         // code running that expects no GC. We need to set this before arguments
         // materialization below (see emitRestoreArguments()).
+
+        // Even though we set Heap::m_expectDoesGC in compileOSRExit(), we also need
+        // to set it here because compileOSRExit() is only called on the first time
+        // we exit from this site, but all subsequent exits will take this compiled
+        // ramp without calling compileOSRExit() first.
         jit.store8(CCallHelpers::TrustedImm32(true), vm.heap.addressOfExpectDoesGC());
     }
 

Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp

@@ -248,6 +248,11 @@ static void compileStub(
         // We're about to exit optimized code. So, there's no longer any optimized
         // code running that expects no GC. We need to set this before object
         // materialization below.
+
+        // Even though we set Heap::m_expectDoesGC in compileFTLOSRExit(), we also need
+        // to set it here because compileFTLOSRExit() is only called on the first time
+        // we exit from this site, but all subsequent exits will take this compiled
+        // ramp without calling compileFTLOSRExit() first.
         jit.store8(CCallHelpers::TrustedImm32(true), vm->heap.addressOfExpectDoesGC());
     }
 
@@ -526,6 +531,13 @@ extern "C" void* compileFTLOSRExit(ExecState* exec, unsigned exitID)
         dataLog("Compiling OSR exit with exitID = ", exitID, "\n");
 
     VM& vm = exec->vm();
+
+    if (validateDFGDoesGC) {
+        // We're about to exit optimized code. So, there's no longer any optimized
+        // code running that expects no GC.
+        vm.heap.setExpectDoesGC(true);
+    }
+
     if (vm.callFrameForCatch)
         RELEASE_ASSERT(vm.callFrameForCatch == exec);
 

Source/JavaScriptCore/heap/CompleteSubspace.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -122,6 +122,9 @@ void* CompleteSubspace::allocateSlow(VM& vm, size_t size, GCDeferralContext* def
 
 void* CompleteSubspace::tryAllocateSlow(VM& vm, size_t size, GCDeferralContext* deferralContext)
 {
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(vm.heap.expectDoesGC());
+
     sanitizeStackForVM(&vm);
 
     if (Allocator allocator = allocatorFor(size, AllocatorForMode::EnsureAllocator))

Source/JavaScriptCore/heap/CompleteSubspaceInlines.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2018-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -29,6 +29,9 @@ namespace JSC {
 
 ALWAYS_INLINE void* CompleteSubspace::allocateNonVirtual(VM& vm, size_t size, GCDeferralContext* deferralContext, AllocationFailureMode failureMode)
 {
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(vm.heap.expectDoesGC());
+
     if (Allocator allocator = allocatorForNonVirtual(size, AllocatorForMode::AllocatorIfExists))
         return allocator.allocate(deferralContext, failureMode);
     return allocateSlow(vm, size, deferralContext, failureMode);

Source/JavaScriptCore/heap/DeferGC.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -33,6 +33,7 @@ namespace JSC {
 
 class DeferGC {
     WTF_MAKE_NONCOPYABLE(DeferGC);
+    WTF_FORBID_HEAP_ALLOCATION;
 public:
     DeferGC(Heap& heap)
         : m_heap(heap)
@@ -42,6 +43,8 @@ class DeferGC {
 
     ~DeferGC()
     {
+        if (validateDFGDoesGC)
+            RELEASE_ASSERT(m_heap.expectDoesGC());
         m_heap.decrementDeferralDepthAndGCIfNeeded();
     }
 
@@ -51,6 +54,7 @@ class DeferGC {
 
 class DeferGCForAWhile {
     WTF_MAKE_NONCOPYABLE(DeferGCForAWhile);
+    WTF_FORBID_HEAP_ALLOCATION;
 public:
     DeferGCForAWhile(Heap& heap)
         : m_heap(heap)
@@ -69,6 +73,7 @@ class DeferGCForAWhile {
 
 class DisallowGC : public DisallowScope<DisallowGC> {
     WTF_MAKE_NONCOPYABLE(DisallowGC);
+    WTF_FORBID_HEAP_ALLOCATION;
     typedef DisallowScope<DisallowGC> Base;
 public:
 #ifdef NDEBUG

Source/JavaScriptCore/heap/GCDeferralContext.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -25,13 +25,17 @@
 
 #pragma once
 
+#include <wtf/ForbidHeapAllocation.h>
+
 namespace JSC {
 
 class Heap;
 class BlockDirectory;
 class LocalAllocator;
 
 class GCDeferralContext {
+    WTF_FORBID_HEAP_ALLOCATION;
+
     friend class Heap;
     friend class BlockDirectory;
     friend class LocalAllocator;

Source/JavaScriptCore/heap/GCDeferralContextInlines.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -37,6 +37,9 @@ ALWAYS_INLINE GCDeferralContext::GCDeferralContext(Heap& heap)
 
 ALWAYS_INLINE GCDeferralContext::~GCDeferralContext()
 {
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(m_heap.expectDoesGC());
+
     if (UNLIKELY(m_shouldGC))
         m_heap.collectIfNecessaryOrDefer();
 }

Source/JavaScriptCore/heap/Heap.cpp

@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2003-2018 Apple Inc. All rights reserved.
+ *  Copyright (C) 2003-2019 Apple Inc. All rights reserved.
  *  Copyright (C) 2007 Eric Seidel <[email protected]>
  *
  *  This library is free software; you can redistribute it and/or
@@ -1029,6 +1029,9 @@ void Heap::collect(Synchronousness synchronousness, GCRequest request)
 
 void Heap::collectNow(Synchronousness synchronousness, GCRequest request)
 {
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(expectDoesGC());
+
     switch (synchronousness) {
     case Async: {
         collectAsync(request);
@@ -1061,6 +1064,9 @@ void Heap::collectNow(Synchronousness synchronousness, GCRequest request)
 
 void Heap::collectAsync(GCRequest request)
 {
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(expectDoesGC());
+
     if (!m_isSafeToCollect)
         return;
 
@@ -1082,9 +1088,12 @@ void Heap::collectAsync(GCRequest request)
 
 void Heap::collectSync(GCRequest request)
 {
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(expectDoesGC());
+
     if (!m_isSafeToCollect)
         return;
-    
+
     waitForCollection(requestCollection(request));
 }
 
@@ -1737,6 +1746,9 @@ NEVER_INLINE void Heap::resumeTheMutator()
 
 void Heap::stopIfNecessarySlow()
 {
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(expectDoesGC());
+
     while (stopIfNecessarySlow(m_worldState.load())) { }
 
     RELEASE_ASSERT(m_worldState.load() & hasAccessBit);
@@ -1749,6 +1761,9 @@ void Heap::stopIfNecessarySlow()
 
 bool Heap::stopIfNecessarySlow(unsigned oldState)
 {
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(expectDoesGC());
+
     RELEASE_ASSERT(oldState & hasAccessBit);
     RELEASE_ASSERT(!(oldState & stoppedBit));
 
@@ -2538,9 +2553,12 @@ void Heap::reportExternalMemoryVisited(size_t size)
 void Heap::collectIfNecessaryOrDefer(GCDeferralContext* deferralContext)
 {
     ASSERT(deferralContext || isDeferred() || !DisallowGC::isInEffectOnCurrentThread());
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(expectDoesGC());
 
     if (!m_isSafeToCollect)
         return;
+
     switch (mutatorState()) {
     case MutatorState::Running:
     case MutatorState::Allocating:

Source/JavaScriptCore/heap/HeapInlines.h

@@ -238,6 +238,9 @@ inline void Heap::deprecatedReportExtraMemory(size_t size)
 
 inline void Heap::acquireAccess()
 {
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(expectDoesGC());
+
     if (m_worldState.compareExchangeWeak(0, hasAccessBit))
         return;
     acquireAccessSlow();
@@ -262,6 +265,9 @@ inline bool Heap::mayNeedToStop()
 
 inline void Heap::stopIfNecessary()
 {
+    if (validateDFGDoesGC)
+        RELEASE_ASSERT(expectDoesGC());
+
     if (mayNeedToStop())
         stopIfNecessarySlow();
 }