diff --git a/benchmark/es/eval.js b/benchmark/es/eval.js new file mode 100644 index 00000000000..b4c6ee2e0a6 --- /dev/null +++ b/benchmark/es/eval.js @@ -0,0 +1,46 @@ +'use strict'; + +const common = require('../common.js'); + +const bench = common.createBenchmark(main, { + method: ['without-sourcemap', 'sourcemap'], + n: [1e6], +}); + +const sourceWithoutSourceMap = ` +'use strict'; +(function() { + let a = 1; + for (let i = 0; i < 1000; i++) { + a++; + } + return a; +})(); +`; +const sourceWithSourceMap = ` +${sourceWithoutSourceMap} +//# sourceMappingURL=https://ci.nodejs.org/405 +`; + +function evalN(n, source) { + bench.start(); + for (let i = 0; i < n; i++) { + eval(source); + } + bench.end(n); +} + +function main({ n, method }) { + switch (method) { + case 'without-sourcemap': + process.setSourceMapsEnabled(false); + evalN(n, sourceWithoutSourceMap); + break; + case 'sourcemap': + process.setSourceMapsEnabled(true); + evalN(n, sourceWithSourceMap); + break; + default: + throw new Error(`Unexpected method "${method}"`); + } +} diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index 551d3d50fee..8420bd230fa 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -61,10 +61,15 @@ const prepareStackTrace = (globalThis, error, trace) => { try { // A stack trace will often have several call sites in a row within the // same file, cache the source map and file content accordingly: - const fileName = t.getFileName(); + let fileName = t.getFileName(); + let generated = false; + if (fileName === undefined) { + fileName = t.getEvalOrigin(); + generated = true; + } const sm = fileName === lastFileName ? lastSourceMap : - findSourceMap(fileName); + findSourceMap(fileName, generated); lastSourceMap = sm; lastFileName = fileName; if (sm) { diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index e3721d024d9..bc212b27ff8 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -7,7 +7,7 @@ const { ObjectKeys, ObjectGetOwnPropertyDescriptor, ObjectPrototypeHasOwnProperty, - RegExpPrototypeTest, + RegExpPrototypeExec, SafeMap, StringPrototypeMatch, StringPrototypeSplit, @@ -30,12 +30,18 @@ const { normalizeReferrerURL, } = require('internal/modules/cjs/helpers'); const { validateBoolean } = require('internal/validators'); +const { setMaybeCacheGeneratedSourceMap } = internalBinding('errors'); + // Since the CJS module cache is mutable, which leads to memory leaks when // modules are deleted, we use a WeakMap so that the source map cache will // be purged automatically: const cjsSourceMapCache = new IterableWeakMap(); // The esm cache is not mutable, so we can use a Map without memory concerns: const esmSourceMapCache = new SafeMap(); +// The generated sources is not mutable, so we can use a Map without memory concerns: +const generatedSourceMapCache = new SafeMap(); +const kLeadingProtocol = /^\w+:\/\//; + const { fileURLToPath, pathToFileURL, URL } = require('internal/url'); let SourceMap; @@ -71,14 +77,13 @@ function setSourceMapsEnabled(val) { sourceMapsEnabled = val; } -function maybeCacheSourceMap(filename, content, cjsModuleInstance) { +function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource) { const sourceMapsEnabled = getSourceMapsEnabled(); if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return; try { filename = normalizeReferrerURL(filename); } catch (err) { - // This is most likely an [eval]-wrapper, which is currently not - // supported. + // This is most likely an invalid filename in sourceURL of [eval]-wrapper. debug(err); return; } @@ -96,8 +101,14 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) { data, url }); + } else if (isGeneratedSource) { + generatedSourceMapCache.set(filename, { + lineLengths: lineLengths(content), + data, + url + }); } else { - // If there is no cjsModuleInstance assume we are in a + // If there is no cjsModuleInstance and is not generated source assume we are in a // "modules/esm" context. esmSourceMapCache.set(filename, { lineLengths: lineLengths(content), @@ -108,6 +119,31 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) { } } +function maybeCacheGeneratedSourceMap(content) { + const sourceMapsEnabled = getSourceMapsEnabled(); + if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return; + + const matchSourceURL = RegExpPrototypeExec( + /\/[*/]#\s+sourceURL=(?[^\s]+)/, + content + ); + if (matchSourceURL == null) { + return; + } + let sourceURL = matchSourceURL.groups.sourceURL; + if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) { + sourceURL = pathToFileURL(sourceURL).href; + } + try { + maybeCacheSourceMap(sourceURL, content, null, true); + } catch (err) { + // This can happen if the filename is not a valid URL. + // If we fail to cache the source map, we should not fail the whole process. + debug(err); + } +} +setMaybeCacheGeneratedSourceMap(maybeCacheGeneratedSourceMap); + function dataFromUrl(sourceURL, sourceMappingURL) { try { const url = new URL(sourceMappingURL); @@ -218,21 +254,26 @@ function appendCJSCache(obj) { } } -function findSourceMap(sourceURL) { - if (!RegExpPrototypeTest(/^\w+:\/\//, sourceURL)) { +function findSourceMap(sourceURL, isGenerated) { + if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) { sourceURL = pathToFileURL(sourceURL).href; } if (!SourceMap) { SourceMap = require('internal/source_map/source_map').SourceMap; } - let sourceMap = esmSourceMapCache.get(sourceURL); - if (sourceMap === undefined) { - for (const value of cjsSourceMapCache) { - const filename = ObjectGetValueSafe(value, 'filename'); - if (sourceURL === filename) { - sourceMap = { - data: ObjectGetValueSafe(value, 'data') - }; + let sourceMap; + if (isGenerated) { + sourceMap = generatedSourceMapCache.get(sourceURL); + } else { + sourceMap = esmSourceMapCache.get(sourceURL); + if (sourceMap === undefined) { + for (const value of cjsSourceMapCache) { + const filename = ObjectGetValueSafe(value, 'filename'); + if (sourceURL === filename) { + sourceMap = { + data: ObjectGetValueSafe(value, 'data') + }; + } } } } diff --git a/src/api/environment.cc b/src/api/environment.cc index 6adf7122426..5df88a9dbab 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -251,6 +251,8 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) { auto* allow_wasm_codegen_cb = s.allow_wasm_code_generation_callback ? s.allow_wasm_code_generation_callback : AllowWasmCodeGenerationCallback; isolate->SetAllowWasmCodeGenerationCallback(allow_wasm_codegen_cb); + isolate->SetModifyCodeGenerationFromStringsCallback( + ModifyCodeGenerationFromStrings); if ((s.flags & SHOULD_NOT_SET_PROMISE_REJECTION_CALLBACK) == 0) { auto* promise_reject_cb = s.promise_reject_callback ? @@ -645,6 +647,9 @@ bool InitializeContextForSnapshot(Local context) { Isolate* isolate = context->GetIsolate(); HandleScope handle_scope(isolate); + context->AllowCodeGenerationFromStrings(false); + context->SetEmbedderData( + ContextEmbedderIndex::kAllowCodeGenerationFromStrings, True(isolate)); context->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration, True(isolate)); return InitializePrimordials(context); diff --git a/src/env.h b/src/env.h index 9b414d2fb0d..8e518dd8316 100644 --- a/src/env.h +++ b/src/env.h @@ -546,6 +546,7 @@ class NoArrayBufferZeroFillScope { V(inspector_console_extension_installer, v8::Function) \ V(inspector_disable_async_hooks, v8::Function) \ V(inspector_enable_async_hooks, v8::Function) \ + V(maybe_cache_generated_source_map, v8::Function) \ V(messaging_deserialize_create_object, v8::Function) \ V(message_port, v8::Object) \ V(native_module_require, v8::Function) \ diff --git a/src/node_context_data.h b/src/node_context_data.h index 912e65b4270..42aae872e60 100644 --- a/src/node_context_data.h +++ b/src/node_context_data.h @@ -29,12 +29,18 @@ namespace node { #define NODE_BINDING_LIST_INDEX 36 #endif +#ifndef NODE_CONTEXT_ALLOW_CODE_GENERATION_FROM_STRINGS_INDEX +#define NODE_CONTEXT_ALLOW_CODE_GENERATION_FROM_STRINGS_INDEX 37 +#endif + enum ContextEmbedderIndex { kEnvironment = NODE_CONTEXT_EMBEDDER_DATA_INDEX, kSandboxObject = NODE_CONTEXT_SANDBOX_OBJECT_INDEX, kAllowWasmCodeGeneration = NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX, kContextTag = NODE_CONTEXT_TAG, - kBindingListIndex = NODE_BINDING_LIST_INDEX + kBindingListIndex = NODE_BINDING_LIST_INDEX, + kAllowCodeGenerationFromStrings = + NODE_CONTEXT_ALLOW_CODE_GENERATION_FROM_STRINGS_INDEX }; } // namespace node diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 9005da50893..ca374d71695 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -233,7 +233,11 @@ MaybeLocal ContextifyContext::CreateV8Context( ctx->Global()); Utf8Value name_val(env->isolate(), options.name); - ctx->AllowCodeGenerationFromStrings(options.allow_code_gen_strings->IsTrue()); + // Delegate the code generation validation to + // node::ModifyCodeGenerationFromStrings. + ctx->AllowCodeGenerationFromStrings(false); + ctx->SetEmbedderData(ContextEmbedderIndex::kAllowCodeGenerationFromStrings, + options.allow_code_gen_strings); ctx->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration, options.allow_code_gen_wasm); diff --git a/src/node_errors.cc b/src/node_errors.cc index 3028892b080..8c29dd85486 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -450,6 +450,38 @@ void OnFatalError(const char* location, const char* message) { ABORT(); } +v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings( + v8::Local context, + v8::Local source, + bool is_code_like) { + HandleScope scope(context->GetIsolate()); + + Environment* env = Environment::GetCurrent(context); + if (env->source_maps_enabled()) { + // We do not expect the maybe_cache_generated_source_map to throw any more + // exceptions. If it does, just ignore it. + errors::TryCatchScope try_catch(env); + Local maybe_cache_source_map = + env->maybe_cache_generated_source_map(); + Local argv[1] = {source}; + + MaybeLocal maybe_cached = maybe_cache_source_map->Call( + context, context->Global(), arraysize(argv), argv); + if (maybe_cached.IsEmpty()) { + DCHECK(try_catch.HasCaught()); + } + } + + Local allow_code_gen = context->GetEmbedderData( + ContextEmbedderIndex::kAllowCodeGenerationFromStrings); + bool codegen_allowed = + allow_code_gen->IsUndefined() || allow_code_gen->IsTrue(); + return { + codegen_allowed, + {}, + }; +} + namespace errors { TryCatchScope::~TryCatchScope() { @@ -832,6 +864,13 @@ static void SetSourceMapsEnabled(const FunctionCallbackInfo& args) { env->set_source_maps_enabled(args[0].As()->Value()); } +static void SetMaybeCacheGeneratedSourceMap( + const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + CHECK(args[0]->IsFunction()); + env->set_maybe_cache_generated_source_map(args[0].As()); +} + static void SetEnhanceStackForFatalException( const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -866,6 +905,7 @@ static void TriggerUncaughtException(const FunctionCallbackInfo& args) { void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(SetPrepareStackTraceCallback); registry->Register(SetSourceMapsEnabled); + registry->Register(SetMaybeCacheGeneratedSourceMap); registry->Register(SetEnhanceStackForFatalException); registry->Register(NoSideEffectsToString); registry->Register(TriggerUncaughtException); @@ -879,6 +919,9 @@ void Initialize(Local target, env->SetMethod( target, "setPrepareStackTraceCallback", SetPrepareStackTraceCallback); env->SetMethod(target, "setSourceMapsEnabled", SetSourceMapsEnabled); + env->SetMethod(target, + "setMaybeCacheGeneratedSourceMap", + SetMaybeCacheGeneratedSourceMap); env->SetMethod(target, "setEnhanceStackForFatalException", SetEnhanceStackForFatalException); diff --git a/src/node_errors.h b/src/node_errors.h index f540b3e2a37..f6a60a1daa4 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -270,6 +270,11 @@ void DecorateErrorStack(Environment* env, const errors::TryCatchScope& try_catch); } // namespace errors +v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings( + v8::Local context, + v8::Local source, + bool is_code_like); + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/test/message/source_map_eval.js b/test/message/source_map_eval.js new file mode 100644 index 00000000000..534d16bd34c --- /dev/null +++ b/test/message/source_map_eval.js @@ -0,0 +1,8 @@ +// Flags: --enable-source-maps + +'use strict'; +require('../common'); +const fs = require('fs'); + +const content = fs.readFileSync(require.resolve('../fixtures/source-map/tabs.js'), 'utf8'); +eval(content); diff --git a/test/message/source_map_eval.out b/test/message/source_map_eval.out new file mode 100644 index 00000000000..7cfd7c84fe6 --- /dev/null +++ b/test/message/source_map_eval.out @@ -0,0 +1,10 @@ +ReferenceError: alert is not defined + at Object.eval (*tabs.coffee:26:2) + at eval (*tabs.coffee:1:14) + at Object. (*source_map_eval.js:8:1) + at Module._compile (node:internal/modules/cjs/loader:*) + at Object.Module._extensions..js (node:internal/modules/cjs/loader:*) + at Module.load (node:internal/modules/cjs/loader:*) + at Function.Module._load (node:internal/modules/cjs/loader:*) + at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:*) + at node:internal/main/run_main_module:*