Pass the pointer of owning object in GCPointer::set() part II (#1509)

Summary: Pull Request resolved: #1509

Differential Revision: D62227030

Author: lavenzg

include/hermes/VM/GCPointer-inline.h

@@ -59,12 +59,16 @@ inline void GCPointerBase::setNonNull(
   setNoBarrier(CompressedPointer::encodeNonNull(ptr, base));
 }
 
-inline void
-GCPointerBase::set(PointerBase &base, CompressedPointer ptr, GC &gc) {
+inline void GCPointerBase::set(
+    PointerBase &base,
+    CompressedPointer ptr,
+    GC &gc,
+    const GCCell *owningObj) {
   assert(
       (!ptr || gc.validPointer(ptr.get(base))) &&
       "Cannot set a GCPointer to an invalid pointer");
   // Write barrier must happen before the write.
+  (void)owningObj;
   gc.writeBarrier(this, ptr.get(base));
   setNoBarrier(ptr);
 }

include/hermes/VM/GCPointer.h

@@ -41,7 +41,11 @@ class GCPointerBase : public CompressedPointer {
   /// \param owningObj The object that contains this GCPointer.
   inline void
   set(PointerBase &base, GCCell *ptr, GC &gc, const GCCell *owningObj);
-  inline void set(PointerBase &base, CompressedPointer ptr, GC &gc);
+  inline void set(
+      PointerBase &base,
+      CompressedPointer ptr,
+      GC &gc,
+      const GCCell *owningObj);
   inline void
   setNonNull(PointerBase &base, GCCell *ptr, GC &gc, const GCCell *owningObj);
 
@@ -102,8 +106,12 @@ class GCPointer : public GCPointerBase {
   }
 
   /// Convenience overload of GCPointer::set for other GCPointers.
-  void set(PointerBase &base, const GCPointer<T> &ptr, GC &gc) {
-    GCPointerBase::set(base, ptr, gc);
+  void set(
+      PointerBase &base,
+      const GCPointer<T> &ptr,
+      GC &gc,
+      const GCCell *owningObj) {
+    GCPointerBase::set(base, ptr, gc, owningObj);
   }
 };
 

lib/VM/HiddenClass.cpp

@@ -213,7 +213,7 @@ Handle<HiddenClass> HiddenClass::copyToNewDictionary(
     initializeMissingPropertyMap(selfHandle, runtime);
 
   newClassHandle->propertyMap_.set(
-      runtime, selfHandle->propertyMap_, runtime.getHeap());
+      runtime, selfHandle->propertyMap_, runtime.getHeap(), *newClassHandle);
   selfHandle->propertyMap_.setNull(runtime.getHeap());
 
   LLVM_DEBUG(
@@ -436,7 +436,7 @@ CallResult<std::pair<Handle<HiddenClass>, SlotIndex>> HiddenClass::addProperty(
         return ExecutionStatus::EXCEPTION;
       }
       childHandle->propertyMap_.set(
-          runtime, selfHandle->propertyMap_, runtime.getHeap());
+          runtime, selfHandle->propertyMap_, runtime.getHeap(), *childHandle);
     } else {
       LLVM_DEBUG(
           dbgs() << "Adding property " << runtime.formatSymbolID(name)
@@ -512,7 +512,7 @@ CallResult<std::pair<Handle<HiddenClass>, SlotIndex>> HiddenClass::addProperty(
 
     // Move the map to the child class.
     childHandle->propertyMap_.set(
-        runtime, selfHandle->propertyMap_, runtime.getHeap());
+        runtime, selfHandle->propertyMap_, runtime.getHeap(), *childHandle);
     selfHandle->propertyMap_.setNull(runtime.getHeap());
 
     if (LLVM_UNLIKELY(
@@ -589,7 +589,7 @@ Handle<HiddenClass> HiddenClass::updateProperty(
 
       descPair->second.flags = newFlags;
       existingChild->propertyMap_.set(
-          runtime, selfHandle->propertyMap_, runtime.getHeap());
+          runtime, selfHandle->propertyMap_, runtime.getHeap(), existingChild);
     } else {
       LLVM_DEBUG(
           dbgs() << "Updating property " << runtime.formatSymbolID(name)
@@ -634,7 +634,7 @@ Handle<HiddenClass> HiddenClass::updateProperty(
 
   // Move the updated map to the child class.
   childHandle->propertyMap_.set(
-      runtime, selfHandle->propertyMap_, runtime.getHeap());
+      runtime, selfHandle->propertyMap_, runtime.getHeap(), *childHandle);
   selfHandle->propertyMap_.setNull(runtime.getHeap());
 
   return childHandle;
@@ -915,7 +915,8 @@ void HiddenClass::stealPropertyMapFromParent(
   self->propertyMap_.set(
       runtime,
       self->parent_.getNonNull(runtime)->propertyMap_,
-      runtime.getHeap());
+      runtime.getHeap(),
+      self);
   self->parent_.getNonNull(runtime)->propertyMap_.setNull(runtime.getHeap());
 
   // Does our class add a new property?

lib/VM/JSObject.cpp

@@ -3015,9 +3015,11 @@ JSObject::checkPropertyUpdate(
 
     // If not setting the getter or the setter, re-use the current one.
     if (!dpFlags.setGetter)
-      newAccessor->getter.set(runtime, curAccessor->getter, runtime.getHeap());
+      newAccessor->getter.set(
+          runtime, curAccessor->getter, runtime.getHeap(), newAccessor);
     if (!dpFlags.setSetter)
-      newAccessor->setter.set(runtime, curAccessor->setter, runtime.getHeap());
+      newAccessor->setter.set(
+          runtime, curAccessor->setter, runtime.getHeap(), newAccessor);
   }
 
   // 8.12.9 [12] For each attribute field of Desc that is present, set the

lib/VM/OrderedHashMap.cpp

@@ -83,15 +83,18 @@ void OrderedHashMapBase<BucketType, Derived>::removeLinkedListNode(
       entry != lastIterationEntry_.get(runtime) &&
       "Cannot remove the last entry");
   if (entry->prevIterationEntry) {
-    entry->prevIterationEntry.getNonNull(runtime)->nextIterationEntry.set(
-        runtime, entry->nextIterationEntry, gc);
+    auto *prevIterationEntry = entry->prevIterationEntry.getNonNull(runtime);
+    prevIterationEntry->nextIterationEntry.set(
+        runtime, entry->nextIterationEntry, gc, prevIterationEntry);
   }
   if (entry->nextIterationEntry) {
-    entry->nextIterationEntry.getNonNull(runtime)->prevIterationEntry.set(
-        runtime, entry->prevIterationEntry, gc);
+    auto *nextIterationEntry = entry->nextIterationEntry.getNonNull(runtime);
+    nextIterationEntry->prevIterationEntry.set(
+        runtime, entry->prevIterationEntry, gc, nextIterationEntry);
   }
   if (entry == firstIterationEntry_.get(runtime)) {
-    firstIterationEntry_.set(runtime, entry->nextIterationEntry, gc);
+    firstIterationEntry_.set(
+        runtime, entry->nextIterationEntry, gc, static_cast<Derived *>(this));
   }
   entry->prevIterationEntry.setNull(runtime.getHeap());
 }
@@ -346,7 +349,7 @@ ExecutionStatus OrderedHashMapBase<BucketType, Derived>::doInsert(
     previousLastEntry->nextIterationEntry.set(
         runtime, newMapEntry.get(), heap, previousLastEntry);
     newMapEntry->prevIterationEntry.set(
-        runtime, rawSelf->lastIterationEntry_, heap);
+        runtime, rawSelf->lastIterationEntry_, heap, *newMapEntry);
 
     rawSelf->lastIterationEntry_.set(runtime, newMapEntry.get(), heap, *self);
 
@@ -443,7 +446,11 @@ void OrderedHashMapBase<BucketType, Derived>::clear(Runtime &runtime) {
   // in case there is an iterator out there
   // pointing to the middle of the iteration chain. We need it to be
   // able to merge back eventually.
-  firstIterationEntry_.set(runtime, lastIterationEntry_, runtime.getHeap());
+  firstIterationEntry_.set(
+      runtime,
+      lastIterationEntry_,
+      runtime.getHeap(),
+      static_cast<Derived *>(this));
   firstIterationEntry_.getNonNull(runtime)->prevIterationEntry.setNull(
       runtime.getHeap());
   size_ = 0;