inspector: add --cond to node inspect probe mode · nodejs/node@445bcce · GitHub
Skip to content

Commit 445bcce

Browse files
joyeecheungaduh95
joyeecheung
authored and
aduh95
committed
inspector: add --cond to node inspect probe mode
On a hot path, the probe can record every hit and require filtering afterwards. This patch adds a per-probe `--cond <expr>` option that allows limiting the hit to only when the expression is truthy at the probe location. V8 evaluates it as the breakpoint's native condition, so the target is not paused when it does not hold, and a condition that throws is treated as false. Since in CDP, a location can only carry one breakpoint per URL pattern, probes sharing a location must share one condition (or none). Conflicting conditions are rejected. Example: ```js // app.js let total = 0; for (let i = 0; i < 10; i++) { total += i; // line 4 } ``` ``` $ out/Release/node inspect --probe app.js:4 --expr 'total' \ --cond 'i % 3 === 0' app.js ``` ``` Hit 1 at file:///path/to/app.js:3:3 total = 0 Hit 2 at file:///path/to/app.js:3:3 total = 3 Hit 3 at file:///path/to/app.js:3:3 total = 15 Hit 4 at file:///path/to/app.js:3:3 total = 36 Completed ``` Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: #64328 Refs: #63646 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 5d90e48 commit 445bcce

9 files changed

Lines changed: 348 additions & 17 deletions

‎doc/api/debugger.md‎

Lines changed: 53 additions & 7 deletions

