Skip to content

Commit

Permalink
Pass the pointer of owning object in GCPointer::set() part II (facebo…
Browse files Browse the repository at this point in the history
…ok#1509)

Summary: Pull Request resolved: facebook#1509

Differential Revision: D62227030
  • Loading branch information
lavenzg authored and facebook-github-bot committed Oct 21, 2024
1 parent 9223b4a commit 7944e4c
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 20 deletions.
8 changes: 6 additions & 2 deletions include/hermes/VM/GCPointer-inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
14 changes: 11 additions & 3 deletions include/hermes/VM/GCPointer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
};

Expand Down
13 changes: 7 additions & 6 deletions lib/VM/HiddenClass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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?
Expand Down
6 changes: 4 additions & 2 deletions lib/VM/JSObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 14 additions & 7 deletions lib/VM/OrderedHashMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 7944e4c

Please sign in to comment.