Add DisallowGarbageCollection and AllowGarbageCollection

Added scopes to diallow/allow GCs from happening using a DCHECK. It is stricter than DisallowHeapAllocation, since this also doesn't allow safepoints. As soon as Turbofan is ready, we can replace all usages of DisallowHeapAllocation with DisallowGarbageCollection. Bug: v8:10315 Change-Id: I12c144ec099d9af57d692ff343adbe7aec46c0c7 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2362960 Reviewed-by: Igor Sheludko <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Reviewed-by: Ulan Degenbaev <[email protected]> Reviewed-by: Georg Neis <[email protected]> Commit-Queue: Dominik Inführ <[email protected]> Cr-Commit-Position: refs/heads/master@{#70042}
zhaoleipeng · Sep 22, 2020 · 2e00b64 · 2e00b64
1 parent fbd3834
commit 2e00b64
Show file tree

Hide file tree

Showing 31 changed files with 92 additions and 51 deletions.
diff --git a/src/api/api.cc b/src/api/api.cc
@@ -786,7 +786,7 @@ StartupData SnapshotCreator::CreateBlob(
   i::Snapshot::ClearReconstructableDataForSerialization(
       isolate, function_code_handling == FunctionCodeHandling::kClear);
 
-  i::DisallowHeapAllocation no_gc_from_here_on;
+  i::DisallowGarbageCollection no_gc_from_here_on;
 
   // Create a vector with all contexts and clear associated Persistent fields.
   // Note these contexts may be dead after calling Clear(), but will not be

diff --git a/src/codegen/compiler.cc b/src/codegen/compiler.cc
@@ -1625,6 +1625,7 @@ bool Compiler::CollectSourcePositions(Isolate* isolate,
   // Collecting source positions requires allocating a new source position
   // table.
   DCHECK(AllowHeapAllocation::IsAllowed());
+  DCHECK(AllowGarbageCollection::IsAllowed());
 
   Handle<BytecodeArray> bytecode =
       handle(shared_info->GetBytecodeArray(), isolate);

diff --git a/src/common/assert-scope.cc b/src/common/assert-scope.cc
@@ -120,6 +120,8 @@ bool PerIsolateAssertScope<kType, kAllow>::IsAllowed(Isolate* isolate) {
 // -----------------------------------------------------------------------------
 // Instantiations.
 
+template class PerThreadAssertScope<GARBAGE_COLLECTION_ASSERT, false>;
+template class PerThreadAssertScope<GARBAGE_COLLECTION_ASSERT, true>;
 template class PerThreadAssertScope<HEAP_ALLOCATION_ASSERT, false>;
 template class PerThreadAssertScope<HEAP_ALLOCATION_ASSERT, true>;
 template class PerThreadAssertScope<HANDLE_ALLOCATION_ASSERT, false>;

diff --git a/src/common/assert-scope.h b/src/common/assert-scope.h
@@ -28,6 +28,7 @@ struct PointerWithPayloadTraits<PerThreadAssertData> {
 };
 
 enum PerThreadAssertType {
+  GARBAGE_COLLECTION_ASSERT,
   HEAP_ALLOCATION_ASSERT,
   HANDLE_ALLOCATION_ASSERT,
   HANDLE_DEREFERENCE_ASSERT,
@@ -126,7 +127,17 @@ using DisallowHandleAllocation =
 using AllowHandleAllocation =
     PerThreadAssertScopeDebugOnly<HANDLE_ALLOCATION_ASSERT, true>;
 
-// Scope to document where we do not expect any allocation and GC.
+// Scope to document where we do not expect garbage collections. It differs from
+// DisallowHeapAllocation by also forbiding safepoints.
+using DisallowGarbageCollection =
+    PerThreadAssertScopeDebugOnly<GARBAGE_COLLECTION_ASSERT, false>;
+
+// Scope to introduce an exception to DisallowGarbageCollection.
+using AllowGarbageCollection =
+    PerThreadAssertScopeDebugOnly<GARBAGE_COLLECTION_ASSERT, true>;
+
+// Scope to document where we do not expect any allocation and GC. Deprecated
+// and will eventually be removed, use DisallowGarbageCollection instead.
 using DisallowHeapAllocation =
     PerThreadAssertScopeDebugOnly<HEAP_ALLOCATION_ASSERT, false>;
 #ifdef DEBUG

diff --git a/src/deoptimizer/deoptimizer.cc b/src/deoptimizer/deoptimizer.cc
@@ -12,6 +12,7 @@
 #include "src/codegen/callable.h"
 #include "src/codegen/macro-assembler.h"
 #include "src/codegen/register-configuration.h"
+#include "src/common/assert-scope.h"
 #include "src/diagnostics/disasm.h"
 #include "src/execution/frames-inl.h"
 #include "src/execution/pointer-authentication.h"
@@ -548,7 +549,8 @@ Deoptimizer::Deoptimizer(Isolate* isolate, JSFunction function,
   DCHECK(function.IsJSFunction());
 #ifdef DEBUG
   DCHECK(AllowHeapAllocation::IsAllowed());
-  disallow_heap_allocation_ = new DisallowHeapAllocation();
+  DCHECK(AllowGarbageCollection::IsAllowed());
+  disallow_garbage_collection_ = new DisallowGarbageCollection();
 #endif  // DEBUG
   CHECK(CodeKindCanDeoptimize(compiled_code_.kind()));
   if (!compiled_code_.deopt_already_counted() &&
@@ -620,7 +622,7 @@ bool Deoptimizer::should_reuse_code() const {
 
 Deoptimizer::~Deoptimizer() {
   DCHECK(input_ == nullptr && output_ == nullptr);
-  DCHECK_NULL(disallow_heap_allocation_);
+  DCHECK_NULL(disallow_garbage_collection_);
 }
 
 void Deoptimizer::DeleteFrameDescriptions() {
@@ -632,10 +634,10 @@ void Deoptimizer::DeleteFrameDescriptions() {
   input_ = nullptr;
   output_ = nullptr;
 #ifdef DEBUG
-  DCHECK(!AllowHeapAllocation::IsAllowed());
-  DCHECK_NOT_NULL(disallow_heap_allocation_);
-  delete disallow_heap_allocation_;
-  disallow_heap_allocation_ = nullptr;
+  DCHECK(!AllowGarbageCollection::IsAllowed());
+  DCHECK_NOT_NULL(disallow_garbage_collection_);
+  delete disallow_garbage_collection_;
+  disallow_garbage_collection_ = nullptr;
 #endif  // DEBUG
 }
 

diff --git a/src/deoptimizer/deoptimizer.h b/src/deoptimizer/deoptimizer.h
@@ -12,6 +12,7 @@
 #include "src/codegen/label.h"
 #include "src/codegen/register-arch.h"
 #include "src/codegen/source-position.h"
+#include "src/common/assert-scope.h"
 #include "src/common/globals.h"
 #include "src/deoptimizer/deoptimize-reason.h"
 #include "src/diagnostics/code-tracer.h"
@@ -643,7 +644,7 @@ class Deoptimizer : public Malloced {
   std::vector<ValueToMaterialize> values_to_materialize_;
 
 #ifdef DEBUG
-  DisallowHeapAllocation* disallow_heap_allocation_;
+  DisallowGarbageCollection* disallow_garbage_collection_;
 #endif  // DEBUG
 
   std::unique_ptr<CodeTracer::Scope> trace_scope_;

diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
@@ -26,6 +26,7 @@
 #include "src/codegen/assembler-inl.h"
 #include "src/codegen/compilation-cache.h"
 #include "src/codegen/flush-instruction-cache.h"
+#include "src/common/assert-scope.h"
 #include "src/common/ptr-compr.h"
 #include "src/compiler-dispatcher/compiler-dispatcher.h"
 #include "src/compiler-dispatcher/optimizing-compile-dispatcher.h"
@@ -1597,7 +1598,8 @@ Object Isolate::Throw(Object raw_exception, MessageLocation* location) {
 // Script::GetLineNumber and Script::GetColumnNumber can allocate on the heap to
 // initialize the line_ends array, so be careful when calling them.
 #ifdef DEBUG
-      if (AllowHeapAllocation::IsAllowed()) {
+      if (AllowHeapAllocation::IsAllowed() &&
+          AllowGarbageCollection::IsAllowed()) {
 #else
       if ((false)) {
 #endif

diff --git a/src/heap/heap-inl.h b/src/heap/heap-inl.h
@@ -13,6 +13,7 @@
 #include "src/base/atomic-utils.h"
 #include "src/base/atomicops.h"
 #include "src/base/platform/platform.h"
+#include "src/common/assert-scope.h"
 #include "src/heap/heap-write-barrier.h"
 #include "src/heap/heap.h"
 #include "src/heap/third-party/heap-api.h"
@@ -169,6 +170,7 @@ AllocationResult Heap::AllocateRaw(int size_in_bytes, AllocationType type,
                                    AllocationAlignment alignment) {
   DCHECK(AllowHandleAllocation::IsAllowed());
   DCHECK(AllowHeapAllocation::IsAllowed());
+  DCHECK(AllowGarbageCollection::IsAllowed());
   DCHECK_IMPLIES(type == AllocationType::kCode,
                  alignment == AllocationAlignment::kCodeAligned);
   DCHECK_EQ(gc_state(), NOT_IN_GC);
@@ -272,6 +274,7 @@ HeapObject Heap::AllocateRawWith(int size, AllocationType allocation,
                                  AllocationAlignment alignment) {
   DCHECK(AllowHandleAllocation::IsAllowed());
   DCHECK(AllowHeapAllocation::IsAllowed());
+  DCHECK(AllowGarbageCollection::IsAllowed());
   if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) {
     AllocationResult result = AllocateRaw(size, allocation, origin, alignment);
     DCHECK(!result.IsRetry());

diff --git a/src/heap/heap.cc b/src/heap/heap.cc
@@ -20,6 +20,7 @@
 #include "src/builtins/accessors.h"
 #include "src/codegen/assembler-inl.h"
 #include "src/codegen/compilation-cache.h"
+#include "src/common/assert-scope.h"
 #include "src/common/globals.h"
 #include "src/debug/debug.h"
 #include "src/deoptimizer/deoptimizer.h"
@@ -1553,7 +1554,9 @@ bool Heap::CollectGarbage(AllocationSpace space,
   {
     tracer()->Start(collector, gc_reason, collector_reason);
     DCHECK(AllowHeapAllocation::IsAllowed());
+    DCHECK(AllowGarbageCollection::IsAllowed());
     DisallowHeapAllocation no_allocation_during_gc;
+    DisallowGarbageCollection no_gc_during_gc;
     GarbageCollectionPrologue();
 
     {
@@ -1584,6 +1587,7 @@ bool Heap::CollectGarbage(AllocationSpace space,
             EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
         if (scope.CheckReenter()) {
           AllowHeapAllocation allow_allocation;
+          AllowGarbageCollection allow_gc;
           AllowJavascriptExecution allow_js(isolate());
           TRACE_GC(tracer(), GCTracer::Scope::HEAP_EXTERNAL_PROLOGUE);
           VMState<EXTERNAL> state(isolate_);
@@ -1608,6 +1612,7 @@ bool Heap::CollectGarbage(AllocationSpace space,
         gc_post_processing_depth_++;
         {
           AllowHeapAllocation allow_allocation;
+          AllowGarbageCollection allow_gc;
           AllowJavascriptExecution allow_js(isolate());
           freed_global_handles +=
               isolate_->global_handles()->PostGarbageCollectionProcessing(
@@ -1620,6 +1625,7 @@ bool Heap::CollectGarbage(AllocationSpace space,
         GCCallbacksScope scope(this);
         if (scope.CheckReenter()) {
           AllowHeapAllocation allow_allocation;
+          AllowGarbageCollection allow_gc;
           AllowJavascriptExecution allow_js(isolate());
           TRACE_GC(tracer(), GCTracer::Scope::HEAP_EXTERNAL_EPILOGUE);
           VMState<EXTERNAL> state(isolate_);

diff --git a/src/heap/local-heap-inl.h b/src/heap/local-heap-inl.h
@@ -19,6 +19,7 @@ AllocationResult LocalHeap::AllocateRaw(int size_in_bytes, AllocationType type,
   DCHECK_EQ(LocalHeap::Current(), this);
   DCHECK(AllowHandleAllocation::IsAllowed());
   DCHECK(AllowHeapAllocation::IsAllowed());
+  DCHECK(AllowGarbageCollection::IsAllowed());
   DCHECK_IMPLIES(type == AllocationType::kCode,
                  alignment == AllocationAlignment::kCodeAligned);
   Heap::HeapState state = heap()->gc_state();

diff --git a/src/heap/local-heap.h b/src/heap/local-heap.h
@@ -10,6 +10,7 @@
 
 #include "src/base/platform/condition-variable.h"
 #include "src/base/platform/mutex.h"
+#include "src/common/assert-scope.h"
 #include "src/execution/isolate.h"
 #include "src/handles/persistent-handles.h"
 #include "src/heap/concurrent-allocator.h"
@@ -35,6 +36,10 @@ class V8_EXPORT_PRIVATE LocalHeap {
   // Frequently invoked by local thread to check whether safepoint was requested
   // from the main thread.
   void Safepoint() {
+    // In case garbage collection is disabled, the thread isn't even allowed to
+    // invoke Safepoint(). Otherwise a GC might happen here.
+    DCHECK(AllowGarbageCollection::IsAllowed());
+
     if (IsSafepointRequested()) {
       ClearSafepointRequested();
       EnterSafepoint();

diff --git a/src/objects/embedder-data-slot-inl.h b/src/objects/embedder-data-slot-inl.h
@@ -99,7 +99,7 @@ bool EmbedderDataSlot::store_aligned_pointer(Isolate* isolate, void* ptr) {
 }
 
 EmbedderDataSlot::RawData EmbedderDataSlot::load_raw(
-    Isolate* isolate, const DisallowHeapAllocation& no_gc) const {
+    Isolate* isolate, const DisallowGarbageCollection& no_gc) const {
   // We don't care about atomicity of access here because embedder slots
   // are accessed this way only by serializer from the main thread when
   // GC is not active (concurrent marker may still look at the tagged part
@@ -120,7 +120,7 @@ EmbedderDataSlot::RawData EmbedderDataSlot::load_raw(
 
 void EmbedderDataSlot::store_raw(Isolate* isolate,
                                  EmbedderDataSlot::RawData data,
-                                 const DisallowHeapAllocation& no_gc) {
+                                 const DisallowGarbageCollection& no_gc) {
   // We currently have to treat zero as nullptr in embedder slots.
   if (data) data = EncodeExternalPointer(isolate, data);
   gc_safe_store(data);

diff --git a/src/objects/embedder-data-slot.h b/src/objects/embedder-data-slot.h
@@ -75,9 +75,9 @@ class EmbedderDataSlot
                                                              void* ptr);
 
   V8_INLINE RawData load_raw(Isolate* isolate,
-                             const DisallowHeapAllocation& no_gc) const;
+                             const DisallowGarbageCollection& no_gc) const;
   V8_INLINE void store_raw(Isolate* isolate, RawData data,
-                           const DisallowHeapAllocation& no_gc);
+                           const DisallowGarbageCollection& no_gc);
 
  private:
   // Stores given value to the embedder data slot in a concurrent-marker

diff --git a/src/objects/string.cc b/src/objects/string.cc
@@ -4,6 +4,7 @@
 
 #include "src/objects/string.h"
 
+#include "src/common/assert-scope.h"
 #include "src/common/globals.h"
 #include "src/handles/handles-inl.h"
 #include "src/heap/heap-inl.h"
@@ -42,6 +43,7 @@ Handle<String> String::SlowFlatten(Isolate* isolate, Handle<ConsString> cons,
   }
 
   DCHECK(AllowHeapAllocation::IsAllowed());
+  DCHECK(AllowGarbageCollection::IsAllowed());
   int length = cons->length();
   allocation =
       ObjectInYoungGeneration(*cons) ? allocation : AllocationType::kOld;

diff --git a/src/runtime/runtime-compiler.cc b/src/runtime/runtime-compiler.cc
@@ -5,6 +5,7 @@
 #include "src/asmjs/asm-js.h"
 #include "src/codegen/compilation-cache.h"
 #include "src/codegen/compiler.h"
+#include "src/common/assert-scope.h"
 #include "src/common/message-template.h"
 #include "src/compiler-dispatcher/optimizing-compile-dispatcher.h"
 #include "src/deoptimizer/deoptimizer.h"
@@ -170,6 +171,7 @@ RUNTIME_FUNCTION(Runtime_NotifyDeoptimized) {
   DCHECK(CodeKindCanDeoptimize(deoptimizer->compiled_code()->kind()));
   DCHECK(deoptimizer->compiled_code()->is_turbofanned());
   DCHECK(AllowHeapAllocation::IsAllowed());
+  DCHECK(AllowGarbageCollection::IsAllowed());
   DCHECK(isolate->context().is_null());
 
   TimerEventScope<TimerEventDeoptimizeCode> timer(isolate);

diff --git a/src/snapshot/code-serializer.cc b/src/snapshot/code-serializer.cc
@@ -66,7 +66,7 @@ ScriptCompiler::CachedData* CodeSerializer::Serialize(
   Handle<String> source(String::cast(script->source()), isolate);
   CodeSerializer cs(isolate, SerializedCodeData::SourceHash(
                                  source, script->origin_options()));
-  DisallowHeapAllocation no_gc;
+  DisallowGarbageCollection no_gc;
   cs.reference_map()->AddAttachedReference(
       reinterpret_cast<void*>(source->ptr()));
   ScriptData* script_data = cs.SerializeSharedFunctionInfo(info);
@@ -88,7 +88,7 @@ ScriptCompiler::CachedData* CodeSerializer::Serialize(
 
 ScriptData* CodeSerializer::SerializeSharedFunctionInfo(
     Handle<SharedFunctionInfo> info) {
-  DisallowHeapAllocation no_gc;
+  DisallowGarbageCollection no_gc;
 
   VisitRootPointer(Root::kHandleScope, nullptr,
                    FullObjectSlot(info.location()));
@@ -384,7 +384,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
             SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate,
                                                                shared_info);
           }
-          DisallowHeapAllocation no_gc;
+          DisallowGarbageCollection no_gc;
           int line_num =
               script->GetLineNumber(shared_info->StartPosition()) + 1;
           int column_num =
@@ -407,7 +407,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
 
 SerializedCodeData::SerializedCodeData(const std::vector<byte>* payload,
                                        const CodeSerializer* cs) {
-  DisallowHeapAllocation no_gc;
+  DisallowGarbageCollection no_gc;
   std::vector<Reservation> reservations = cs->EncodeReservations();
 
   // Calculate sizes.
@@ -520,7 +520,7 @@ SerializedCodeData::SerializedCodeData(ScriptData* data)
 SerializedCodeData SerializedCodeData::FromCachedData(
     ScriptData* cached_data, uint32_t expected_source_hash,
     SanityCheckResult* rejection_result) {
-  DisallowHeapAllocation no_gc;
+  DisallowGarbageCollection no_gc;
   SerializedCodeData scd(cached_data);
   *rejection_result = scd.SanityCheck(expected_source_hash);
   if (*rejection_result != CHECK_SUCCESS) {

diff --git a/src/snapshot/context-deserializer.cc b/src/snapshot/context-deserializer.cc
@@ -42,7 +42,7 @@ MaybeHandle<Object> ContextDeserializer::Deserialize(
 
   Handle<Object> result;
   {
-    DisallowHeapAllocation no_gc;
+    DisallowGarbageCollection no_gc;
     // Keep track of the code space start and end pointers in case new
     // code objects were unserialized
     CodeSpace* code_space = isolate->heap()->code_space();
@@ -83,7 +83,7 @@ void ContextDeserializer::SetupOffHeapArrayBufferBackingStores() {
 void ContextDeserializer::DeserializeEmbedderFields(
     v8::DeserializeEmbedderFieldsCallback embedder_fields_deserializer) {
   if (!source()->HasMore() || source()->Get() != kEmbedderFieldsData) return;
-  DisallowHeapAllocation no_gc;
+  DisallowGarbageCollection no_gc;
   DisallowJavascriptExecution no_js(isolate());
   DisallowCompilation no_compile(isolate());
   DCHECK_NOT_NULL(embedder_fields_deserializer.callback);

diff --git a/src/snapshot/context-serializer.cc b/src/snapshot/context-serializer.cc
@@ -24,7 +24,7 @@ class SanitizeNativeContextScope final {
  public:
   SanitizeNativeContextScope(Isolate* isolate, NativeContext native_context,
                              bool allow_active_isolate_for_testing,
-                             const DisallowHeapAllocation& no_gc)
+                             const DisallowGarbageCollection& no_gc)
       : isolate_(isolate),
         native_context_(native_context),
         microtask_queue_(native_context.microtask_queue()),
@@ -82,7 +82,7 @@ ContextSerializer::~ContextSerializer() {
 }
 
 void ContextSerializer::Serialize(Context* o,
-                                  const DisallowHeapAllocation& no_gc) {
+                                  const DisallowGarbageCollection& no_gc) {
   context_ = *o;
   DCHECK(context_.IsNativeContext());
 
@@ -212,7 +212,7 @@ bool ContextSerializer::SerializeJSObjectWithEmbedderFields(Object obj) {
   CHECK_GT(embedder_fields_count, 0);
   DCHECK(!js_obj.NeedsRehashing());
 
-  DisallowHeapAllocation no_gc;
+  DisallowGarbageCollection no_gc;
   DisallowJavascriptExecution no_js(isolate());
   DisallowCompilation no_compile(isolate());
 

diff --git a/src/snapshot/context-serializer.h b/src/snapshot/context-serializer.h
@@ -23,7 +23,7 @@ class V8_EXPORT_PRIVATE ContextSerializer : public Serializer {
   ~ContextSerializer() override;
 
   // Serialize the objects reachable from a single object pointer.
-  void Serialize(Context* o, const DisallowHeapAllocation& no_gc);
+  void Serialize(Context* o, const DisallowGarbageCollection& no_gc);
 
   bool can_be_rehashed() const { return can_be_rehashed_; }