Skip to content

Commit 3a32d20

Browse files
committed
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.
1 parent 511e289 commit 3a32d20

File tree

10 files changed

+152
-23
lines changed

10 files changed

+152
-23
lines changed

lib/internal/source_map/prepare_stack_trace.js

Lines changed: 7 additions & 2 deletions
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

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@ const {
77
ObjectKeys,
88
ObjectGetOwnPropertyDescriptor,
99
ObjectPrototypeHasOwnProperty,
10-
RegExpPrototypeTest,
10+
RegExpPrototypeExec,
1111
SafeMap,
12-
StringPrototypeMatch,
1312
StringPrototypeSplit,
1413
} = primordials;
1514

@@ -30,12 +29,18 @@ const {
3029
normalizeReferrerURL,
3130
} = require('internal/modules/cjs/helpers');
3231
const { validateBoolean } = require('internal/validators');
32+
const { setMaybeCacheGeneratedSourceMap } = internalBinding('errors');
33+
3334
// Since the CJS module cache is mutable, which leads to memory leaks when
3435
// modules are deleted, we use a WeakMap so that the source map cache will
3536
// be purged automatically:
3637
const cjsSourceMapCache = new IterableWeakMap();
3738
// The esm cache is not mutable, so we can use a Map without memory concerns:
3839
const esmSourceMapCache = new SafeMap();
40+
// The generated sources is not mutable, so we can use a Map without memory concerns:
41+
const generatedSourceMapCache = new SafeMap();
42+
const kLeadingProtocol = /^\w+:\/\//;
43+
3944
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
4045
let SourceMap;
4146

@@ -71,20 +76,19 @@ function setSourceMapsEnabled(val) {
7176
sourceMapsEnabled = val;
7277
}
7378

74-
function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
79+
function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource) {
7580
const sourceMapsEnabled = getSourceMapsEnabled();
7681
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
7782
try {
7883
filename = normalizeReferrerURL(filename);
7984
} catch (err) {
80-
// This is most likely an [eval]-wrapper, which is currently not
81-
// supported.
85+
// This is most likely an invalid filename in sourceURL of [eval]-wrapper.
8286
debug(err);
8387
return;
8488
}
85-
const match = StringPrototypeMatch(
86-
content,
87-
/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/
89+
const match = RegExpPrototypeExec(
90+
/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/,
91+
content
8892
);
8993
if (match) {
9094
const data = dataFromUrl(filename, match.groups.sourceMappingURL);
@@ -96,8 +100,14 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
96100
data,
97101
url
98102
});
103+
} else if (isGeneratedSource) {
104+
generatedSourceMapCache.set(filename, {
105+
lineLengths: lineLengths(content),
106+
data,
107+
url
108+
});
99109
} else {
100-
// If there is no cjsModuleInstance assume we are in a
110+
// If there is no cjsModuleInstance and is not generated source assume we are in a
101111
// "modules/esm" context.
102112
esmSourceMapCache.set(filename, {
103113
lineLengths: lineLengths(content),
@@ -108,6 +118,31 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
108118
}
109119
}
110120

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

221-
function findSourceMap(sourceURL) {
222-
if (!RegExpPrototypeTest(/^\w+:\/\//, sourceURL)) {
256+
function findSourceMap(sourceURL, isGenerated) {
257+
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
223258
sourceURL = pathToFileURL(sourceURL).href;
224259
}
225260
if (!SourceMap) {
226261
SourceMap = require('internal/source_map/source_map').SourceMap;
227262
}
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-
};
263+
let sourceMap;
264+
if (isGenerated) {
265+
sourceMap = generatedSourceMapCache.get(sourceURL);
266+
} else {
267+
sourceMap = esmSourceMapCache.get(sourceURL);
268+
if (sourceMap === undefined) {
269+
for (const value of cjsSourceMapCache) {
270+
const filename = ObjectGetValueSafe(value, 'filename');
271+
if (sourceURL === filename) {
272+
sourceMap = {
273+
data: ObjectGetValueSafe(value, 'data')
274+
};
275+
}
236276
}
237277
}
238278
}

src/api/environment.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,8 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
254254
auto* allow_wasm_codegen_cb = s.allow_wasm_code_generation_callback ?
255255
s.allow_wasm_code_generation_callback : AllowWasmCodeGenerationCallback;
256256
isolate->SetAllowWasmCodeGenerationCallback(allow_wasm_codegen_cb);
257+
isolate->SetModifyCodeGenerationFromStringsCallback(
258+
ModifyCodeGenerationFromStrings);
257259

258260
Mutex::ScopedLock lock(node::per_process::cli_options_mutex);
259261
if (per_process::cli_options->get_per_isolate_options()
@@ -665,6 +667,9 @@ Maybe<bool> InitializeContextForSnapshot(Local<Context> context) {
665667
Isolate* isolate = context->GetIsolate();
666668
HandleScope handle_scope(isolate);
667669

670+
context->AllowCodeGenerationFromStrings(false);
671+
context->SetEmbedderData(
672+
ContextEmbedderIndex::kAllowCodeGenerationFromStrings, True(isolate));
668673
context->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration,
669674
True(isolate));
670675

src/env.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,7 @@ class NoArrayBufferZeroFillScope {
541541
V(inspector_console_extension_installer, v8::Function) \
542542
V(inspector_disable_async_hooks, v8::Function) \
543543
V(inspector_enable_async_hooks, v8::Function) \
544+
V(maybe_cache_generated_source_map, v8::Function) \
544545
V(messaging_deserialize_create_object, v8::Function) \
545546
V(message_port, v8::Object) \
546547
V(native_module_require, v8::Function) \

src/node_context_data.h

Lines changed: 7 additions & 1 deletion
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

Lines changed: 5 additions & 1 deletion
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

Lines changed: 43 additions & 0 deletions
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+
Local<Function> maybe_cache_source_map =
465+
env->maybe_cache_generated_source_map();
466+
{
467+
// We do not expect the maybe_cache_generated_source_map to throw any more
468+
// exceptions. If it does, just ignore it.
469+
errors::TryCatchScope try_catch(env);
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

Lines changed: 5 additions & 0 deletions
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

Lines changed: 8 additions & 0 deletions
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

Lines changed: 12 additions & 0 deletions
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)