github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-01-09 23:48:01 -05:00
Code Issues Packages Projects Releases Wiki Activity
Files
135f127db8966f60750817fcba39d09681ab3a32
electron/patches/v8/preserve_field_repr_in_property_array_extension.patch
Shelley Vohr 6b797a84a8 chore: cherry-pick 4cf9311 from v8 (#48948)
2025-11-13 14:57:18 -08:00

303 lines
14 KiB
Diff
Raw Blame History

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Leszek Swirski <leszeks@chromium.org>
Date: Wed, 12 Nov 2025 16:46:01 +0100
Subject: Preserve field repr in property array extension
Walk the descriptor array in lockstep with the property array when
extending the latter.
Fixed: 460017370
Change-Id: If0b4fc3c5f62fc0cc373588cbddc3c0a95c7225c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7146166
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#103674}
diff --git a/src/compiler/access-builder.cc b/src/compiler/access-builder.cc
index d8f618f2bbec1c854956225dd568c235dea780cb..ad6172b555f86cb7d9f5fdf8912f407cd69139d4 100644
--- a/src/compiler/access-builder.cc
+++ b/src/compiler/access-builder.cc
@@ -4,6 +4,8 @@
#include "src/compiler/access-builder.h"
+#include "src/codegen/machine-type.h"
+#include "src/compiler/property-access-builder.h"
#include "src/compiler/type-cache.h"
#include "src/handles/handles-inl.h"
#include "src/objects/arguments.h"
@@ -1070,12 +1072,16 @@ FieldAccess AccessBuilder::ForFeedbackVectorSlot(int index) {
}
// static
-FieldAccess AccessBuilder::ForPropertyArraySlot(int index) {
+FieldAccess AccessBuilder::ForPropertyArraySlot(int index,
+ Representation representation) {
int offset = PropertyArray::OffsetOfElementAt(index);
- FieldAccess access = {kTaggedBase, offset,
- Handle<Name>(), OptionalMapRef(),
- Type::Any(), MachineType::AnyTagged(),
- kFullWriteBarrier, "PropertyArraySlot"};
+ MachineType machine_type =
+ representation.IsHeapObject() || representation.IsDouble()
+ ? MachineType::TaggedPointer()
+ : MachineType::AnyTagged();
+ FieldAccess access = {
+ kTaggedBase, offset, Handle<Name>(), OptionalMapRef(),
+ Type::Any(), machine_type, kFullWriteBarrier, "PropertyArraySlot"};
return access;
}
diff --git a/src/compiler/access-builder.h b/src/compiler/access-builder.h
index 8e7d9cdf979f5daebd4daf605293320b1b105c77..4feaefc0a6cf401cd528b162f212d931c4d71cbf 100644
--- a/src/compiler/access-builder.h
+++ b/src/compiler/access-builder.h
@@ -11,6 +11,7 @@
#include "src/compiler/write-barrier-kind.h"
#include "src/objects/elements-kind.h"
#include "src/objects/js-objects.h"
+#include "src/objects/property-details.h"
namespace v8 {
namespace internal {
@@ -318,7 +319,8 @@ class V8_EXPORT_PRIVATE AccessBuilder final
static FieldAccess ForFeedbackVectorSlot(int index);
// Provides access to PropertyArray slots.
- static FieldAccess ForPropertyArraySlot(int index);
+ static FieldAccess ForPropertyArraySlot(int index,
+ Representation representation);
// Provides access to ScopeInfo flags.
static FieldAccess ForScopeInfoFlags();
diff --git a/src/compiler/js-native-context-specialization.cc b/src/compiler/js-native-context-specialization.cc
index faeda3e2623afbd453acc61ea05ec534b060a31e..dc8fcd4c65658c6dd32606a3ce3ff4952b4d9364 100644
--- a/src/compiler/js-native-context-specialization.cc
+++ b/src/compiler/js-native-context-specialization.cc
@@ -38,6 +38,7 @@
#include "src/objects/elements-kind.h"
#include "src/objects/feedback-vector.h"
#include "src/objects/heap-number.h"
+#include "src/objects/property-details.h"
#include "src/objects/string.h"
namespace v8 {
@@ -4235,25 +4236,59 @@ Node* JSNativeContextSpecialization::BuildExtendPropertiesBackingStore(
// for intermediate states of chains of property additions. That makes
// it unclear what the best approach is here.
DCHECK_EQ(map.UnusedPropertyFields(), 0);
- int length = map.NextFreePropertyIndex() - map.GetInObjectProperties();
+ int in_object_length = map.GetInObjectProperties();
+ int length = map.NextFreePropertyIndex() - in_object_length;
// Under normal circumstances, NextFreePropertyIndex() will always be larger
// than GetInObjectProperties(). However, an attacker able to corrupt heap
// memory can break this invariant, in which case we'll get confused here,
// potentially causing a sandbox violation. This CHECK defends against that.
SBXCHECK_GE(length, 0);
int new_length = length + JSObject::kFieldsAdded;
+
+ // Find the descriptor index corresponding to the first out-of-object
+ // property.
+ DescriptorArrayRef descs = map.instance_descriptors(broker());
+ InternalIndex first_out_of_object_descriptor(in_object_length);
+ InternalIndex number_of_descriptors(descs.object()->number_of_descriptors());
+ for (InternalIndex i(in_object_length); i < number_of_descriptors; ++i) {
+ PropertyDetails details = descs.GetPropertyDetails(i);
+ // Skip over non-field properties.
+ if (details.location() != PropertyLocation::kField) {
+ continue;
+ }
+ // Skip over in-object fields.
+ // TODO(leszeks): We could make this smarter, like a binary search.
+ if (details.field_index() < in_object_length) {
+ continue;
+ }
+ first_out_of_object_descriptor = i;
+ break;
+ }
+
// Collect the field values from the {properties}.
- ZoneVector<Node*> values(zone());
+ ZoneVector<std::pair<Node*, Representation>> values(zone());
values.reserve(new_length);
- for (int i = 0; i < length; ++i) {
+
+ // Walk the property descriptors alongside the property values, to make
+ // sure to get and store them with the right machine type.
+ InternalIndex descriptor = first_out_of_object_descriptor;
+ for (int i = 0; i < length; ++i, ++descriptor) {
+ PropertyDetails details = descs.GetPropertyDetails(descriptor);
+ while (details.location() != PropertyLocation::kField) {
+ ++descriptor;
+ details = descs.GetPropertyDetails(descriptor);
+ }
+ DCHECK_EQ(i, details.field_index() - in_object_length);
Node* value = effect = graph()->NewNode(
- simplified()->LoadField(AccessBuilder::ForFixedArraySlot(i)),
+ simplified()->LoadField(
+ AccessBuilder::ForPropertyArraySlot(i, details.representation())),
properties, effect, control);
- values.push_back(value);
+ values.push_back({value, details.representation()});
}
// Initialize the new fields to undefined.
for (int i = 0; i < JSObject::kFieldsAdded; ++i) {
- values.push_back(jsgraph()->UndefinedConstant());
+ values.push_back(
+ {jsgraph()->UndefinedConstant(), Representation::Tagged()});
}
// Compute new length and hash.
@@ -4291,7 +4326,8 @@ Node* JSNativeContextSpecialization::BuildExtendPropertiesBackingStore(
a.Store(AccessBuilder::ForMap(), jsgraph()->PropertyArrayMapConstant());
a.Store(AccessBuilder::ForPropertyArrayLengthAndHash(), new_length_and_hash);
for (int i = 0; i < new_length; ++i) {
- a.Store(AccessBuilder::ForFixedArraySlot(i), values[i]);
+ a.Store(AccessBuilder::ForPropertyArraySlot(i, values[i].second),
+ values[i].first);
}
return a.Finish();
}
diff --git a/src/compiler/turboshaft/turbolev-early-lowering-reducer-inl.h b/src/compiler/turboshaft/turbolev-early-lowering-reducer-inl.h
index 0c63f5ca2083976cd8cf3c1df003562acdadca76..d8b5571b3ca5b5d4e10ca87985b8bdb814ab46f2 100644
--- a/src/compiler/turboshaft/turbolev-early-lowering-reducer-inl.h
+++ b/src/compiler/turboshaft/turbolev-early-lowering-reducer-inl.h
@@ -14,6 +14,7 @@
#include "src/compiler/turboshaft/representations.h"
#include "src/deoptimizer/deoptimize-reason.h"
#include "src/objects/contexts.h"
+#include "src/objects/descriptor-array-inl.h"
#include "src/objects/instance-type-inl.h"
namespace v8::internal::compiler::turboshaft {
@@ -318,8 +319,32 @@ class TurbolevEarlyLoweringReducer : public Next {
}
V<PropertyArray> ExtendPropertiesBackingStore(
- V<PropertyArray> old_property_array, V<JSObject> object, int old_length,
+ V<PropertyArray> old_property_array, V<JSObject> object,
+ const compiler::MapRef& old_map, int old_length,
V<FrameState> frame_state, const FeedbackSource& feedback) {
+ int in_object_length = old_map.GetInObjectProperties();
+
+ // Find the descriptor index corresponding to the first out-of-object
+ // property.
+ DescriptorArrayRef descs = old_map.instance_descriptors(broker_);
+ InternalIndex first_out_of_object_descriptor(in_object_length);
+ InternalIndex number_of_descriptors(
+ descs.object()->number_of_descriptors());
+ for (InternalIndex i(in_object_length); i < number_of_descriptors; ++i) {
+ PropertyDetails details = descs.GetPropertyDetails(i);
+ // Skip over non-field properties.
+ if (details.location() != PropertyLocation::kField) {
+ continue;
+ }
+ // Skip over in-object fields.
+ // TODO(leszeks): We could make this smarter, like a binary search.
+ if (details.field_index() < in_object_length) {
+ continue;
+ }
+ first_out_of_object_descriptor = i;
+ break;
+ }
+
// Allocate new PropertyArray.
int new_length = old_length + JSObject::kFieldsAdded;
Uninitialized<PropertyArray> new_property_array =
@@ -330,18 +355,28 @@ class TurbolevEarlyLoweringReducer : public Next {
__ HeapConstant(factory_->property_array_map()));
// Copy existing properties over.
- for (int i = 0; i < old_length; i++) {
+ InternalIndex descriptor = first_out_of_object_descriptor;
+ for (int i = 0; i < old_length; ++i, ++descriptor) {
+ PropertyDetails details = descs.GetPropertyDetails(descriptor);
+ while (details.location() != PropertyLocation::kField) {
+ ++descriptor;
+ details = descs.GetPropertyDetails(descriptor);
+ }
+ DCHECK_EQ(i, details.field_index() - in_object_length);
+ Representation r = details.representation();
+
V<Object> old_value = __ template LoadField<Object>(
- old_property_array, AccessBuilder::ForPropertyArraySlot(i));
+ old_property_array, AccessBuilder::ForPropertyArraySlot(i, r));
__ InitializeField(new_property_array,
- AccessBuilder::ForPropertyArraySlot(i), old_value);
+ AccessBuilder::ForPropertyArraySlot(i, r), old_value);
}
// Initialize new properties to undefined.
V<Undefined> undefined = __ HeapConstant(factory_->undefined_value());
for (int i = 0; i < JSObject::kFieldsAdded; ++i) {
__ InitializeField(new_property_array,
- AccessBuilder::ForPropertyArraySlot(old_length + i),
+ AccessBuilder::ForPropertyArraySlot(
+ old_length + i, Representation::Tagged()),
undefined);
}
diff --git a/src/compiler/turboshaft/turbolev-graph-builder.cc b/src/compiler/turboshaft/turbolev-graph-builder.cc
index 34b06d2ba836f60bf940612564c92b52dceaf5b3..819ac484904cd72aad64a4193f82db47a02dabd8 100644
--- a/src/compiler/turboshaft/turbolev-graph-builder.cc
+++ b/src/compiler/turboshaft/turbolev-graph-builder.cc
@@ -2789,10 +2789,11 @@ class GraphBuildingNodeProcessor {
maglev::ProcessResult Process(maglev::ExtendPropertiesBackingStore* node,
const maglev::ProcessingState& state) {
GET_FRAME_STATE_MAYBE_ABORT(frame_state, node->eager_deopt_info());
- SetMap(node, __ ExtendPropertiesBackingStore(
- Map(node->property_array_input()),
- Map(node->object_input()), node->old_length(), frame_state,
- node->eager_deopt_info()->feedback_to_update()));
+ SetMap(node,
+ __ ExtendPropertiesBackingStore(
+ Map(node->property_array_input()), Map(node->object_input()),
+ node->old_map(), node->old_length(), frame_state,
+ node->eager_deopt_info()->feedback_to_update()));
return maglev::ProcessResult::kContinue;
}
diff --git a/src/maglev/maglev-graph-builder.cc b/src/maglev/maglev-graph-builder.cc
index 6cfe938ae94d86e023be208f3ec09822ec569fb3..ba28636e41a4a46e08c957b2640c9f20c911b9ce 100644
--- a/src/maglev/maglev-graph-builder.cc
+++ b/src/maglev/maglev-graph-builder.cc
@@ -5189,7 +5189,7 @@ ReduceResult MaglevGraphBuilder::BuildExtendPropertiesBackingStore(
// potentially causing a sandbox violation. This CHECK defends against that.
SBXCHECK_GE(length, 0);
return AddNewNode<ExtendPropertiesBackingStore>({property_array, receiver},
- length);
+ map, length);
}
MaybeReduceResult MaglevGraphBuilder::TryBuildStoreField(
diff --git a/src/maglev/maglev-ir.h b/src/maglev/maglev-ir.h
index 4792ca5387c9f4229179a66ff6c15346e222342b..233eed0baf18aa814caae4b3241130ff289157ca 100644
--- a/src/maglev/maglev-ir.h
+++ b/src/maglev/maglev-ir.h
@@ -9165,8 +9165,10 @@ class ExtendPropertiesBackingStore
using Base = FixedInputValueNodeT<2, ExtendPropertiesBackingStore>;
public:
- explicit ExtendPropertiesBackingStore(uint64_t bitfield, int old_length)
- : Base(bitfield), old_length_(old_length) {}
+ explicit ExtendPropertiesBackingStore(uint64_t bitfield,
+ const compiler::MapRef& old_map,
+ int old_length)
+ : Base(bitfield), old_map_(old_map), old_length_(old_length) {}
static constexpr OpProperties kProperties =
OpProperties::CanAllocate() | OpProperties::CanRead() |
@@ -9186,9 +9188,11 @@ class ExtendPropertiesBackingStore
void GenerateCode(MaglevAssembler*, const ProcessingState&);
void PrintParams(std::ostream&) const;
+ const compiler::MapRef& old_map() const { return old_map_; }
int old_length() const { return old_length_; }
private:
+ const compiler::MapRef old_map_;
const int old_length_;
};
Reference in New Issue View Git Blame Copy Permalink