From 37c298bcafcb2904b31184212822ddef51b22fca Mon Sep 17 00:00:00 2001 From: Gang Zhao Date: Mon, 21 Oct 2024 11:49:18 -0700 Subject: [PATCH] Add pointer of owning object to GCPointer::set II Differential Revision: D62227030 --- include/hermes/VM/GCPointer-inline.h | 8 ++++++-- include/hermes/VM/GCPointer.h | 14 +++++++++++--- lib/VM/HiddenClass.cpp | 13 +++++++------ lib/VM/JSObject.cpp | 6 ++++-- lib/VM/OrderedHashMap.cpp | 21 ++++++++++++++------- 5 files changed, 42 insertions(+), 20 deletions(-) diff --git a/include/hermes/VM/GCPointer-inline.h b/include/hermes/VM/GCPointer-inline.h index 582bdd5d010..413204525f6 100644 --- a/include/hermes/VM/GCPointer-inline.h +++ b/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); } diff --git a/include/hermes/VM/GCPointer.h b/include/hermes/VM/GCPointer.h index 5b6d545b5d4..60d86966caf 100644 --- a/include/hermes/VM/GCPointer.h +++ b/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 &ptr, GC &gc) { - GCPointerBase::set(base, ptr, gc); + void set( + PointerBase &base, + const GCPointer &ptr, + GC &gc, + const GCCell *owningObj) { + GCPointerBase::set(base, ptr, gc, owningObj); } }; diff --git a/lib/VM/HiddenClass.cpp b/lib/VM/HiddenClass.cpp index cbc32ffdc55..0ada602aced 100644 --- a/lib/VM/HiddenClass.cpp +++ b/lib/VM/HiddenClass.cpp @@ -213,7 +213,7 @@ Handle 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, 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, 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::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::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? diff --git a/lib/VM/JSObject.cpp b/lib/VM/JSObject.cpp index 01bc5547169..ad80219d23f 100644 --- a/lib/VM/JSObject.cpp +++ b/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 diff --git a/lib/VM/OrderedHashMap.cpp b/lib/VM/OrderedHashMap.cpp index 5c45095cd3b..ab6438a7e76 100644 --- a/lib/VM/OrderedHashMap.cpp +++ b/lib/VM/OrderedHashMap.cpp @@ -83,15 +83,18 @@ void OrderedHashMapBase::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(this)); } entry->prevIterationEntry.setNull(runtime.getHeap()); } @@ -346,7 +349,7 @@ ExecutionStatus OrderedHashMapBase::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::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(this)); firstIterationEntry_.getNonNull(runtime)->prevIterationEntry.setNull( runtime.getHeap()); size_ = 0;