Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: zero-initialize data that are copied into the snapshot #53563

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/aliased_buffer-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

namespace node {

typedef uint64_t AliasedBufferIndex;
typedef size_t AliasedBufferIndex;

template <typename NativeT, typename V8T>
AliasedBufferBase<NativeT, V8T>::AliasedBufferBase(
Expand Down
2 changes: 1 addition & 1 deletion src/aliased_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace node {

typedef uint64_t AliasedBufferIndex;
typedef size_t AliasedBufferIndex;

/**
* Do not use this class directly when creating instances of it - use the
Expand Down
7 changes: 2 additions & 5 deletions src/base_object_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,10 @@ namespace node {
SERIALIZABLE_NON_BINDING_TYPES(V)

#define V(TypeId, NativeType) k_##TypeId,
// To avoid padding, the enums are uint64_t.
enum class BindingDataType : uint64_t {
BINDING_TYPES(V) kBindingDataTypeCount
};
enum class BindingDataType : uint8_t { BINDING_TYPES(V) kBindingDataTypeCount };
// Make sure that we put the bindings first so that we can also use the enums
// for the bindings as index to the binding data store.
enum class EmbedderObjectType : uint64_t {
enum class EmbedderObjectType : uint8_t {
BINDING_TYPES(V) SERIALIZABLE_NON_BINDING_TYPES(V)
// We do not need to know about all the unserializable non-binding types for
// now so we do not list them.
Expand Down
6 changes: 0 additions & 6 deletions src/encoding_binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ class BindingData : public SnapshotableObject {
AliasedBufferIndex encode_into_results_buffer;
};

// Make sure that there's no padding in the struct since we will memcpy
// them into the snapshot blob and they need to be reproducible.
static_assert(sizeof(InternalFieldInfo) ==
sizeof(InternalFieldInfoBase) + sizeof(AliasedBufferIndex),
"InternalFieldInfo should have no padding");

BindingData(Realm* realm,
v8::Local<v8::Object> obj,
InternalFieldInfo* info = nullptr);
Expand Down
6 changes: 0 additions & 6 deletions src/node_file.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,6 @@ class BindingData : public SnapshotableObject {
AliasedBufferIndex statfs_field_bigint_array;
};

// Make sure that there's no padding in the struct since we will memcpy
// them into the snapshot blob and they need to be reproducible.
static_assert(sizeof(InternalFieldInfo) == sizeof(InternalFieldInfoBase) +
sizeof(AliasedBufferIndex) * 4,
"InternalFieldInfo should have no padding");

enum class FilePathIsFileReturnType {
kIsFile = 0,
kIsNotFile,
Expand Down
6 changes: 0 additions & 6 deletions src/node_process.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,6 @@ class BindingData : public SnapshotableObject {
AliasedBufferIndex hrtime_buffer;
};

// Make sure that there's no padding in the struct since we will memcpy
// them into the snapshot blob and they need to be reproducible.
static_assert(sizeof(InternalFieldInfo) ==
sizeof(InternalFieldInfoBase) + sizeof(AliasedBufferIndex),
"InternalFieldInfo should have no padding");

static void AddMethods(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> target);
static void RegisterExternalReferences(ExternalReferenceRegistry* registry);
Expand Down
16 changes: 9 additions & 7 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,9 @@ size_t SnapshotSerializer::Write(const PropInfo& data) {
}

// Layout of AsyncHooks::SerializeInfo
// [ 8 bytes ] snapshot index of async_ids_stack
// [ 8 bytes ] snapshot index of fields
// [ 8 bytes ] snapshot index of async_id_fields
// [ 4/8 bytes ] snapshot index of async_ids_stack
// [ 4/8 bytes ] snapshot index of fields
// [ 4/8 bytes ] snapshot index of async_id_fields
// [ 4/8 bytes ] snapshot index of js_execution_async_resources
// [ 4/8 bytes ] length of native_execution_async_resources
// [ ... ] snapshot indices of each element in
Expand Down Expand Up @@ -387,9 +387,9 @@ size_t SnapshotSerializer::Write(const ImmediateInfo::SerializeInfo& data) {
}

// Layout of PerformanceState::SerializeInfo
// [ 8 bytes ] snapshot index of root
// [ 8 bytes ] snapshot index of milestones
// [ 8 bytes ] snapshot index of observers
// [ 4/8 bytes ] snapshot index of root
// [ 4/8 bytes ] snapshot index of milestones
// [ 4/8 bytes ] snapshot index of observers
template <>
performance::PerformanceState::SerializeInfo SnapshotDeserializer::Read() {
Debug("Read<PerformanceState::SerializeInfo>()\n");
Expand Down Expand Up @@ -1415,9 +1415,11 @@ StartupData SerializeNodeContextInternalFields(Local<Object> holder,
// To serialize the type field, save data in a EmbedderTypeInfo.
if (index == BaseObject::kEmbedderType) {
int size = sizeof(EmbedderTypeInfo);
char* data = new char[size];
// We need to use placement new because V8 calls delete[] on the returned
// data.
// The () syntax at the end would zero-initialize the block and make
// the padding reproducible.
char* data = new char[size]();
// TODO(joyeecheung): support cppgc objects.
new (data) EmbedderTypeInfo(obj->type(),
EmbedderTypeInfo::MemoryMode::kBaseObject);
Expand Down
38 changes: 5 additions & 33 deletions src/node_snapshotable.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include <cassert> // For static_assert
#include <cstddef> // For offsetof
#include "aliased_buffer.h"
#include "base_object.h"
#include "util.h"
Expand Down Expand Up @@ -35,20 +33,21 @@ bool WithoutCodeCache(const SnapshotConfig& config);
// and pass it into the V8 callback as the payload of StartupData.
// The memory chunk looks like this:
//
// [ type ] - EmbedderObjectType (a uint64_t)
// [ length ] - a uint64_t
// [ type ] - EmbedderObjectType (a uint8_t)
// [ length ] - a size_t
// [ ... ] - custom bytes of size |length - header size|
struct InternalFieldInfoBase {
public:
EmbedderObjectType type;
uint64_t length;
size_t length;

template <typename T>
static T* New(EmbedderObjectType type) {
static_assert(std::is_base_of_v<InternalFieldInfoBase, T> ||
std::is_same_v<InternalFieldInfoBase, T>,
"Can only accept InternalFieldInfoBase subclasses");
void* buf = ::operator new[](sizeof(T));
memset(buf, 0, sizeof(T)); // Make the padding reproducible.
T* result = new (buf) T;
result->type = type;
result->length = sizeof(T);
Expand All @@ -73,35 +72,14 @@ struct InternalFieldInfoBase {
InternalFieldInfoBase() = default;
};

// Make sure that there's no padding in the struct since we will memcpy
// them into the snapshot blob and they need to be reproducible.
static_assert(offsetof(InternalFieldInfoBase, type) == 0,
"InternalFieldInfoBase::type should start from offset 0");
static_assert(offsetof(InternalFieldInfoBase, length) ==
sizeof(EmbedderObjectType),
"InternalFieldInfoBase::type should have no padding");

struct EmbedderTypeInfo {
// To avoid padding, the enum is uint64_t.
enum class MemoryMode : uint64_t { kBaseObject = 0, kCppGC };
enum class MemoryMode : uint8_t { kBaseObject, kCppGC };
EmbedderTypeInfo(EmbedderObjectType t, MemoryMode m) : type(t), mode(m) {}
EmbedderTypeInfo() = default;

EmbedderObjectType type;
MemoryMode mode;
};

// Make sure that there's no padding in the struct since we will memcpy
// them into the snapshot blob and they need to be reproducible.
static_assert(offsetof(EmbedderTypeInfo, type) == 0,
"EmbedderTypeInfo::type should start from offset 0");
static_assert(offsetof(EmbedderTypeInfo, mode) == sizeof(EmbedderObjectType),
"EmbedderTypeInfo::type should have no padding");
static_assert(sizeof(EmbedderTypeInfo) ==
sizeof(EmbedderObjectType) +
sizeof(EmbedderTypeInfo::MemoryMode),
"EmbedderTypeInfo::mode should have no padding");

// An interface for snapshotable native objects to inherit from.
// Use the SERIALIZABLE_OBJECT_METHODS() macro in the class to define
// the following methods to implement:
Expand Down Expand Up @@ -173,12 +151,6 @@ class BindingData : public SnapshotableObject {
AliasedBufferIndex is_building_snapshot_buffer;
};

// Make sure that there's no padding in the struct since we will memcpy
// them into the snapshot blob and they need to be reproducible.
static_assert(sizeof(InternalFieldInfo) ==
sizeof(InternalFieldInfoBase) + sizeof(AliasedBufferIndex),
"InternalFieldInfo should have no padding");

BindingData(Realm* realm,
v8::Local<v8::Object> obj,
InternalFieldInfo* info = nullptr);
Expand Down
7 changes: 0 additions & 7 deletions src/node_v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ class BindingData : public SnapshotableObject {
AliasedBufferIndex heap_space_statistics_buffer;
AliasedBufferIndex heap_code_statistics_buffer;
};

// Make sure that there's no padding in the struct since we will memcpy
// them into the snapshot blob and they need to be reproducible.
static_assert(sizeof(InternalFieldInfo) == sizeof(InternalFieldInfoBase) +
sizeof(AliasedBufferIndex) * 3,
"InternalFieldInfo should have no padding");

BindingData(Realm* realm,
v8::Local<v8::Object> obj,
InternalFieldInfo* info = nullptr);
Expand Down
Loading