From 9f4f472b336e7483fb1d6b97d4506d0c00ffd0f9 Mon Sep 17 00:00:00 2001 From: Camillo Bruni Date: Wed, 26 Jan 2022 18:09:56 +0100 Subject: [PATCH] [runtime] Avoid handles in PropertyCell-related code Bug: v8:11263 Change-Id: I02c51fae400a9a5d67376ed645ea01be4ef1dc1e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3417437 Reviewed-by: Igor Sheludko Commit-Queue: Camillo Bruni Cr-Commit-Position: refs/heads/main@{#78810} --- src/objects/lookup.cc | 2 +- src/objects/objects.cc | 42 ++++++++++++++++---------------- src/objects/property-cell.h | 8 +++--- src/web-snapshot/web-snapshot.cc | 2 +- 4 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/objects/lookup.cc b/src/objects/lookup.cc index dd087b49c5..c5cb6f38ba 100644 --- a/src/objects/lookup.cc +++ b/src/objects/lookup.cc @@ -587,7 +587,7 @@ void LookupIterator::PrepareTransitionToDataProperty( // Don't set enumeration index (it will be set during value store). property_details_ = PropertyDetails(PropertyKind::kData, attributes, - PropertyCell::InitialType(isolate_, value)); + PropertyCell::InitialType(isolate_, *value)); transition_ = isolate_->factory()->NewPropertyCell( name(), property_details_, value); has_property_ = true; diff --git a/src/objects/objects.cc b/src/objects/objects.cc index 1df89d2593..2cb899356b 100644 --- a/src/objects/objects.cc +++ b/src/objects/objects.cc @@ -6516,38 +6516,36 @@ Handle PropertyCell::InvalidateAndReplaceEntry( return new_cell; } -static bool RemainsConstantType(Handle cell, - Handle value) { +static bool RemainsConstantType(PropertyCell cell, Object value) { + DisallowGarbageCollection no_gc; // TODO(dcarney): double->smi and smi->double transition from kConstant - if (cell->value().IsSmi() && value->IsSmi()) { + if (cell.value().IsSmi() && value.IsSmi()) { return true; - } else if (cell->value().IsHeapObject() && value->IsHeapObject()) { - return HeapObject::cast(cell->value()).map() == - HeapObject::cast(*value).map() && - HeapObject::cast(*value).map().is_stable(); + } else if (cell.value().IsHeapObject() && value.IsHeapObject()) { + Map map = HeapObject::cast(value).map(); + return HeapObject::cast(cell.value()).map() == map && map.is_stable(); } return false; } // static -PropertyCellType PropertyCell::InitialType(Isolate* isolate, - Handle value) { - return value->IsUndefined(isolate) ? PropertyCellType::kUndefined - : PropertyCellType::kConstant; +PropertyCellType PropertyCell::InitialType(Isolate* isolate, Object value) { + return value.IsUndefined(isolate) ? PropertyCellType::kUndefined + : PropertyCellType::kConstant; } // static -PropertyCellType PropertyCell::UpdatedType(Isolate* isolate, - Handle cell, - Handle value, +PropertyCellType PropertyCell::UpdatedType(Isolate* isolate, PropertyCell cell, + Object value, PropertyDetails details) { - DCHECK(!value->IsTheHole(isolate)); - DCHECK(!cell->value().IsTheHole(isolate)); + DisallowGarbageCollection no_gc; + DCHECK(!value.IsTheHole(isolate)); + DCHECK(!cell.value().IsTheHole(isolate)); switch (details.cell_type()) { case PropertyCellType::kUndefined: return PropertyCellType::kConstant; case PropertyCellType::kConstant: - if (*value == cell->value()) return PropertyCellType::kConstant; + if (value == cell.value()) return PropertyCellType::kConstant; V8_FALLTHROUGH; case PropertyCellType::kConstantType: if (RemainsConstantType(cell, value)) { @@ -6565,9 +6563,9 @@ Handle PropertyCell::PrepareForAndSetValue( Isolate* isolate, Handle dictionary, InternalIndex entry, Handle value, PropertyDetails details) { DCHECK(!value->IsTheHole(isolate)); - Handle cell(dictionary->CellAt(entry), isolate); - CHECK(!cell->value().IsTheHole(isolate)); - const PropertyDetails original_details = cell->property_details(); + PropertyCell raw_cell = dictionary->CellAt(entry); + CHECK(!raw_cell.value().IsTheHole(isolate)); + const PropertyDetails original_details = raw_cell.property_details(); // Data accesses could be cached in ics or optimized code. bool invalidate = original_details.kind() == PropertyKind::kData && details.kind() == PropertyKind::kAccessor; @@ -6576,9 +6574,11 @@ Handle PropertyCell::PrepareForAndSetValue( details = details.set_index(index); PropertyCellType new_type = - UpdatedType(isolate, cell, value, original_details); + UpdatedType(isolate, raw_cell, *value, original_details); details = details.set_cell_type(new_type); + Handle cell(raw_cell, isolate); + if (invalidate) { cell = PropertyCell::InvalidateAndReplaceEntry(isolate, dictionary, entry, details, value); diff --git a/src/objects/property-cell.h b/src/objects/property-cell.h index a85bc1e4df..7b607cd90b 100644 --- a/src/objects/property-cell.h +++ b/src/objects/property-cell.h @@ -41,14 +41,12 @@ class PropertyCell // For protectors: void InvalidateProtector(); - static PropertyCellType InitialType(Isolate* isolate, Handle value); + static PropertyCellType InitialType(Isolate* isolate, Object value); // Computes the new type of the cell's contents for the given value, but // without actually modifying the details. - static PropertyCellType UpdatedType(Isolate* isolate, - Handle cell, - Handle value, - PropertyDetails details); + static PropertyCellType UpdatedType(Isolate* isolate, PropertyCell cell, + Object value, PropertyDetails details); // Prepares property cell at given entry for receiving given value and sets // that value. As a result the old cell could be invalidated and/or dependent diff --git a/src/web-snapshot/web-snapshot.cc b/src/web-snapshot/web-snapshot.cc index f8bb6a9c5f..ee91151bb4 100644 --- a/src/web-snapshot/web-snapshot.cc +++ b/src/web-snapshot/web-snapshot.cc @@ -1788,7 +1788,7 @@ void WebSnapshotDeserializer::DeserializeExports() { PropertyDetails property_details = PropertyDetails(PropertyKind::kData, NONE, - PropertyCell::InitialType(isolate_, export_value)); + PropertyCell::InitialType(isolate_, *export_value)); Handle transition_cell = isolate_->factory()->NewPropertyCell( export_name, property_details, export_value);