Skip to content

Commit d02ad40

Browse files
addaleaxaddaleax
committed
inspector,vm: remove --eval wrapper
Report the actual source code when running with `--eval` and `--inspect-brk`, by telling the vm module to break on the first line of the script being executed rather than wrapping the source code in a function. PR-URL: #25832 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 84358b5 commit d02ad40

10 files changed

Lines changed: 63 additions & 34 deletions

‎lib/internal/process/execution.js‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,26 +33,30 @@ function tryGetCwd() {
3333
}
3434
}
3535

36-
function evalScript(name, body, breakFristLine) {
36+
function evalScript(name, body, breakFirstLine) {
3737
const CJSModule = require('internal/modules/cjs/loader');
38-
if (breakFristLine) {
39-
const fn = `function() {\n\n${body};\n\n}`;
40-
body = `process.binding('inspector').callAndPauseOnStart(${fn}, {})`;
41-
}
38+
const { kVmBreakFirstLineSymbol } = require('internal/util');
4239

4340
const cwd = tryGetCwd();
4441

4542
const module = new CJSModule(name);
4643
module.filename = path.join(cwd, name);
4744
module.paths = CJSModule._nodeModulePaths(cwd);
48-
const script = `global.__filename = ${JSON.stringify(name)};\n` +
49-
'global.exports = exports;\n' +
50-
'global.module = module;\n' +
51-
'global.__dirname = __dirname;\n' +
52-
'global.require = require;\n' +
53-
'return require("vm").runInThisContext(' +
54-
`${JSON.stringify(body)}, { filename: ` +
55-
`${JSON.stringify(name)}, displayErrors: true });\n`;
45+
global.kVmBreakFirstLineSymbol = kVmBreakFirstLineSymbol;
46+
const script = `
47+
global.__filename = ${JSON.stringify(name)};
48+
global.exports = exports;
49+
global.module = module;
50+
global.__dirname = __dirname;
51+
global.require = require;
52+
const { kVmBreakFirstLineSymbol } = global;
53+
delete global.kVmBreakFirstLineSymbol;
54+
return require("vm").runInThisContext(
55+
${JSON.stringify(body)}, {
56+
filename: ${JSON.stringify(name)},
57+
displayErrors: true,
58+
[kVmBreakFirstLineSymbol]: ${!!breakFirstLine}
59+
});\n`;
5660
const result = module._compile(script, `${name}-wrapper`);
5761
if (require('internal/options').getOptionValue('--print')) {
5862
console.log(result);

‎lib/internal/util.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,5 +418,6 @@ module.exports = {
418418
// Used by the buffer module to capture an internal reference to the
419419
// default isEncoding implementation, just in case userland overrides it.
420420
kIsEncodingSymbol: Symbol('kIsEncodingSymbol'),
421-
kExpandStackSymbol: Symbol('kExpandStackSymbol')
421+
kExpandStackSymbol: Symbol('kExpandStackSymbol'),
422+
kVmBreakFirstLineSymbol: Symbol('kVmBreakFirstLineSymbol')
422423
};

‎lib/vm.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ const {
3838
validateUint32,
3939
validateString
4040
} = require('internal/validators');
41+
const { kVmBreakFirstLineSymbol } = require('internal/util');
4142
const kParsingContext = Symbol('script parsing context');
4243

4344
const ArrayForEach = Function.call.bind(Array.prototype.forEach);
@@ -175,7 +176,8 @@ function getRunInContextArgs(options = {}) {
175176

176177
const {
177178
displayErrors = true,
178-
breakOnSigint = false
179+
breakOnSigint = false,
180+
[kVmBreakFirstLineSymbol]: breakFirstLine = false,
179181
} = options;
180182

181183
if (typeof displayErrors !== 'boolean') {
@@ -189,7 +191,7 @@ function getRunInContextArgs(options = {}) {
189191

190192
return {
191193
breakOnSigint,
192-
args: [timeout, displayErrors, breakOnSigint]
194+
args: [timeout, displayErrors, breakOnSigint, breakFirstLine]
193195
};
194196
}
195197

‎src/node_contextify.cc‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,9 @@ void ContextifyScript::RunInThisContext(
795795
TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(
796796
TRACING_CATEGORY_NODE2(vm, script), "RunInThisContext", wrapped_script);
797797

798-
CHECK_EQ(args.Length(), 3);
798+
// TODO(addaleax): Use an options object or otherwise merge this with
799+
// RunInContext().
800+
CHECK_EQ(args.Length(), 4);
799801

800802
CHECK(args[0]->IsNumber());
801803
int64_t timeout = args[0]->IntegerValue(env->context()).FromJust();
@@ -806,8 +808,16 @@ void ContextifyScript::RunInThisContext(
806808
CHECK(args[2]->IsBoolean());
807809
bool break_on_sigint = args[2]->IsTrue();
808810

811+
CHECK(args[3]->IsBoolean());
812+
bool break_on_first_line = args[3]->IsTrue();
813+
809814
// Do the eval within this context
810-
EvalMachine(env, timeout, display_errors, break_on_sigint, args);
815+
EvalMachine(env,
816+
timeout,
817+
display_errors,
818+
break_on_sigint,
819+
break_on_first_line,
820+
args);
811821

812822
TRACE_EVENT_NESTABLE_ASYNC_END0(
813823
TRACING_CATEGORY_NODE2(vm, script), "RunInThisContext", wrapped_script);
@@ -819,7 +829,7 @@ void ContextifyScript::RunInContext(const FunctionCallbackInfo<Value>& args) {
819829
ContextifyScript* wrapped_script;
820830
ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder());
821831

822-
CHECK_EQ(args.Length(), 4);
832+
CHECK_EQ(args.Length(), 5);
823833

824834
CHECK(args[0]->IsObject());
825835
Local<Object> sandbox = args[0].As<Object>();
@@ -843,12 +853,16 @@ void ContextifyScript::RunInContext(const FunctionCallbackInfo<Value>& args) {
843853
CHECK(args[3]->IsBoolean());
844854
bool break_on_sigint = args[3]->IsTrue();
845855

856+
CHECK(args[4]->IsBoolean());
857+
bool break_on_first_line = args[4]->IsTrue();
858+
846859
// Do the eval within the context
847860
Context::Scope context_scope(contextify_context->context());
848861
EvalMachine(contextify_context->env(),
849862
timeout,
850863
display_errors,
851864
break_on_sigint,
865+
break_on_first_line,
852866
args);
853867

854868
TRACE_EVENT_NESTABLE_ASYNC_END0(
@@ -859,6 +873,7 @@ bool ContextifyScript::EvalMachine(Environment* env,
859873
const int64_t timeout,
860874
const bool display_errors,
861875
const bool break_on_sigint,
876+
const bool break_on_first_line,
862877
const FunctionCallbackInfo<Value>& args) {
863878
if (!env->can_call_into_js())
864879
return false;
@@ -874,6 +889,12 @@ bool ContextifyScript::EvalMachine(Environment* env,
874889
PersistentToLocal::Default(env->isolate(), wrapped_script->script_);
875890
Local<Script> script = unbound_script->BindToCurrentContext();
876891

892+
#if HAVE_INSPECTOR
893+
if (break_on_first_line) {
894+
env->inspector_agent()->PauseOnNextJavascriptStatement("Break on start");
895+
}
896+
#endif
897+
877898
MaybeLocal<Value> result;
878899
bool timed_out = false;
879900
bool received_signal = false;

‎src/node_contextify.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ class ContextifyScript : public BaseObject {
121121
const int64_t timeout,
122122
const bool display_errors,
123123
const bool break_on_sigint,
124+
const bool break_on_first_line,
124125
const v8::FunctionCallbackInfo<v8::Value>& args);
125126

126127
inline uint32_t id() { return id_; }

‎test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ setTimeout(() => {
1414
`;
1515

1616
async function skipBreakpointAtStart(session) {
17-
await session.waitForBreakOnLine(3, '[eval]');
17+
await session.waitForBreakOnLine(1, '[eval]');
1818
await session.send({ 'method': 'Debugger.resume' });
1919
}
2020

2121
async function checkAsyncStackTrace(session) {
2222
console.error('[test]', 'Verify basic properties of asyncStackTrace');
23-
const paused = await session.waitForBreakOnLine(4, '[eval]');
23+
const paused = await session.waitForBreakOnLine(2, '[eval]');
2424
assert(paused.params.asyncStackTrace,
2525
`${Object.keys(paused.params)} contains "asyncStackTrace" property`);
2626
assert(paused.params.asyncStackTrace.description, 'Timeout');

‎test/sequential/test-inspector-async-stack-traces-promise-then.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,18 @@ async function runTests() {
3131
{ 'method': 'Runtime.runIfWaitingForDebugger' }
3232
]);
3333

34-
await session.waitForBreakOnLine(2, '[eval]');
34+
await session.waitForBreakOnLine(0, '[eval]');
3535
await session.send({ 'method': 'Debugger.resume' });
3636

3737
console.error('[test] Waiting for break1');
38-
debuggerPausedAt(await session.waitForBreakOnLine(6, '[eval]'),
39-
'break1', 'runTest:5');
38+
debuggerPausedAt(await session.waitForBreakOnLine(4, '[eval]'),
39+
'break1', 'runTest:3');
4040

4141
await session.send({ 'method': 'Debugger.resume' });
4242

4343
console.error('[test] Waiting for break2');
44-
debuggerPausedAt(await session.waitForBreakOnLine(9, '[eval]'),
45-
'break2', 'runTest:8');
44+
debuggerPausedAt(await session.waitForBreakOnLine(7, '[eval]'),
45+
'break2', 'runTest:6');
4646

4747
await session.runToCompletion();
4848
assert.strictEqual((await instance.expectShutdown()).exitCode, 0);

‎test/sequential/test-inspector-async-stack-traces-set-interval.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@ const script = 'setInterval(() => { debugger; }, 50);';
1010

1111
async function skipFirstBreakpoint(session) {
1212
console.log('[test]', 'Skipping the first breakpoint in the eval script');
13-
await session.waitForBreakOnLine(2, '[eval]');
13+
await session.waitForBreakOnLine(0, '[eval]');
1414
await session.send({ 'method': 'Debugger.resume' });
1515
}
1616

1717
async function checkAsyncStackTrace(session) {
1818
console.error('[test]', 'Verify basic properties of asyncStackTrace');
19-
const paused = await session.waitForBreakOnLine(2, '[eval]');
19+
const paused = await session.waitForBreakOnLine(0, '[eval]');
2020
assert(paused.params.asyncStackTrace,
2121
`${Object.keys(paused.params)} contains "asyncStackTrace" property`);
2222
assert(paused.params.asyncStackTrace.description, 'Timeout');

‎test/sequential/test-inspector-break-e.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ async function runTests() {
1414
{ 'method': 'Debugger.enable' },
1515
{ 'method': 'Runtime.runIfWaitingForDebugger' }
1616
]);
17-
await session.waitForBreakOnLine(2, '[eval]');
17+
await session.waitForBreakOnLine(0, '[eval]');
1818
await session.runToCompletion();
1919
assert.strictEqual((await instance.expectShutdown()).exitCode, 0);
2020
}

‎test/sequential/test-inspector-scriptparsed-context.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,28 +51,28 @@ async function runTests() {
5151
{ 'method': 'Debugger.enable' },
5252
{ 'method': 'Runtime.runIfWaitingForDebugger' }
5353
]);
54-
await session.waitForBreakOnLine(4, '[eval]');
54+
await session.waitForBreakOnLine(2, '[eval]');
5555

5656
await session.send({ 'method': 'Runtime.enable' });
5757
await getContext(session);
5858
await session.send({ 'method': 'Debugger.resume' });
5959
const childContext = await getContext(session);
60-
await session.waitForBreakOnLine(13, '[eval]');
60+
await session.waitForBreakOnLine(11, '[eval]');
6161

6262
console.error('[test]', 'Script is unbound');
6363
await session.send({ 'method': 'Debugger.resume' });
64-
await session.waitForBreakOnLine(17, '[eval]');
64+
await session.waitForBreakOnLine(15, '[eval]');
6565

6666
console.error('[test]', 'vm.runInContext associates script with context');
6767
await session.send({ 'method': 'Debugger.resume' });
6868
await checkScriptContext(session, childContext);
69-
await session.waitForBreakOnLine(20, '[eval]');
69+
await session.waitForBreakOnLine(18, '[eval]');
7070

7171
console.error('[test]', 'vm.runInNewContext associates script with context');
7272
await session.send({ 'method': 'Debugger.resume' });
7373
const thirdContext = await getContext(session);
7474
await checkScriptContext(session, thirdContext);
75-
await session.waitForBreakOnLine(23, '[eval]');
75+
await session.waitForBreakOnLine(21, '[eval]');
7676

7777
console.error('[test]', 'vm.runInNewContext can contain debugger statements');
7878
await session.send({ 'method': 'Debugger.resume' });

0 commit comments

Comments
 (0)