Skip to content

Commit 10ecb83

Browse files
lavenzgfacebook-github-bot
lavenzg
authored and
facebook-github-bot
committed
Pass the pointer of owning object in GCPointer::set() part I (#1502)
Summary: Pull Request resolved: #1502 Differential Revision: D62222257
1 parent 7ec226b commit 10ecb83

21 files changed

+87
-57
lines changed

‎include/hermes/VM/GCPointer-inline.h

+12-2
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,29 @@ GCPointerBase::GCPointerBase(
3232
}
3333
}
3434

35-
inline void GCPointerBase::set(PointerBase &base, GCCell *ptr, GC &gc) {
35+
inline void GCPointerBase::set(
36+
PointerBase &base,
37+
GCCell *ptr,
38+
GC &gc,
39+
const GCCell *owningObj) {
3640
assert(
3741
(!ptr || gc.validPointer(ptr)) &&
3842
"Cannot set a GCPointer to an invalid pointer");
3943
// Write barrier must happen before the write.
44+
(void)owningObj;
4045
gc.writeBarrier(this, ptr);
4146
setNoBarrier(CompressedPointer::encode(ptr, base));
4247
}
4348

44-
inline void GCPointerBase::setNonNull(PointerBase &base, GCCell *ptr, GC &gc) {
49+
inline void GCPointerBase::setNonNull(
50+
PointerBase &base,
51+
GCCell *ptr,
52+
GC &gc,
53+
const GCCell *owningObj) {
4554
assert(
4655
gc.validPointer(ptr) && "Cannot set a GCPointer to an invalid pointer");
4756
// Write barrier must happen before the write.
57+
(void)owningObj;
4858
gc.writeBarrier(this, ptr);
4959
setNoBarrier(CompressedPointer::encodeNonNull(ptr, base));
5060
}

‎include/hermes/VM/GCPointer.h

+10-6
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,12 @@ class GCPointerBase : public CompressedPointer {
3838
/// \param ptr The memory being pointed to.
3939
/// \param base The base of ptr.
4040
/// \param gc Used for write barriers.
41-
inline void set(PointerBase &base, GCCell *ptr, GC &gc);
41+
/// \param owningObj The object that contains this GCPointer.
42+
inline void
43+
set(PointerBase &base, GCCell *ptr, GC &gc, const GCCell *owningObj);
4244
inline void set(PointerBase &base, CompressedPointer ptr, GC &gc);
43-
inline void setNonNull(PointerBase &base, GCCell *ptr, GC &gc);
45+
inline void
46+
setNonNull(PointerBase &base, GCCell *ptr, GC &gc, const GCCell *owningObj);
4447

4548
/// Set this pointer to null. This needs a write barrier in some types of
4649
/// garbage collectors.
@@ -90,11 +93,12 @@ class GCPointer : public GCPointerBase {
9093
/// \param base The base of ptr.
9194
/// \param ptr The memory being pointed to.
9295
/// \param gc Used for write barriers.
93-
void set(PointerBase &base, T *ptr, GC &gc) {
94-
GCPointerBase::set(base, ptr, gc);
96+
/// \param owningObj The object that contains this GCPointer.
97+
void set(PointerBase &base, T *ptr, GC &gc, const GCCell *owningObj) {
98+
GCPointerBase::set(base, ptr, gc, owningObj);
9599
}
96-
void setNonNull(PointerBase &base, T *ptr, GC &gc) {
97-
GCPointerBase::setNonNull(base, ptr, gc);
100+
void setNonNull(PointerBase &base, T *ptr, GC &gc, const GCCell *owningObj) {
101+
GCPointerBase::setNonNull(base, ptr, gc, owningObj);
98102
}
99103

100104
/// Convenience overload of GCPointer::set for other GCPointers.

‎include/hermes/VM/HiddenClass.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ class HiddenClass final : public GCCell {
326326
}
327327

328328
void setForInCache(BigStorage *arr, Runtime &runtime) {
329-
forInCache_.set(runtime, arr, runtime.getHeap());
329+
forInCache_.set(runtime, arr, runtime.getHeap(), this);
330330
}
331331

332332
void clearForInCache(Runtime &runtime) {

‎include/hermes/VM/JSArray.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ class ArrayImpl : public JSObject {
121121
/// Set the indexed storage of this array to be \p p. The pointer is allowed
122122
/// to be null.
123123
void setIndexedStorage(PointerBase &base, StorageType *p, GC &gc) {
124-
indexedStorage_.set(base, p, gc);
124+
indexedStorage_.set(base, p, gc, this);
125125
}
126126

127127
/// @}

‎include/hermes/VM/JSDataView.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ class JSDataView final : public JSObject {
9292
assert(
9393
offset + length <= buffer->size() &&
9494
"A DataView cannot be looking outside of the storage");
95-
buffer_.setNonNull(runtime, buffer, runtime.getHeap());
95+
buffer_.setNonNull(runtime, buffer, runtime.getHeap(), this);
9696
offset_ = offset;
9797
length_ = length;
9898
}

‎include/hermes/VM/JSMapImpl.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class JSMapIteratorImpl final : public JSObject {
145145
Runtime &runtime,
146146
Handle<JSMapImpl<JSMapTypeTraits<C>::ContainerKind>> data,
147147
IterationKind kind) {
148-
data_.set(runtime, data.get(), runtime.getHeap());
148+
data_.set(runtime, data.get(), runtime.getHeap(), this);
149149
iterationKind_ = kind;
150150

151151
assert(data_ && "Invalid storage data");
@@ -171,7 +171,8 @@ class JSMapIteratorImpl final : public JSObject {
171171
runtime,
172172
self->data_.getNonNull(runtime)->iteratorNext(
173173
runtime, self->itr_.get(runtime)),
174-
runtime.getHeap());
174+
runtime.getHeap(),
175+
*self);
175176
if (self->itr_) {
176177
switch (self->iterationKind_) {
177178
case IterationKind::Key:

‎include/hermes/VM/JSObject.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ class JSObject : public GCCell {
463463
/// cycle checking.
464464
static void
465465
unsafeSetParentInternal(JSObject *self, Runtime &runtime, JSObject *parent) {
466-
self->parent_.set(runtime, parent, runtime.getHeap());
466+
self->parent_.set(runtime, parent, runtime.getHeap(), self);
467467
}
468468

469469
/// Return the value of an internal property slot. Use getDirectSlotValue if
@@ -1640,7 +1640,7 @@ inline ExecutionStatus JSObject::allocatePropStorage(
16401640
return ExecutionStatus::EXCEPTION;
16411641

16421642
selfHandle->propStorage_.setNonNull(
1643-
runtime, vmcast<PropStorage>(*res), runtime.getHeap());
1643+
runtime, vmcast<PropStorage>(*res), runtime.getHeap(), *selfHandle);
16441644
return ExecutionStatus::RETURNED;
16451645
}
16461646

‎include/hermes/VM/OrderedHashMap.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ class OrderedHashMapBase {
200200
return ExecutionStatus::EXCEPTION;
201201
}
202202

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

‎lib/VM/Domain.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ ExecutionStatus Domain::importCJSModuleTable(
167167
return ExecutionStatus::EXCEPTION;
168168
}
169169

170-
self->throwingRequire_.set(runtime, *requireFn, runtime.getHeap());
170+
self->throwingRequire_.set(runtime, *requireFn, runtime.getHeap(), *self);
171171
} else {
172172
cjsModules = self->cjsModules_.get(runtime);
173173
}
@@ -308,7 +308,7 @@ ExecutionStatus Domain::importCJSModuleTable(
308308
}
309309
}
310310

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

@@ -343,8 +343,8 @@ Handle<RequireContext> RequireContext::create(
343343
runtime.getHiddenClassForPrototype(
344344
*objProto, numOverlapSlots<RequireContext>()));
345345
auto self = JSObjectInit::initToHandle(runtime, cell);
346-
self->domain_.set(runtime, *domain, runtime.getHeap());
347-
self->dirname_.set(runtime, *dirname, runtime.getHeap());
346+
self->domain_.set(runtime, *domain, runtime.getHeap(), *self);
347+
self->dirname_.set(runtime, *dirname, runtime.getHeap(), *self);
348348
return self;
349349
}
350350

‎lib/VM/DummyObject.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ void DummyObject::releaseExtMem(GC &gc) {
5353
}
5454

5555
void DummyObject::setPointer(GC &gc, DummyObject *obj) {
56-
other.set(gc.getPointerBase(), obj, gc);
56+
other.set(gc.getPointerBase(), obj, gc, this);
5757
}
5858

5959
/* static */ constexpr CellKind DummyObject::getCellKind() {

‎lib/VM/FastArray.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ CallResult<HermesValue> FastArray::create(Runtime &runtime, size_t capacity) {
104104
return ExecutionStatus::EXCEPTION;
105105

106106
lv.self->indexedStorage_.setNonNull(
107-
runtime, vmcast<ArrayStorageSmall>(*arrRes), runtime.getHeap());
107+
runtime, vmcast<ArrayStorageSmall>(*arrRes), runtime.getHeap(), *lv.self);
108108

109109
auto shv = SmallHermesValue::encodeNumberValue(0, runtime);
110110
lv.self->setLength(runtime, shv);
@@ -122,7 +122,7 @@ FastArray::pushSlow(Handle<FastArray> self, Runtime &runtime, Handle<> val) {
122122
ExecutionStatus::EXCEPTION))
123123
return ExecutionStatus::EXCEPTION;
124124

125-
self->indexedStorage_.setNonNull(runtime, *storage, runtime.getHeap());
125+
self->indexedStorage_.setNonNull(runtime, *storage, runtime.getHeap(), *self);
126126
auto newSz = SmallHermesValue::encodeNumberValue(storage->size(), runtime);
127127
self->setLength(runtime, newSz);
128128
return ExecutionStatus::RETURNED;
@@ -141,7 +141,7 @@ ExecutionStatus FastArray::appendSlow(
141141
ArrayStorageSmall::append(storage, runtime, otherStorage) ==
142142
ExecutionStatus::EXCEPTION))
143143
return ExecutionStatus::EXCEPTION;
144-
self->indexedStorage_.setNonNull(runtime, *storage, runtime.getHeap());
144+
self->indexedStorage_.setNonNull(runtime, *storage, runtime.getHeap(), *self);
145145
auto newSz = SmallHermesValue::encodeNumberValue(storage->size(), runtime);
146146
self->setLength(runtime, newSz);
147147
return ExecutionStatus::RETURNED;

‎lib/VM/HiddenClass.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -826,7 +826,8 @@ ExecutionStatus HiddenClass::addToPropertyMap(
826826
return ExecutionStatus::EXCEPTION;
827827
}
828828

829-
selfHandle->propertyMap_.setNonNull(runtime, *updatedMap, runtime.getHeap());
829+
selfHandle->propertyMap_.setNonNull(
830+
runtime, *updatedMap, runtime.getHeap(), *selfHandle);
830831
return ExecutionStatus::RETURNED;
831832
}
832833

@@ -889,7 +890,8 @@ void HiddenClass::initializeMissingPropertyMap(
889890
inserted->first->slot = slotIndex++;
890891
}
891892

892-
selfHandle->propertyMap_.setNonNull(runtime, *mapHandle, runtime.getHeap());
893+
selfHandle->propertyMap_.setNonNull(
894+
runtime, *mapHandle, runtime.getHeap(), *selfHandle);
893895
}
894896

895897
void HiddenClass::stealPropertyMapFromParent(

‎lib/VM/JSCallableProxy.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ void JSCallableProxy::setTargetAndHandler(
6464
Runtime &runtime,
6565
Handle<JSObject> target,
6666
Handle<JSObject> handler) {
67-
slots_.target.set(runtime, target.get(), runtime.getHeap());
68-
slots_.handler.set(runtime, handler.get(), runtime.getHeap());
67+
slots_.target.set(runtime, target.get(), runtime.getHeap(), this);
68+
slots_.handler.set(runtime, handler.get(), runtime.getHeap(), this);
6969
}
7070

7171
CallResult<HermesValue>

‎lib/VM/JSError.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,8 @@ ExecutionStatus JSError::recordStackTrace(
495495
}
496496
}
497497
}
498-
selfHandle->domains_.set(runtime, domains.get(), runtime.getHeap());
498+
selfHandle->domains_.set(
499+
runtime, domains.get(), runtime.getHeap(), *selfHandle);
499500

500501
// Remove the last entry.
501502
stack->pop_back();
@@ -509,7 +510,8 @@ ExecutionStatus JSError::recordStackTrace(
509510
"Function names and stack trace must have same size.");
510511

511512
selfHandle->stacktrace_ = std::move(stack);
512-
selfHandle->funcNames_.set(runtime, *funcNames, runtime.getHeap());
513+
selfHandle->funcNames_.set(
514+
runtime, *funcNames, runtime.getHeap(), *selfHandle);
513515
return ExecutionStatus::RETURNED;
514516
}
515517

‎lib/VM/JSGeneratorObject.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ CallResult<PseudoHandle<JSGeneratorObject>> JSGeneratorObject::create(
4343
parentHandle,
4444
runtime.getHiddenClassForPrototype(
4545
*parentHandle, numOverlapSlots<JSGeneratorObject>()));
46-
cell->innerFunction_.set(runtime, *innerFunction, runtime.getHeap());
46+
cell->innerFunction_.set(runtime, *innerFunction, runtime.getHeap(), cell);
4747
return JSObjectInit::initToPseudoHandle(runtime, cell);
4848
}
4949

‎lib/VM/JSObject.cpp

+20-12
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ PseudoHandle<JSObject> JSObject::create(
101101
Runtime &runtime,
102102
Handle<HiddenClass> clazz) {
103103
auto obj = JSObject::create(runtime, clazz->getNumProperties());
104-
obj->clazz_.setNonNull(runtime, *clazz, runtime.getHeap());
104+
obj->clazz_.setNonNull(runtime, *clazz, runtime.getHeap(), obj.get());
105105
// If the hidden class has index like property, we need to clear the fast path
106106
// flag.
107107
if (LLVM_UNLIKELY(
@@ -115,7 +115,7 @@ PseudoHandle<JSObject> JSObject::create(
115115
Handle<JSObject> parentHandle,
116116
Handle<HiddenClass> clazz) {
117117
PseudoHandle<JSObject> obj = JSObject::create(runtime, clazz);
118-
obj->parent_.set(runtime, parentHandle.get(), runtime.getHeap());
118+
obj->parent_.set(runtime, parentHandle.get(), runtime.getHeap(), obj.get());
119119
return obj;
120120
}
121121

@@ -224,7 +224,7 @@ CallResult<bool> JSObject::setParent(
224224
}
225225
}
226226
// 9.
227-
self->parent_.set(runtime, parent, runtime.getHeap());
227+
self->parent_.set(runtime, parent, runtime.getHeap(), self);
228228
// 10.
229229
return true;
230230
}
@@ -252,7 +252,7 @@ void JSObject::allocateNewSlotStorage(
252252
auto arrRes = runtime.ignoreAllocationFailure(
253253
PropStorage::create(runtime, DEFAULT_PROPERTY_CAPACITY));
254254
selfHandle->propStorage_.setNonNull(
255-
runtime, vmcast<PropStorage>(arrRes), runtime.getHeap());
255+
runtime, vmcast<PropStorage>(arrRes), runtime.getHeap(), *selfHandle);
256256
} else if (LLVM_UNLIKELY(
257257
newSlotIndex >=
258258
selfHandle->propStorage_.getNonNull(runtime)->capacity())) {
@@ -262,7 +262,8 @@ void JSObject::allocateNewSlotStorage(
262262
"allocated slot must be at end");
263263
auto hnd = runtime.makeMutableHandle(selfHandle->propStorage_);
264264
PropStorage::resize(hnd, runtime, newSlotIndex + 1);
265-
selfHandle->propStorage_.setNonNull(runtime, *hnd, runtime.getHeap());
265+
selfHandle->propStorage_.setNonNull(
266+
runtime, *hnd, runtime.getHeap(), *selfHandle);
266267
}
267268

268269
{
@@ -1924,7 +1925,8 @@ CallResult<bool> JSObject::deleteNamed(
19241925
// Perform the actual deletion.
19251926
auto newClazz = HiddenClass::deleteProperty(
19261927
runtime.makeHandle(selfHandle->clazz_), runtime, *pos);
1927-
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
1928+
selfHandle->clazz_.setNonNull(
1929+
runtime, *newClazz, runtime.getHeap(), *selfHandle);
19281930

19291931
return true;
19301932
}
@@ -2024,7 +2026,8 @@ CallResult<bool> JSObject::deleteComputed(
20242026
// Remove the property descriptor.
20252027
auto newClazz = HiddenClass::deleteProperty(
20262028
runtime.makeHandle(selfHandle->clazz_), runtime, *pos);
2027-
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
2029+
selfHandle->clazz_.setNonNull(
2030+
runtime, *newClazz, runtime.getHeap(), *selfHandle);
20282031
} else if (LLVM_UNLIKELY(selfHandle->flags_.proxyObject)) {
20292032
CallResult<Handle<>> key = toPropertyKey(runtime, nameValPrimitiveHandle);
20302033
if (key == ExecutionStatus::EXCEPTION)
@@ -2613,7 +2616,8 @@ ExecutionStatus JSObject::seal(Handle<JSObject> selfHandle, Runtime &runtime) {
26132616

26142617
auto newClazz = HiddenClass::makeAllNonConfigurable(
26152618
runtime.makeHandle(selfHandle->clazz_), runtime);
2616-
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
2619+
selfHandle->clazz_.setNonNull(
2620+
runtime, *newClazz, runtime.getHeap(), *selfHandle);
26172621

26182622
selfHandle->flags_.sealed = true;
26192623

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

26392643
auto newClazz = HiddenClass::makeAllReadOnly(
26402644
runtime.makeHandle(selfHandle->clazz_), runtime);
2641-
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
2645+
selfHandle->clazz_.setNonNull(
2646+
runtime, *newClazz, runtime.getHeap(), *selfHandle);
26422647

26432648
selfHandle->flags_.frozen = true;
26442649
selfHandle->flags_.sealed = true;
@@ -2658,7 +2663,8 @@ void JSObject::updatePropertyFlagsWithoutTransitions(
26582663
flagsToClear,
26592664
flagsToSet,
26602665
props);
2661-
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
2666+
selfHandle->clazz_.setNonNull(
2667+
runtime, *newClazz, runtime.getHeap(), *selfHandle);
26622668
}
26632669

26642670
CallResult<bool> JSObject::isExtensible(
@@ -2783,7 +2789,8 @@ ExecutionStatus JSObject::addOwnPropertyImpl(
27832789
if (LLVM_UNLIKELY(addResult == ExecutionStatus::EXCEPTION)) {
27842790
return ExecutionStatus::EXCEPTION;
27852791
}
2786-
selfHandle->clazz_.setNonNull(runtime, *addResult->first, runtime.getHeap());
2792+
selfHandle->clazz_.setNonNull(
2793+
runtime, *addResult->first, runtime.getHeap(), *selfHandle);
27872794

27882795
allocateNewSlotStorage(
27892796
selfHandle, runtime, addResult->second, valueOrAccessor);
@@ -2826,7 +2833,8 @@ CallResult<bool> JSObject::updateOwnProperty(
28262833
runtime,
28272834
propertyPos,
28282835
desc.flags);
2829-
selfHandle->clazz_.setNonNull(runtime, *newClazz, runtime.getHeap());
2836+
selfHandle->clazz_.setNonNull(
2837+
runtime, *newClazz, runtime.getHeap(), *selfHandle);
28302838
}
28312839

28322840
if (updateStatus->first == PropertyUpdateStatus::done)

‎lib/VM/JSProxy.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ void JSProxy::setTargetAndHandler(
108108
Handle<JSObject> target,
109109
Handle<JSObject> handler) {
110110
auto &slots = detail::slots(*selfHandle);
111-
slots.target.set(runtime, target.get(), runtime.getHeap());
112-
slots.handler.set(runtime, handler.get(), runtime.getHeap());
111+
slots.target.set(runtime, target.get(), runtime.getHeap(), *selfHandle);
112+
slots.handler.set(runtime, handler.get(), runtime.getHeap(), *selfHandle);
113113
}
114114

115115
namespace {

0 commit comments

Comments
 (0)