Skip to content

Commit 61e6d78

Browse files
legendecastargos
authored andcommitted
lib,src: add source map support for global eval
Dynamic sources with FunctionConstructor is not supported yet as V8 prepends lines to the sources which makes the stack line number incorrect. PR-URL: #43428 Refs: #43047 Reviewed-By: Ben Coe <bencoe@gmail.com>
1 parent 38e92fd commit 61e6d78

11 files changed

+194
-18
lines changed

benchmark/es/eval.js

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
3+
const common = require('../common.js');
4+
5+
const bench = common.createBenchmark(main, {
6+
method: ['without-sourcemap', 'sourcemap'],
7+
n: [1e6],
8+
});
9+
10+
const sourceWithoutSourceMap = `
11+
'use strict';
12+
(function() {
13+
let a = 1;
14+
for (let i = 0; i < 1000; i++) {
15+
a++;
16+
}
17+
return a;
18+
})();
19+
`;
20+
const sourceWithSourceMap = `
21+
${sourceWithoutSourceMap}
22+
//# sourceMappingURL=https://ci.nodejs.org/405
23+
`;
24+
25+
function evalN(n, source) {
26+
bench.start();
27+
for (let i = 0; i < n; i++) {
28+
eval(source);
29+
}
30+
bench.end(n);
31+
}
32+
33+
function main({ n, method }) {
34+
switch (method) {
35+
case 'without-sourcemap':
36+
process.setSourceMapsEnabled(false);
37+
evalN(n, sourceWithoutSourceMap);
38+
break;
39+
case 'sourcemap':
40+
process.setSourceMapsEnabled(true);
41+
evalN(n, sourceWithSourceMap);
42+
break;
43+
default:
44+
throw new Error(`Unexpected method "${method}"`);
45+
}
46+
}

lib/internal/source_map/prepare_stack_trace.js

