From 59d874d1514943f215b812dec7d4134df427c27c Mon Sep 17 00:00:00 2001 From: Georgi Davidkov Date: Tue, 5 May 2026 18:44:09 +0300 Subject: [PATCH] Fix v8::External::Value/New calls for V8 13 V8 13 added an ExternalPointerTypeTag parameter (uint16_t pointer-tag for the sandbox) to both v8::External::New and v8::External::Value: static Local New(Isolate*, void*, ExternalPointerTypeTag); void* Value(ExternalPointerTypeTag) const; The old zero-arg Value() and two-arg New(Isolate*, void*) overloads are gone, so any nan-using addon fails to compile against Electron 42 / Node 24's V8: nan_callbacks_12_inl.h(189): error C2664: 'void *v8::External::Value(v8::ExternalPointerTypeTag) const': cannot convert argument 1 from 'v8::Isolate *' to 'v8::ExternalPointerTypeTag' nan_implementation_12_inl.h(79): error C2660: 'v8::External::New': function does not take 2 arguments Add two private inline helpers in the existing imp:: namespace: imp::GetExternalPointer(v8::Local) imp::NewExternal(v8::Isolate*, void*) Each selects the V8 13+ signature when V8_MAJOR_VERSION >= 13 (passing v8::kExternalPointerTypeTagDefault, the documented no-tagging default for embedders that do not partition externals by type) and falls back to the legacy signature otherwise. All 28 Value() trampoline callsites in nan_callbacks_12_inl.h and all 3 New() factory callsites in nan_implementation_12_inl.h are routed through the helpers. Both helpers share kExternalPointerTypeTagDefault so the tag matches between create and read. No new macros are introduced. Tests ----- The existing news.cpp::NewExternal and nannew.cpp::testExternal cases call v8::External::Value() directly (consumer-side raw-V8 calls, not through nan), so they failed to compile against V8 13 in the same way as nan's headers. Added the same V8 13 guard inline at those two sites so the tests build and run on V8 13 while remaining identical on older V8. Together these two existing tests already cover the round-trip exercised by this fix: - news.cpp::NewExternal exercises the factory path: Nan::New(...) -> Factory::New(value) -> imp::NewExternal(isolate, value) -> v8::External::New(isolate, value, tag) - The trampoline path (imp::GetExternalPointer) is exercised every time any NAN_METHOD test is invoked from JS, including news.cpp::NewExternal itself, since FunctionCallbackWrapper unpacks the registered callback pointer through that helper before dispatching. Scope and what is not changed ----------------------------- This patch addresses the v8::External break specifically. Other unrelated V8 13 API changes in nan, if any, are out of scope (mirroring the prior nan_weak.h fix which was a separate, surgical commit). nan_callbacks_pre_12_inl.h and nan_implementation_pre_12_inl.h are intentionally untouched: they target NODE_MODULE_VERSION <= NODE_0_12_MODULE_VERSION (Node <= 0.12, pre-V8-3.x), where neither overload exists. Backwards compatibility ----------------------- Every change is gated on `defined(V8_MAJOR_VERSION) && V8_MAJOR_VERSION >= 13`. On V8 <= 12 the else branch reproduces the original code byte-for-byte, so the patch is a no-op for every V8 version nan currently supports. --- nan_callbacks_12_inl.h | 129 ++++++++++++++++++++---------------- nan_implementation_12_inl.h | 22 +++++- test/cpp/nannew.cpp | 4 ++ test/cpp/news.cpp | 5 ++ 4 files changed, 101 insertions(+), 59 deletions(-) diff --git a/nan_callbacks_12_inl.h b/nan_callbacks_12_inl.h index ff3b654d..835580dc 100644 --- a/nan_callbacks_12_inl.h +++ b/nan_callbacks_12_inl.h @@ -179,13 +179,30 @@ class PropertyCallbackInfo { }; namespace imp { + +// Recent V8 versions (currently Chromium/Electron's V8 snapshot; not yet in +// any released Node) gained an ExternalPointerTypeTag parameter on +// v8::External::Value(). Detect that API via the V8_EXTERNAL_POINTER_TAG_COUNT +// macro (defined in alongside the new signature) rather than a +// V8_MAJOR_VERSION cutoff, since Node and Chromium ship divergent V8 +// snapshots under the same major version. Externals created by nan use +// kExternalPointerTypeTagDefault (see imp::NewExternal in +// nan_implementation_12_inl.h), so the same tag is used here on read. +inline void* GetExternalPointer(v8::Local ext) { +#ifdef V8_EXTERNAL_POINTER_TAG_COUNT + return ext->Value(v8::kExternalPointerTypeTagDefault); +#else + return ext->Value(); +#endif +} + static void FunctionCallbackWrapper(const v8::FunctionCallbackInfo &info) { v8::Local obj = info.Data().As(); FunctionCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kFunctionIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kFunctionIndex) + .As().As()))); FunctionCallbackInfo cbinfo(info, obj->GetInternalField(kDataIndex).As()); callback(cbinfo); @@ -203,8 +220,8 @@ void GetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); GetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kGetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kGetterIndex) + .As().As()))); callback(property.As(), cbinfo); } @@ -221,8 +238,8 @@ void SetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); SetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kSetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kSetterIndex) + .As().As()))); callback(property.As(), value, cbinfo); } @@ -240,8 +257,8 @@ void GetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); GetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kGetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kGetterIndex) + .As().As()))); callback(property, cbinfo); } @@ -258,8 +275,8 @@ void SetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); SetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kSetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kSetterIndex) + .As().As()))); callback(property, value, cbinfo); } @@ -282,8 +299,8 @@ v8::Intercepted PropertyGetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyGetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertyGetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyGetterIndex) + .As().As()))); return callback(property.As(), cbinfo); } @@ -300,8 +317,8 @@ v8::Intercepted PropertySetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertySetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertySetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertySetterIndex) + .As().As()))); return callback(property.As(), value, cbinfo); } @@ -320,8 +337,8 @@ void PropertyGetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyGetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertyGetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyGetterIndex) + .As().As()))); callback(property.As(), cbinfo); } @@ -338,8 +355,8 @@ void PropertySetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertySetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertySetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertySetterIndex) + .As().As()))); callback(property.As(), value, cbinfo); } @@ -357,8 +374,8 @@ void PropertyEnumeratorCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyEnumeratorCallback callback = reinterpret_cast(reinterpret_cast( - obj->GetInternalField(kPropertyEnumeratorIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyEnumeratorIndex) + .As().As()))); callback(cbinfo); } @@ -376,8 +393,8 @@ v8::Intercepted PropertyDeleterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyDeleterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertyDeleterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyDeleterIndex) + .As().As()))); return callback(property.As(), cbinfo); } @@ -394,8 +411,8 @@ v8::Intercepted PropertyQueryCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyQueryCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertyQueryIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyQueryIndex) + .As().As()))); return callback(property.As(), cbinfo); } @@ -411,8 +428,8 @@ void PropertyDeleterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyDeleterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertyDeleterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyDeleterIndex) + .As().As()))); callback(property.As(), cbinfo); } @@ -428,8 +445,8 @@ void PropertyQueryCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyQueryCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertyQueryIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyQueryIndex) + .As().As()))); callback(property.As(), cbinfo); } @@ -446,8 +463,8 @@ void PropertyGetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyGetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertyGetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyGetterIndex) + .As().As()))); callback(property, cbinfo); } @@ -464,8 +481,8 @@ void PropertySetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertySetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertySetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertySetterIndex) + .As().As()))); callback(property, value, cbinfo); } @@ -482,8 +499,8 @@ void PropertyEnumeratorCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyEnumeratorCallback callback = reinterpret_cast(reinterpret_cast( - obj->GetInternalField(kPropertyEnumeratorIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyEnumeratorIndex) + .As().As()))); callback(cbinfo); } @@ -499,8 +516,8 @@ void PropertyDeleterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyDeleterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertyDeleterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyDeleterIndex) + .As().As()))); callback(property, cbinfo); } @@ -516,8 +533,8 @@ void PropertyQueryCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); PropertyQueryCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kPropertyQueryIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kPropertyQueryIndex) + .As().As()))); callback(property, cbinfo); } @@ -535,8 +552,8 @@ v8::Intercepted IndexGetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); IndexGetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kIndexPropertyGetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyGetterIndex) + .As().As()))); return callback(index, cbinfo); } @@ -553,8 +570,8 @@ v8::Intercepted IndexSetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); IndexSetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kIndexPropertySetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kIndexPropertySetterIndex) + .As().As()))); return callback(index, value, cbinfo); } @@ -572,8 +589,8 @@ void IndexGetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); IndexGetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kIndexPropertyGetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyGetterIndex) + .As().As()))); callback(index, cbinfo); } @@ -589,8 +606,8 @@ void IndexSetterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); IndexSetterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kIndexPropertySetterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kIndexPropertySetterIndex) + .As().As()))); callback(index, value, cbinfo); } @@ -609,9 +626,9 @@ void IndexEnumeratorCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); IndexEnumeratorCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField( + imp::GetExternalPointer(obj->GetInternalField( kIndexPropertyEnumeratorIndex) - .As().As()->Value())); + .As().As()))); callback(cbinfo); } @@ -628,8 +645,8 @@ v8::Intercepted IndexDeleterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); IndexDeleterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kIndexPropertyDeleterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyDeleterIndex) + .As().As()))); return callback(index, cbinfo); } @@ -644,8 +661,8 @@ v8::Intercepted IndexQueryCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); IndexQueryCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kIndexPropertyQueryIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyQueryIndex) + .As().As()))); return callback(index, cbinfo); } @@ -660,8 +677,8 @@ void IndexDeleterCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); IndexDeleterCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kIndexPropertyDeleterIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyDeleterIndex) + .As().As()))); callback(index, cbinfo); } @@ -676,8 +693,8 @@ void IndexQueryCallbackWrapper( cbinfo(info, obj->GetInternalField(kDataIndex).As()); IndexQueryCallback callback = reinterpret_cast( reinterpret_cast( - obj->GetInternalField(kIndexPropertyQueryIndex) - .As().As()->Value())); + imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyQueryIndex) + .As().As()))); callback(index, cbinfo); } diff --git a/nan_implementation_12_inl.h b/nan_implementation_12_inl.h index 255293ac..79731c76 100644 --- a/nan_implementation_12_inl.h +++ b/nan_implementation_12_inl.h @@ -14,6 +14,22 @@ namespace imp { +// Recent V8 versions (currently Chromium/Electron's V8 snapshot; not yet in +// any released Node) gained an ExternalPointerTypeTag parameter on +// v8::External::New(). Detect that API via the V8_EXTERNAL_POINTER_TAG_COUNT +// macro (defined in alongside the new signature) rather than +// a V8_MAJOR_VERSION cutoff, since Node and Chromium ship divergent V8 +// snapshots under the same major version. Externals created via this helper +// are read back through imp::GetExternalPointer in nan_callbacks_12_inl.h, +// which uses the matching kExternalPointerTypeTagDefault tag. +inline v8::Local NewExternal(v8::Isolate* isolate, void* value) { +#ifdef V8_EXTERNAL_POINTER_TAG_COUNT + return v8::External::New(isolate, value, v8::kExternalPointerTypeTagDefault); +#else + return v8::External::New(isolate, value); +#endif +} + //=== Array ==================================================================== Factory::return_t @@ -76,7 +92,7 @@ Factory::New(double value) { Factory::return_t Factory::New(void * value) { - return v8::External::New(v8::Isolate::GetCurrent(), value); + return imp::NewExternal(v8::Isolate::GetCurrent(), value); } //=== Function ================================================================= @@ -92,7 +108,7 @@ Factory::New( FunctionCallback callback obj->SetInternalField( imp::kFunctionIndex - , v8::External::New(isolate, reinterpret_cast(callback))); + , imp::NewExternal(isolate, reinterpret_cast(callback))); v8::Local val = v8::Local::New(isolate, data); @@ -128,7 +144,7 @@ Factory::New( FunctionCallback callback obj->SetInternalField( imp::kFunctionIndex - , v8::External::New(isolate, reinterpret_cast(callback))); + , imp::NewExternal(isolate, reinterpret_cast(callback))); v8::Local val = v8::Local::New(isolate, data); if (!val.IsEmpty()) { diff --git a/test/cpp/nannew.cpp b/test/cpp/nannew.cpp index e36ce4f8..267b6b21 100644 --- a/test/cpp/nannew.cpp +++ b/test/cpp/nannew.cpp @@ -136,7 +136,11 @@ NAN_METHOD(testExternal) { t.plan(2); +#ifdef V8_EXTERNAL_POINTER_TAG_COUNT + t.ok(_(New(&ttt)->Value(v8::kExternalPointerTypeTagDefault) == &ttt)); +#else t.ok(_(New(&ttt)->Value() == &ttt)); +#endif t.ok(_( assertType(New(&ttt)))); info.GetReturnValue().SetUndefined(); diff --git a/test/cpp/news.cpp b/test/cpp/news.cpp index 3597f0fc..c966af37 100644 --- a/test/cpp/news.cpp +++ b/test/cpp/news.cpp @@ -93,7 +93,12 @@ NAN_METHOD(NewBooleanObject) { NAN_METHOD(NewExternal) { v8::Local ext = New(&magic); +#ifdef V8_EXTERNAL_POINTER_TAG_COUNT + assert(*static_cast( + ext->Value(v8::kExternalPointerTypeTagDefault)) == 1337); +#else assert(*static_cast(ext->Value()) == 1337); +#endif info.GetReturnValue().Set(New("passed").ToLocalChecked()); }