src: add receiver to fast api callback methods

When creating an fast api the callback might use the receiver. In that case if the internal binding is destructured the method won't have access to the reciver and it will throw. Passing the receiver as second argument ensures the receiver is available. PR-URL: #54408 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
nodejs · Oct 4, 2024 · 0bb5584 · 0bb5584
1 parent 7186ede
commit 0bb5584
Show file tree

Hide file tree

Showing 18 changed files with 123 additions and 53 deletions.
diff --git 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
@@ -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
@@ -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<Object> unused,
+      Local<Object> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -169,7 +169,8 @@ void HistogramBase::RecordDelta(const FunctionCallbackInfo<Value>& args) {
   (*histogram)->RecordDelta();
 }
 
-void HistogramBase::FastRecordDelta(Local<Value> receiver) {
+void HistogramBase::FastRecordDelta(Local<Value> unused,
+                                    Local<Value> receiver) {
   HistogramBase* histogram;
   ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
   (*histogram)->RecordDelta();
@@ -189,7 +190,8 @@ void HistogramBase::Record(const FunctionCallbackInfo<Value>& args) {
   (*histogram)->Record(value);
 }
 
-void HistogramBase::FastRecord(Local<Value> receiver,
+void HistogramBase::FastRecord(Local<Value> unused,
+                               Local<Value> receiver,
                                const int64_t value,
                                FastApiCallbackOptions& options) {
   if (value < 1) {
@@ -436,7 +438,9 @@ void IntervalHistogram::Start(const FunctionCallbackInfo<Value>& args) {
   histogram->OnStart(args[0]->IsTrue() ? StartFlags::RESET : StartFlags::NONE);
 }
 
-void IntervalHistogram::FastStart(Local<Value> receiver, bool reset) {
+void IntervalHistogram::FastStart(Local<Value> unused,
+                                  Local<Value> 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<Value>& args) {
   histogram->OnStop();
 }
 
-void IntervalHistogram::FastStop(Local<Value> receiver) {
+void IntervalHistogram::FastStop(Local<Value> unused, Local<Value> receiver) {
   IntervalHistogram* histogram;
   ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
   histogram->OnStop();
@@ -564,42 +568,45 @@ void HistogramImpl::DoReset(const FunctionCallbackInfo<Value>& args) {
   (*histogram)->Reset();
 }
 
-void HistogramImpl::FastReset(Local<Value> receiver) {
+void HistogramImpl::FastReset(Local<Value> unused, Local<Value> receiver) {
   HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
   (*histogram)->Reset();
 }
 
-double HistogramImpl::FastGetCount(Local<Value> receiver) {
+double HistogramImpl::FastGetCount(Local<Value> unused, Local<Value> receiver) {
   HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
   return static_cast<double>((*histogram)->Count());
 }
 
-double HistogramImpl::FastGetMin(Local<Value> receiver) {
+double HistogramImpl::FastGetMin(Local<Value> unused, Local<Value> receiver) {
   HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
   return static_cast<double>((*histogram)->Min());
 }
 
-double HistogramImpl::FastGetMax(Local<Value> receiver) {
+double HistogramImpl::FastGetMax(Local<Value> unused, Local<Value> receiver) {
   HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
   return static_cast<double>((*histogram)->Max());
 }
 
-double HistogramImpl::FastGetMean(Local<Value> receiver) {
+double HistogramImpl::FastGetMean(Local<Value> unused, Local<Value> receiver) {
   HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
   return (*histogram)->Mean();
 }
 
-double HistogramImpl::FastGetExceeds(Local<Value> receiver) {
+double HistogramImpl::FastGetExceeds(Local<Value> unused,
+                                     Local<Value> receiver) {
   HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
   return static_cast<double>((*histogram)->Exceeds());
 }
 
-double HistogramImpl::FastGetStddev(Local<Value> receiver) {
+double HistogramImpl::FastGetStddev(Local<Value> unused,
+                                    Local<Value> receiver) {
   HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
   return (*histogram)->Stddev();
 }
 
-double HistogramImpl::FastGetPercentile(Local<Value> receiver,
+double HistogramImpl::FastGetPercentile(Local<Value> unused,
+                                        Local<Value> receiver,
                                         const double percentile) {
   HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
   return static_cast<double>((*histogram)->Percentile(percentile));

diff --git a/src/histogram.h b/src/histogram.h
@@ -101,14 +101,22 @@ class HistogramImpl {
   static void GetPercentilesBigInt(
       const v8::FunctionCallbackInfo<v8::Value>& args);
 
-  static void FastReset(v8::Local<v8::Value> receiver);
-  static double FastGetCount(v8::Local<v8::Value> receiver);
-  static double FastGetMin(v8::Local<v8::Value> receiver);
-  static double FastGetMax(v8::Local<v8::Value> receiver);
-  static double FastGetMean(v8::Local<v8::Value> receiver);
-  static double FastGetExceeds(v8::Local<v8::Value> receiver);
-  static double FastGetStddev(v8::Local<v8::Value> receiver);
-  static double FastGetPercentile(v8::Local<v8::Value> receiver,
+  static void FastReset(v8::Local<v8::Value> unused,
+                        v8::Local<v8::Value> receiver);
+  static double FastGetCount(v8::Local<v8::Value> unused,
+                             v8::Local<v8::Value> receiver);
+  static double FastGetMin(v8::Local<v8::Value> unused,
+                           v8::Local<v8::Value> receiver);
+  static double FastGetMax(v8::Local<v8::Value> unused,
+                           v8::Local<v8::Value> receiver);
+  static double FastGetMean(v8::Local<v8::Value> unused,
+                            v8::Local<v8::Value> receiver);
+  static double FastGetExceeds(v8::Local<v8::Value> unused,
+                               v8::Local<v8::Value> receiver);
+  static double FastGetStddev(v8::Local<v8::Value> unused,
+                              v8::Local<v8::Value> receiver);
+  static double FastGetPercentile(v8::Local<v8::Value> unused,
+                                  v8::Local<v8::Value> 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<v8::Value>& args);
 
   static void FastRecord(
+      v8::Local<v8::Value> unused,
       v8::Local<v8::Value> receiver,
       const int64_t value,
       v8::FastApiCallbackOptions& options);  // NOLINT(runtime/references)
-  static void FastRecordDelta(v8::Local<v8::Value> receiver);
+  static void FastRecordDelta(v8::Local<v8::Value> unused,
+                              v8::Local<v8::Value> receiver);
 
   HistogramBase(
       Environment* env,
@@ -233,8 +243,11 @@ class IntervalHistogram final : public HandleWrap, public HistogramImpl {
   static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Stop(const v8::FunctionCallbackInfo<v8::Value>& args);
 
-  static void FastStart(v8::Local<v8::Value> receiver, bool reset);
-  static void FastStop(v8::Local<v8::Value> receiver);
+  static void FastStart(v8::Local<v8::Value> unused,
+                        v8::Local<v8::Value> receiver,
+                        bool reset);
+  static void FastStop(v8::Local<v8::Value> unused,
+                       v8::Local<v8::Value> receiver);
 
   BaseObject::TransferMode GetTransferMode() const override {
     return TransferMode::kCloneable;

diff --git a/src/node_external_reference.h b/src/node_external_reference.h
@@ -12,19 +12,25 @@ namespace node {
 
 using CFunctionCallbackWithOneByteString =
     uint32_t (*)(v8::Local<v8::Value>, const v8::FastOneByteString&);
-using CFunctionCallback = void (*)(v8::Local<v8::Value> receiver);
+using CFunctionCallback = void (*)(v8::Local<v8::Value> unused,
+                                   v8::Local<v8::Value> receiver);
 using CFunctionCallbackReturnDouble =
-    double (*)(v8::Local<v8::Object> receiver);
+    double (*)(v8::Local<v8::Object> unused, v8::Local<v8::Object> receiver);
 using CFunctionCallbackReturnInt32 =
-    int32_t (*)(v8::Local<v8::Object> receiver,
+    int32_t (*)(v8::Local<v8::Object> unused,
+                v8::Local<v8::Object> receiver,
                 const v8::FastOneByteString& input,
                 // NOLINTNEXTLINE(runtime/references) This is V8 api.
                 v8::FastApiCallbackOptions& options);
 using CFunctionCallbackValueReturnDouble =
     double (*)(v8::Local<v8::Value> receiver);
-using CFunctionCallbackWithInt64 = void (*)(v8::Local<v8::Object> receiver,
+using CFunctionCallbackValueReturnDoubleUnusedReceiver =
+    double (*)(v8::Local<v8::Value> unused, v8::Local<v8::Value> receiver);
+using CFunctionCallbackWithInt64 = void (*)(v8::Local<v8::Object> unused,
+                                            v8::Local<v8::Object> receiver,
                                             int64_t);
-using CFunctionCallbackWithBool = void (*)(v8::Local<v8::Object> receiver,
+using CFunctionCallbackWithBool = void (*)(v8::Local<v8::Object> unused,
+                                           v8::Local<v8::Object> receiver,
                                            bool);
 using CFunctionCallbackWithString =
     bool (*)(v8::Local<v8::Value>, const v8::FastOneByteString& input);
@@ -50,11 +56,15 @@ using CFunctionCallbackWithUint8ArrayUint32Int64Bool =
 using CFunctionWithUint32 = uint32_t (*)(v8::Local<v8::Value>,
                                          const uint32_t input);
 using CFunctionWithDoubleReturnDouble = double (*)(v8::Local<v8::Value>,
+                                                   v8::Local<v8::Value>,
                                                    const double);
 using CFunctionWithInt64Fallback = void (*)(v8::Local<v8::Value>,
+                                            v8::Local<v8::Value>,
                                             const int64_t,
                                             v8::FastApiCallbackOptions&);
-using CFunctionWithBool = void (*)(v8::Local<v8::Value>, bool);
+using CFunctionWithBool = void (*)(v8::Local<v8::Value>,
+                                   v8::Local<v8::Value>,
+                                   bool);
 
 using CFunctionWriteString =
     uint32_t (*)(v8::Local<v8::Value> 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
@@ -1034,8 +1034,9 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& args) {
 static void InternalModuleStat(const FunctionCallbackInfo<Value>& 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<Value>& args) {
 }
 
 static int32_t FastInternalModuleStat(
+    Local<Object> unused,
     Local<Object> recv,
     const FastOneByteString& input,
     // NOLINTNEXTLINE(runtime/references) This is V8 api.

diff --git a/src/node_process.h b/src/node_process.h
@@ -72,15 +72,17 @@ class BindingData : public SnapshotableObject {
   static BindingData* FromV8Value(v8::Local<v8::Value> receiver);
   static void NumberImpl(BindingData* receiver);
 
-  static void FastNumber(v8::Local<v8::Value> receiver) {
+  static void FastNumber(v8::Local<v8::Value> unused,
+                         v8::Local<v8::Value> receiver) {
     NumberImpl(FromV8Value(receiver));
   }
 
   static void SlowNumber(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   static void BigIntImpl(BindingData* receiver);
 
-  static void FastBigInt(v8::Local<v8::Value> receiver) {
+  static void FastBigInt(v8::Local<v8::Value> unused,
+                         v8::Local<v8::Value> receiver) {
     BigIntImpl(FromV8Value(receiver));
   }
 

diff --git a/src/node_wasi.cc b/src/node_wasi.cc
@@ -241,6 +241,7 @@ inline void EinvalError() {}
 
 template <typename FT, FT F, typename R, typename... Args>
 R WASI::WasiFunction<FT, F, R, Args...>::FastCallback(
+    Local<Object> unused,
     Local<Object> receiver,
     Args... args,
     // NOLINTNEXTLINE(runtime/references) This is V8 api.

diff --git a/src/node_wasi.h b/src/node_wasi.h
@@ -160,7 +160,8 @@ class WASI : public BaseObject,
                             v8::Local<v8::FunctionTemplate>);
 
    private:
-    static R FastCallback(v8::Local<v8::Object> receiver,
+    static R FastCallback(v8::Local<v8::Object> unused,
+                          v8::Local<v8::Object> receiver,
                           Args...,
                           v8::FastApiCallbackOptions&);