[turbofan] validate more concurrent reads

tebbi · V8 LUCI CQ · commit ebe5675360e4 · 2022-10-06T12:26:47.000Z
Bug: chromium:1369871 Change-Id: Ib8786b97b2f9555cfcb84a197182c4f2ab5c30e8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3936273 Reviewed-by: Jakob Linke <jgruber@chromium.org> Auto-Submit: Tobias Tebbi <tebbi@chromium.org> Commit-Queue: Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/heads/main@{#83555}
diff --git a/src/compiler/compilation-dependencies.cc b/src/compiler/compilation-dependencies.cc
@@ -34,7 +34,8 @@ namespace compiler {
   V(Protector)                          \
   V(PrototypeProperty)                  \
   V(StableMap)                          \
-  V(Transition)
+  V(Transition)                         \
+  V(ObjectSlotValue)
 
 CompilationDependencies::CompilationDependencies(JSHeapBroker* broker,
                                                  Zone* zone)
@@ -868,6 +869,42 @@ class ProtectorDependency final : public CompilationDependency {
   const PropertyCellRef cell_;
 };
 
+// Check that an object slot will not change during compilation.
+class ObjectSlotValueDependency final : public CompilationDependency {
+ public:
+  explicit ObjectSlotValueDependency(const HeapObjectRef& object, int offset,
+                                     const ObjectRef& value)
+      : CompilationDependency(kObjectSlotValue),
+        object_(object.object()),
+        offset_(offset),
+        value_(value.object()) {}
+
+  bool IsValid() const override {
+    PtrComprCageBase cage_base = GetPtrComprCageBase(*object_);
+    Object current_value =
+        offset_ == HeapObject::kMapOffset
+            ? object_->map()
+            : TaggedField<Object>::Relaxed_Load(cage_base, *object_, offset_);
+    return *value_ == current_value;
+  }
+  void Install(PendingDependencies* deps) const override {}
+
+ private:
+  size_t Hash() const override {
+    return base::hash_combine(object_.address(), offset_, value_.address());
+  }
+
+  bool Equals(const CompilationDependency* that) const override {
+    const ObjectSlotValueDependency* const zat = that->AsObjectSlotValue();
+    return object_->address() == zat->object_->address() &&
+           offset_ == zat->offset_ && value_.address() == zat->value_.address();
+  }
+
+  Handle<HeapObject> object_;
+  int offset_;
+  Handle<Object> value_;
+};
+
 class ElementsKindDependency final : public CompilationDependency {
  public:
   ElementsKindDependency(const AllocationSiteRef& site, ElementsKind kind)
@@ -1120,6 +1157,12 @@ void CompilationDependencies::DependOnElementsKind(
   }
 }
 
+void CompilationDependencies::DependOnObjectSlotValue(
+    const HeapObjectRef& object, int offset, const ObjectRef& value) {
+  RecordDependency(
+      zone_->New<ObjectSlotValueDependency>(object, offset, value));
+}
+
 void CompilationDependencies::DependOnOwnConstantElement(
     const JSObjectRef& holder, uint32_t index, const ObjectRef& element) {
   RecordDependency(
diff --git a/src/compiler/compilation-dependencies.h b/src/compiler/compilation-dependencies.h
@@ -93,6 +93,10 @@ class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
   // Record the assumption that {site}'s {ElementsKind} doesn't change.
   void DependOnElementsKind(const AllocationSiteRef& site);
 
+  // Check that an object slot will not change during compilation.
+  void DependOnObjectSlotValue(const HeapObjectRef& object, int offset,
+                               const ObjectRef& value);
+
   void DependOnOwnConstantElement(const JSObjectRef& holder, uint32_t index,
                                   const ObjectRef& element);
 
diff --git a/src/compiler/js-create-lowering.cc b/src/compiler/js-create-lowering.cc
@@ -1673,6 +1673,10 @@ base::Optional<Node*> JSCreateLowering::TryAllocateFastLiteral(
 
   // Now that we hold the migration lock, get the current map.
   MapRef boilerplate_map = boilerplate.map();
+  // Protect against concurrent changes to the boilerplate object by checking
+  // for an identical value at the end of the compilation.
+  dependencies()->DependOnObjectSlotValue(boilerplate, HeapObject::kMapOffset,
+                                          boilerplate_map);
   {
     base::Optional<MapRef> current_boilerplate_map =
         boilerplate.map_direct_read();
@@ -1838,10 +1842,18 @@ base::Optional<Node*> JSCreateLowering::TryAllocateFastLiteralElements(
       boilerplate.elements(kRelaxedLoad);
   if (!maybe_boilerplate_elements.has_value()) return {};
   FixedArrayBaseRef boilerplate_elements = maybe_boilerplate_elements.value();
+  // Protect against concurrent changes to the boilerplate object by checking
+  // for an identical value at the end of the compilation.
+  dependencies()->DependOnObjectSlotValue(
+      boilerplate, JSObject::kElementsOffset, boilerplate_elements);
 
   // Empty or copy-on-write elements just store a constant.
   int const elements_length = boilerplate_elements.length();
   MapRef elements_map = boilerplate_elements.map();
+  // Protect against concurrent changes to the boilerplate object by checking
+  // for an identical value at the end of the compilation.
+  dependencies()->DependOnObjectSlotValue(boilerplate_elements,
+                                          HeapObject::kMapOffset, elements_map);
   if (boilerplate_elements.length() == 0 || elements_map.IsFixedCowArrayMap()) {
     if (allocation == AllocationType::kOld &&
         !boilerplate.IsElementsTenured(boilerplate_elements)) {
