From 3f641945dcdb7905e0c3043026a6167123491316 Mon Sep 17 00:00:00 2001 From: Jakob Gruber Date: Thu, 22 Jul 2021 08:31:12 +0200 Subject: [PATCH] [compiler] Don't construct refs inside DisallowGarbageCollection scopes The MapRef constructor contains a ParkedSharedMutexGuard which may trigger gc; and MapRefs may be created for any HeapObjectRef (or subclass) creation. Thus, calls to (Try)MakeRef must happen in contexts in which garbage collection is allowed. Bug: v8:7790,v8:12012 Change-Id: If0cb9e2dae7150b0aa5193a90ec3bc9cd9ac3b81 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3043951 Reviewed-by: Georg Neis Commit-Queue: Georg Neis Auto-Submit: Jakob Gruber Cr-Commit-Position: refs/heads/master@{#75850} --- src/compiler/heap-refs.cc | 97 ++++++++++++++++++++++----------------- 1 file changed, 56 insertions(+), 41 deletions(-) diff --git a/src/compiler/heap-refs.cc b/src/compiler/heap-refs.cc index acd01cd58f..a26cdad097 100644 --- a/src/compiler/heap-refs.cc +++ b/src/compiler/heap-refs.cc @@ -517,14 +517,18 @@ base::Optional GetOwnFastDataPropertyFromHeap( // object, we are guaranteed to see valid heap words even if the data is wrong. base::Optional GetOwnDictionaryPropertyFromHeap( JSHeapBroker* broker, Handle receiver, InternalIndex dict_index) { - DisallowGarbageCollection no_gc; - // DictionaryPropertyAt will check that we are within the bounds of the - // object. - base::Optional maybe_constant = JSObject::DictionaryPropertyAt( - receiver, dict_index, broker->isolate()->heap()); - DCHECK_IMPLIES(broker->IsMainThread(), maybe_constant); - if (!maybe_constant) return {}; - return TryMakeRef(broker, maybe_constant.value()); + Handle constant; + { + DisallowGarbageCollection no_gc; + // DictionaryPropertyAt will check that we are within the bounds of the + // object. + base::Optional maybe_constant = JSObject::DictionaryPropertyAt( + receiver, dict_index, broker->isolate()->heap()); + DCHECK_IMPLIES(broker->IsMainThread(), maybe_constant); + if (!maybe_constant) return {}; + constant = broker->CanonicalPersistentHandle(maybe_constant.value()); + } + return TryMakeRef(broker, constant); } } // namespace @@ -2691,24 +2695,28 @@ base::Optional JSObjectRef::RawInobjectPropertyAt( FieldIndex index) const { CHECK(index.is_inobject()); if (data_->should_access_heap() || broker()->is_concurrent_inlining()) { - DisallowGarbageCollection no_gc; - Map current_map = object()->map(kAcquireLoad); + Handle value; + { + DisallowGarbageCollection no_gc; + Map current_map = object()->map(kAcquireLoad); - // If the map changed in some prior GC epoch, our {index} could be - // outside the valid bounds of the cached map. - if (*map().object() != current_map) { - TRACE_BROKER_MISSING(broker(), "Map change detected in " << *this); - return {}; - } + // If the map changed in some prior GC epoch, our {index} could be + // outside the valid bounds of the cached map. + if (*map().object() != current_map) { + TRACE_BROKER_MISSING(broker(), "Map change detected in " << *this); + return {}; + } - base::Optional value = - object()->RawInobjectPropertyAt(current_map, index); - if (!value.has_value()) { - TRACE_BROKER_MISSING(broker(), - "Unable to safely read property in " << *this); - return {}; + base::Optional maybe_value = + object()->RawInobjectPropertyAt(current_map, index); + if (!maybe_value.has_value()) { + TRACE_BROKER_MISSING(broker(), + "Unable to safely read property in " << *this); + return {}; + } + value = broker()->CanonicalPersistentHandle(maybe_value.value()); } - return TryMakeRef(broker(), value.value()); + return TryMakeRef(broker(), value); } JSObjectData* object_data = data()->AsJSObject(); return ObjectRef(broker(), @@ -2870,13 +2878,16 @@ int ArrayBoilerplateDescriptionRef::constants_elements_length() const { ObjectRef FixedArrayRef::get(int i) const { return TryGet(i).value(); } base::Optional FixedArrayRef::TryGet(int i) const { - DisallowGarbageCollection no_gc; - CHECK_GE(i, 0); - Object value = object()->get(i, kAcquireLoad); - if (i >= object()->length(kAcquireLoad)) { - // Right-trimming happened. - CHECK_LT(i, length()); - return {}; + Handle value; + { + DisallowGarbageCollection no_gc; + CHECK_GE(i, 0); + value = broker()->CanonicalPersistentHandle(object()->get(i, kAcquireLoad)); + if (i >= object()->length(kAcquireLoad)) { + // Right-trimming happened. + CHECK_LT(i, length()); + return {}; + } } return TryMakeRef(broker(), value); } @@ -3115,17 +3126,22 @@ HolderLookupResult FunctionTemplateInfoRef::LookupHolderOfExpectedType( DCHECK(has_call_code()); - DisallowGarbageCollection no_gc; - HeapObject signature = object()->signature(); - if (signature.IsUndefined()) { - return HolderLookupResult(CallOptimization::kHolderIsReceiver); - } - auto expected_receiver_type = FunctionTemplateInfo::cast(signature); - if (expected_receiver_type.IsTemplateFor(*receiver_map.object())) { - return HolderLookupResult(CallOptimization::kHolderIsReceiver); + Handle expected_receiver_type; + { + DisallowGarbageCollection no_gc; + HeapObject signature = object()->signature(); + if (signature.IsUndefined()) { + return HolderLookupResult(CallOptimization::kHolderIsReceiver); + } + expected_receiver_type = broker()->CanonicalPersistentHandle( + FunctionTemplateInfo::cast(signature)); + if (expected_receiver_type->IsTemplateFor(*receiver_map.object())) { + return HolderLookupResult(CallOptimization::kHolderIsReceiver); + } + + if (!receiver_map.IsJSGlobalProxyMap()) return not_found; } - if (!receiver_map.IsJSGlobalProxyMap()) return not_found; if (policy == SerializationPolicy::kSerializeIfNeeded) { receiver_map.SerializePrototype(NotConcurrentInliningTag{broker()}); } @@ -3133,8 +3149,7 @@ HolderLookupResult FunctionTemplateInfoRef::LookupHolderOfExpectedType( if (!prototype.has_value()) return not_found; if (prototype->IsNull()) return not_found; - JSObject raw_prototype = JSObject::cast(*prototype->object()); - if (!expected_receiver_type.IsTemplateFor(raw_prototype.map())) { + if (!expected_receiver_type->IsTemplateFor(prototype->object()->map())) { return not_found; } return HolderLookupResult(CallOptimization::kHolderFound,