Skip to content

Commit

Permalink
Revert "src: make sure that memcpy-ed structs in snapshot have no pad…
Browse files Browse the repository at this point in the history
…ding"

This reverts commit 4e58cde.

PR-URL: #53563
Refs: #50983
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung authored and aduh95 committed Jul 16, 2024
1 parent 933359a commit 8cda2db
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 71 deletions.
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
12 changes: 6 additions & 6 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
37 changes: 4 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,13 +33,13 @@ 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) {
Expand Down Expand Up @@ -73,35 +71,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 +150,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

0 comments on commit 8cda2db

Please sign in to comment.