Skip to content

Commit 0fac393

Browse files
jasnelljasnell
committedMar 2, 2020
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: nodejs#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 68e36ad commit 0fac393

40 files changed

+151
-92
lines changed
 

‎src/async_wrap.cc

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

177177
class PromiseWrap : public AsyncWrap {
178178
public:
179+
enum InternalFields {
180+
kIsChainedPromiseField = AsyncWrap::kInternalFieldCount,
181+
kInternalFieldCount
182+
};
179183
PromiseWrap(Environment* env, Local<Object> object, bool silent)
180184
: AsyncWrap(env, object, PROVIDER_PROMISE, kInvalidAsyncId, silent) {
181185
MakeWeak();
@@ -185,9 +189,6 @@ class PromiseWrap : public AsyncWrap {
185189
SET_MEMORY_INFO_NAME(PromiseWrap)
186190
SET_SELF_SIZE(PromiseWrap)
187191

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

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

228230
static void PromiseHook(PromiseHookType type, Local<Promise> promise,
@@ -560,7 +562,7 @@ void AsyncWrap::Initialize(Local<Object> target,
560562
function_template->SetClassName(class_name);
561563
function_template->Inherit(AsyncWrap::GetConstructorTemplate(env));
562564
auto instance_template = function_template->InstanceTemplate();
563-
instance_template->SetInternalFieldCount(1);
565+
instance_template->SetInternalFieldCount(AsyncWrap::kInternalFieldCount);
564566
auto function =
565567
function_template->GetFunction(env->context()).ToLocalChecked();
566568
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
@@ -2223,7 +2223,8 @@ void Initialize(Local<Object> target,
22232223

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

22292230
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
@@ -1636,7 +1636,8 @@ void ModuleWrap::Initialize(Local<Object> target,
16361636

16371637
Local<FunctionTemplate> tpl = env->NewFunctionTemplate(New);
16381638
tpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"));
1639-
tpl->InstanceTemplate()->SetInternalFieldCount(1);
1639+
tpl->InstanceTemplate()->SetInternalFieldCount(
1640+
ModuleWrap::kInternalFieldCount);
16401641

16411642
env->SetProtoMethod(tpl, "link", Link);
16421643
env->SetProtoMethod(tpl, "instantiate", Instantiate);

‎src/node_contextify.cc

+9-5
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ MaybeLocal<Object> ContextifyContext::CreateDataWrapper(Environment* env) {
145145
return MaybeLocal<Object>();
146146
}
147147

148-
wrapper->SetAlignedPointerInInternalField(0, this);
148+
wrapper->SetAlignedPointerInInternalField(ContextifyContext::kSlot, this);
149149
return wrapper;
150150
}
151151

@@ -232,7 +232,8 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
232232
void ContextifyContext::Init(Environment* env, Local<Object> target) {
233233
Local<FunctionTemplate> function_template =
234234
FunctionTemplate::New(env->isolate());
235-
function_template->InstanceTemplate()->SetInternalFieldCount(1);
235+
function_template->InstanceTemplate()->SetInternalFieldCount(
236+
ContextifyContext::kInternalFieldCount);
236237
env->set_script_data_constructor_function(
237238
function_template->GetFunction(env->context()).ToLocalChecked());
238239

@@ -331,7 +332,8 @@ template <typename T>
331332
ContextifyContext* ContextifyContext::Get(const PropertyCallbackInfo<T>& args) {
332333
Local<Value> data = args.Data();
333334
return static_cast<ContextifyContext*>(
334-
data.As<Object>()->GetAlignedPointerFromInternalField(0));
335+
data.As<Object>()->GetAlignedPointerFromInternalField(
336+
ContextifyContext::kSlot));
335337
}
336338

337339
// static
@@ -628,7 +630,8 @@ void ContextifyScript::Init(Environment* env, Local<Object> target) {
628630
FIXED_ONE_BYTE_STRING(env->isolate(), "ContextifyScript");
629631

630632
Local<FunctionTemplate> script_tmpl = env->NewFunctionTemplate(New);
631-
script_tmpl->InstanceTemplate()->SetInternalFieldCount(1);
633+
script_tmpl->InstanceTemplate()->SetInternalFieldCount(
634+
ContextifyScript::kInternalFieldCount);
632635
script_tmpl->SetClassName(class_name);
633636
env->SetProtoMethod(script_tmpl, "createCachedData", CreateCachedData);
634637
env->SetProtoMethod(script_tmpl, "runInContext", RunInContext);
@@ -1251,7 +1254,8 @@ void Initialize(Local<Object> target,
12511254
{
12521255
Local<FunctionTemplate> tpl = FunctionTemplate::New(env->isolate());
12531256
tpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "CompiledFnEntry"));
1254-
tpl->InstanceTemplate()->SetInternalFieldCount(1);
1257+
tpl->InstanceTemplate()->SetInternalFieldCount(
1258+
CompiledFnEntry::kInternalFieldCount);
12551259

12561260
env->set_compiled_fn_entry_template(tpl->InstanceTemplate());
12571261
}

‎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
@@ -464,7 +464,8 @@ static T* MallocOpenSSL(size_t count) {
464464

465465
void SecureContext::Initialize(Environment* env, Local<Object> target) {
466466
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
467-
t->InstanceTemplate()->SetInternalFieldCount(1);
467+
t->InstanceTemplate()->SetInternalFieldCount(
468+
SecureContext::kInternalFieldCount);
468469
Local<String> secureContextString =
469470
FIXED_ONE_BYTE_STRING(env->isolate(), "SecureContext");
470471
t->SetClassName(secureContextString);
@@ -3803,7 +3804,8 @@ EVP_PKEY* ManagedEVPPKey::get() const {
38033804

38043805
Local<Function> KeyObject::Initialize(Environment* env, Local<Object> target) {
38053806
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
3806-
t->InstanceTemplate()->SetInternalFieldCount(1);
3807+
t->InstanceTemplate()->SetInternalFieldCount(
3808+
KeyObject::kInternalFieldCount);
38073809

38083810
env->SetProtoMethod(t, "init", Init);
38093811
env->SetProtoMethodNoSideEffect(t, "getSymmetricKeySize",
@@ -4036,7 +4038,8 @@ CipherBase::CipherBase(Environment* env,
40364038
void CipherBase::Initialize(Environment* env, Local<Object> target) {
40374039
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
40384040

4039-
t->InstanceTemplate()->SetInternalFieldCount(1);
4041+
t->InstanceTemplate()->SetInternalFieldCount(
4042+
CipherBase::kInternalFieldCount);
40404043

40414044
env->SetProtoMethod(t, "init", Init);
40424045
env->SetProtoMethod(t, "initiv", InitIv);
@@ -4663,7 +4666,8 @@ Hmac::Hmac(Environment* env, v8::Local<v8::Object> wrap)
46634666
void Hmac::Initialize(Environment* env, Local<Object> target) {
46644667
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
46654668

4666-
t->InstanceTemplate()->SetInternalFieldCount(1);
4669+
t->InstanceTemplate()->SetInternalFieldCount(
4670+
Hmac::kInternalFieldCount);
46674671

46684672
env->SetProtoMethod(t, "init", HmacInit);
46694673
env->SetProtoMethod(t, "update", HmacUpdate);
@@ -4789,7 +4793,8 @@ Hash::Hash(Environment* env, v8::Local<v8::Object> wrap)
47894793
void Hash::Initialize(Environment* env, Local<Object> target) {
47904794
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
47914795

4792-
t->InstanceTemplate()->SetInternalFieldCount(1);
4796+
t->InstanceTemplate()->SetInternalFieldCount(
4797+
Hash::kInternalFieldCount);
47934798

47944799
env->SetProtoMethod(t, "update", HashUpdate);
47954800
env->SetProtoMethod(t, "digest", HashDigest);
@@ -5061,7 +5066,8 @@ Sign::Sign(Environment* env, v8::Local<v8::Object> wrap) : SignBase(env, wrap) {
50615066
void Sign::Initialize(Environment* env, Local<Object> target) {
50625067
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
50635068

5064-
t->InstanceTemplate()->SetInternalFieldCount(1);
5069+
t->InstanceTemplate()->SetInternalFieldCount(
5070+
SignBase::kInternalFieldCount);
50655071

50665072
env->SetProtoMethod(t, "init", SignInit);
50675073
env->SetProtoMethod(t, "update", SignUpdate);
@@ -5385,7 +5391,8 @@ Verify::Verify(Environment* env, v8::Local<v8::Object> wrap) :
53855391
void Verify::Initialize(Environment* env, Local<Object> target) {
53865392
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
53875393

5388-
t->InstanceTemplate()->SetInternalFieldCount(1);
5394+
t->InstanceTemplate()->SetInternalFieldCount(
5395+
SignBase::kInternalFieldCount);
53895396

53905397
env->SetProtoMethod(t, "init", VerifyInit);
53915398
env->SetProtoMethod(t, "update", VerifyUpdate);
@@ -5697,7 +5704,8 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
56975704
const PropertyAttribute attributes =
56985705
static_cast<PropertyAttribute>(ReadOnly | DontDelete);
56995706

5700-
t->InstanceTemplate()->SetInternalFieldCount(1);
5707+
t->InstanceTemplate()->SetInternalFieldCount(
5708+
DiffieHellman::kInternalFieldCount);
57015709

57025710
env->SetProtoMethod(t, "generateKeys", GenerateKeys);
57035711
env->SetProtoMethod(t, "computeSecret", ComputeSecret);
@@ -6036,7 +6044,7 @@ void ECDH::Initialize(Environment* env, Local<Object> target) {
60366044

60376045
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
60386046

6039-
t->InstanceTemplate()->SetInternalFieldCount(1);
6047+
t->InstanceTemplate()->SetInternalFieldCount(ECDH::kInternalFieldCount);
60406048

60416049
env->SetProtoMethod(t, "generateKeys", GenerateKeys);
60426050
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
@@ -2290,7 +2290,8 @@ void Initialize(Local<Object> target,
22902290

22912291
// Create FunctionTemplate for FSReqCallback
22922292
Local<FunctionTemplate> fst = env->NewFunctionTemplate(NewFSReqCallback);
2293-
fst->InstanceTemplate()->SetInternalFieldCount(1);
2293+
fst->InstanceTemplate()->SetInternalFieldCount(
2294+
FSReqBase::kInternalFieldCount);
22942295
fst->Inherit(AsyncWrap::GetConstructorTemplate(env));
22952296
Local<String> wrapString =
22962297
FIXED_ONE_BYTE_STRING(isolate, "FSReqCallback");
@@ -2303,7 +2304,8 @@ void Initialize(Local<Object> target,
23032304
// Create FunctionTemplate for FileHandleReadWrap. There’s no need
23042305
// to do anything in the constructor, so we only store the instance template.
23052306
Local<FunctionTemplate> fh_rw = FunctionTemplate::New(isolate);
2306-
fh_rw->InstanceTemplate()->SetInternalFieldCount(1);
2307+
fh_rw->InstanceTemplate()->SetInternalFieldCount(
2308+
FSReqBase::kInternalFieldCount);
23072309
fh_rw->Inherit(AsyncWrap::GetConstructorTemplate(env));
23082310
Local<String> fhWrapString =
23092311
FIXED_ONE_BYTE_STRING(isolate, "FileHandleReqWrap");
@@ -2318,7 +2320,7 @@ void Initialize(Local<Object> target,
23182320
FIXED_ONE_BYTE_STRING(isolate, "FSReqPromise");
23192321
fpt->SetClassName(promiseString);
23202322
Local<ObjectTemplate> fpo = fpt->InstanceTemplate();
2321-
fpo->SetInternalFieldCount(1);
2323+
fpo->SetInternalFieldCount(FSReqBase::kInternalFieldCount);
23222324
env->set_fsreqpromise_constructor_template(fpo);
23232325

23242326
// Create FunctionTemplate for FileHandle
@@ -2327,7 +2329,7 @@ void Initialize(Local<Object> target,
23272329
env->SetProtoMethod(fd, "close", FileHandle::Close);
23282330
env->SetProtoMethod(fd, "releaseFD", FileHandle::ReleaseFD);
23292331
Local<ObjectTemplate> fdt = fd->InstanceTemplate();
2330-
fdt->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
2332+
fdt->SetInternalFieldCount(StreamBase::kInternalFieldCount);
23312333
Local<String> handleString =
23322334
FIXED_ONE_BYTE_STRING(isolate, "FileHandle");
23332335
fd->SetClassName(handleString);
@@ -2344,7 +2346,7 @@ void Initialize(Local<Object> target,
23442346
"FileHandleCloseReq"));
23452347
fdclose->Inherit(AsyncWrap::GetConstructorTemplate(env));
23462348
Local<ObjectTemplate> fdcloset = fdclose->InstanceTemplate();
2347-
fdcloset->SetInternalFieldCount(1);
2349+
fdcloset->SetInternalFieldCount(FSReqBase::kInternalFieldCount);
23482350
env->set_fdclose_constructor_template(fdcloset);
23492351

23502352
Local<Symbol> use_promises_symbol =

0 commit comments

Comments
 (0)
Please sign in to comment.