diff --git a/doc/api/cli.md b/doc/api/cli.md index 23e08096510..96139b83b0e 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -220,7 +220,7 @@ The initializer module also needs to be allowed. Consider the following example: ```console $ node --experimental-permission t.js node:internal/modules/cjs/loader:162 - const result = internalModuleStat(filename); + const result = internalModuleStat(receiver, filename); ^ Error: Access to this API has been restricted diff --git a/doc/api/permissions.md b/doc/api/permissions.md index fd1a8e5859f..e37c2982bc1 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -48,7 +48,7 @@ will be restricted. ```console $ node --experimental-permission index.js node:internal/modules/cjs/loader:171 - const result = internalModuleStat(filename); + const result = internalModuleStat(receiver, filename); ^ Error: Access to this API has been restricted diff --git a/doc/contributing/adding-v8-fast-api.md b/doc/contributing/adding-v8-fast-api.md index 7f7da32656d..1869c12731f 100644 --- a/doc/contributing/adding-v8-fast-api.md +++ b/doc/contributing/adding-v8-fast-api.md @@ -24,7 +24,7 @@ for example, they may not trigger garbage collection. [`node_external_reference.h`](../../src/node_external_reference.h) file. Although, it would not start failing or crashing until the function ends up in a snapshot (either the built-in or a user-land one). Please refer to the - [binding functions documentation](../../src#binding-functions) for more + [binding functions documentation](../../src/README.md#binding-functions) for more information. * To test fast APIs, make sure to run the tests in a loop with a decent iterations count to trigger relevant optimizations that prefer the fast API @@ -38,6 +38,23 @@ for example, they may not trigger garbage collection. * The fast callback must be idempotent up to the point where error and fallback conditions are checked, because otherwise executing the slow callback might produce visible side effects twice. +* If the receiver is used in the callback, it must be passed as a second argument, + leaving the first one unused, to prevent the JS land from accidentally omitting the receiver when + invoking the fast API method. + + ```cpp + // Instead of invoking the method as `receiver.internalModuleStat(input)`, the JS land should + // invoke it as `internalModuleStat(binding, input)` to make sure the binding is available to + // the native land. + static int32_t FastInternalModuleStat( + Local unused, + Local recv, + const FastOneByteString& input, + FastApiCallbackOptions& options) { + Environment* env = Environment::GetCurrent(recv->GetCreationContextChecked()); + // More code + } + ``` ## Fallback to slow path diff --git a/lib/fs.js b/lib/fs.js index 1497c0935a1..11c53abc917 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1414,7 +1414,7 @@ function readdirSyncRecursive(basePath, options) { for (let i = 0; i < readdirResult.length; i++) { const resultPath = pathModule.join(path, readdirResult[i]); const relativeResultPath = pathModule.relative(basePath, resultPath); - const stat = binding.internalModuleStat(resultPath); + const stat = binding.internalModuleStat(binding, resultPath); ArrayPrototypePush(readdirResults, relativeResultPath); // 1 indicates directory if (stat === 1) { diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 88473afc1a5..2f37c8d7f8b 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -914,7 +914,7 @@ async function readdirRecursive(originalPath, options) { const { 0: path, 1: readdir } = ArrayPrototypePop(queue); for (const ent of readdir) { const direntPath = pathModule.join(path, ent); - const stat = binding.internalModuleStat(direntPath); + const stat = binding.internalModuleStat(binding, direntPath); ArrayPrototypePush( result, pathModule.relative(originalPath, direntPath), diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index c959170e573..b0210e4d963 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -235,9 +235,9 @@ function stat(filename) { const result = statCache.get(filename); if (result !== undefined) { return result; } } - const result = internalFsBinding.internalModuleStat(filename); + const result = internalFsBinding.internalModuleStat(internalFsBinding, filename); if (statCache !== null && result >= 0) { - // Only set cache when `internalModuleStat(filename)` succeeds. + // Only set cache when `internalModuleStat(internalFsBinding, filename)` succeeds. statCache.set(filename, result); } return result; diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index c83778cc00e..35925ef0817 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -241,8 +241,10 @@ function finalizeResolution(resolved, base, preserveSymlinks) { throw err; } - const stats = internalFsBinding.internalModuleStat(StringPrototypeEndsWith(path, '/') ? - StringPrototypeSlice(path, -1) : path); + const stats = internalFsBinding.internalModuleStat( + internalFsBinding, + StringPrototypeEndsWith(internalFsBinding, path, '/') ? StringPrototypeSlice(path, -1) : path, + ); // Check for stats.isDirectory() if (stats === 1) { @@ -802,6 +804,7 @@ function packageResolve(specifier, base, conditions) { let lastPath; do { const stat = internalFsBinding.internalModuleStat( + internalFsBinding, StringPrototypeSlice(packageJSONPath, 0, packageJSONPath.length - 13), ); // Check for !stat.isDirectory() diff --git a/src/histogram.cc b/src/histogram.cc index 50605515dc8..c62a5b89524 100644 --- a/src/histogram.cc +++ b/src/histogram.cc @@ -169,7 +169,8 @@ void HistogramBase::RecordDelta(const FunctionCallbackInfo& args) { (*histogram)->RecordDelta(); } -void HistogramBase::FastRecordDelta(Local receiver) { +void HistogramBase::FastRecordDelta(Local unused, + Local receiver) { HistogramBase* histogram; ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver); (*histogram)->RecordDelta(); @@ -189,7 +190,8 @@ void HistogramBase::Record(const FunctionCallbackInfo& args) { (*histogram)->Record(value); } -void HistogramBase::FastRecord(Local receiver, +void HistogramBase::FastRecord(Local unused, + Local receiver, const int64_t value, FastApiCallbackOptions& options) { if (value < 1) { @@ -436,7 +438,9 @@ void IntervalHistogram::Start(const FunctionCallbackInfo& args) { histogram->OnStart(args[0]->IsTrue() ? StartFlags::RESET : StartFlags::NONE); } -void IntervalHistogram::FastStart(Local receiver, bool reset) { +void IntervalHistogram::FastStart(Local unused, + Local receiver, + bool reset) { IntervalHistogram* histogram; ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver); histogram->OnStart(reset ? StartFlags::RESET : StartFlags::NONE); @@ -448,7 +452,7 @@ void IntervalHistogram::Stop(const FunctionCallbackInfo& args) { histogram->OnStop(); } -void IntervalHistogram::FastStop(Local receiver) { +void IntervalHistogram::FastStop(Local unused, Local receiver) { IntervalHistogram* histogram; ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver); histogram->OnStop(); @@ -564,42 +568,45 @@ void HistogramImpl::DoReset(const FunctionCallbackInfo& args) { (*histogram)->Reset(); } -void HistogramImpl::FastReset(Local receiver) { +void HistogramImpl::FastReset(Local unused, Local receiver) { HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); (*histogram)->Reset(); } -double HistogramImpl::FastGetCount(Local receiver) { +double HistogramImpl::FastGetCount(Local unused, Local receiver) { HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return static_cast((*histogram)->Count()); } -double HistogramImpl::FastGetMin(Local receiver) { +double HistogramImpl::FastGetMin(Local unused, Local receiver) { HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return static_cast((*histogram)->Min()); } -double HistogramImpl::FastGetMax(Local receiver) { +double HistogramImpl::FastGetMax(Local unused, Local receiver) { HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return static_cast((*histogram)->Max()); } -double HistogramImpl::FastGetMean(Local receiver) { +double HistogramImpl::FastGetMean(Local unused, Local receiver) { HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return (*histogram)->Mean(); } -double HistogramImpl::FastGetExceeds(Local receiver) { +double HistogramImpl::FastGetExceeds(Local unused, + Local receiver) { HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return static_cast((*histogram)->Exceeds()); } -double HistogramImpl::FastGetStddev(Local receiver) { +double HistogramImpl::FastGetStddev(Local unused, + Local receiver) { HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return (*histogram)->Stddev(); } -double HistogramImpl::FastGetPercentile(Local receiver, +double HistogramImpl::FastGetPercentile(Local unused, + Local receiver, const double percentile) { HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return static_cast((*histogram)->Percentile(percentile)); diff --git a/src/histogram.h b/src/histogram.h index c3d8fe5f207..362e82e4436 100644 --- a/src/histogram.h +++ b/src/histogram.h @@ -101,14 +101,22 @@ class HistogramImpl { static void GetPercentilesBigInt( const v8::FunctionCallbackInfo& args); - static void FastReset(v8::Local receiver); - static double FastGetCount(v8::Local receiver); - static double FastGetMin(v8::Local receiver); - static double FastGetMax(v8::Local receiver); - static double FastGetMean(v8::Local receiver); - static double FastGetExceeds(v8::Local receiver); - static double FastGetStddev(v8::Local receiver); - static double FastGetPercentile(v8::Local receiver, + static void FastReset(v8::Local unused, + v8::Local receiver); + static double FastGetCount(v8::Local unused, + v8::Local receiver); + static double FastGetMin(v8::Local unused, + v8::Local receiver); + static double FastGetMax(v8::Local unused, + v8::Local receiver); + static double FastGetMean(v8::Local unused, + v8::Local receiver); + static double FastGetExceeds(v8::Local unused, + v8::Local receiver); + static double FastGetStddev(v8::Local unused, + v8::Local receiver); + static double FastGetPercentile(v8::Local unused, + v8::Local receiver, const double percentile); static void AddMethods(v8::Isolate* isolate, @@ -158,10 +166,12 @@ class HistogramBase final : public BaseObject, public HistogramImpl { static void Add(const v8::FunctionCallbackInfo& args); static void FastRecord( + v8::Local unused, v8::Local receiver, const int64_t value, v8::FastApiCallbackOptions& options); // NOLINT(runtime/references) - static void FastRecordDelta(v8::Local receiver); + static void FastRecordDelta(v8::Local unused, + v8::Local receiver); HistogramBase( Environment* env, @@ -233,8 +243,11 @@ class IntervalHistogram final : public HandleWrap, public HistogramImpl { static void Start(const v8::FunctionCallbackInfo& args); static void Stop(const v8::FunctionCallbackInfo& args); - static void FastStart(v8::Local receiver, bool reset); - static void FastStop(v8::Local receiver); + static void FastStart(v8::Local unused, + v8::Local receiver, + bool reset); + static void FastStop(v8::Local unused, + v8::Local receiver); BaseObject::TransferMode GetTransferMode() const override { return TransferMode::kCloneable; diff --git a/src/node_external_reference.h b/src/node_external_reference.h index 38a4ff7e6c2..8d49a119c21 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -12,19 +12,25 @@ namespace node { using CFunctionCallbackWithOneByteString = uint32_t (*)(v8::Local, const v8::FastOneByteString&); -using CFunctionCallback = void (*)(v8::Local receiver); +using CFunctionCallback = void (*)(v8::Local unused, + v8::Local receiver); using CFunctionCallbackReturnDouble = - double (*)(v8::Local receiver); + double (*)(v8::Local unused, v8::Local receiver); using CFunctionCallbackReturnInt32 = - int32_t (*)(v8::Local receiver, + int32_t (*)(v8::Local unused, + v8::Local receiver, const v8::FastOneByteString& input, // NOLINTNEXTLINE(runtime/references) This is V8 api. v8::FastApiCallbackOptions& options); using CFunctionCallbackValueReturnDouble = double (*)(v8::Local receiver); -using CFunctionCallbackWithInt64 = void (*)(v8::Local receiver, +using CFunctionCallbackValueReturnDoubleUnusedReceiver = + double (*)(v8::Local unused, v8::Local receiver); +using CFunctionCallbackWithInt64 = void (*)(v8::Local unused, + v8::Local receiver, int64_t); -using CFunctionCallbackWithBool = void (*)(v8::Local receiver, +using CFunctionCallbackWithBool = void (*)(v8::Local unused, + v8::Local receiver, bool); using CFunctionCallbackWithString = bool (*)(v8::Local, const v8::FastOneByteString& input); @@ -50,11 +56,15 @@ using CFunctionCallbackWithUint8ArrayUint32Int64Bool = using CFunctionWithUint32 = uint32_t (*)(v8::Local, const uint32_t input); using CFunctionWithDoubleReturnDouble = double (*)(v8::Local, + v8::Local, const double); using CFunctionWithInt64Fallback = void (*)(v8::Local, + v8::Local, const int64_t, v8::FastApiCallbackOptions&); -using CFunctionWithBool = void (*)(v8::Local, bool); +using CFunctionWithBool = void (*)(v8::Local, + v8::Local, + bool); using CFunctionWriteString = uint32_t (*)(v8::Local receiver, @@ -83,6 +93,7 @@ class ExternalReferenceRegistry { V(CFunctionCallbackReturnDouble) \ V(CFunctionCallbackReturnInt32) \ V(CFunctionCallbackValueReturnDouble) \ + V(CFunctionCallbackValueReturnDoubleUnusedReceiver) \ V(CFunctionCallbackWithInt64) \ V(CFunctionCallbackWithBool) \ V(CFunctionCallbackWithString) \ diff --git a/src/node_file.cc b/src/node_file.cc index 0b40cb63fae..ab2961ca8cf 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1034,8 +1034,9 @@ static void ExistsSync(const FunctionCallbackInfo& args) { static void InternalModuleStat(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - CHECK(args[0]->IsString()); - BufferValue path(env->isolate(), args[0]); + CHECK_GE(args.Length(), 2); + CHECK(args[1]->IsString()); + BufferValue path(env->isolate(), args[1]); CHECK_NOT_NULL(*path); ToNamespacedPath(env, &path); THROW_IF_INSUFFICIENT_PERMISSIONS( @@ -1053,6 +1054,7 @@ static void InternalModuleStat(const FunctionCallbackInfo& args) { } static int32_t FastInternalModuleStat( + Local unused, Local recv, const FastOneByteString& input, // NOLINTNEXTLINE(runtime/references) This is V8 api. diff --git a/src/node_process.h b/src/node_process.h index 142d0e63e18..d4f1c0d45de 100644 --- a/src/node_process.h +++ b/src/node_process.h @@ -72,7 +72,8 @@ class BindingData : public SnapshotableObject { static BindingData* FromV8Value(v8::Local receiver); static void NumberImpl(BindingData* receiver); - static void FastNumber(v8::Local receiver) { + static void FastNumber(v8::Local unused, + v8::Local receiver) { NumberImpl(FromV8Value(receiver)); } @@ -80,7 +81,8 @@ class BindingData : public SnapshotableObject { static void BigIntImpl(BindingData* receiver); - static void FastBigInt(v8::Local receiver) { + static void FastBigInt(v8::Local unused, + v8::Local receiver) { BigIntImpl(FromV8Value(receiver)); } diff --git a/src/node_wasi.cc b/src/node_wasi.cc index ad1da44a01f..d8b226f40a9 100644 --- a/src/node_wasi.cc +++ b/src/node_wasi.cc @@ -241,6 +241,7 @@ inline void EinvalError() {} template R WASI::WasiFunction::FastCallback( + Local unused, Local receiver, Args... args, // NOLINTNEXTLINE(runtime/references) This is V8 api. diff --git a/src/node_wasi.h b/src/node_wasi.h index 25551936e6b..ef7d2e83b67 100644 --- a/src/node_wasi.h +++ b/src/node_wasi.h @@ -160,7 +160,8 @@ class WASI : public BaseObject, v8::Local); private: - static R FastCallback(v8::Local receiver, + static R FastCallback(v8::Local unused, + v8::Local receiver, Args..., v8::FastApiCallbackOptions&); diff --git a/src/timers.cc b/src/timers.cc index 127806fbcdf..c7ba22a078f 100644 --- a/src/timers.cc +++ b/src/timers.cc @@ -33,7 +33,8 @@ void BindingData::SlowGetLibuvNow(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(Number::New(args.GetIsolate(), now)); } -double BindingData::FastGetLibuvNow(Local receiver) { +double BindingData::FastGetLibuvNow(Local unused, + Local receiver) { return GetLibuvNowImpl(FromJSObject(receiver)); } @@ -47,7 +48,9 @@ void BindingData::SlowScheduleTimer(const FunctionCallbackInfo& args) { ScheduleTimerImpl(Realm::GetBindingData(args), duration); } -void BindingData::FastScheduleTimer(Local receiver, int64_t duration) { +void BindingData::FastScheduleTimer(Local unused, + Local receiver, + int64_t duration) { ScheduleTimerImpl(FromJSObject(receiver), duration); } @@ -61,7 +64,9 @@ void BindingData::SlowToggleTimerRef( args[0]->IsTrue()); } -void BindingData::FastToggleTimerRef(Local receiver, bool ref) { +void BindingData::FastToggleTimerRef(Local unused, + Local receiver, + bool ref) { ToggleTimerRefImpl(FromJSObject(receiver), ref); } @@ -75,7 +80,9 @@ void BindingData::SlowToggleImmediateRef( args[0]->IsTrue()); } -void BindingData::FastToggleImmediateRef(Local receiver, bool ref) { +void BindingData::FastToggleImmediateRef(Local unused, + Local receiver, + bool ref) { ToggleImmediateRefImpl(FromJSObject(receiver), ref); } diff --git a/src/timers.h b/src/timers.h index d514a70fc25..fd3cbf66f7c 100644 --- a/src/timers.h +++ b/src/timers.h @@ -26,23 +26,29 @@ class BindingData : public SnapshotableObject { static void SetupTimers(const v8::FunctionCallbackInfo& args); static void SlowGetLibuvNow(const v8::FunctionCallbackInfo& args); - static double FastGetLibuvNow(v8::Local receiver); + static double FastGetLibuvNow(v8::Local unused, + v8::Local receiver); static double GetLibuvNowImpl(BindingData* data); static void SlowScheduleTimer( const v8::FunctionCallbackInfo& args); - static void FastScheduleTimer(v8::Local receiver, + static void FastScheduleTimer(v8::Local unused, + v8::Local receiver, int64_t duration); static void ScheduleTimerImpl(BindingData* data, int64_t duration); static void SlowToggleTimerRef( const v8::FunctionCallbackInfo& args); - static void FastToggleTimerRef(v8::Local receiver, bool ref); + static void FastToggleTimerRef(v8::Local unused, + v8::Local receiver, + bool ref); static void ToggleTimerRefImpl(BindingData* data, bool ref); static void SlowToggleImmediateRef( const v8::FunctionCallbackInfo& args); - static void FastToggleImmediateRef(v8::Local receiver, bool ref); + static void FastToggleImmediateRef(v8::Local unused, + v8::Local receiver, + bool ref); static void ToggleImmediateRefImpl(BindingData* data, bool ref); static void CreatePerIsolateProperties(IsolateData* isolate_data, diff --git a/test/parallel/test-permission-fs-internal-module-stat.js b/test/parallel/test-permission-fs-internal-module-stat.js index b39c276f3c1..0075afb8bb5 100644 --- a/test/parallel/test-permission-fs-internal-module-stat.js +++ b/test/parallel/test-permission-fs-internal-module-stat.js @@ -18,7 +18,7 @@ const internalFsBinding = internalBinding('fs'); // Run this inside a for loop to trigger the fast API for (let i = 0; i < 10_000; i++) { assert.throws(() => { - internalFsBinding.internalModuleStat(blockedFile); + internalFsBinding.internalModuleStat(internalFsBinding, blockedFile); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemRead', diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index c8af56bcc8a..889769de209 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -113,7 +113,7 @@ declare namespace InternalFSBinding { function futimes(fd: number, atime: number, mtime: number): void; function futimes(fd: number, atime: number, mtime: number, usePromises: typeof kUsePromises): Promise; - function internalModuleStat(path: string): number; + function internalModuleStat(receiver: unknown, path: string): number; function lchown(path: string, uid: number, gid: number, req: FSReqCallback): void; function lchown(path: string, uid: number, gid: number, req: undefined, ctx: FSSyncContext): void;