Skip to content

Commit c979aea

Browse files
jasnelltargos
jasnell
authored and
targos
committed
src: improve handling of internal field counting
Change suggested by bnoordhuis. Improve handing of internal field counting by using enums. Helps protect against future possible breakage if field indexes are ever changed or added to. Signed-off-by: James M Snell <[email protected]> PR-URL: #31960 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 0847bc3 commit c979aea

40 files changed

+151
-92
lines changed

‎src/async_wrap.cc

+12-10
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,10 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) {
175175

176176
class PromiseWrap : public AsyncWrap {
177177
public:
178+
enum InternalFields {
179+
kIsChainedPromiseField = AsyncWrap::kInternalFieldCount,
180+
kInternalFieldCount
181+
};
178182
PromiseWrap(Environment* env, Local<Object> object, bool silent)
179183
: AsyncWrap(env, object, PROVIDER_PROMISE, kInvalidAsyncId, silent) {
180184
MakeWeak();
@@ -184,9 +188,6 @@ class PromiseWrap : public AsyncWrap {
184188
SET_MEMORY_INFO_NAME(PromiseWrap)
185189
SET_SELF_SIZE(PromiseWrap)
186190

187-
static constexpr int kIsChainedPromiseField = 1;
188-
static constexpr int kInternalFieldCount = 2;
189-
190191
static PromiseWrap* New(Environment* env,
191192
Local<Promise> promise,
192193
PromiseWrap* parent_wrap,
@@ -213,15 +214,16 @@ PromiseWrap* PromiseWrap::New(Environment* env,
213214
void PromiseWrap::getIsChainedPromise(Local<String> property,
214215
const PropertyCallbackInfo<Value>& info) {
215216
info.GetReturnValue().Set(
216-
info.Holder()->GetInternalField(kIsChainedPromiseField));
217+
info.Holder()->GetInternalField(PromiseWrap::kIsChainedPromiseField));
217218
}
218219

219220
static PromiseWrap* extractPromiseWrap(Local<Promise> promise) {
220-
Local<Value> resource_object_value = promise->GetInternalField(0);
221-
if (resource_object_value->IsObject()) {
222-
return Unwrap<PromiseWrap>(resource_object_value.As<Object>());
223-
}
224-
return nullptr;
221+
// This check is imperfect. If the internal field is set, it should
222+
// be an object. If it's not, we just ignore it. Ideally v8 would
223+
// have had GetInternalField returning a MaybeLocal but this works
224+
// for now.
225+
Local<Value> obj = promise->GetInternalField(0);
226+
return obj->IsObject() ? Unwrap<PromiseWrap>(obj.As<Object>()) : nullptr;
225227
}
226228

227229
static void PromiseHook(PromiseHookType type, Local<Promise> promise,
@@ -566,7 +568,7 @@ void AsyncWrap::Initialize(Local<Object> target,
566568
function_template->SetClassName(class_name);
567569
function_template->Inherit(AsyncWrap::GetConstructorTemplate(env));
568570
auto instance_template = function_template->InstanceTemplate();
569-
instance_template->SetInternalFieldCount(1);
571+
instance_template->SetInternalFieldCount(AsyncWrap::kInternalFieldCount);
570572
auto function =
571573
function_template->GetFunction(env->context()).ToLocalChecked();
572574
target->Set(env->context(), class_name, function).Check();

‎src/base_object-inl.h

+9-5
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object)
4343
: persistent_handle_(env->isolate(), object), env_(env) {
4444
CHECK_EQ(false, object.IsEmpty());
4545
CHECK_GT(object->InternalFieldCount(), 0);
46-
object->SetAlignedPointerInInternalField(0, static_cast<void*>(this));
46+
object->SetAlignedPointerInInternalField(
47+
BaseObject::kSlot,
48+
static_cast<void*>(this));
4749
env->AddCleanupHook(DeleteMe, static_cast<void*>(this));
4850
env->modify_base_object_count(1);
4951
}
@@ -67,7 +69,7 @@ BaseObject::~BaseObject() {
6769

6870
{
6971
v8::HandleScope handle_scope(env()->isolate());
70-
object()->SetAlignedPointerInInternalField(0, nullptr);
72+
object()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
7173
}
7274
}
7375

@@ -100,7 +102,8 @@ Environment* BaseObject::env() const {
100102

101103
BaseObject* BaseObject::FromJSObject(v8::Local<v8::Object> obj) {
102104
CHECK_GT(obj->InternalFieldCount(), 0);
103-
return static_cast<BaseObject*>(obj->GetAlignedPointerFromInternalField(0));
105+
return static_cast<BaseObject*>(
106+
obj->GetAlignedPointerFromInternalField(BaseObject::kSlot));
104107
}
105108

106109

@@ -148,11 +151,12 @@ BaseObject::MakeLazilyInitializedJSTemplate(Environment* env) {
148151
auto constructor = [](const v8::FunctionCallbackInfo<v8::Value>& args) {
149152
DCHECK(args.IsConstructCall());
150153
DCHECK_GT(args.This()->InternalFieldCount(), 0);
151-
args.This()->SetAlignedPointerInInternalField(0, nullptr);
154+
args.This()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
152155
};
153156

154157
v8::Local<v8::FunctionTemplate> t = env->NewFunctionTemplate(constructor);
155-
t->InstanceTemplate()->SetInternalFieldCount(1);
158+
t->InstanceTemplate()->SetInternalFieldCount(
159+
BaseObject::kInternalFieldCount);
156160
return t;
157161
}
158162

‎src/base_object.h

+2
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ class BaseObjectPtrImpl;
3636

3737
class BaseObject : public MemoryRetainer {
3838
public:
39+
enum InternalFields { kSlot, kInternalFieldCount };
40+
3941
// Associates this object with `object`. It uses the 0th internal field for
4042
// that, and in particular aborts if there is no such field.
4143
inline BaseObject(Environment* env, v8::Local<v8::Object> object);

‎src/cares_wrap.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -2224,7 +2224,8 @@ void Initialize(Local<Object> target,
22242224

22252225
Local<FunctionTemplate> channel_wrap =
22262226
env->NewFunctionTemplate(ChannelWrap::New);
2227-
channel_wrap->InstanceTemplate()->SetInternalFieldCount(1);
2227+
channel_wrap->InstanceTemplate()->SetInternalFieldCount(
2228+
ChannelWrap::kInternalFieldCount);
22282229
channel_wrap->Inherit(AsyncWrap::GetConstructorTemplate(env));
22292230

22302231
env->SetProtoMethod(channel_wrap, "queryAny", Query<QueryAnyWrap>);

‎src/fs_event_wrap.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ void FSEventWrap::Initialize(Local<Object> target,
9797

9898
auto fsevent_string = FIXED_ONE_BYTE_STRING(env->isolate(), "FSEvent");
9999
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
100-
t->InstanceTemplate()->SetInternalFieldCount(1);
100+
t->InstanceTemplate()->SetInternalFieldCount(
101+
FSEventWrap::kInternalFieldCount);
101102
t->SetClassName(fsevent_string);
102103

103104
t->Inherit(AsyncWrap::GetConstructorTemplate(env));

‎src/heap_utils.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ BaseObjectPtr<AsyncWrap> CreateHeapSnapshotStream(
341341
Local<FunctionTemplate> os = FunctionTemplate::New(env->isolate());
342342
os->Inherit(AsyncWrap::GetConstructorTemplate(env));
343343
Local<ObjectTemplate> ost = os->InstanceTemplate();
344-
ost->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
344+
ost->SetInternalFieldCount(StreamBase::kInternalFieldCount);
345345
os->SetClassName(
346346
FIXED_ONE_BYTE_STRING(env->isolate(), "HeapSnapshotStream"));
347347
StreamBase::AddMethods(env, os);

‎src/inspector_js_api.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ class JSBindingsConnection : public AsyncWrap {
105105
Local<String> class_name = ConnectionType::GetClassName(env);
106106
Local<FunctionTemplate> tmpl =
107107
env->NewFunctionTemplate(JSBindingsConnection::New);
108-
tmpl->InstanceTemplate()->SetInternalFieldCount(1);
108+
tmpl->InstanceTemplate()->SetInternalFieldCount(
109+
JSBindingsConnection::kInternalFieldCount);
109110
tmpl->SetClassName(class_name);
110111
tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env));
111112
env->SetProtoMethod(tmpl, "dispatch", JSBindingsConnection::Dispatch);

‎src/js_stream.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ void JSStream::Initialize(Local<Object> target,
204204
FIXED_ONE_BYTE_STRING(env->isolate(), "JSStream");
205205
t->SetClassName(jsStreamString);
206206
t->InstanceTemplate()
207-
->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
207+
->SetInternalFieldCount(StreamBase::kInternalFieldCount);
208208
t->Inherit(AsyncWrap::GetConstructorTemplate(env));
209209

210210
env->SetProtoMethod(t, "finishWrite", Finish<WriteWrap>);

‎src/module_wrap.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -1639,7 +1639,8 @@ void ModuleWrap::Initialize(Local<Object> target,
16391639

16401640
Local<FunctionTemplate> tpl = env->NewFunctionTemplate(New);
16411641
tpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"));
1642-
tpl->InstanceTemplate()->SetInternalFieldCount(1);
1642+
tpl->InstanceTemplate()->SetInternalFieldCount(
1643+
ModuleWrap::kInternalFieldCount);
16431644

16441645
env->SetProtoMethod(tpl, "link", Link);
16451646
env->SetProtoMethod(tpl, "instantiate", Instantiate);

‎src/node_contextify.cc

+9-5
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ MaybeLocal<Object> ContextifyContext::CreateDataWrapper(Environment* env) {
142142
return MaybeLocal<Object>();
143143
}
144144

145-
wrapper->SetAlignedPointerInInternalField(0, this);
145+
wrapper->SetAlignedPointerInInternalField(ContextifyContext::kSlot, this);
146146
return wrapper;
147147
}
148148

@@ -229,7 +229,8 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
229229
void ContextifyContext::Init(Environment* env, Local<Object> target) {
230230
Local<FunctionTemplate> function_template =
231231
FunctionTemplate::New(env->isolate());
232-
function_template->InstanceTemplate()->SetInternalFieldCount(1);
232+
function_template->InstanceTemplate()->SetInternalFieldCount(
233+
ContextifyContext::kInternalFieldCount);
233234
env->set_script_data_constructor_function(
234235
function_template->GetFunction(env->context()).ToLocalChecked());
235236

@@ -328,7 +329,8 @@ template <typename T>
328329
ContextifyContext* ContextifyContext::Get(const PropertyCallbackInfo<T>& args) {
329330
Local<Value> data = args.Data();
330331
return static_cast<ContextifyContext*>(
331-
data.As<Object>()->GetAlignedPointerFromInternalField(0));
332+
data.As<Object>()->GetAlignedPointerFromInternalField(
333+
ContextifyContext::kSlot));
332334
}
333335

334336
// static
@@ -625,7 +627,8 @@ void ContextifyScript::Init(Environment* env, Local<Object> target) {
625627
FIXED_ONE_BYTE_STRING(env->isolate(), "ContextifyScript");
626628

627629
Local<FunctionTemplate> script_tmpl = env->NewFunctionTemplate(New);
628-
script_tmpl->InstanceTemplate()->SetInternalFieldCount(1);
630+
script_tmpl->InstanceTemplate()->SetInternalFieldCount(
631+
ContextifyScript::kInternalFieldCount);
629632
script_tmpl->SetClassName(class_name);
630633
env->SetProtoMethod(script_tmpl, "createCachedData", CreateCachedData);
631634
env->SetProtoMethod(script_tmpl, "runInContext", RunInContext);
@@ -1220,7 +1223,8 @@ void Initialize(Local<Object> target,
12201223
{
12211224
Local<FunctionTemplate> tpl = FunctionTemplate::New(env->isolate());
12221225
tpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "CompiledFnEntry"));
1223-
tpl->InstanceTemplate()->SetInternalFieldCount(1);
1226+
tpl->InstanceTemplate()->SetInternalFieldCount(
1227+
CompiledFnEntry::kInternalFieldCount);
12241228

12251229
env->set_compiled_fn_entry_template(tpl->InstanceTemplate());
12261230
}

‎src/node_contextify.h

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ struct ContextOptions {
1919

2020
class ContextifyContext {
2121
public:
22+
enum InternalFields { kSlot, kInternalFieldCount };
2223
ContextifyContext(Environment* env,
2324
v8::Local<v8::Object> sandbox_obj,
2425
const ContextOptions& options);

‎src/node_crypto.cc

+17-9
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,8 @@ bool EntropySource(unsigned char* buffer, size_t length) {
432432

433433
void SecureContext::Initialize(Environment* env, Local<Object> target) {
434434
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
435-
t->InstanceTemplate()->SetInternalFieldCount(1);
435+
t->InstanceTemplate()->SetInternalFieldCount(
436+
SecureContext::kInternalFieldCount);
436437
Local<String> secureContextString =
437438
FIXED_ONE_BYTE_STRING(env->isolate(), "SecureContext");
438439
t->SetClassName(secureContextString);
@@ -3217,7 +3218,8 @@ EVP_PKEY* ManagedEVPPKey::get() const {
32173218

32183219
Local<Function> KeyObject::Initialize(Environment* env, Local<Object> target) {
32193220
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
3220-
t->InstanceTemplate()->SetInternalFieldCount(1);
3221+
t->InstanceTemplate()->SetInternalFieldCount(
3222+
KeyObject::kInternalFieldCount);
32213223

32223224
env->SetProtoMethod(t, "init", Init);
32233225
env->SetProtoMethodNoSideEffect(t, "getSymmetricKeySize",
@@ -3450,7 +3452,8 @@ CipherBase::CipherBase(Environment* env,
34503452
void CipherBase::Initialize(Environment* env, Local<Object> target) {
34513453
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
34523454

3453-
t->InstanceTemplate()->SetInternalFieldCount(1);
3455+
t->InstanceTemplate()->SetInternalFieldCount(
3456+
CipherBase::kInternalFieldCount);
34543457

34553458
env->SetProtoMethod(t, "init", Init);
34563459
env->SetProtoMethod(t, "initiv", InitIv);
@@ -4076,7 +4079,8 @@ Hmac::Hmac(Environment* env, Local<Object> wrap)
40764079
void Hmac::Initialize(Environment* env, Local<Object> target) {
40774080
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
40784081

4079-
t->InstanceTemplate()->SetInternalFieldCount(1);
4082+
t->InstanceTemplate()->SetInternalFieldCount(
4083+
Hmac::kInternalFieldCount);
40804084

40814085
env->SetProtoMethod(t, "init", HmacInit);
40824086
env->SetProtoMethod(t, "update", HmacUpdate);
@@ -4201,7 +4205,8 @@ Hash::Hash(Environment* env, Local<Object> wrap)
42014205
void Hash::Initialize(Environment* env, Local<Object> target) {
42024206
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
42034207

4204-
t->InstanceTemplate()->SetInternalFieldCount(1);
4208+
t->InstanceTemplate()->SetInternalFieldCount(
4209+
Hash::kInternalFieldCount);
42054210

42064211
env->SetProtoMethod(t, "update", HashUpdate);
42074212
env->SetProtoMethod(t, "digest", HashDigest);
@@ -4472,7 +4477,8 @@ Sign::Sign(Environment* env, Local<Object> wrap) : SignBase(env, wrap) {
44724477
void Sign::Initialize(Environment* env, Local<Object> target) {
44734478
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
44744479

4475-
t->InstanceTemplate()->SetInternalFieldCount(1);
4480+
t->InstanceTemplate()->SetInternalFieldCount(
4481+
SignBase::kInternalFieldCount);
44764482

44774483
env->SetProtoMethod(t, "init", SignInit);
44784484
env->SetProtoMethod(t, "update", SignUpdate);
@@ -4796,7 +4802,8 @@ Verify::Verify(Environment* env, Local<Object> wrap)
47964802
void Verify::Initialize(Environment* env, Local<Object> target) {
47974803
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
47984804

4799-
t->InstanceTemplate()->SetInternalFieldCount(1);
4805+
t->InstanceTemplate()->SetInternalFieldCount(
4806+
SignBase::kInternalFieldCount);
48004807

48014808
env->SetProtoMethod(t, "init", VerifyInit);
48024809
env->SetProtoMethod(t, "update", VerifyUpdate);
@@ -5107,7 +5114,8 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
51075114
const PropertyAttribute attributes =
51085115
static_cast<PropertyAttribute>(ReadOnly | DontDelete);
51095116

5110-
t->InstanceTemplate()->SetInternalFieldCount(1);
5117+
t->InstanceTemplate()->SetInternalFieldCount(
5118+
DiffieHellman::kInternalFieldCount);
51115119

51125120
env->SetProtoMethod(t, "generateKeys", GenerateKeys);
51135121
env->SetProtoMethod(t, "computeSecret", ComputeSecret);
@@ -5446,7 +5454,7 @@ void ECDH::Initialize(Environment* env, Local<Object> target) {
54465454

54475455
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
54485456

5449-
t->InstanceTemplate()->SetInternalFieldCount(1);
5457+
t->InstanceTemplate()->SetInternalFieldCount(ECDH::kInternalFieldCount);
54505458

54515459
env->SetProtoMethod(t, "generateKeys", GenerateKeys);
54525460
env->SetProtoMethod(t, "computeSecret", ComputeSecret);

‎src/node_dir.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ void Initialize(Local<Object> target,
358358
env->SetProtoMethod(dir, "read", DirHandle::Read);
359359
env->SetProtoMethod(dir, "close", DirHandle::Close);
360360
Local<ObjectTemplate> dirt = dir->InstanceTemplate();
361-
dirt->SetInternalFieldCount(DirHandle::kDirHandleFieldCount);
361+
dirt->SetInternalFieldCount(DirHandle::kInternalFieldCount);
362362
Local<String> handleString =
363363
FIXED_ONE_BYTE_STRING(isolate, "DirHandle");
364364
dir->SetClassName(handleString);

‎src/node_dir.h

-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ namespace fs_dir {
1212
// Needed to propagate `uv_dir_t`.
1313
class DirHandle : public AsyncWrap {
1414
public:
15-
static constexpr int kDirHandleFieldCount = 1;
16-
1715
static DirHandle* New(Environment* env, uv_dir_t* dir);
1816
~DirHandle() override;
1917

‎src/node_file.cc

+7-5
Original file line numberDiff line numberDiff line change
@@ -2277,7 +2277,8 @@ void Initialize(Local<Object> target,
22772277

22782278
// Create FunctionTemplate for FSReqCallback
22792279
Local<FunctionTemplate> fst = env->NewFunctionTemplate(NewFSReqCallback);
2280-
fst->InstanceTemplate()->SetInternalFieldCount(1);
2280+
fst->InstanceTemplate()->SetInternalFieldCount(
2281+
FSReqBase::kInternalFieldCount);
22812282
fst->Inherit(AsyncWrap::GetConstructorTemplate(env));
22822283
Local<String> wrapString =
22832284
FIXED_ONE_BYTE_STRING(isolate, "FSReqCallback");
@@ -2290,7 +2291,8 @@ void Initialize(Local<Object> target,
22902291
// Create FunctionTemplate for FileHandleReadWrap. There’s no need
22912292
// to do anything in the constructor, so we only store the instance template.
22922293
Local<FunctionTemplate> fh_rw = FunctionTemplate::New(isolate);
2293-
fh_rw->InstanceTemplate()->SetInternalFieldCount(1);
2294+
fh_rw->InstanceTemplate()->SetInternalFieldCount(
2295+
FSReqBase::kInternalFieldCount);
22942296
fh_rw->Inherit(AsyncWrap::GetConstructorTemplate(env));
22952297
Local<String> fhWrapString =
22962298
FIXED_ONE_BYTE_STRING(isolate, "FileHandleReqWrap");
@@ -2305,7 +2307,7 @@ void Initialize(Local<Object> target,
23052307
FIXED_ONE_BYTE_STRING(isolate, "FSReqPromise");
23062308
fpt->SetClassName(promiseString);
23072309
Local<ObjectTemplate> fpo = fpt->InstanceTemplate();
2308-
fpo->SetInternalFieldCount(1);
2310+
fpo->SetInternalFieldCount(FSReqBase::kInternalFieldCount);
23092311
env->set_fsreqpromise_constructor_template(fpo);
23102312

23112313
// Create FunctionTemplate for FileHandle
@@ -2314,7 +2316,7 @@ void Initialize(Local<Object> target,
23142316
env->SetProtoMethod(fd, "close", FileHandle::Close);
23152317
env->SetProtoMethod(fd, "releaseFD", FileHandle::ReleaseFD);
23162318
Local<ObjectTemplate> fdt = fd->InstanceTemplate();
2317-
fdt->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
2319+
fdt->SetInternalFieldCount(StreamBase::kInternalFieldCount);
23182320
Local<String> handleString =
23192321
FIXED_ONE_BYTE_STRING(isolate, "FileHandle");
23202322
fd->SetClassName(handleString);
@@ -2331,7 +2333,7 @@ void Initialize(Local<Object> target,
23312333
"FileHandleCloseReq"));
23322334
fdclose->Inherit(AsyncWrap::GetConstructorTemplate(env));
23332335
Local<ObjectTemplate> fdcloset = fdclose->InstanceTemplate();
2334-
fdcloset->SetInternalFieldCount(1);
2336+
fdcloset->SetInternalFieldCount(FSReqBase::kInternalFieldCount);
23352337
env->set_fdclose_constructor_template(fdcloset);
23362338

23372339
Local<Symbol> use_promises_symbol =

0 commit comments

Comments
 (0)