Skip to content

Commit

Permalink
Pass the pointer of owning object in ctor of GCPointer (facebook#1503)
Browse files Browse the repository at this point in the history
Summary: Pull Request resolved: facebook#1503

Differential Revision: D62227037
  • Loading branch information
lavenzg authored and facebook-github-bot committed Oct 21, 2024
1 parent 37c298b commit cff90fd
Show file tree
Hide file tree
Showing 11 changed files with 44 additions and 26 deletions.
17 changes: 11 additions & 6 deletions include/hermes/VM/Callable.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,11 @@ class Environment final
Runtime &runtime,
Handle<Environment> parentEnvironment,
uint32_t size)
: parentEnvironment_(runtime, parentEnvironment.get(), runtime.getHeap()),
: parentEnvironment_(
runtime,
parentEnvironment.get(),
runtime.getHeap(),
this),
size_(size) {
// Initialize all slots to 'undefined'.
GCHermesValue::uninitialized_fill(
Expand Down Expand Up @@ -340,7 +344,7 @@ class Callable : public JSObject {
HiddenClass *clazz,
Handle<Environment> env)
: JSObject(runtime, parent, clazz),
environment_(runtime, *env, runtime.getHeap()) {}
environment_(runtime, *env, runtime.getHeap(), this) {}
Callable(Runtime &runtime, JSObject *parent, HiddenClass *clazz)
: JSObject(runtime, parent, clazz), environment_() {}

Expand All @@ -363,7 +367,8 @@ Environment::Environment(
runtime,
// TODO: Consider keeping the parent as a compressed pointer.
parentFn->getEnvironment(runtime),
runtime.getHeap()),
runtime.getHeap(),
this),
size_(size) {
// Initialize all slots to 'undefined'.
GCHermesValue::uninitialized_fill(
Expand Down Expand Up @@ -441,8 +446,8 @@ class BoundFunction final : public Callable {
Handle<Callable> target,
Handle<ArrayStorage> argStorage)
: Callable(runtime, *parent, *clazz),
target_(runtime, *target, runtime.getHeap()),
argStorage_(runtime, *argStorage, runtime.getHeap()) {}
target_(runtime, *target, runtime.getHeap(), this),
argStorage_(runtime, *argStorage, runtime.getHeap(), this) {}

private:
/// Return a pointer to the stored arguments, including \c this. \c this is
Expand Down Expand Up @@ -1021,7 +1026,7 @@ class JSFunction : public Callable {
CodeBlock *codeBlock)
: Callable(runtime, *parent, *clazz, environment),
codeBlock_(codeBlock),
domain_(runtime, *domain, runtime.getHeap()) {
domain_(runtime, *domain, runtime.getHeap(), this) {
assert(
!vt.finalize_ == (kHasFinalizer != HasFinalizer::Yes) &&
"kHasFinalizer invalid value");
Expand Down
2 changes: 2 additions & 0 deletions include/hermes/VM/GCPointer-inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ GCPointerBase::GCPointerBase(
PointerBase &base,
GCCell *ptr,
GC &gc,
const GCCell *owningObj,
NeedsBarriers)
: CompressedPointer(CompressedPointer::encode(ptr, base)) {
assert(
(!ptr || gc.validPointer(ptr)) &&
"Cannot construct a GCPointer from an invalid pointer");
(void)owningObj;
if (NeedsBarriers::value) {
gc.constructorWriteBarrier(this, ptr);
} else {
Expand Down
20 changes: 15 additions & 5 deletions include/hermes/VM/GCPointer.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@ class GCPointerBase : public CompressedPointer {
explicit GCPointerBase(std::nullptr_t) : CompressedPointer(nullptr) {}

template <typename NeedsBarriers>
inline GCPointerBase(PointerBase &base, GCCell *ptr, GC &gc, NeedsBarriers);
inline GCPointerBase(
PointerBase &base,
GCCell *ptr,
GC &gc,
const GCCell *owningObj,
NeedsBarriers);

public:
// These classes are used as arguments to GCPointer constructors, to
Expand Down Expand Up @@ -69,14 +74,19 @@ class GCPointer : public GCPointerBase {
/// this argument is unused, but its type's boolean value constant indicates
/// whether barriers are required.)
template <typename NeedsBarriers>
GCPointer(PointerBase &base, T *ptr, GC &gc, NeedsBarriers needsBarriers)
: GCPointerBase(base, ptr, gc, needsBarriers) {}
GCPointer(
PointerBase &base,
T *ptr,
GC &gc,
const GCCell *owningObj,
NeedsBarriers needsBarriers)
: GCPointerBase(base, ptr, gc, owningObj, needsBarriers) {}

/// Same as the constructor above, with the default for
/// NeedsBarriers as "YesBarriers". (We can't use default template
/// arguments with the idiom used above.)
inline GCPointer(PointerBase &base, T *ptr, GC &gc)
: GCPointer<T>(base, ptr, gc, YesBarriers()) {}
inline GCPointer(PointerBase &base, T *ptr, GC &gc, const GCCell *owningObj)
: GCPointer<T>(base, ptr, gc, owningObj, YesBarriers()) {}

/// We are not allowed to copy-construct or assign GCPointers.
GCPointer(const GCPointerBase &) = delete;
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 @@ -495,7 +495,7 @@ class HiddenClass final : public GCCell {
propertyFlags_(propertyFlags),
flags_(flags),
numProperties_(numProperties),
parent_(runtime, *parent, runtime.getHeap()) {
parent_(runtime, *parent, runtime.getHeap(), this) {
assert(propertyFlags.isValid() && "propertyFlags must be valid");
}

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 @@ -430,7 +430,7 @@ class JSArrayIterator : public JSObject {
Handle<JSObject> iteratedObject,
IterationKind iterationKind)
: JSObject(runtime, *parent, *clazz),
iteratedObject_(runtime, *iteratedObject, runtime.getHeap()),
iteratedObject_(runtime, *iteratedObject, runtime.getHeap(), this),
iterationKind_(iterationKind) {}

private:
Expand Down
8 changes: 4 additions & 4 deletions include/hermes/VM/JSObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,8 @@ class JSObject : public GCCell {
JSObject *parent,
HiddenClass *clazz,
NeedsBarriers needsBarriers)
: parent_(runtime, parent, runtime.getHeap(), needsBarriers),
clazz_(runtime, clazz, runtime.getHeap(), needsBarriers),
: parent_(runtime, parent, runtime.getHeap(), this, needsBarriers),
clazz_(runtime, clazz, runtime.getHeap(), this, needsBarriers),
propStorage_(nullptr) {
// Direct property slots are initialized by initDirectPropStorage.
}
Expand All @@ -291,8 +291,8 @@ class JSObject : public GCCell {
Handle<JSObject> parent,
Handle<HiddenClass> clazz,
NeedsBarriers needsBarriers)
: parent_(runtime, *parent, runtime.getHeap(), needsBarriers),
clazz_(runtime, *clazz, runtime.getHeap(), needsBarriers),
: parent_(runtime, *parent, runtime.getHeap(), this, needsBarriers),
clazz_(runtime, *clazz, runtime.getHeap(), this, needsBarriers),
propStorage_(nullptr) {
// Direct property slots are initialized by initDirectPropStorage.
}
Expand Down
3 changes: 2 additions & 1 deletion include/hermes/VM/JSRegExp.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ class JSRegExp final : public JSObject {
pattern_(
runtime,
runtime.getPredefinedString(Predefined::emptyString),
runtime.getHeap()) {}
runtime.getHeap(),
this) {}

private:
~JSRegExp();
Expand Down
4 changes: 2 additions & 2 deletions include/hermes/VM/JSRegExpStringIterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ class JSRegExpStringIterator : public JSObject {
bool global,
bool unicode)
: JSObject(runtime, *parent, *clazz),
iteratedRegExp_(runtime, *iteratedRegExp, runtime.getHeap()),
iteratedString_(runtime, *iteratedString, runtime.getHeap()),
iteratedRegExp_(runtime, *iteratedRegExp, runtime.getHeap(), this),
iteratedString_(runtime, *iteratedString, runtime.getHeap(), this),
global_(global),
unicode_(unicode) {}

Expand Down
6 changes: 3 additions & 3 deletions include/hermes/VM/PrimitiveBox.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class JSString final : public JSObject {
Handle<JSObject> parent,
Handle<HiddenClass> clazz)
: JSObject(runtime, *parent, *clazz),
primitiveValue_(runtime, *value, runtime.getHeap()) {
primitiveValue_(runtime, *value, runtime.getHeap(), this) {
flags_.indexedStorage = true;
flags_.fastIndexProperties = true;
}
Expand Down Expand Up @@ -157,7 +157,7 @@ class JSStringIterator : public JSObject {
Handle<HiddenClass> clazz,
Handle<StringPrimitive> iteratedString)
: JSObject(runtime, *parent, *clazz),
iteratedString_(runtime, *iteratedString, runtime.getHeap()) {}
iteratedString_(runtime, *iteratedString, runtime.getHeap(), this) {}

private:
/// [[IteratedString]]
Expand Down Expand Up @@ -221,7 +221,7 @@ class JSBigInt final : public JSObject {
Handle<JSObject> parent,
Handle<HiddenClass> clazz)
: JSObject(runtime, *parent, *clazz),
primitiveValue_(runtime, *value, runtime.getHeap()) {}
primitiveValue_(runtime, *value, runtime.getHeap(), this) {}

private:
GCPointer<BigIntPrimitive> primitiveValue_;
Expand Down
4 changes: 2 additions & 2 deletions include/hermes/VM/PropertyAccessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ class PropertyAccessor final : public GCCell {
Runtime &runtime,
Handle<Callable> getter,
Handle<Callable> setter)
: getter(runtime, *getter, runtime.getHeap()),
setter(runtime, *setter, runtime.getHeap()) {}
: getter(runtime, *getter, runtime.getHeap(), this),
setter(runtime, *setter, runtime.getHeap(), this) {}

static const VTable vt;

Expand Down
2 changes: 1 addition & 1 deletion lib/VM/JSCallSite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ JSCallSite::JSCallSite(
Handle<JSError> error,
size_t stackFrameIndex)
: JSObject(runtime, *parent, *clazz),
error_(runtime, *error, runtime.getHeap()),
error_(runtime, *error, runtime.getHeap(), this),
stackFrameIndex_(stackFrameIndex) {
assert(
error_.getNonNull(runtime)->getStackTrace() &&
Expand Down

0 comments on commit cff90fd

Please sign in to comment.