Skip to content

Commit

Permalink
Avoid naked sizeof(C) for GCCell classes
Browse files Browse the repository at this point in the history 
Summary:
To prepare for a future where the builtin `sizeof` operator doesn't always give the correct allocation size, even for a fixed-sized GCCell (e.g. due to trailing direct object properties), reroute all usage of `sizeof` on fixed-size GCCells through a template function `cellSize<C>`. The current implementation just returns `sizeof(C)`, so the behavior is currently unchanged.

The plan is to override this for JSObject and its descendants, but for consistency and reduced risk of mistakes, it should be used for all fixed-sized cells.

Reviewed By: tmikov

Differential Revision: D18153281

fbshipit-source-id: 4d4fd19e3a788c8e8e9ab8d98d619c50134d382b
  • Loading branch information
@kodafb @facebook-github-bot
kodafb authored and facebook-github-bot committed Oct 29, 2019
1 parent e0747bd commit 06147ee
Show file tree
Hide file tree
Showing 24 changed files with 152 additions and 119 deletions.
4 changes: 2 additions & 2 deletions include/hermes/VM/Callable.h
View file
Expand Up @@ -682,7 +682,7 @@ class NativeConstructor final : public NativeFunction {
unsigned paramCount,
CreatorFunction *creator,
CellKind targetKind) {
void *mem = runtime->alloc(sizeof(NativeConstructor));
void *mem = runtime->alloc(cellSize<NativeConstructor>());
return createPseudoHandle(new (mem) NativeConstructor(
runtime,
*parentHandle,
Expand All @@ -706,7 +706,7 @@ class NativeConstructor final : public NativeFunction {
NativeFunctionPtr functionPtr,
CreatorFunction *creator,
CellKind targetKind) {
void *mem = runtime->alloc(sizeof(NativeConstructor));
void *mem = runtime->alloc(cellSize<NativeConstructor>());
return createPseudoHandle(new (mem) NativeConstructor(
runtime,
*parentHandle,
Expand Down
31 changes: 31 additions & 0 deletions include/hermes/VM/GCCell.h
View file
Expand Up @@ -19,6 +19,23 @@
namespace hermes {
namespace vm {

template <typename T>
class ExternalStringPrimitive;
class VariableSizeRuntimeCell;

/// Return the allocation size for a fixed-size cell corresponding to class C.
/// This must be used instead of the builtin sizeof operator, since classes
/// further down the GCCell hirerarchy may add (fixed-size) trailing objects and
/// redefine this method. Example usage:
///
/// CallResult<HermesValue> MyCell::create(Runtime *runtime, ...) {
/// void *mem = runtime->alloc(cellSize<MyCell>());
/// ...
template <class C>
static constexpr uint32_t cellSize() {
return C::template cellSizeImpl<C>();
}

/// This include file defines a GCCell that allows forward heap
/// traversal in a contiguous space: given a pointer to the head, you
/// can get the size, and thus get to the head of the next cell.
Expand Down Expand Up @@ -62,6 +79,20 @@ class GCCell {
/// when vtp is incorrect.)
uint32_t getAllocatedSize(const VTable *vtp) const;

/// Implementation of cellSize. Do not use this directly.
template <class C>
static constexpr uint32_t cellSizeImpl() {
static_assert(std::is_convertible<C *, GCCell *>::value, "must be a cell");
// ExternalStringPrimitive<T> is special: it extends VariableSizeRuntimeCell
// but it's actually fixed-size.
static_assert(
!std::is_convertible<C *, VariableSizeRuntimeCell *>::value ||
std::is_same<C, ExternalStringPrimitive<char>>::value ||
std::is_same<C, ExternalStringPrimitive<char16_t>>::value,
"must be fixed-size");
return sizeof(C);
}

/// Is true if the cell has a variable size, false if it has a fixed size.
bool isVariableSize() const {
return isVariableSize(getVT());
Expand Down
4 changes: 2 additions & 2 deletions include/hermes/VM/SingleObject.h
View file
Expand Up @@ -32,7 +32,7 @@ class SingleObject final : public JSObject {
static CallResult<HermesValue> create(
Runtime *runtime,
Handle<JSObject> parentHandle) {
void *mem = runtime->alloc(sizeof(SingleObject));
void *mem = runtime->alloc(cellSize<SingleObject>());
return HermesValue::encodeObjectValue(
JSObject::allocateSmallPropStorage<NEEDED_PROPERTY_SLOTS>(
new (mem) SingleObject(
Expand All @@ -53,7 +53,7 @@ struct IsGCObject<SingleObject<kind>> {

template <CellKind kind>
const ObjectVTable SingleObject<kind>::vt = {
VTable(kind, sizeof(SingleObject<kind>), nullptr, nullptr),
VTable(kind, cellSize<SingleObject<kind>>(), nullptr, nullptr),
SingleObject::_getOwnIndexedRangeImpl,
SingleObject::_haveOwnIndexedImpl,
SingleObject::_getOwnIndexedPropertyFlagsImpl,
Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/StringPrimitive.h
View file
Expand Up @@ -815,7 +815,7 @@ using DynamicUniquedASCIIStringPrimitive =
template <typename T>
const VTable ExternalStringPrimitive<T>::vt = VTable(
ExternalStringPrimitive<T>::getCellKind(),
sizeof(ExternalStringPrimitive<T>),
cellSize<ExternalStringPrimitive<T>>(),
ExternalStringPrimitive<T>::_finalizeImpl,
nullptr, // markWeak.
ExternalStringPrimitive<T>::_mallocSizeImpl,
Expand Down
36 changes: 18 additions & 18 deletions lib/VM/Callable.cpp
View file
Expand Up @@ -419,7 +419,7 @@ CallableVTable BoundFunction::vt{
{
VTable(
CellKind::BoundFunctionKind,
sizeof(BoundFunction),
cellSize<BoundFunction>(),
nullptr,
nullptr,
nullptr,
Expand Down Expand Up @@ -475,7 +475,7 @@ void BoundFunctionSerialize(Serializer &s, const GCCell *cell) {

void BoundFunctionDeserialize(Deserializer &d, CellKind kind) {
assert(kind == CellKind::BoundFunctionKind && "Expected BoundFunction");
void *mem = d.getRuntime()->alloc(sizeof(BoundFunction));
void *mem = d.getRuntime()->alloc(cellSize<BoundFunction>());
void *cell = new (mem) BoundFunction(d);

d.endObject(cell);
Expand All @@ -495,7 +495,7 @@ CallResult<HermesValue> BoundFunction::create(
}
auto argStorageHandle = runtime->makeHandle<ArrayStorage>(*arrRes);

void *mem = runtime->alloc(sizeof(BoundFunction));
void *mem = runtime->alloc(cellSize<BoundFunction>());
auto selfHandle = runtime->makeHandle(new (mem) BoundFunction(
runtime,
runtime->functionPrototypeRawPtr,
Expand Down Expand Up @@ -826,7 +826,7 @@ CallableVTable NativeFunction::vt{
{
VTable(
CellKind::NativeFunctionKind,
sizeof(NativeFunction),
cellSize<NativeFunction>(),
nullptr,
nullptr,
nullptr,
Expand Down Expand Up @@ -889,7 +889,7 @@ void NativeFunctionDeserialize(Deserializer &d, CellKind kind) {
void *context = (void *)d.readInt<uint64_t>();
void *functionPtr = d.ptrRelocationOrNull(d.readInt<uint32_t>());
assert(functionPtr && "functionPtr not in relocation map");
void *mem = d.getRuntime()->alloc(sizeof(NativeFunction));
void *mem = d.getRuntime()->alloc(cellSize<NativeFunction>());
auto *cell = new (mem) NativeFunction(
d,
&NativeFunction::vt.base.base,
Expand All @@ -916,7 +916,7 @@ Handle<NativeFunction> NativeFunction::create(
SymbolID name,
unsigned paramCount,
Handle<JSObject> prototypeObjectHandle) {
void *mem = runtime->alloc(sizeof(NativeFunction));
void *mem = runtime->alloc(cellSize<NativeFunction>());
auto selfHandle = runtime->makeHandle(new (mem) NativeFunction(
runtime,
&vt.base.base,
Expand Down Expand Up @@ -949,7 +949,7 @@ Handle<NativeFunction> NativeFunction::create(
SymbolID name,
unsigned paramCount,
Handle<JSObject> prototypeObjectHandle) {
void *mem = runtime->alloc(sizeof(NativeFunction));
void *mem = runtime->alloc(cellSize<NativeFunction>());
auto selfHandle = runtime->makeHandle(new (mem) NativeFunction(
runtime,
&vt.base.base,
Expand Down Expand Up @@ -995,7 +995,7 @@ const CallableVTable NativeConstructor::vt{
{
VTable(
CellKind::NativeConstructorKind,
sizeof(NativeConstructor),
cellSize<NativeConstructor>(),
nullptr,
nullptr,
nullptr,
Expand Down Expand Up @@ -1071,7 +1071,7 @@ void NativeConstructorDeserialize(Deserializer &d, CellKind kind) {
#endif
void *creatorPtr = d.ptrRelocationOrNull(d.readInt<uint32_t>());
assert(creatorPtr && "funtion pointer must have been mapped already");
void *mem = d.getRuntime()->alloc(sizeof(NativeConstructor));
void *mem = d.getRuntime()->alloc(cellSize<NativeConstructor>());
auto *cell = new (mem) NativeConstructor(
d,
context,
Expand Down Expand Up @@ -1106,7 +1106,7 @@ CallableVTable JSFunction::vt{
{
VTable(
CellKind::FunctionKind,
sizeof(JSFunction),
cellSize<JSFunction>(),
nullptr,
nullptr,
nullptr,
Expand Down Expand Up @@ -1155,7 +1155,7 @@ void FunctionSerialize(Serializer &s, const GCCell *cell) {
void FunctionDeserialize(Deserializer &d, CellKind kind) {
assert(kind == CellKind::FunctionKind && "Expected Function");
void *mem = d.getRuntime()->alloc</*fixedSize*/ true, HasFinalizer::No>(
sizeof(JSFunction));
cellSize<JSFunction>());
auto *cell = new (mem) JSFunction(d, &JSFunction::vt.base.base);
d.endObject(cell);
}
Expand All @@ -1168,7 +1168,7 @@ CallResult<HermesValue> JSFunction::create(
Handle<Environment> envHandle,
CodeBlock *codeBlock) {
void *mem =
runtime->alloc</*fixedSize*/ true, kHasFinalizer>(sizeof(JSFunction));
runtime->alloc</*fixedSize*/ true, kHasFinalizer>(cellSize<JSFunction>());
auto *self = new (mem) JSFunction(
runtime,
*domain,
Expand Down Expand Up @@ -1196,7 +1196,7 @@ CallableVTable JSGeneratorFunction::vt{
{
VTable(
CellKind::GeneratorFunctionKind,
sizeof(JSGeneratorFunction),
cellSize<JSGeneratorFunction>(),
nullptr,
nullptr,
nullptr,
Expand Down Expand Up @@ -1237,7 +1237,7 @@ void GeneratorFunctionDeserialize(Deserializer &d, CellKind kind) {
assert(
kind == CellKind::GeneratorFunctionKind && "Expected GeneratorFunction");
void *mem = d.getRuntime()->alloc</*fixedSize*/ true, HasFinalizer::No>(
sizeof(JSFunction));
cellSize<JSFunction>());
auto *cell = new (mem) JSGeneratorFunction(d);
d.endObject(cell);
}
Expand All @@ -1250,7 +1250,7 @@ CallResult<HermesValue> JSGeneratorFunction::create(
Handle<Environment> envHandle,
CodeBlock *codeBlock) {
void *mem =
runtime->alloc</*fixedSize*/ true, kHasFinalizer>(sizeof(JSFunction));
runtime->alloc</*fixedSize*/ true, kHasFinalizer>(cellSize<JSFunction>());
auto *self = new (mem) JSGeneratorFunction(
runtime,
*domain,
Expand All @@ -1269,7 +1269,7 @@ CallableVTable GeneratorInnerFunction::vt{
{
VTable(
CellKind::GeneratorInnerFunctionKind,
sizeof(GeneratorInnerFunction),
cellSize<GeneratorInnerFunction>(),
nullptr,
nullptr,
nullptr,
Expand Down Expand Up @@ -1338,7 +1338,7 @@ void GeneratorInnerFunctionDeserialize(Deserializer &d, CellKind kind) {
assert(
kind == CellKind::GeneratorInnerFunctionKind &&
"Expected GeneratorInnerFunction");
void *mem = d.getRuntime()->alloc(sizeof(GeneratorInnerFunction));
void *mem = d.getRuntime()->alloc(cellSize<GeneratorInnerFunction>());
auto *cell = new (mem) GeneratorInnerFunction(d);
d.endObject(cell);
}
Expand All @@ -1351,7 +1351,7 @@ CallResult<Handle<GeneratorInnerFunction>> GeneratorInnerFunction::create(
Handle<Environment> envHandle,
CodeBlock *codeBlock,
NativeArgs args) {
void *mem = runtime->alloc(sizeof(GeneratorInnerFunction));
void *mem = runtime->alloc(cellSize<GeneratorInnerFunction>());
auto self = runtime->makeHandle(new (mem) GeneratorInnerFunction(
runtime,
*domain,
Expand Down
12 changes: 6 additions & 6 deletions lib/VM/Domain.cpp
View file
Expand Up @@ -19,7 +19,7 @@ namespace hermes {
namespace vm {

VTable Domain::vt{CellKind::DomainKind,
sizeof(Domain),
cellSize<Domain>(),
_finalizeImpl,
_markWeakImpl,
_mallocSizeImpl};
Expand Down Expand Up @@ -94,7 +94,7 @@ void DomainSerialize(Serializer &s, const GCCell *cell) {
void DomainDeserialize(Deserializer &d, CellKind kind) {
assert(kind == CellKind::DomainKind && "Expected Domain");
void *mem = d.getRuntime()->alloc</*fixedSize*/ true, HasFinalizer::Yes>(
sizeof(Domain));
cellSize<Domain>());
auto *cell = new (mem) Domain(d);
auto &samplingProfiler = SamplingProfiler::getInstance();
samplingProfiler->increaseDomainCount();
Expand Down Expand Up @@ -139,7 +139,7 @@ ArrayStorage *Domain::deserializeArrayStorage(Deserializer &d) {

PseudoHandle<Domain> Domain::create(Runtime *runtime) {
void *mem =
runtime->alloc</*fixedSize*/ true, HasFinalizer::Yes>(sizeof(Domain));
runtime->alloc</*fixedSize*/ true, HasFinalizer::Yes>(cellSize<Domain>());
auto self = createPseudoHandle(new (mem) Domain(runtime));
auto &samplingProfiler = SamplingProfiler::getInstance();
samplingProfiler->increaseDomainCount();
Expand Down Expand Up @@ -300,7 +300,7 @@ ExecutionStatus Domain::importCJSModuleTable(
}

ObjectVTable RequireContext::vt{
VTable(CellKind::RequireContextKind, sizeof(RequireContext)),
VTable(CellKind::RequireContextKind, cellSize<RequireContext>()),
RequireContext::_getOwnIndexedRangeImpl,
RequireContext::_haveOwnIndexedImpl,
RequireContext::_getOwnIndexedPropertyFlagsImpl,
Expand All @@ -325,7 +325,7 @@ void RequireContextSerialize(Serializer &s, const GCCell *cell) {
void RequireContextDeserialize(Deserializer &d, CellKind kind) {
assert(kind == CellKind::RequireContextKind && "Expected RequireContext");
void *mem = d.getRuntime()->alloc</*fixedSize*/ true, HasFinalizer::No>(
sizeof(RequireContext));
cellSize<RequireContext>());
auto *cell = new (mem) RequireContext(d);
d.endObject(cell);
}
Expand All @@ -336,7 +336,7 @@ Handle<RequireContext> RequireContext::create(
Handle<Domain> domain,
Handle<StringPrimitive> dirname) {
void *mem = runtime->alloc</*fixedSize*/ true, HasFinalizer::No>(
sizeof(RequireContext));
cellSize<RequireContext>());
auto self = runtime->makeHandle(
JSObject::allocateSmallPropStorage<NEEDED_PROPERTY_SLOTS>(
new (mem) RequireContext(
Expand Down
7 changes: 4 additions & 3 deletions lib/VM/HiddenClass.cpp
View file
Expand Up @@ -55,7 +55,7 @@ void TransitionMap::uncleanMakeLarge() {
} // namespace detail

VTable HiddenClass::vt{CellKind::HiddenClassKind,
sizeof(HiddenClass),
cellSize<HiddenClass>(),
_finalizeImpl,
_markWeakImpl,
_mallocSizeImpl};
Expand Down Expand Up @@ -114,7 +114,7 @@ void HiddenClassDeserialize(Deserializer &d, CellKind kind) {
unsigned numProperties = d.readInt<uint32_t>();

void *mem = d.getRuntime()->alloc</*fixedSize*/ true, HasFinalizer::Yes>(
sizeof(HiddenClass));
cellSize<HiddenClass>());
auto *cell = new (mem) HiddenClass(
d.getRuntime(),
classFlags,
Expand Down Expand Up @@ -188,7 +188,8 @@ CallResult<HermesValue> HiddenClass::create(
assert(
(flags.dictionaryMode || numProperties == 0 || *parent) &&
"non-empty non-dictionary orphan");
void *mem = runtime->allocLongLived<HasFinalizer::Yes>(sizeof(HiddenClass));
void *mem =
runtime->allocLongLived<HasFinalizer::Yes>(cellSize<HiddenClass>());
return HermesValue::encodeObjectValue(new (mem) HiddenClass(
runtime, flags, parent, symbolID, propertyFlags, numProperties));
}
Expand Down
10 changes: 5 additions & 5 deletions lib/VM/HostModel.cpp
View file
Expand Up @@ -22,7 +22,7 @@ CallableVTable FinalizableNativeFunction::vt{
{
VTable(
CellKind::FinalizableNativeFunctionKind,
sizeof(FinalizableNativeFunction),
cellSize<FinalizableNativeFunction>(),
FinalizableNativeFunction::_finalizeImpl),
FinalizableNativeFunction::_getOwnIndexedRangeImpl,
FinalizableNativeFunction::_haveOwnIndexedImpl,
Expand Down Expand Up @@ -63,7 +63,7 @@ CallResult<HermesValue> FinalizableNativeFunction::createWithoutPrototype(
auto parentHandle = Handle<JSObject>::vmcast(&runtime->functionPrototype);

void *mem = runtime->alloc</*fixedSize*/ true, HasFinalizer::Yes>(
sizeof(FinalizableNativeFunction));
cellSize<FinalizableNativeFunction>());
auto selfHandle = runtime->makeHandle(new (mem) FinalizableNativeFunction(
runtime,
parentHandle,
Expand Down Expand Up @@ -94,7 +94,7 @@ HostObjectProxy::~HostObjectProxy() {}
ObjectVTable HostObject::vt{
VTable(
CellKind::HostObjectKind,
sizeof(HostObject),
cellSize<HostObject>(),
HostObject::_finalizeImpl),
HostObject::_getOwnIndexedRangeImpl,
HostObject::_haveOwnIndexedImpl,
Expand Down Expand Up @@ -126,8 +126,8 @@ CallResult<HermesValue> HostObject::createWithoutPrototype(
std::shared_ptr<HostObjectProxy> proxy) {
auto parentHandle = Handle<JSObject>::vmcast(&runtime->objectPrototype);

void *mem =
runtime->alloc</*fixedSize*/ true, HasFinalizer::Yes>(sizeof(HostObject));
void *mem = runtime->alloc</*fixedSize*/ true, HasFinalizer::Yes>(
cellSize<HostObject>());
HostObject *hostObj = new (mem) HostObject(
runtime,
*parentHandle,
Expand Down

0 comments on commit 06147ee

Please sign in to comment.