+7-2
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,15 @@ const prepareStackTrace = (globalThis, error, trace) => {
6161
try {
6262
// A stack trace will often have several call sites in a row within the
6363
// same file, cache the source map and file content accordingly:
64-
const fileName = t.getFileName();
64+
let fileName = t.getFileName();
65+
let generated = false;
66+
if (fileName === undefined) {
67+
fileName = t.getEvalOrigin();
68+
generated = true;
69+
}
6570
const sm = fileName === lastFileName ?
6671
lastSourceMap :
67-
findSourceMap(fileName);
72+
findSourceMap(fileName, generated);
6873
lastSourceMap = sm;
6974
lastFileName = fileName;
7075
if (sm) {

lib/internal/source_map/source_map_cache.js

+55-14
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,18 @@ const {
3030
normalizeReferrerURL,
3131
} = require('internal/modules/cjs/helpers');
3232
const { validateBoolean } = require('internal/validators');
33+
const { setMaybeCacheGeneratedSourceMap } = internalBinding('errors');
34+
3335
// Since the CJS module cache is mutable, which leads to memory leaks when
3436
// modules are deleted, we use a WeakMap so that the source map cache will
3537
// be purged automatically:
3638
const cjsSourceMapCache = new IterableWeakMap();
3739
// The esm cache is not mutable, so we can use a Map without memory concerns:
3840
const esmSourceMapCache = new SafeMap();
41+
// The generated sources is not mutable, so we can use a Map without memory concerns:
42+
const generatedSourceMapCache = new SafeMap();
43+
const kLeadingProtocol = /^\w+:\/\//;
44+
3945
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
4046
let SourceMap;
4147

@@ -71,14 +77,13 @@ function setSourceMapsEnabled(val) {
7177
sourceMapsEnabled = val;
7278
}
7379

74-
function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
80+
function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource) {
7581
const sourceMapsEnabled = getSourceMapsEnabled();
7682
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
7783
try {
7884
filename = normalizeReferrerURL(filename);
7985
} catch (err) {
80-
// This is most likely an [eval]-wrapper, which is currently not
81-
// supported.
86+
// This is most likely an invalid filename in sourceURL of [eval]-wrapper.
8287
debug(err);
8388
return;
8489
}
@@ -96,8 +101,14 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
96101
data,
97102
url
98103
});
104+
} else if (isGeneratedSource) {
105+
generatedSourceMapCache.set(filename, {
106+
lineLengths: lineLengths(content),
107+
data,
108+
url
109+
});
99110
} else {
100-
// If there is no cjsModuleInstance assume we are in a
111+
// If there is no cjsModuleInstance and is not generated source assume we are in a
101112
// "modules/esm" context.
102113
esmSourceMapCache.set(filename, {
103114
lineLengths: lineLengths(content),
@@ -108,6 +119,31 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
108119
}
109120
}
110121

122+
function maybeCacheGeneratedSourceMap(content) {
123+
const sourceMapsEnabled = getSourceMapsEnabled();
124+
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
125+
126+
const matchSourceURL = RegExpPrototypeExec(
127+
/\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/,
128+
content
129+
);
130+
if (matchSourceURL == null) {
131+
return;
132+
}
133+
let sourceURL = matchSourceURL.groups.sourceURL;
134+
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
135+
sourceURL = pathToFileURL(sourceURL).href;
136+
}
137+
try {
138+
maybeCacheSourceMap(sourceURL, content, null, true);
139+
} catch (err) {
140+
// This can happen if the filename is not a valid URL.
141+
// If we fail to cache the source map, we should not fail the whole process.
142+
debug(err);
143+
}
144+
}
145+
setMaybeCacheGeneratedSourceMap(maybeCacheGeneratedSourceMap);
146+
111147
function dataFromUrl(sourceURL, sourceMappingURL) {
112148
try {
113149
const url = new URL(sourceMappingURL);
@@ -218,21 +254,26 @@ function appendCJSCache(obj) {
218254
}
219255
}
220256

221-
function findSourceMap(sourceURL) {
222-
if (RegExpPrototypeExec(/^\w+:\/\//, sourceURL) === null) {
257+
function findSourceMap(sourceURL, isGenerated) {
258+
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
223259
sourceURL = pathToFileURL(sourceURL).href;
224260
}
225261
if (!SourceMap) {
226262
SourceMap = require('internal/source_map/source_map').SourceMap;
227263
}
228-
let sourceMap = esmSourceMapCache.get(sourceURL);
229-
if (sourceMap === undefined) {
230-
for (const value of cjsSourceMapCache) {
231-
const filename = ObjectGetValueSafe(value, 'filename');
232-
if (sourceURL === filename) {
233-
sourceMap = {
234-
data: ObjectGetValueSafe(value, 'data')
235-
};
264+
let sourceMap;
265+
if (isGenerated) {
266+
sourceMap = generatedSourceMapCache.get(sourceURL);
267+
} else {
268+
sourceMap = esmSourceMapCache.get(sourceURL);
269+
if (sourceMap === undefined) {
270+
for (const value of cjsSourceMapCache) {
271+
const filename = ObjectGetValueSafe(value, 'filename');
272+
if (sourceURL === filename) {
273+
sourceMap = {
274+
data: ObjectGetValueSafe(value, 'data')
275+
};
276+
}
236277
}
237278
}
238279
}

src/api/environment.cc

+5
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,8 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
253253
auto* allow_wasm_codegen_cb = s.allow_wasm_code_generation_callback ?
254254
s.allow_wasm_code_generation_callback : AllowWasmCodeGenerationCallback;
255255
isolate->SetAllowWasmCodeGenerationCallback(allow_wasm_codegen_cb);
256+
isolate->SetModifyCodeGenerationFromStringsCallback(
257+
ModifyCodeGenerationFromStrings);
256258

257259
Mutex::ScopedLock lock(node::per_process::cli_options_mutex);
258260
if (per_process::cli_options->get_per_isolate_options()
@@ -658,6 +660,9 @@ Maybe<bool> InitializeContextForSnapshot(Local<Context> context) {
658660
Isolate* isolate = context->GetIsolate();
659661
HandleScope handle_scope(isolate);
660662

663+
context->AllowCodeGenerationFromStrings(false);
664+
context->SetEmbedderData(
665+
ContextEmbedderIndex::kAllowCodeGenerationFromStrings, True(isolate));
661666
context->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration,
662667
True(isolate));
663668

src/env.h

+1
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,7 @@ class NoArrayBufferZeroFillScope {
543543
V(inspector_console_extension_installer, v8::Function) \
544544
V(inspector_disable_async_hooks, v8::Function) \
545545
V(inspector_enable_async_hooks, v8::Function) \
546+
V(maybe_cache_generated_source_map, v8::Function) \
546547
V(messaging_deserialize_create_object, v8::Function) \
547548
V(message_port, v8::Object) \
548549
V(native_module_require, v8::Function) \

src/node_context_data.h

+7-1
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,18 @@ namespace node {
2929
#define NODE_BINDING_LIST_INDEX 36
3030
#endif
3131

32+
#ifndef NODE_CONTEXT_ALLOW_CODE_GENERATION_FROM_STRINGS_INDEX
33+
#define NODE_CONTEXT_ALLOW_CODE_GENERATION_FROM_STRINGS_INDEX 37
34+
#endif
35+
3236
enum ContextEmbedderIndex {
3337
kEnvironment = NODE_CONTEXT_EMBEDDER_DATA_INDEX,
3438
kSandboxObject = NODE_CONTEXT_SANDBOX_OBJECT_INDEX,
3539
kAllowWasmCodeGeneration = NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX,
3640
kContextTag = NODE_CONTEXT_TAG,
37-
kBindingListIndex = NODE_BINDING_LIST_INDEX
41+
kBindingListIndex = NODE_BINDING_LIST_INDEX,
42+
kAllowCodeGenerationFromStrings =
43+
NODE_CONTEXT_ALLOW_CODE_GENERATION_FROM_STRINGS_INDEX
3844
};
3945

4046
} // namespace node

src/node_contextify.cc

+5-1
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,11 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
233233
ctx->Global());
234234

235235
Utf8Value name_val(env->isolate(), options.name);
236-
ctx->AllowCodeGenerationFromStrings(options.allow_code_gen_strings->IsTrue());
236+
// Delegate the code generation validation to
237+
// node::ModifyCodeGenerationFromStrings.
238+
ctx->AllowCodeGenerationFromStrings(false);
239+
ctx->SetEmbedderData(ContextEmbedderIndex::kAllowCodeGenerationFromStrings,
240+
options.allow_code_gen_strings);
237241
ctx->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration,
238242
options.allow_code_gen_wasm);
239243

src/node_errors.cc

+43
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,38 @@ void OnFatalError(const char* location, const char* message) {
454454
ABORT();
455455
}
456456

457+
v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings(
458+
v8::Local<v8::Context> context,
459+
v8::Local<v8::Value> source,
460+
bool is_code_like) {
461+
HandleScope scope(context->GetIsolate());
462+
463+
Environment* env = Environment::GetCurrent(context);
464+
if (env->source_maps_enabled()) {
465+
// We do not expect the maybe_cache_generated_source_map to throw any more
466+
// exceptions. If it does, just ignore it.
467+
errors::TryCatchScope try_catch(env);
468+
Local<Function> maybe_cache_source_map =
469+
env->maybe_cache_generated_source_map();
470+
Local<Value> argv[1] = {source};
471+
472+
MaybeLocal<Value> maybe_cached = maybe_cache_source_map->Call(
473+
context, context->Global(), arraysize(argv), argv);
474+
if (maybe_cached.IsEmpty()) {
475+
DCHECK(try_catch.HasCaught());
476+
}
477+
}
478+
479+
Local<Value> allow_code_gen = context->GetEmbedderData(
480+
ContextEmbedderIndex::kAllowCodeGenerationFromStrings);
481+
bool codegen_allowed =
482+
allow_code_gen->IsUndefined() || allow_code_gen->IsTrue();
483+
return {
484+
codegen_allowed,
485+
{},
486+
};
487+
}
488+
457489
namespace errors {
458490

459491
TryCatchScope::~TryCatchScope() {
@@ -836,6 +868,13 @@ static void SetSourceMapsEnabled(const FunctionCallbackInfo<Value>& args) {
836868
env->set_source_maps_enabled(args[0].As<Boolean>()->Value());
837869
}
838870

871+
static void SetMaybeCacheGeneratedSourceMap(
872+
const FunctionCallbackInfo<Value>& args) {
873+
Environment* env = Environment::GetCurrent(args);
874+
CHECK(args[0]->IsFunction());
875+
env->set_maybe_cache_generated_source_map(args[0].As<Function>());
876+
}
877+
839878
static void SetEnhanceStackForFatalException(
840879
const FunctionCallbackInfo<Value>& args) {
841880
Environment* env = Environment::GetCurrent(args);
@@ -870,6 +909,7 @@ static void TriggerUncaughtException(const FunctionCallbackInfo<Value>& args) {
870909
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
871910
registry->Register(SetPrepareStackTraceCallback);
872911
registry->Register(SetSourceMapsEnabled);
912+
registry->Register(SetMaybeCacheGeneratedSourceMap);
873913
registry->Register(SetEnhanceStackForFatalException);
874914
registry->Register(NoSideEffectsToString);
875915
registry->Register(TriggerUncaughtException);
@@ -883,6 +923,9 @@ void Initialize(Local<Object> target,
883923
env->SetMethod(
884924
target, "setPrepareStackTraceCallback", SetPrepareStackTraceCallback);
885925
env->SetMethod(target, "setSourceMapsEnabled", SetSourceMapsEnabled);
926+
env->SetMethod(target,
927+
"setMaybeCacheGeneratedSourceMap",
928+
SetMaybeCacheGeneratedSourceMap);
886929
env->SetMethod(target,
887930
"setEnhanceStackForFatalException",
888931
SetEnhanceStackForFatalException);

src/node_errors.h

+5
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,11 @@ void DecorateErrorStack(Environment* env,
271271
const errors::TryCatchScope& try_catch);
272272
} // namespace errors
273273

274+
v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings(
275+
v8::Local<v8::Context> context,
276+
v8::Local<v8::Value> source,
277+
bool is_code_like);
278+
274279
} // namespace node
275280

276281
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

test/message/source_map_eval.js

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Flags: --enable-source-maps
2+
3+
'use strict';
4+
require('../common');
5+
const fs = require('fs');
6+
7+
const content = fs.readFileSync(require.resolve('../fixtures/source-map/tabs.js'), 'utf8');
8+
eval(content);

test/message/source_map_eval.out

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
ReferenceError: alert is not defined
2+
at Object.eval (*tabs.coffee:26:2)
3+
at eval (*tabs.coffee:1:14)
4+
at Object.<anonymous> (*source_map_eval.js:8:1)
5+
at Module._compile (node:internal/modules/cjs/loader:*)
6+
at Module._extensions..js (node:internal/modules/cjs/loader:*)
7+
at Module.load (node:internal/modules/cjs/loader:*)
8+
at Module._load (node:internal/modules/cjs/loader:*)
9+
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:*)
10+
at node:internal/main/run_main_module:*
11+
12+
Node.js *

0 commit comments

Comments
 (0)