vm: use internal versions of compileFunction and Script

Instead of using the public versions of the vm APIs internally, use the internal versions so that we can skip unnecessary argument validation. The public versions would need special care to the generation of host-defined options to hit the isolate compilation cache when imporModuleDynamically isn't used, while internally it's almost always used, so this allows us to handle the host-defined options separately. PR-URL: #50137 Refs: #35375 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
nodejs · Oct 23, 2023 · 3999362 · 3999362
1 parent a54179f
commit 3999362
Show file tree

Hide file tree

Showing 8 changed files with 268 additions and 186 deletions.
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -52,6 +52,7 @@ const {
   SafeMap,
   SafeWeakMap,
   String,
+  Symbol,
   StringPrototypeCharAt,
   StringPrototypeCharCodeAt,
   StringPrototypeEndsWith,
@@ -84,7 +85,12 @@ const {
   setOwnProperty,
   getLazy,
 } = require('internal/util');
-const { internalCompileFunction } = require('internal/vm');
+const {
+  internalCompileFunction,
+  makeContextifyScript,
+  runScriptInThisContext,
+} = require('internal/vm');
+
 const assert = require('internal/assert');
 const fs = require('fs');
 const path = require('path');
@@ -1240,7 +1246,6 @@ Module.prototype.require = function(id) {
 let resolvedArgv;
 let hasPausedEntry = false;
 /** @type {import('vm').Script} */
-let Script;
 
 /**
  * Wraps the given content in a script and runs it in a new context.
@@ -1250,47 +1255,49 @@ let Script;
  * @param {object} codeCache The SEA code cache
  */
 function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
+  const hostDefinedOptionId = Symbol(`cjs:${filename}`);
+  async function importModuleDynamically(specifier, _, importAttributes) {
+    const cascadedLoader = getCascadedLoader();
+    return cascadedLoader.import(specifier, normalizeReferrerURL(filename),
+                                 importAttributes);
+  }
   if (patched) {
-    const wrapper = Module.wrap(content);
-    if (Script === undefined) {
-      ({ Script } = require('vm'));
-    }
-    const script = new Script(wrapper, {
-      filename,
-      lineOffset: 0,
-      importModuleDynamically: async (specifier, _, importAttributes) => {
-        const cascadedLoader = getCascadedLoader();
-        return cascadedLoader.import(specifier, normalizeReferrerURL(filename),
-                                     importAttributes);
-      },
-    });
+    const wrapped = Module.wrap(content);
+    const script = makeContextifyScript(
+      wrapped,                 // code
+      filename,                // filename
+      0,                       // lineOffset
+      0,                       // columnOffset
+      undefined,               // cachedData
+      false,                   // produceCachedData
+      undefined,               // parsingContext
+      hostDefinedOptionId,     // hostDefinedOptionId
+      importModuleDynamically, // importModuleDynamically
+    );
 
     // Cache the source map for the module if present.
     if (script.sourceMapURL) {
       maybeCacheSourceMap(filename, content, this, false, undefined, script.sourceMapURL);
     }
 
-    return script.runInThisContext({
-      displayErrors: true,
-    });
+    return runScriptInThisContext(script, true, false);
   }
 
+  const params = [ 'exports', 'require', 'module', '__filename', '__dirname' ];
   try {
-    const result = internalCompileFunction(content, [
-      'exports',
-      'require',
-      'module',
-      '__filename',
-      '__dirname',
-    ], {
-      filename,
-      cachedData: codeCache,
-      importModuleDynamically(specifier, _, importAttributes) {
-        const cascadedLoader = getCascadedLoader();
-        return cascadedLoader.import(specifier, normalizeReferrerURL(filename),
-                                     importAttributes);
-      },
-    });
+    const result = internalCompileFunction(
+      content,                           // code,
+      filename,                          // filename
+      0,                                 // lineOffset
+      0,                                 // columnOffset,
+      codeCache,                         // cachedData
+      false,                             // produceCachedData
+      undefined,                         // parsingContext
+      undefined,                         // contextExtensions
+      params,                            // params
+      hostDefinedOptionId,               // hostDefinedOptionId
+      importModuleDynamically,           // importModuleDynamically
+    );
 
     // The code cache is used for SEAs only.
     if (codeCache &&

diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
@@ -15,6 +15,7 @@ const {
   StringPrototypeReplaceAll,
   StringPrototypeSlice,
   StringPrototypeStartsWith,
+  Symbol,
   SyntaxErrorPrototype,
   globalThis: { WebAssembly },
 } = primordials;
@@ -192,19 +193,29 @@ function enrichCJSError(err, content, filename) {
  */
 function loadCJSModule(module, source, url, filename) {
   let compiledWrapper;
+  async function importModuleDynamically(specifier, _, importAttributes) {
+    return asyncESM.esmLoader.import(specifier, url, importAttributes);
+  }
   try {
-    compiledWrapper = internalCompileFunction(source, [
-      'exports',
-      'require',
-      'module',
-      '__filename',
-      '__dirname',
-    ], {
-      filename,
-      importModuleDynamically(specifier, _, importAttributes) {
-        return asyncESM.esmLoader.import(specifier, url, importAttributes);
-      },
-    }).function;
+    compiledWrapper = internalCompileFunction(
+      source,                         // code,
+      filename,                       // filename
+      0,                              // lineOffset
+      0,                              // columnOffset,
+      undefined,                      // cachedData
+      false,                          // produceCachedData
+      undefined,                      // parsingContext
+      undefined,                      // contextExtensions
+      [                               // params
+        'exports',
+        'require',
+        'module',
+        '__filename',
+        '__dirname',
+      ],
+      Symbol(`cjs:${filename}`),      // hostDefinedOptionsId
+      importModuleDynamically,        // importModuleDynamically
+    ).function;
   } catch (err) {
     enrichCJSError(err, source, url);
     throw err;

diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const {
+  Symbol,
   RegExpPrototypeExec,
   globalThis,
 } = primordials;
@@ -25,7 +26,9 @@ const {
   emitAfter,
   popAsyncContext,
 } = require('internal/async_hooks');
-
+const {
+  makeContextifyScript, runScriptInThisContext,
+} = require('internal/vm');
 // shouldAbortOnUncaughtToggle is a typed array for faster
 // communication with JS.
 const { shouldAbortOnUncaughtToggle } = internalBinding('util');
@@ -53,7 +56,6 @@ function evalModule(source, print) {
 
 function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) {
   const CJSModule = require('internal/modules/cjs/loader').Module;
-  const { kVmBreakFirstLineSymbol } = require('internal/util');
   const { pathToFileURL } = require('internal/url');
 
   const cwd = tryGetCwd();
@@ -79,16 +81,25 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) {
     `;
     globalThis.__filename = name;
     RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs.
-    const result = module._compile(script, `${name}-wrapper`)(() =>
-      require('vm').runInThisContext(body, {
-        filename: name,
-        displayErrors: true,
-        [kVmBreakFirstLineSymbol]: !!breakFirstLine,
-        importModuleDynamically(specifier, _, importAttributes) {
-          const loader = asyncESM.esmLoader;
-          return loader.import(specifier, baseUrl, importAttributes);
-        },
-      }));
+    const result = module._compile(script, `${name}-wrapper`)(() => {
+      const hostDefinedOptionId = Symbol(name);
+      async function importModuleDynamically(specifier, _, importAttributes) {
+        const loader = asyncESM.esmLoader;
+        return loader.import(specifier, baseUrl, importAttributes);
+      }
+      const script = makeContextifyScript(
+        body,                    // code
+        name,                    // filename,
+        0,                       // lineOffset
+        0,                       // columnOffset,
+        undefined,               // cachedData
+        false,                   // produceCachedData
+        undefined,               // parsingContext
+        hostDefinedOptionId,     // hostDefinedOptionId
+        importModuleDynamically, // importModuleDynamically
+      );
+      return runScriptInThisContext(script, true, !!breakFirstLine);
+    });
     if (print) {
       const { log } = require('internal/console/global');
       log(result);

diff --git a/lib/internal/vm.js b/lib/internal/vm.js
@@ -1,32 +1,26 @@
 'use strict';
 
 const {
-  ArrayPrototypeForEach,
+  ReflectApply,
   Symbol,
 } = primordials;
 
 const {
+  ContextifyScript,
   compileFunction,
   isContext: _isContext,
 } = internalBinding('contextify');
+const {
+  runInContext,
+} = ContextifyScript.prototype;
 const {
   default_host_defined_options,
 } = internalBinding('symbols');
 const {
-  validateArray,
-  validateBoolean,
-  validateBuffer,
   validateFunction,
   validateObject,
-  validateString,
-  validateStringArray,
   kValidateObjectAllowArray,
-  kValidateObjectAllowNullable,
-  validateInt32,
 } = require('internal/validators');
-const {
-  ERR_INVALID_ARG_TYPE,
-} = require('internal/errors').codes;
 
 function isContext(object) {
   validateObject(object, 'object', kValidateObjectAllowArray);
@@ -50,49 +44,20 @@ function getHostDefinedOptionId(importModuleDynamically, filename) {
   return Symbol(filename);
 }
 
-function internalCompileFunction(code, params, options) {
-  validateString(code, 'code');
-  if (params !== undefined) {
-    validateStringArray(params, 'params');
-  }
-  const {
-    filename = '',
-    columnOffset = 0,
-    lineOffset = 0,
-    cachedData = undefined,
-    produceCachedData = false,
-    parsingContext = undefined,
-    contextExtensions = [],
-    importModuleDynamically,
-  } = options;
-
-  validateString(filename, 'options.filename');
-  validateInt32(columnOffset, 'options.columnOffset');
-  validateInt32(lineOffset, 'options.lineOffset');
-  if (cachedData !== undefined)
-    validateBuffer(cachedData, 'options.cachedData');
-  validateBoolean(produceCachedData, 'options.produceCachedData');
-  if (parsingContext !== undefined) {
-    if (
-      typeof parsingContext !== 'object' ||
-      parsingContext === null ||
-      !isContext(parsingContext)
-    ) {
-      throw new ERR_INVALID_ARG_TYPE(
-        'options.parsingContext',
-        'Context',
-        parsingContext,
-      );
-    }
-  }
-  validateArray(contextExtensions, 'options.contextExtensions');
-  ArrayPrototypeForEach(contextExtensions, (extension, i) => {
-    const name = `options.contextExtensions[${i}]`;
-    validateObject(extension, name, kValidateObjectAllowNullable);
+function registerImportModuleDynamically(referrer, importModuleDynamically) {
+  const { importModuleDynamicallyWrap } = require('internal/vm/module');
+  const { registerModule } = require('internal/modules/esm/utils');
+  registerModule(referrer, {
+    __proto__: null,
+    importModuleDynamically:
+      importModuleDynamicallyWrap(importModuleDynamically),
   });
+}
 
-  const hostDefinedOptionId =
-      getHostDefinedOptionId(importModuleDynamically, filename);
+function internalCompileFunction(
+  code, filename, lineOffset, columnOffset,
+  cachedData, produceCachedData, parsingContext, contextExtensions,
+  params, hostDefinedOptionId, importModuleDynamically) {
   const result = compileFunction(
     code,
     filename,
@@ -119,23 +84,65 @@ function internalCompileFunction(code, params, options) {
   }
 
   if (importModuleDynamically !== undefined) {
-    validateFunction(importModuleDynamically,
-                     'options.importModuleDynamically');
-    const { importModuleDynamicallyWrap } = require('internal/vm/module');
-    const wrapped = importModuleDynamicallyWrap(importModuleDynamically);
-    const func = result.function;
-    const { registerModule } = require('internal/modules/esm/utils');
-    registerModule(func, {
-      __proto__: null,
-      importModuleDynamically: wrapped,
-    });
+    registerImportModuleDynamically(result.function, importModuleDynamically);
   }
 
   return result;
 }
 
+function makeContextifyScript(code,
+                              filename,
+                              lineOffset,
+                              columnOffset,
+                              cachedData,
+                              produceCachedData,
+                              parsingContext,
+                              hostDefinedOptionId,
+                              importModuleDynamically) {
+  let script;
+  // Calling `ReThrow()` on a native TryCatch does not generate a new
+  // abort-on-uncaught-exception check. A dummy try/catch in JS land
+  // protects against that.
+  try { // eslint-disable-line no-useless-catch
+    script = new ContextifyScript(code,
+                                  filename,
+                                  lineOffset,
+                                  columnOffset,
+                                  cachedData,
+                                  produceCachedData,
+                                  parsingContext,
+                                  hostDefinedOptionId);
+  } catch (e) {
+    throw e; /* node-do-not-add-exception-line */
+  }
+
+  if (importModuleDynamically !== undefined) {
+    registerImportModuleDynamically(script, importModuleDynamically);
+  }
+  return script;
+}
+
+// Internal version of vm.Script.prototype.runInThisContext() which skips
+// argument validation.
+function runScriptInThisContext(script, displayErrors, breakOnFirstLine) {
+  return ReflectApply(
+    runInContext,
+    script,
+    [
+      null,                // sandbox - use current context
+      -1,                  // timeout
+      displayErrors,       // displayErrors
+      false,               // breakOnSigint
+      breakOnFirstLine,    // breakOnFirstLine
+    ],
+  );
+}
+
 module.exports = {
   getHostDefinedOptionId,
   internalCompileFunction,
   isContext,
+  makeContextifyScript,
+  registerImportModuleDynamically,
+  runScriptInThisContext,
 };