‎lib/internal/debugger/inspect.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ function parseInteractiveArgs(args) {
268268

269269
const kInspectArgOptions = {
270270
'__proto__': null,
271+
'cond': { type: 'string' },
271272
'expr': { type: 'string' },
272273
'help': { type: 'boolean', short: 'h' },
273274
'json': { type: 'boolean' },

‎lib/internal/debugger/inspect_helpers.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ function writeInspectUsageAndExit(invokedAs, message, exitCode) {
6969
}
7070
out.write(`Usage: ${invokedAs} [--port=<port>] [<node-option> ...]
7171
[<script> [<script-args>] | <host>:<port> | -p <pid>]
72-
${invokedAs} --probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>]
73-
[--probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>] ...]
72+
${invokedAs} --probe <file>:<line>[:<col>] --expr <expr> [--cond <expr>] [--max-hit <n>]
73+
[--probe <file>:<line>[:<col>] --expr <expr> [--cond <expr>] [--max-hit <n>] ...]
7474
[--json] [--preview] [--timeout=<ms>] [--port=<port>]
7575
[--] [<node-option> ...] <script> [<script-args> ...]
7676
@@ -109,6 +109,9 @@ Options:
109109
preceding --probe each time execution reaches it.
110110
Avoid probing let/const-bound variables at their
111111
declaration site or a ReferenceError may be thrown.
112+
--cond <expr> Optional condition for the probe location. The probe only
113+
records a hit when <expr> is truthy at the location. A
114+
condition that throws is treated as false.
112115
--max-hit <n> Per-probe limit on evaluated hits. When not specified,
113116
there's no hit limit. When any probe reaches its hit LIMIT,
114117
the probing process will detach and report the results.
@@ -121,6 +124,9 @@ Options:
121124
Semantics:
122125
* Multiple --probe/--expr pairs are allowed. Same-location --probes share
123126
a pause and scope, their --exprs are evaluated in command-line order.
127+
* --max-hit scopes to one --probe/--expr pair, so same-location pairs may set
128+
different limits. --cond scopes to the location, probes sharing a location
129+
must all share one condition (or none).
124130
* --probe utils.js:<line>[:<col>] matches every loaded utils.js. Pass a
125131
fuller path e.g. src/utils.js to narrow the match.
126132
* Use -- before any Node.js flags intended for the child process.

‎lib/internal/debugger/inspect_probe.js‎

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const {
2121
StringPrototypeIncludes,
2222
StringPrototypeSlice,
2323
StringPrototypeStartsWith,
24+
StringPrototypeTrim,
2425
Symbol,
2526
} = primordials;
2627

@@ -87,6 +88,8 @@ const kInspectPortRegex = /^--inspect-port=(\d+)$/;
8788
* @typedef {object} Probe
8889
* @property {string} expr Expression to evaluate on hit.
8990
* @property {ProbeTarget} target User's original --probe request shape.
91+
* @property {string} [condition] Condition from --cond. V8 only breaks when it is truthy.
92+
* Scoped to the location, so probes sharing a location all carry the same value.
9093
* @property {number} maxHit Per-probe hit limit from --max-hit. Infinity when unlimited.
9194
* @property {number} hits Count of hits observed.
9295
*/
@@ -130,6 +133,12 @@ function formatTargetText(target) {
130133
return column === undefined ? `${suffix}:${line}` : `${suffix}:${line}:${column}`;
131134
}
132135

136+
// Identity of a probe location. Probes sharing a key share one V8 breakpoint,
137+
// so this must stay in sync between condition validation and breakpoint setup.
138+
function locationKey(target) {
139+
return `${target.suffix}\n${target.line}\n${target.column ?? ''}`;
140+
}
141+
133142
function formatPendingProbeLocations(probes, pending) {
134143
const seen = new SafeSet();
135144
for (const probeIndex of pending) {
@@ -391,6 +400,22 @@ function parseProbeTokens(tokens, args) {
391400
probe.maxHit = parseUnsignedInteger(token.value, 'max-hit');
392401
break;
393402
}
403+
case 'cond': {
404+
if (probes.length === 0) {
405+
throw new ERR_DEBUGGER_STARTUP_ERROR('Unexpected --cond before --probe');
406+
}
407+
// A blank condition does not act as a real predicate in V8 (an empty
408+
// string always breaks), so reject it rather than silently mislead.
409+
if (token.value === undefined || StringPrototypeTrim(token.value) === '') {
410+
throw new ERR_DEBUGGER_STARTUP_ERROR(`Missing value for ${token.rawName}`);
411+
}
412+
const probe = probes[probes.length - 1];
413+
if (probe.condition !== undefined) {
414+
throw new ERR_DEBUGGER_STARTUP_ERROR('A --probe can have at most one --cond');
415+
}
416+
probe.condition = token.value;
417+
break;
418+
}
394419
default:
395420
if (probes.length > 0) {
396421
throw new ERR_DEBUGGER_STARTUP_ERROR(
@@ -410,6 +435,24 @@ function parseProbeTokens(tokens, args) {
410435
'Probe mode requires at least one --probe <loc> --expr <expr> group');
411436
}
412437

438+
// V8 allows only one breakpoint per location, so probes sharing a location
439+
// cannot carry different conditions.
440+
const conditionByLocation = new SafeMap();
441+
for (const { target, condition } of probes) {
442+
const key = locationKey(target);
443+
// All probes at the same location must share one condition. We split the
444+
// existence check since if one probe does not have a condition (= undefined),
445+
// then all probes at the location must also omit it.
446+
if (conditionByLocation.has(key)) {
447+
if (conditionByLocation.get(key) !== condition) {
448+
throw new ERR_DEBUGGER_STARTUP_ERROR(
449+
`Probes at ${formatTargetText(target)} must use the same --cond (or none)`);
450+
}
451+
} else {
452+
conditionByLocation.set(key, condition);
453+
}
454+
}
455+
413456
const childArgv = ArrayPrototypeSlice(args, childStartIndex);
414457
if (childArgv.length === 0) {
415458
throw new ERR_DEBUGGER_STARTUP_ERROR('Probe mode requires a child script');
@@ -479,8 +522,8 @@ class ProbeInspectorSession {
479522
this.resolveCompletion = resolve;
480523
/** @type {Probe[]} */
481524
this.probes = ArrayPrototypeMap(options.probes,
482-
({ expr, target, maxHit }) =>
483-
({ expr, target, maxHit: maxHit ?? Infinity, hits: 0 }));
525+
({ expr, target, maxHit, condition }) =>
526+
({ expr, target, condition, maxHit: maxHit ?? Infinity, hits: 0 }));
484527
this.onChildOutput = FunctionPrototypeBind(this.onChildOutput, this);
485528
this.onChildExit = FunctionPrototypeBind(this.onChildExit, this);
486529
this.onClientClose = FunctionPrototypeBind(this.onClientClose, this);
@@ -887,17 +930,19 @@ class ProbeInspectorSession {
887930
const uniqueTargets = new SafeMap();
888931

889932
for (let probeIndex = 0; probeIndex < this.probes.length; probeIndex++) {
890-
const { target } = this.probes[probeIndex];
891-
const key = `${target.suffix}\n${target.line}\n${target.column ?? ''}`;
933+
const { target, condition } = this.probes[probeIndex];
934+
// Probes at the same location share one V8 breakpoint. parseProbeTokens has
935+
// already ensured they carry the same condition.
936+
const key = locationKey(target);
892937
let entry = uniqueTargets.get(key);
893938
if (entry === undefined) {
894-
entry = { target, probeIndices: [] };
939+
entry = { target, condition, probeIndices: [] };
895940
uniqueTargets.set(key, entry);
896941
}
897942
ArrayPrototypePush(entry.probeIndices, probeIndex);
898943
}
899944

900-
for (const { target, probeIndices } of uniqueTargets.values()) {
945+
for (const { target, condition, probeIndices } of uniqueTargets.values()) {
901946
// On Windows, normalize backslashes to forward slashes so the regex matches
902947
// V8 script URLs which always use forward slashes.
903948
const normalizedFile = process.platform === 'win32' ?
@@ -918,6 +963,9 @@ class ProbeInspectorSession {
918963
// the inspector bind to the first executable column.
919964
params.columnNumber = target.column - 1;
920965
}
966+
if (condition !== undefined) {
967+
params.condition = condition;
968+
}
921969

922970
const result = await this.callCdp('Debugger.setBreakpointByUrl', params);
923971
debug('breakpoint set: id=%s urlRegex=%s locations=%j',
@@ -943,9 +991,10 @@ class ProbeInspectorSession {
943991
code: exitCode,
944992
report: {
945993
v: kProbeVersion,
946-
probes: ArrayPrototypeMap(this.probes, ({ expr, target, maxHit }) => {
947-
// Omit an unlimited maxHit, as Infinity would serialize to null in JSON.
994+
probes: ArrayPrototypeMap(this.probes, ({ expr, target, maxHit, condition }) => {
948995
const probe = { expr, target };
996+
if (condition !== undefined) { probe.condition = condition; }
997+
// Omit an unlimited maxHit, as Infinity would serialize to null in JSON.
949998
if (maxHit !== Infinity) { probe.maxHit = maxHit; }
950999
return probe;
9511000
}),

‎test/parallel/test-debugger-probe-cond-invalid.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// This tests that probe mode rejects malformed --cond usage.
2+
'use strict';
3+
4+
const common = require('../common');
5+
common.skipIfInspectorDisabled();
6+
7+
const fixtures = require('../common/fixtures');
8+
const { assertProbeCliError } = require('../common/debugger-probe');
9+
10+
const cwd = fixtures.path('debugger');
11+
12+
assertProbeCliError(
13+
['--cond', 'x', '--probe', 'probe.js:12', '--expr', 'finalValue', 'probe.js'],
14+
/Unexpected --cond before --probe/, { cwd });
15+
16+
assertProbeCliError(
17+
['--probe', 'probe.js:12', '--cond', 'x', '--expr', 'finalValue', 'probe.js'],
18+
/Each --probe must be followed immediately by --expr/, { cwd });
19+
20+
assertProbeCliError(
21+
['--probe', 'probe.js:12', '--expr', 'finalValue', '--cond', 'x', '--cond', 'y', 'probe.js'],
22+
/A --probe can have at most one --cond/, { cwd });
23+
24+
assertProbeCliError(
25+
['--probe', 'probe.js:12', '--expr', 'finalValue', '--cond', ' ', 'probe.js'],
26+
/Missing value for --cond/, { cwd });
27+
28+
assertProbeCliError(
29+
['--probe', 'probe.js:12', '--expr', 'finalValue', '--cond'],
30+
/Missing value for --cond/, { cwd });
31+
32+
assertProbeCliError(
33+
['--probe', 'probe.js:12', '--expr', 'a', '--cond', 'x',
34+
'--probe', 'probe.js:12', '--expr', 'b', '--cond', 'y', 'probe.js'],
35+
/Probes at probe\.js:12 must use the same --cond \(or none\)/, { cwd });
36+
37+
assertProbeCliError(
38+
['--probe', 'probe.js:12', '--expr', 'a', '--cond', 'x',
39+
'--probe', 'probe.js:12', '--expr', 'b', 'probe.js'],
40+
/Probes at probe\.js:12 must use the same --cond \(or none\)/, { cwd });

‎test/parallel/test-debugger-probe-cond-max-hit.js‎

Lines changed: 47 additions & 0 deletions

0 commit comments

Comments
 (0)