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 <jasnell@gmail.com>

PR-URL: #31960
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

src/async_wrap.cc

@@ -175,6 +175,10 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) {
 
 class PromiseWrap : public AsyncWrap {
  public:
+  enum InternalFields {
+    kIsChainedPromiseField = AsyncWrap::kInternalFieldCount,
+    kInternalFieldCount
+  };
   PromiseWrap(Environment* env, Local<Object> object, bool silent)
       : AsyncWrap(env, object, PROVIDER_PROMISE, kInvalidAsyncId, silent) {
     MakeWeak();
@@ -184,9 +188,6 @@ class PromiseWrap : public AsyncWrap {
   SET_MEMORY_INFO_NAME(PromiseWrap)
   SET_SELF_SIZE(PromiseWrap)
 
-  static constexpr int kIsChainedPromiseField = 1;
-  static constexpr int kInternalFieldCount = 2;
-
   static PromiseWrap* New(Environment* env,
                           Local<Promise> promise,
                           PromiseWrap* parent_wrap,
@@ -213,15 +214,16 @@ PromiseWrap* PromiseWrap::New(Environment* env,
 void PromiseWrap::getIsChainedPromise(Local<String> property,
                                       const PropertyCallbackInfo<Value>& info) {
   info.GetReturnValue().Set(
-    info.Holder()->GetInternalField(kIsChainedPromiseField));
+      info.Holder()->GetInternalField(PromiseWrap::kIsChainedPromiseField));
 }
 
 static PromiseWrap* extractPromiseWrap(Local<Promise> promise) {
-  Local<Value> resource_object_value = promise->GetInternalField(0);
-  if (resource_object_value->IsObject()) {
-    return Unwrap<PromiseWrap>(resource_object_value.As<Object>());
-  }
-  return nullptr;
+  // This check is imperfect. If the internal field is set, it should
+  // be an object. If it's not, we just ignore it. Ideally v8 would
+  // have had GetInternalField returning a MaybeLocal but this works
+  // for now.
+  Local<Value> obj = promise->GetInternalField(0);
+  return obj->IsObject() ? Unwrap<PromiseWrap>(obj.As<Object>()) : nullptr;
 }
 
 static void PromiseHook(PromiseHookType type, Local<Promise> promise,
@@ -566,7 +568,7 @@ void AsyncWrap::Initialize(Local<Object> target,
     function_template->SetClassName(class_name);
     function_template->Inherit(AsyncWrap::GetConstructorTemplate(env));
     auto instance_template = function_template->InstanceTemplate();
-    instance_template->SetInternalFieldCount(1);
+    instance_template->SetInternalFieldCount(AsyncWrap::kInternalFieldCount);
     auto function =
         function_template->GetFunction(env->context()).ToLocalChecked();
     target->Set(env->context(), class_name, function).Check();

src/base_object-inl.h

@@ -43,7 +43,9 @@ BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object)
     : persistent_handle_(env->isolate(), object), env_(env) {
   CHECK_EQ(false, object.IsEmpty());
   CHECK_GT(object->InternalFieldCount(), 0);
-  object->SetAlignedPointerInInternalField(0, static_cast<void*>(this));
+  object->SetAlignedPointerInInternalField(
+      BaseObject::kSlot,
+      static_cast<void*>(this));
   env->AddCleanupHook(DeleteMe, static_cast<void*>(this));
   env->modify_base_object_count(1);
 }
@@ -67,7 +69,7 @@ BaseObject::~BaseObject() {
 
   {
     v8::HandleScope handle_scope(env()->isolate());
-    object()->SetAlignedPointerInInternalField(0, nullptr);
+    object()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
   }
 }
 
@@ -100,7 +102,8 @@ Environment* BaseObject::env() const {
 
 BaseObject* BaseObject::FromJSObject(v8::Local<v8::Object> obj) {
   CHECK_GT(obj->InternalFieldCount(), 0);
-  return static_cast<BaseObject*>(obj->GetAlignedPointerFromInternalField(0));
+  return static_cast<BaseObject*>(
+      obj->GetAlignedPointerFromInternalField(BaseObject::kSlot));
 }
 
 
@@ -148,11 +151,12 @@ BaseObject::MakeLazilyInitializedJSTemplate(Environment* env) {
   auto constructor = [](const v8::FunctionCallbackInfo<v8::Value>& args) {
     DCHECK(args.IsConstructCall());
     DCHECK_GT(args.This()->InternalFieldCount(), 0);
-    args.This()->SetAlignedPointerInInternalField(0, nullptr);
+    args.This()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
   };
 
   v8::Local<v8::FunctionTemplate> t = env->NewFunctionTemplate(constructor);
-  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->InstanceTemplate()->SetInternalFieldCount(
+      BaseObject::kInternalFieldCount);
   return t;
 }
 

src/base_object.h

@@ -36,6 +36,8 @@ class BaseObjectPtrImpl;
 
 class BaseObject : public MemoryRetainer {
  public:
+  enum InternalFields { kSlot, kInternalFieldCount };
+
   // Associates this object with `object`. It uses the 0th internal field for
   // that, and in particular aborts if there is no such field.
   inline BaseObject(Environment* env, v8::Local<v8::Object> object);

src/cares_wrap.cc

@@ -2224,7 +2224,8 @@ void Initialize(Local<Object> target,
 
   Local<FunctionTemplate> channel_wrap =
       env->NewFunctionTemplate(ChannelWrap::New);
-  channel_wrap->InstanceTemplate()->SetInternalFieldCount(1);
+  channel_wrap->InstanceTemplate()->SetInternalFieldCount(
+      ChannelWrap::kInternalFieldCount);
   channel_wrap->Inherit(AsyncWrap::GetConstructorTemplate(env));
 
   env->SetProtoMethod(channel_wrap, "queryAny", Query<QueryAnyWrap>);

src/fs_event_wrap.cc

@@ -97,7 +97,8 @@ void FSEventWrap::Initialize(Local<Object> target,
 
   auto fsevent_string = FIXED_ONE_BYTE_STRING(env->isolate(), "FSEvent");
   Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
-  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->InstanceTemplate()->SetInternalFieldCount(
+      FSEventWrap::kInternalFieldCount);
   t->SetClassName(fsevent_string);
 
   t->Inherit(AsyncWrap::GetConstructorTemplate(env));

src/heap_utils.cc

@@ -341,7 +341,7 @@ BaseObjectPtr<AsyncWrap> CreateHeapSnapshotStream(
     Local<FunctionTemplate> os = FunctionTemplate::New(env->isolate());
     os->Inherit(AsyncWrap::GetConstructorTemplate(env));
     Local<ObjectTemplate> ost = os->InstanceTemplate();
-    ost->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
+    ost->SetInternalFieldCount(StreamBase::kInternalFieldCount);
     os->SetClassName(
         FIXED_ONE_BYTE_STRING(env->isolate(), "HeapSnapshotStream"));
     StreamBase::AddMethods(env, os);

src/inspector_js_api.cc

@@ -105,7 +105,8 @@ class JSBindingsConnection : public AsyncWrap {
     Local<String> class_name = ConnectionType::GetClassName(env);
     Local<FunctionTemplate> tmpl =
         env->NewFunctionTemplate(JSBindingsConnection::New);
-    tmpl->InstanceTemplate()->SetInternalFieldCount(1);
+    tmpl->InstanceTemplate()->SetInternalFieldCount(
+        JSBindingsConnection::kInternalFieldCount);
     tmpl->SetClassName(class_name);
     tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env));
     env->SetProtoMethod(tmpl, "dispatch", JSBindingsConnection::Dispatch);

src/js_stream.cc

@@ -204,7 +204,7 @@ void JSStream::Initialize(Local<Object> target,
       FIXED_ONE_BYTE_STRING(env->isolate(), "JSStream");
   t->SetClassName(jsStreamString);
   t->InstanceTemplate()
-    ->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
+    ->SetInternalFieldCount(StreamBase::kInternalFieldCount);
   t->Inherit(AsyncWrap::GetConstructorTemplate(env));
 
   env->SetProtoMethod(t, "finishWrite", Finish<WriteWrap>);

src/module_wrap.cc

@@ -1639,7 +1639,8 @@ void ModuleWrap::Initialize(Local<Object> target,
 
   Local<FunctionTemplate> tpl = env->NewFunctionTemplate(New);
   tpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"));
-  tpl->InstanceTemplate()->SetInternalFieldCount(1);
+  tpl->InstanceTemplate()->SetInternalFieldCount(
+      ModuleWrap::kInternalFieldCount);
 
   env->SetProtoMethod(tpl, "link", Link);
   env->SetProtoMethod(tpl, "instantiate", Instantiate);

src/node_contextify.cc

@@ -142,7 +142,7 @@ MaybeLocal<Object> ContextifyContext::CreateDataWrapper(Environment* env) {
     return MaybeLocal<Object>();
   }
 
-  wrapper->SetAlignedPointerInInternalField(0, this);
+  wrapper->SetAlignedPointerInInternalField(ContextifyContext::kSlot, this);
   return wrapper;
 }
 
@@ -229,7 +229,8 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
 void ContextifyContext::Init(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> function_template =
       FunctionTemplate::New(env->isolate());
-  function_template->InstanceTemplate()->SetInternalFieldCount(1);
+  function_template->InstanceTemplate()->SetInternalFieldCount(
+      ContextifyContext::kInternalFieldCount);
   env->set_script_data_constructor_function(
       function_template->GetFunction(env->context()).ToLocalChecked());
 
@@ -328,7 +329,8 @@ template <typename T>
 ContextifyContext* ContextifyContext::Get(const PropertyCallbackInfo<T>& args) {
   Local<Value> data = args.Data();
   return static_cast<ContextifyContext*>(
-      data.As<Object>()->GetAlignedPointerFromInternalField(0));
+      data.As<Object>()->GetAlignedPointerFromInternalField(
+          ContextifyContext::kSlot));
 }
 
 // static
@@ -625,7 +627,8 @@ void ContextifyScript::Init(Environment* env, Local<Object> target) {
       FIXED_ONE_BYTE_STRING(env->isolate(), "ContextifyScript");
 
   Local<FunctionTemplate> script_tmpl = env->NewFunctionTemplate(New);
-  script_tmpl->InstanceTemplate()->SetInternalFieldCount(1);
+  script_tmpl->InstanceTemplate()->SetInternalFieldCount(
+      ContextifyScript::kInternalFieldCount);
   script_tmpl->SetClassName(class_name);
   env->SetProtoMethod(script_tmpl, "createCachedData", CreateCachedData);
   env->SetProtoMethod(script_tmpl, "runInContext", RunInContext);
@@ -1220,7 +1223,8 @@ void Initialize(Local<Object> target,
   {
     Local<FunctionTemplate> tpl = FunctionTemplate::New(env->isolate());
     tpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "CompiledFnEntry"));
-    tpl->InstanceTemplate()->SetInternalFieldCount(1);
+    tpl->InstanceTemplate()->SetInternalFieldCount(
+        CompiledFnEntry::kInternalFieldCount);
 
     env->set_compiled_fn_entry_template(tpl->InstanceTemplate());
   }

src/node_contextify.h

@@ -19,6 +19,7 @@ struct ContextOptions {
 
 class ContextifyContext {
  public:
+  enum InternalFields { kSlot, kInternalFieldCount };
   ContextifyContext(Environment* env,
                     v8::Local<v8::Object> sandbox_obj,
                     const ContextOptions& options);

src/node_crypto.cc

@@ -432,7 +432,8 @@ bool EntropySource(unsigned char* buffer, size_t length) {
 
 void SecureContext::Initialize(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
-  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->InstanceTemplate()->SetInternalFieldCount(
+      SecureContext::kInternalFieldCount);
   Local<String> secureContextString =
       FIXED_ONE_BYTE_STRING(env->isolate(), "SecureContext");
   t->SetClassName(secureContextString);
@@ -3217,7 +3218,8 @@ EVP_PKEY* ManagedEVPPKey::get() const {
 
 Local<Function> KeyObject::Initialize(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
-  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->InstanceTemplate()->SetInternalFieldCount(
+      KeyObject::kInternalFieldCount);
 
   env->SetProtoMethod(t, "init", Init);
   env->SetProtoMethodNoSideEffect(t, "getSymmetricKeySize",
@@ -3450,7 +3452,8 @@ CipherBase::CipherBase(Environment* env,
 void CipherBase::Initialize(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
 
-  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->InstanceTemplate()->SetInternalFieldCount(
+      CipherBase::kInternalFieldCount);
 
   env->SetProtoMethod(t, "init", Init);
   env->SetProtoMethod(t, "initiv", InitIv);
@@ -4076,7 +4079,8 @@ Hmac::Hmac(Environment* env, Local<Object> wrap)
 void Hmac::Initialize(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
 
-  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->InstanceTemplate()->SetInternalFieldCount(
+      Hmac::kInternalFieldCount);
 
   env->SetProtoMethod(t, "init", HmacInit);
   env->SetProtoMethod(t, "update", HmacUpdate);
@@ -4201,7 +4205,8 @@ Hash::Hash(Environment* env, Local<Object> wrap)
 void Hash::Initialize(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
 
-  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->InstanceTemplate()->SetInternalFieldCount(
+      Hash::kInternalFieldCount);
 
   env->SetProtoMethod(t, "update", HashUpdate);
   env->SetProtoMethod(t, "digest", HashDigest);
@@ -4472,7 +4477,8 @@ Sign::Sign(Environment* env, Local<Object> wrap) : SignBase(env, wrap) {
 void Sign::Initialize(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
 
-  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->InstanceTemplate()->SetInternalFieldCount(
+      SignBase::kInternalFieldCount);
 
   env->SetProtoMethod(t, "init", SignInit);
   env->SetProtoMethod(t, "update", SignUpdate);
@@ -4796,7 +4802,8 @@ Verify::Verify(Environment* env, Local<Object> wrap)
 void Verify::Initialize(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
 
-  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->InstanceTemplate()->SetInternalFieldCount(
+      SignBase::kInternalFieldCount);
 
   env->SetProtoMethod(t, "init", VerifyInit);
   env->SetProtoMethod(t, "update", VerifyUpdate);
@@ -5107,7 +5114,8 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
     const PropertyAttribute attributes =
         static_cast<PropertyAttribute>(ReadOnly | DontDelete);
 
-    t->InstanceTemplate()->SetInternalFieldCount(1);
+    t->InstanceTemplate()->SetInternalFieldCount(
+        DiffieHellman::kInternalFieldCount);
 
     env->SetProtoMethod(t, "generateKeys", GenerateKeys);
     env->SetProtoMethod(t, "computeSecret", ComputeSecret);
@@ -5446,7 +5454,7 @@ void ECDH::Initialize(Environment* env, Local<Object> target) {
 
   Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
 
-  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->InstanceTemplate()->SetInternalFieldCount(ECDH::kInternalFieldCount);
 
   env->SetProtoMethod(t, "generateKeys", GenerateKeys);
   env->SetProtoMethod(t, "computeSecret", ComputeSecret);

src/node_dir.cc

@@ -358,7 +358,7 @@ void Initialize(Local<Object> target,
   env->SetProtoMethod(dir, "read", DirHandle::Read);
   env->SetProtoMethod(dir, "close", DirHandle::Close);
   Local<ObjectTemplate> dirt = dir->InstanceTemplate();
-  dirt->SetInternalFieldCount(DirHandle::kDirHandleFieldCount);
+  dirt->SetInternalFieldCount(DirHandle::kInternalFieldCount);
   Local<String> handleString =
        FIXED_ONE_BYTE_STRING(isolate, "DirHandle");
   dir->SetClassName(handleString);

src/node_dir.h

@@ -12,8 +12,6 @@ namespace fs_dir {
 // Needed to propagate `uv_dir_t`.
 class DirHandle : public AsyncWrap {
  public:
-  static constexpr int kDirHandleFieldCount = 1;
-
   static DirHandle* New(Environment* env, uv_dir_t* dir);
   ~DirHandle() override;
 

src/node_file.cc

@@ -2277,7 +2277,8 @@ void Initialize(Local<Object> target,
 
   // Create FunctionTemplate for FSReqCallback
   Local<FunctionTemplate> fst = env->NewFunctionTemplate(NewFSReqCallback);
-  fst->InstanceTemplate()->SetInternalFieldCount(1);
+  fst->InstanceTemplate()->SetInternalFieldCount(
+      FSReqBase::kInternalFieldCount);
   fst->Inherit(AsyncWrap::GetConstructorTemplate(env));
   Local<String> wrapString =
       FIXED_ONE_BYTE_STRING(isolate, "FSReqCallback");
@@ -2290,7 +2291,8 @@ void Initialize(Local<Object> target,
   // Create FunctionTemplate for FileHandleReadWrap. There’s no need
   // to do anything in the constructor, so we only store the instance template.
   Local<FunctionTemplate> fh_rw = FunctionTemplate::New(isolate);
-  fh_rw->InstanceTemplate()->SetInternalFieldCount(1);
+  fh_rw->InstanceTemplate()->SetInternalFieldCount(
+      FSReqBase::kInternalFieldCount);
   fh_rw->Inherit(AsyncWrap::GetConstructorTemplate(env));
   Local<String> fhWrapString =
       FIXED_ONE_BYTE_STRING(isolate, "FileHandleReqWrap");
@@ -2305,7 +2307,7 @@ void Initialize(Local<Object> target,
       FIXED_ONE_BYTE_STRING(isolate, "FSReqPromise");
   fpt->SetClassName(promiseString);
   Local<ObjectTemplate> fpo = fpt->InstanceTemplate();
-  fpo->SetInternalFieldCount(1);
+  fpo->SetInternalFieldCount(FSReqBase::kInternalFieldCount);
   env->set_fsreqpromise_constructor_template(fpo);
 
   // Create FunctionTemplate for FileHandle
@@ -2314,7 +2316,7 @@ void Initialize(Local<Object> target,
   env->SetProtoMethod(fd, "close", FileHandle::Close);
   env->SetProtoMethod(fd, "releaseFD", FileHandle::ReleaseFD);
   Local<ObjectTemplate> fdt = fd->InstanceTemplate();
-  fdt->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
+  fdt->SetInternalFieldCount(StreamBase::kInternalFieldCount);
   Local<String> handleString =
        FIXED_ONE_BYTE_STRING(isolate, "FileHandle");
   fd->SetClassName(handleString);
@@ -2331,7 +2333,7 @@ void Initialize(Local<Object> target,
                         "FileHandleCloseReq"));
   fdclose->Inherit(AsyncWrap::GetConstructorTemplate(env));
   Local<ObjectTemplate> fdcloset = fdclose->InstanceTemplate();
-  fdcloset->SetInternalFieldCount(1);
+  fdcloset->SetInternalFieldCount(FSReqBase::kInternalFieldCount);
   env->set_fdclose_constructor_template(fdcloset);
 
   Local<Symbol> use_promises_symbol =