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: https://github.com/nodejs/node/pull/25832
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

10 files changed, 63 insertions, 34 deletions

diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js
index 4ee890e9db..a35feaacce 100644
--- a/lib/internal/process/execution.js
+++ b/lib/internal/process/execution.js
@@ -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);
diff --git a/lib/internal/util.js b/lib/internal/util.js
index fb3eed6f5e..b3fdff2762 100644
--- a/lib/internal/util.js
+++ b/lib/internal/util.js
@@ -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')
 };
diff --git a/lib/vm.js b/lib/vm.js
index 464724071a..b2875d1e11 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -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);
@@ -175,7 +176,8 @@ function getRunInContextArgs(options = {}) {
 
   const {
     displayErrors = true,
-    breakOnSigint = false
+    breakOnSigint = false,
+    [kVmBreakFirstLineSymbol]: breakFirstLine = false,
   } = options;
 
   if (typeof displayErrors !== 'boolean') {
@@ -189,7 +191,7 @@ function getRunInContextArgs(options = {}) {
 
   return {
     breakOnSigint,
-    args: [timeout, displayErrors, breakOnSigint]
+    args: [timeout, displayErrors, breakOnSigint, breakFirstLine]
   };
 }
 
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index b7c7f38eb6..952acfe06f 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -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();
@@ -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);
@@ -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>();
@@ -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(
@@ -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;
@@ -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;
diff --git a/src/node_contextify.h b/src/node_contextify.h
index cb3ca83553..71c4bfc0d6 100644
--- a/src/node_contextify.h
+++ b/src/node_contextify.h
@@ -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_; }
diff --git a/test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js b/test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js
index a1b8f301e0..85a612be21 100644
--- a/test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js
+++ b/test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js
@@ -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');
diff --git a/test/sequential/test-inspector-async-stack-traces-promise-then.js b/test/sequential/test-inspector-async-stack-traces-promise-then.js
index 11905431d5..a6bbd2686c 100644
--- a/test/sequential/test-inspector-async-stack-traces-promise-then.js
+++ b/test/sequential/test-inspector-async-stack-traces-promise-then.js
@@ -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);
diff --git a/test/sequential/test-inspector-async-stack-traces-set-interval.js b/test/sequential/test-inspector-async-stack-traces-set-interval.js
index 240074f631..451cc14b1c 100644
--- a/test/sequential/test-inspector-async-stack-traces-set-interval.js
+++ b/test/sequential/test-inspector-async-stack-traces-set-interval.js
@@ -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');
diff --git a/test/sequential/test-inspector-break-e.js b/test/sequential/test-inspector-break-e.js
index 954ce6b897..6d4fbbb474 100644
--- a/test/sequential/test-inspector-break-e.js
+++ b/test/sequential/test-inspector-break-e.js
@@ -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);
 }
diff --git a/test/sequential/test-inspector-scriptparsed-context.js b/test/sequential/test-inspector-scriptparsed-context.js
index 56cc41dd86..944829f59d 100644
--- a/test/sequential/test-inspector-scriptparsed-context.js
+++ b/test/sequential/test-inspector-scriptparsed-context.js
@@ -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' });