Skip to content

Commit

Permalink
inspector,vm: remove --eval wrapper
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@addaleax
addaleax committed Feb 3, 2019
1 parent f722540 commit cca897e
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 34 deletions.
30 changes: 17 additions & 13 deletions lib/internal/process/execution.js
View file
Expand Up @@ -33,26 +33,30 @@ function tryGetCwd() {
}
}

function evalScript(name, body, breakFristLine) {
function evalScript(name, body, breakFirstLine) {
const CJSModule = require('internal/modules/cjs/loader');
if (breakFristLine) {
const fn = `function() {\n\n${body};\n\n}`;
body = `process.binding('inspector').callAndPauseOnStart(${fn}, {})`;
}
const { kVmBreakFirstLineSymbol } = require('internal/util');

const cwd = tryGetCwd();

const module = new CJSModule(name);
module.filename = path.join(cwd, name);
module.paths = CJSModule._nodeModulePaths(cwd);
const script = `global.__filename = ${JSON.stringify(name)};\n` +
'global.exports = exports;\n' +
'global.module = module;\n' +
'global.__dirname = __dirname;\n' +
'global.require = require;\n' +
'return require("vm").runInThisContext(' +
`${JSON.stringify(body)}, { filename: ` +
`${JSON.stringify(name)}, displayErrors: true });\n`;
global.kVmBreakFirstLineSymbol = kVmBreakFirstLineSymbol;
const script = `
global.__filename = ${JSON.stringify(name)};
global.exports = exports;
global.module = module;
global.__dirname = __dirname;
global.require = require;
const { kVmBreakFirstLineSymbol } = global;
delete global.kVmBreakFirstLineSymbol;
return require("vm").runInThisContext(
${JSON.stringify(body)}, {
filename: ${JSON.stringify(name)},
displayErrors: true,
[kVmBreakFirstLineSymbol]: ${!!breakFirstLine}
});\n`;
const result = module._compile(script, `${name}-wrapper`);
if (require('internal/options').getOptionValue('--print')) {
console.log(result);
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/util.js
View file
Expand Up @@ -418,5 +418,6 @@ module.exports = {
// Used by the buffer module to capture an internal reference to the
// default isEncoding implementation, just in case userland overrides it.
kIsEncodingSymbol: Symbol('kIsEncodingSymbol'),
kExpandStackSymbol: Symbol('kExpandStackSymbol')
kExpandStackSymbol: Symbol('kExpandStackSymbol'),
kVmBreakFirstLineSymbol: Symbol('kVmBreakFirstLineSymbol')
};
6 changes: 4 additions & 2 deletions lib/vm.js
View file
Expand Up @@ -38,6 +38,7 @@ const {
validateUint32,
validateString
} = require('internal/validators');
const { kVmBreakFirstLineSymbol } = require('internal/util');
const kParsingContext = Symbol('script parsing context');

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

const {
displayErrors = true,
breakOnSigint = false
breakOnSigint = false,
[kVmBreakFirstLineSymbol]: breakFirstLine = false,
} = options;

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

return {
breakOnSigint,
args: [timeout, displayErrors, breakOnSigint]
args: [timeout, displayErrors, breakOnSigint, breakFirstLine]
};
}

Expand Down
27 changes: 24 additions & 3 deletions src/node_contextify.cc
View file
Expand Up @@ -795,7 +795,9 @@ void ContextifyScript::RunInThisContext(
TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(
TRACING_CATEGORY_NODE2(vm, script), "RunInThisContext", wrapped_script);

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

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

CHECK(args[3]->IsBoolean());
bool break_on_first_line = args[3]->IsTrue();

// Do the eval within this context
EvalMachine(env, timeout, display_errors, break_on_sigint, args);
EvalMachine(env,
timeout,
display_errors,
break_on_sigint,
break_on_first_line,
args);

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

CHECK_EQ(args.Length(), 4);
CHECK_EQ(args.Length(), 5);

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

CHECK(args[4]->IsBoolean());
bool break_on_first_line = args[4]->IsTrue();

// Do the eval within the context
Context::Scope context_scope(contextify_context->context());
EvalMachine(contextify_context->env(),
timeout,
display_errors,
break_on_sigint,
break_on_first_line,
args);

TRACE_EVENT_NESTABLE_ASYNC_END0(
Expand All @@ -859,6 +873,7 @@ bool ContextifyScript::EvalMachine(Environment* env,
const int64_t timeout,
const bool display_errors,
const bool break_on_sigint,
const bool break_on_first_line,
const FunctionCallbackInfo<Value>& args) {
if (!env->can_call_into_js())
return false;
Expand All @@ -874,6 +889,12 @@ bool ContextifyScript::EvalMachine(Environment* env,
PersistentToLocal::Default(env->isolate(), wrapped_script->script_);
Local<Script> script = unbound_script->BindToCurrentContext();

#if HAVE_INSPECTOR
if (break_on_first_line) {
env->inspector_agent()->PauseOnNextJavascriptStatement("Break on start");
}
#endif

MaybeLocal<Value> result;
bool timed_out = false;
bool received_signal = false;
Expand Down
1 change: 1 addition & 0 deletions src/node_contextify.h
View file
Expand Up @@ -121,6 +121,7 @@ class ContextifyScript : public BaseObject {
const int64_t timeout,
const bool display_errors,
const bool break_on_sigint,
const bool break_on_first_line,
const v8::FunctionCallbackInfo<v8::Value>& args);

inline uint32_t id() { return id_; }
Expand Down
4 changes: 2 additions & 2 deletions test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js
View file
Expand Up @@ -14,13 +14,13 @@ setTimeout(() => {
`;

async function skipBreakpointAtStart(session) {
await session.waitForBreakOnLine(3, '[eval]');
await session.waitForBreakOnLine(1, '[eval]');
await session.send({ 'method': 'Debugger.resume' });
}

async function checkAsyncStackTrace(session) {
console.error('[test]', 'Verify basic properties of asyncStackTrace');
const paused = await session.waitForBreakOnLine(4, '[eval]');
const paused = await session.waitForBreakOnLine(2, '[eval]');
assert(paused.params.asyncStackTrace,
`${Object.keys(paused.params)} contains "asyncStackTrace" property`);
assert(paused.params.asyncStackTrace.description, 'Timeout');
Expand Down
10 changes: 5 additions & 5 deletions test/sequential/test-inspector-async-stack-traces-promise-then.js
View file
Expand Up @@ -31,18 +31,18 @@ async function runTests() {
{ 'method': 'Runtime.runIfWaitingForDebugger' }
]);

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

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

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

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

await session.runToCompletion();
assert.strictEqual((await instance.expectShutdown()).exitCode, 0);
Expand Down
4 changes: 2 additions & 2 deletions test/sequential/test-inspector-async-stack-traces-set-interval.js
View file
Expand Up @@ -10,13 +10,13 @@ const script = 'setInterval(() => { debugger; }, 50);';

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

async function checkAsyncStackTrace(session) {
console.error('[test]', 'Verify basic properties of asyncStackTrace');
const paused = await session.waitForBreakOnLine(2, '[eval]');
const paused = await session.waitForBreakOnLine(0, '[eval]');
assert(paused.params.asyncStackTrace,
`${Object.keys(paused.params)} contains "asyncStackTrace" property`);
assert(paused.params.asyncStackTrace.description, 'Timeout');
Expand Down
2 changes: 1 addition & 1 deletion test/sequential/test-inspector-break-e.js
View file
Expand Up @@ -14,7 +14,7 @@ async function runTests() {
{ 'method': 'Debugger.enable' },
{ 'method': 'Runtime.runIfWaitingForDebugger' }
]);
await session.waitForBreakOnLine(2, '[eval]');
await session.waitForBreakOnLine(0, '[eval]');
await session.runToCompletion();
assert.strictEqual((await instance.expectShutdown()).exitCode, 0);
}
Expand Down
10 changes: 5 additions & 5 deletions test/sequential/test-inspector-scriptparsed-context.js
View file
Expand Up @@ -51,28 +51,28 @@ async function runTests() {
{ 'method': 'Debugger.enable' },
{ 'method': 'Runtime.runIfWaitingForDebugger' }
]);
await session.waitForBreakOnLine(4, '[eval]');
await session.waitForBreakOnLine(2, '[eval]');

await session.send({ 'method': 'Runtime.enable' });
await getContext(session);
await session.send({ 'method': 'Debugger.resume' });
const childContext = await getContext(session);
await session.waitForBreakOnLine(13, '[eval]');
await session.waitForBreakOnLine(11, '[eval]');

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

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

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

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

0 comments on commit cca897e

Please sign in to comment.