Skip to content

Commit

Permalink
Add pointer of owning object to GCPointer::set
Browse files Browse the repository at this point in the history
Differential Revision: D62222257
  • Loading branch information
lavenzg authored and facebook-github-bot committed Sep 5, 2024
1 parent 16698e5 commit 6703f45
Show file tree
Hide file tree
Showing 21 changed files with 87 additions and 57 deletions.
14 changes: 12 additions & 2 deletions include/hermes/VM/GCPointer-inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,29 @@ GCPointerBase::GCPointerBase(
}
}

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

inline void GCPointerBase::setNonNull(PointerBase &base, GCCell *ptr, GC &gc) {
inline void GCPointerBase::setNonNull(
PointerBase &base,
GCCell *ptr,
GC &gc,
const GCCell *owningObj) {
assert(
gc.validPointer(ptr) && "Cannot set a GCPointer to an invalid pointer");
// Write barrier must happen before the write.
(void)owningObj;
gc.writeBarrier(this, ptr);
setNoBarrier(CompressedPointer::encodeNonNull(ptr, base));
}
Expand Down
16 changes: 10 additions & 6 deletions include/hermes/VM/GCPointer.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,12 @@ class GCPointerBase : public CompressedPointer {
/// \param ptr The memory being pointed to.
/// \param base The base of ptr.
/// \param gc Used for write barriers.
inline void set(PointerBase &base, GCCell *ptr, GC &gc);
/// \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 setNonNull(PointerBase &base, GCCell *ptr, GC &gc);
inline void
setNonNull(PointerBase &base, GCCell *ptr, GC &gc, const GCCell *owningObj);

/// Set this pointer to null. This needs a write barrier in some types of
/// garbage collectors.
Expand Down Expand Up @@ -90,11 +93,12 @@ class GCPointer : public GCPointerBase {
/// \param base The base of ptr.
/// \param ptr The memory being pointed to.
/// \param gc Used for write barriers.
void set(PointerBase &base, T *ptr, GC &gc) {
GCPointerBase::set(base, ptr, gc);
/// \param owningObj The object that contains this GCPointer.
void set(PointerBase &base, T *ptr, GC &gc, const GCCell *owningObj) {
GCPointerBase::set(base, ptr, gc, owningObj);
}
void setNonNull(PointerBase &base, T *ptr, GC &gc) {
GCPointerBase::setNonNull(base, ptr, gc);
void setNonNull(PointerBase &base, T *ptr, GC &gc, const GCCell *owningObj) {
GCPointerBase::setNonNull(base, ptr, gc, owningObj);
}

/// Convenience overload of GCPointer::set for other GCPointers.
Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/HiddenClass.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ class HiddenClass final : public GCCell {
}

void setForInCache(BigStorage *arr, Runtime &runtime) {
forInCache_.set(runtime, arr, runtime.getHeap());
forInCache_.set(runtime, arr, runtime.getHeap(), this);
}

void clearForInCache(Runtime &runtime) {
Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/JSArray.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class ArrayImpl : public JSObject {
/// Set the indexed storage of this array to be \p p. The pointer is allowed
/// to be null.
void setIndexedStorage(PointerBase &base, StorageType *p, GC &gc) {
indexedStorage_.set(base, p, gc);
indexedStorage_.set(base, p, gc, this);
}

/// @}
Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/JSDataView.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class JSDataView final : public JSObject {
assert(
offset + length <= buffer->size() &&
"A DataView cannot be looking outside of the storage");
buffer_.setNonNull(runtime, buffer, runtime.getHeap());
buffer_.setNonNull(runtime, buffer, runtime.getHeap(), this);
offset_ = offset;
length_ = length;
}
Expand Down
5 changes: 3 additions & 2 deletions include/hermes/VM/JSMapImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class JSMapIteratorImpl final : public JSObject {
Runtime &runtime,
Handle<JSMapImpl<JSMapTypeTraits<C>::ContainerKind>> data,
IterationKind kind) {
data_.set(runtime, data.get(), runtime.getHeap());
data_.set(runtime, data.get(), runtime.getHeap(), this);
iterationKind_ = kind;

assert(data_ && "Invalid storage data");
Expand All @@ -172,7 +172,8 @@ class JSMapIteratorImpl final : public JSObject {
runtime,
self->data_.getNonNull(runtime)->iteratorNext(
runtime, self->itr_.get(runtime)),
runtime.getHeap());
runtime.getHeap(),
*self);
if (self->itr_) {
switch (self->iterationKind_) {
case IterationKind::Key:
Expand Down
4 changes: 2 additions & 2 deletions include/hermes/VM/JSObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ class JSObject : public GCCell {
/// cycle checking.
static void
unsafeSetParentInternal(JSObject *self, Runtime &runtime, JSObject *parent) {
self->parent_.set(runtime, parent, runtime.getHeap());
self->parent_.set(runtime, parent, runtime.getHeap(), self);
}

/// Return the value of an internal property slot. Use getDirectSlotValue if
Expand Down Expand Up @@ -1686,7 +1686,7 @@ inline ExecutionStatus JSObject::allocatePropStorage(
return ExecutionStatus::EXCEPTION;

selfHandle->propStorage_.setNonNull(
runtime, vmcast<PropStorage>(*res), runtime.getHeap());
runtime, vmcast<PropStorage>(*res), runtime.getHeap(), *selfHandle);
return ExecutionStatus::RETURNED;
}

Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/OrderedHashMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class OrderedHashMapBase {
return ExecutionStatus::EXCEPTION;
}

self->hashTable_.set(runtime, arrRes->get(), runtime.getHeap());
self->hashTable_.set(runtime, arrRes->get(), runtime.getHeap(), *self);
return ExecutionStatus::RETURNED;
}

Expand Down
8 changes: 4 additions & 4 deletions lib/VM/Domain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ ExecutionStatus Domain::importCJSModuleTable(
return ExecutionStatus::EXCEPTION;
}

self->throwingRequire_.set(runtime, *requireFn, runtime.getHeap());
self->throwingRequire_.set(runtime, *requireFn, runtime.getHeap(), *self);
} else {
cjsModules = self->cjsModules_.get(runtime);
}
Expand Down Expand Up @@ -308,7 +308,7 @@ ExecutionStatus Domain::importCJSModuleTable(
}
}

self->cjsModules_.set(runtime, cjsModules.get(), runtime.getHeap());
self->cjsModules_.set(runtime, cjsModules.get(), runtime.getHeap(), *self);
return ExecutionStatus::RETURNED;
}

Expand Down Expand Up @@ -343,8 +343,8 @@ Handle<RequireContext> RequireContext::create(
runtime.getHiddenClassForPrototype(
*objProto, numOverlapSlots<RequireContext>()));
auto self = JSObjectInit::initToHandle(runtime, cell);
self->domain_.set(runtime, *domain, runtime.getHeap());
self->dirname_.set(runtime, *dirname, runtime.getHeap());
self->domain_.set(runtime, *domain, runtime.getHeap(), *self);
self->dirname_.set(runtime, *dirname, runtime.getHeap(), *self);
return self;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/VM/DummyObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ void DummyObject::releaseExtMem(GC &gc) {
}

void DummyObject::setPointer(GC &gc, DummyObject *obj) {
other.set(gc.getPointerBase(), obj, gc);
other.set(gc.getPointerBase(), obj, gc, this);
}

/* static */ constexpr CellKind DummyObject::getCellKind() {
Expand Down
6 changes: 3 additions & 3 deletions lib/VM/FastArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ CallResult<Handle<FastArray>> FastArray::create(
return ExecutionStatus::EXCEPTION;

self->indexedStorage_.setNonNull(
runtime, vmcast<ArrayStorageSmall>(*arrRes), runtime.getHeap());
runtime, vmcast<ArrayStorageSmall>(*arrRes), runtime.getHeap(), *self);

auto shv = SmallHermesValue::encodeNumberValue(0, runtime);
self->setLength(runtime, shv);
Expand All @@ -119,7 +119,7 @@ FastArray::pushSlow(Handle<FastArray> self, Runtime &runtime, Handle<> val) {
ExecutionStatus::EXCEPTION))
return ExecutionStatus::EXCEPTION;

self->indexedStorage_.setNonNull(runtime, *storage, runtime.getHeap());
self->indexedStorage_.setNonNull(runtime, *storage, runtime.getHeap(), *self);
auto newSz = SmallHermesValue::encodeNumberValue(storage->size(), runtime);
self->setLength(runtime, newSz);
return ExecutionStatus::RETURNED;
Expand All @@ -138,7 +138,7 @@ ExecutionStatus FastArray::appendSlow(
ArrayStorageSmall::append(storage, runtime, otherStorage) ==
ExecutionStatus::EXCEPTION))
return ExecutionStatus::EXCEPTION;
self->indexedStorage_.setNonNull(runtime, *storage, runtime.getHeap());
self->indexedStorage_.setNonNull(runtime, *storage, runtime.getHeap(), *self);
auto newSz = SmallHermesValue::encodeNumberValue(storage->size(), runtime);
self->setLength(runtime, newSz);
return ExecutionStatus::RETURNED;
Expand Down
6 changes: 4 additions & 2 deletions lib/VM/HiddenClass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,8 @@ ExecutionStatus HiddenClass::addToPropertyMap(
return ExecutionStatus::EXCEPTION;
}

selfHandle->propertyMap_.setNonNull(runtime, *updatedMap, runtime.getHeap());
selfHandle->propertyMap_.setNonNull(
runtime, *updatedMap, runtime.getHeap(), *selfHandle);
return ExecutionStatus::RETURNED;
}

Expand Down Expand Up @@ -889,7 +890,8 @@ void HiddenClass::initializeMissingPropertyMap(
inserted->first->slot = slotIndex++;
}

selfHandle->propertyMap_.setNonNull(runtime, *mapHandle, runtime.getHeap());
selfHandle->propertyMap_.setNonNull(
runtime, *mapHandle, runtime.getHeap(), *selfHandle);
}

void HiddenClass::stealPropertyMapFromParent(
Expand Down
4 changes: 2 additions & 2 deletions lib/VM/JSCallableProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ void JSCallableProxy::setTargetAndHandler(
Runtime &runtime,
Handle<JSObject> target,
Handle<JSObject> handler) {
slots_.target.set(runtime, target.get(), runtime.getHeap());
slots_.handler.set(runtime, handler.get(), runtime.getHeap());
slots_.target.set(runtime, target.get(), runtime.getHeap(), this);
slots_.handler.set(runtime, handler.get(), runtime.getHeap(), this);
}

CallResult<bool> JSCallableProxy::isConstructor(Runtime &runtime) {
Expand Down
6 changes: 4 additions & 2 deletions lib/VM/JSError.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,8 @@ ExecutionStatus JSError::recordStackTrace(
}
}
}
selfHandle->domains_.set(runtime, domains.get(), runtime.getHeap());
selfHandle->domains_.set(
runtime, domains.get(), runtime.getHeap(), *selfHandle);

// Remove the last entry.
stack->pop_back();
Expand All @@ -521,7 +522,8 @@ ExecutionStatus JSError::recordStackTrace(
"Function names and stack trace must have same size.");

selfHandle->stacktrace_ = std::move(stack);
selfHandle->funcNames_.set(runtime, *funcNames, runtime.getHeap());
selfHandle->funcNames_.set(
runtime, *funcNames, runtime.getHeap(), *selfHandle);
return ExecutionStatus::RETURNED;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/VM/JSGeneratorObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ CallResult<PseudoHandle<JSGeneratorObject>> JSGeneratorObject::create(
parentHandle,
runtime.getHiddenClassForPrototype(
*parentHandle, numOverlapSlots<JSGeneratorObject>()));
cell->innerFunction_.set(runtime, *innerFunction, runtime.getHeap());
cell->innerFunction_.set(runtime, *innerFunction, runtime.getHeap(), cell);
return JSObjectInit::initToPseudoHandle(runtime, cell);
}

Expand Down
32 changes: 20 additions & 12 deletions lib/VM/JSObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ PseudoHandle<JSObject> JSObject::create(
Runtime &runtime,
Handle<HiddenClass> clazz) {
auto obj = JSObject::create(runtime, clazz->getNumProperties());
obj->clazz_.setNonNull(runtime, *clazz, runtime.getHeap());
obj->clazz_.setNonNull(runtime, *clazz, runtime.getHeap(), obj.get());
// If the hidden class has index like property, we need to clear the fast path
// flag.
if (LLVM_UNLIKELY(
Expand All @@ -115,7 +115,7 @@ PseudoHandle<JSObject> JSObject::create(
Handle<JSObject> parentHandle,
Handle<HiddenClass> clazz) {
PseudoHandle<JSObject> obj = JSObject::create(runtime, clazz);
obj->parent_.set(runtime, parentHandle.get(), runtime.getHeap());
obj->parent_.set(runtime, parentHandle.get(), runtime.getHeap(), obj.get());
return obj;
}

Expand Down Expand Up @@ -224,7 +224,7 @@ CallResult<bool> JSObject::setParent(
}
}
// 9.
self->parent_.set(runtime, parent, runtime.getHeap());
self->parent_.set(runtime, parent, runtime.getHeap(), self);
// 10.
return true;
}
Expand Down Expand Up @@ -252,7 +252,7 @@ void JSObject::allocateNewSlotStorage(
auto arrRes = runtime.ignoreAllocationFailure(
PropStorage::create(runtime, DEFAULT_PROPERTY_CAPACITY));
selfHandle->propStorage_.setNonNull(
runtime, vmcast<PropStorage>(arrRes), runtime.getHeap());
runtime, vmcast<PropStorage>(arrRes), runtime.getHeap(), *selfHandle);
} else if (LLVM_UNLIKELY(
newSlotIndex >=
selfHandle->propStorage_.getNonNull(runtime)->capacity())) {
Expand All @@ -262,7 +262,8 @@ void JSObject::allocateNewSlotStorage(
"allocated slot must be at end");
auto hnd = runtime.makeMutableHandle(selfHandle->propStorage_);
PropStorage::resize(hnd, runtime, newSlotIndex + 1);
selfHandle->propStorage_.setNonNull(runtime, *hnd, runtime.getHeap());
selfHandle->propStorage_.setNonNull(
runtime, *hnd, runtime.getHeap(), *selfHandle);
}

{
Expand Down Expand Up @@ -1924,7 +1925,8 @@ CallResult<bool> JSObject::deleteNamed(
// Perform the actual deletion.
auto newClazz = HiddenClass::deleteProperty(
runtime.makeHandle(selfHandle->clazz_), runtime, *pos);
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
selfHandle->clazz_.setNonNull(
runtime, *newClazz, runtime.getHeap(), *selfHandle);

return true;
}
Expand Down Expand Up @@ -2024,7 +2026,8 @@ CallResult<bool> JSObject::deleteComputed(
// Remove the property descriptor.
auto newClazz = HiddenClass::deleteProperty(
runtime.makeHandle(selfHandle->clazz_), runtime, *pos);
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
selfHandle->clazz_.setNonNull(
runtime, *newClazz, runtime.getHeap(), *selfHandle);
} else if (LLVM_UNLIKELY(selfHandle->flags_.proxyObject)) {
CallResult<Handle<>> key = toPropertyKey(runtime, nameValPrimitiveHandle);
if (key == ExecutionStatus::EXCEPTION)
Expand Down Expand Up @@ -2613,7 +2616,8 @@ ExecutionStatus JSObject::seal(Handle<JSObject> selfHandle, Runtime &runtime) {

auto newClazz = HiddenClass::makeAllNonConfigurable(
runtime.makeHandle(selfHandle->clazz_), runtime);
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
selfHandle->clazz_.setNonNull(
runtime, *newClazz, runtime.getHeap(), *selfHandle);

selfHandle->flags_.sealed = true;

Expand All @@ -2638,7 +2642,8 @@ ExecutionStatus JSObject::freeze(

auto newClazz = HiddenClass::makeAllReadOnly(
runtime.makeHandle(selfHandle->clazz_), runtime);
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
selfHandle->clazz_.setNonNull(
runtime, *newClazz, runtime.getHeap(), *selfHandle);

selfHandle->flags_.frozen = true;
selfHandle->flags_.sealed = true;
Expand All @@ -2658,7 +2663,8 @@ void JSObject::updatePropertyFlagsWithoutTransitions(
flagsToClear,
flagsToSet,
props);
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
selfHandle->clazz_.setNonNull(
runtime, *newClazz, runtime.getHeap(), *selfHandle);
}

CallResult<bool> JSObject::isExtensible(
Expand Down Expand Up @@ -2783,7 +2789,8 @@ ExecutionStatus JSObject::addOwnPropertyImpl(
if (LLVM_UNLIKELY(addResult == ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
selfHandle->clazz_.setNonNull(runtime, *addResult->first, runtime.getHeap());
selfHandle->clazz_.setNonNull(
runtime, *addResult->first, runtime.getHeap(), *selfHandle);

allocateNewSlotStorage(
selfHandle, runtime, addResult->second, valueOrAccessor);
Expand Down Expand Up @@ -2826,7 +2833,8 @@ CallResult<bool> JSObject::updateOwnProperty(
runtime,
propertyPos,
desc.flags);
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
selfHandle->clazz_.setNonNull(
runtime, *newClazz, runtime.getHeap(), *selfHandle);
}

if (updateStatus->first == PropertyUpdateStatus::done)
Expand Down
4 changes: 2 additions & 2 deletions lib/VM/JSProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ void JSProxy::setTargetAndHandler(
Handle<JSObject> target,
Handle<JSObject> handler) {
auto &slots = detail::slots(*selfHandle);
slots.target.set(runtime, target.get(), runtime.getHeap());
slots.handler.set(runtime, handler.get(), runtime.getHeap());
slots.target.set(runtime, target.get(), runtime.getHeap(), *selfHandle);
slots.handler.set(runtime, handler.get(), runtime.getHeap(), *selfHandle);
}

namespace {
Expand Down
Loading

0 comments on commit 6703f45

Please sign in to comment.