Skip to content

Commit 50c258e

Browse files
Refine source stepping: line-start cursor (S19) and end-of-trace termination (S20)
Statement stepping rested the cursor on the DWARF column of a run's first record — an arbitrary interior sub-expression that differed between visits of the same line — and a forward step at the outermost function's closing brace clamped in place, re-reporting the same line. S19: the source frame now reports the line's first non-whitespace column (falling back to the DWARF column only when the line text is unavailable), so the cursor lands predictably at the start of the statement. Disassembly-row columns are unchanged. S20: a forward statement step (next/stepIn/stepOut) with no stop point ahead now terminates the session — the replayed contract has returned from its outermost recorded frame. Instruction stepping and wasm-less replay still clamp (S2), continue/reverseContinue still settle (S14), reverse steps still clamp to the first stop (S3/S8), and the epilogue-brace stop is still kept (S18). Developed test-first (red -> green -> review): 24 tests pinned the new behavior before implementation; full suite green (270 passing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 13e4a4c commit 50c258e

6 files changed

Lines changed: 299 additions & 117 deletions

File tree

docs/stepping.md

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,14 @@ frame) with no stop point at all.
6464
index 0. The user's first view is a real statement inside the invoked
6565
function, never the `#[contractimpl]` export shim or a bare signature line,
6666
and never a sourceless, addressless frame.
67-
- **S2** (forward exhaustion): a forward step (any kind, any granularity) with
68-
no further stop point ahead moves to the **last** stop point of the trace
69-
(staying put when already there) and still reports a stop. It never
70-
strands the cursor on trailing invisible/unmapped records.
67+
- **S2** (forward exhaustion): a forward step with no further stop point ahead
68+
never strands the cursor on trailing invisible/unmapped records. At
69+
**instruction** granularity — and in wasm-less replay, where statement
70+
stepping falls back to the visible records — it moves to the **last** stop
71+
point of the trace (staying put when already there) and still reports a stop.
72+
At **statement** granularity it instead **terminates the session** (S20): a
73+
forward source step past the last source statement finishes the replay rather
74+
than re-reporting the same line.
7175
- **S3** (backward exhaustion): a backward step with no earlier stop point
7276
moves to the **first** stop point (staying put when already there). It never
7377
falls off onto the unmapped records at the head of the trace.
@@ -80,13 +84,15 @@ frame) with no stop point at all.
8084
- **S5** (step over): `next` moves to the next run start whose depth is ≤ the
8185
current depth: a call inside the current line is stepped over in one press —
8286
including calls whose return is implicit (no `return` record). One press,
83-
one new line.
87+
one new line. Exhausted forward (no such run start ahead), it terminates the
88+
session (S20).
8489
- **S6** (per-iteration loop stops): a source line that executes again (loop
8590
back-edge) is a **new** run: `next`/`stepIn` stop once per iteration, even
8691
when consecutive iterations are separated only by unmapped records.
8792
- **S7** (step out): `stepOut` moves to the next run start whose depth is <
88-
the current depth. At the outermost recorded depth it behaves like S2
89-
(runs to the last stop point).
93+
the current depth. At the outermost recorded depth there is no shallower
94+
stop, so a statement `stepOut` terminates the session (S20); at instruction
95+
granularity it clamps to the last visible record (S2).
9096
- **S8** (reverse step): `stepBack` at statement granularity is the reverse of
9197
`next`: it moves to the start of the previous run with depth ≤ the current
9298
depth, skipping deeper-frame records. Landing is always on a run **start**
@@ -130,6 +136,27 @@ frame) with no stop point at all.
130136
the stack frame carries that record's source and line; the frame is
131137
sourceless only when the cursor legitimately rests on an unmapped stop point
132138
(instruction granularity, or no line info at all).
139+
- **S19** (line-start cursor): whenever the cursor rests on a mapped record, the
140+
stack frame's source **column** is the 1-based position of the **first
141+
non-whitespace character** of that source line — never the DWARF column,
142+
which for an opt-0 line table points at an arbitrary interior sub-expression
143+
(the `0` of `let mut acc = 0`, the `3` of `match x % 3`) and jumps
144+
unpredictably between visits of the same line. A statement stop already rests
145+
on the run start — the line's first executed instruction, before any
146+
sub-expression's result is in hand — so with S19 the cursor lands at the
147+
start of the statement, predictably, on every visit. Disassembly rows keep
148+
their precise per-instruction DWARF columns (`disassembleRequest` is
149+
unaffected); S19 governs only the source stack frame.
150+
- **S20** (statement forward-exhaustion terminates): a forward *statement* step
151+
(`next`/`stepIn`/`stepOut`, granularity ≠ `instruction`, over the source
152+
run-start stop set) that finds no qualifying stop point ahead ends the debug
153+
session with a `TerminatedEvent` instead of clamping in place. The replayed
154+
contract has returned from its outermost recorded frame; a user pressing
155+
step-over at the closing brace expects the program to finish, not the cursor
156+
to sit on the same line. Instruction-granularity forward steps still clamp
157+
(S2), `continue`/`reverseContinue` still settle on the last/first stop (S14),
158+
and reverse steps still clamp to the first stop (S3/S8) — only forward
159+
*source* stepping past the end terminates.
133160

134161
### Statement stops: declarations and braces
135162

src/debugAdapter/SorobanDebugSession.ts

Lines changed: 78 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,13 @@ import {
2323
import { DebugProtocol } from '@vscode/debugprotocol';
2424
import * as path from 'path';
2525
import { TraceModel } from './TraceModel';
26-
import { classifyLineRole, computeDepths, computeRunStarts, statementStops } from './stops';
26+
import {
27+
classifyLineRole,
28+
computeDepths,
29+
computeRunStarts,
30+
firstNonWhitespaceColumn,
31+
statementStops,
32+
} from './stops';
2733
import { SourceMapper } from '../sourcemap/SourceMapper';
2834
import { Disassembly } from '../wasm/Disassembly';
2935
import { ResolvedTrace, SessionBackend, SorobanLaunchArgs } from './types';
@@ -304,9 +310,18 @@ export class SorobanDebugSession extends DebugSession {
304310
const frameName = `${renderInstr(rec.instr)} [${this.model.cursor}/${this.model.length - 1}]`;
305311

306312
// Unmapped records get no Source at all (and line 0): the client keeps
307-
// showing the frame name instead of opening a wrong file.
313+
// showing the frame name instead of opening a wrong file. S19: a mapped
314+
// frame reports the line's first non-whitespace column, not the arbitrary
315+
// DWARF sub-expression column; fall back to the DWARF column when the line
316+
// text is unavailable or all-whitespace.
308317
const frame: DebugProtocol.StackFrame = loc
309-
? new StackFrame(FRAME_ID, frameName, new Source(path.basename(loc.path), loc.path), loc.line, loc.column ?? 0)
318+
? new StackFrame(
319+
FRAME_ID,
320+
frameName,
321+
new Source(path.basename(loc.path), loc.path),
322+
loc.line,
323+
firstNonWhitespaceColumn(this.source.sourceTextForIndex(this.model.cursor)) ?? loc.column ?? 0,
324+
)
310325
: new StackFrame(FRAME_ID, frameName);
311326
const reference = this.instructionPointerReference();
312327
if (reference !== undefined) {
@@ -443,7 +458,7 @@ export class SorobanDebugSession extends DebugSession {
443458
): void {
444459
this.sendResponse(response);
445460
// S5/S10: step over — the next stop point not in a deeper frame.
446-
this.stopAfter(() => this.moveCursor(this.stopPoints(args.granularity), 1, this.currentDepth()));
461+
this.stepForward(args.granularity, this.currentDepth());
447462
}
448463

449464
protected stepInRequest(
@@ -452,7 +467,7 @@ export class SorobanDebugSession extends DebugSession {
452467
): void {
453468
this.sendResponse(response);
454469
// S4/S10: step in — the next stop point regardless of depth.
455-
this.stopAfter(() => this.moveCursor(this.stopPoints(args.granularity), 1, Infinity));
470+
this.stepForward(args.granularity, Infinity);
456471
}
457472

458473
protected stepOutRequest(
@@ -461,8 +476,9 @@ export class SorobanDebugSession extends DebugSession {
461476
): void {
462477
this.sendResponse(response);
463478
// S7: step out — the next stop point in a shallower frame; at the
464-
// outermost recorded depth this exhausts and clamps like S2.
465-
this.stopAfter(() => this.moveCursor(this.stopPoints(args.granularity), 1, this.currentDepth() - 1));
479+
// outermost recorded depth this exhausts and terminates (S20) at statement
480+
// granularity, or clamps like S2 at instruction granularity.
481+
this.stepForward(args.granularity, this.currentDepth() - 1);
466482
}
467483

468484
// --- Reverse stepping (time travel) ----------------------------------
@@ -472,8 +488,10 @@ export class SorobanDebugSession extends DebugSession {
472488
args: DebugProtocol.StepBackArguments,
473489
): void {
474490
this.sendResponse(response);
475-
// S8/S10: reverse step over — the previous stop point not in a deeper frame.
476-
this.stopAfter(() => this.moveCursor(this.stopPoints(args.granularity), -1, this.currentDepth()));
491+
// S8/S10: reverse step over — the previous stop point not in a deeper
492+
// frame. Reverse steps always CLAMP to the first stop (S3/S8) and never
493+
// terminate; S20 is forward-only.
494+
this.stopAfter(() => this.moveCursorBackward(this.stopPoints(args.granularity), this.currentDepth()));
477495
}
478496

479497
protected reverseContinueRequest(
@@ -563,34 +581,65 @@ export class SorobanDebugSession extends DebugSession {
563581
}
564582

565583
/**
566-
* Move the cursor to the nearest stop point in `direction` whose depth is
567-
* <= `maxDepth`. With none ahead/behind, clamp to the last/first stop point
568-
* (S2/S3/S7) — staying put when already there. An empty stop-point list
569-
* (trace with no visible record at all) leaves the cursor where it is.
584+
* Forward step (S2/S5/S7/S20): seek to the first stop point after the cursor
585+
* whose depth is <= `maxDepth`, then report a stop. With no such stop ahead,
586+
* a STATEMENT step (granularity !== 'instruction' over the source run-start
587+
* stop set) TERMINATES the session (S20) — the replayed contract has returned
588+
* from its outermost recorded frame — while an INSTRUCTION step, or a
589+
* wasm-less replay with no run starts, clamps to the last stop point and
590+
* reports a stop (S2). Emits its own event, so it is called directly (not via
591+
* stopAfter): a termination must NOT be followed by a StoppedEvent.
570592
*/
571-
private moveCursor(points: readonly number[], direction: 1 | -1, maxDepth: number): void {
572-
if (!this.model || points.length === 0) {
593+
private stepForward(
594+
granularity: DebugProtocol.SteppingGranularity | undefined,
595+
maxDepth: number,
596+
): void {
597+
if (!this.model) {
573598
return;
574599
}
600+
const points = this.stopPoints(granularity);
575601
const cursor = this.model.cursor;
576-
if (direction > 0) {
577-
for (const i of points) {
578-
if (i > cursor && this.depths[i] <= maxDepth) {
579-
this.model.seek(i);
580-
return;
581-
}
602+
for (const i of points) {
603+
if (i > cursor && this.depths[i] <= maxDepth) {
604+
this.model.seek(i);
605+
this.sendEvent(new StoppedEvent('step', THREAD_ID));
606+
return;
582607
}
608+
}
609+
// Nothing qualifying ahead.
610+
if (granularity !== 'instruction' && this.runStarts.length > 0) {
611+
// S20: a forward source step past the last statement ends the session.
612+
this.sendEvent(new TerminatedEvent());
613+
return;
614+
}
615+
// S2: instruction granularity / wasm-less replay clamps to the last stop
616+
// point (staying put when already there) and still reports a stop.
617+
if (points.length > 0) {
583618
this.model.seek(points[points.length - 1]);
584-
} else {
585-
for (let k = points.length - 1; k >= 0; k--) {
586-
const i = points[k];
587-
if (i < cursor && this.depths[i] <= maxDepth) {
588-
this.model.seek(i);
589-
return;
590-
}
619+
}
620+
this.sendEvent(new StoppedEvent('step', THREAD_ID));
621+
}
622+
623+
/**
624+
* Reverse step (S3/S8): move the cursor to the nearest earlier stop point
625+
* whose depth is <= `maxDepth`. With none behind, clamp to the first stop
626+
* point (staying put when already there). An empty stop-point list (trace
627+
* with no visible record at all) leaves the cursor where it is. Reverse steps
628+
* always clamp and never terminate.
629+
*/
630+
private moveCursorBackward(points: readonly number[], maxDepth: number): void {
631+
if (!this.model || points.length === 0) {
632+
return;
633+
}
634+
const cursor = this.model.cursor;
635+
for (let k = points.length - 1; k >= 0; k--) {
636+
const i = points[k];
637+
if (i < cursor && this.depths[i] <= maxDepth) {
638+
this.model.seek(i);
639+
return;
591640
}
592-
this.model.seek(points[0]);
593641
}
642+
this.model.seek(points[0]);
594643
}
595644

596645
/** The trace's first stop point: run start, else visible record, else 0. */

src/debugAdapter/stops.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,24 @@ export function statementStops(
230230
return result;
231231
}
232232

233+
/**
234+
* The 1-based column of the first non-whitespace character of a source line
235+
* (S19). Returns null when the text is null or entirely whitespace, so the
236+
* caller can fall back to the DWARF column. This is what a statement stop's
237+
* stack frame reports instead of the arbitrary DWARF sub-expression column.
238+
*/
239+
export function firstNonWhitespaceColumn(text: string | null): number | null {
240+
if (text === null) {
241+
return null;
242+
}
243+
for (let i = 0; i < text.length; i++) {
244+
if (!/\s/.test(text[i])) {
245+
return i + 1;
246+
}
247+
}
248+
return null;
249+
}
250+
233251
/** Index of the sorted range containing `pos`, or -1 when outside all of them. */
234252
function functionIndexAt(ranges: readonly FunctionRange[], pos: number): number {
235253
let lo = 0;

test/dap.test.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -184,12 +184,9 @@ describe('SorobanDebugSession (DAP replay)', () => {
184184
assert.strictEqual(frame.line, 16);
185185
assert.ok(frame.name.includes('[29/40]'), `unexpected frame name: ${frame.name}`);
186186

187-
// Statement granularity: the :16 body is the only statement stop, so a
188-
// step stays put on it (the :12 shim runs are dropped by S17).
189-
await stopAfter(dc.nextRequest({ ...THREAD, granularity: 'statement' }), 'step');
190-
frame = await topFrame();
191-
assert.ok(frame.source?.path?.endsWith(LIB_RS_SUFFIX));
192-
assert.strictEqual(frame.line, 16);
187+
// (A forward statement next here would exhaust the single stop and END the
188+
// session under S20 — see the dedicated S20 test below — so it is omitted
189+
// from this breakpoint/reverse-continue flow.)
193190

194191
// Move forward off the run start, then reverse-continue lands on the same
195192
// run START (29), not on the run's last record (33).
@@ -231,16 +228,21 @@ describe('SorobanDebugSession (DAP replay)', () => {
231228
assert.ok(frame.name.includes('[40/40]'), `unexpected frame name: ${frame.name}`);
232229
});
233230

234-
it('default-granularity next clamps on the single statement stop', async () => {
231+
it('S20: default-granularity next past the single statement stop terminates', async () => {
235232
await launchAndStop(WITH_WASM);
236233
// The entry stop is the only statement stop, lib.rs:16 (record 29); the
237-
// :12 #[contractimpl] runs are dropped by S17, so a default-granularity
238-
// next has nowhere to advance and stays put (S2).
239-
await stopAfter(dc.nextRequest(THREAD), 'step');
240-
const frame = await topFrame();
241-
assert.ok(frame.source?.path?.endsWith(LIB_RS_SUFFIX), `unexpected source: ${frame.source?.path}`);
242-
assert.strictEqual(frame.line, 16);
243-
assert.ok(frame.name.includes('[29/40]'), `unexpected frame name: ${frame.name}`);
234+
// :12 #[contractimpl] runs are dropped by S17. A default-granularity next
235+
// (granularity !== 'instruction', so it is a statement step) has nowhere to
236+
// advance and ENDS the session (S20) instead of clamping in place. Race
237+
// 'terminated' vs 'stopped' so a clamp-in-place regression fails fast
238+
// rather than blocking on a 'stopped' that never comes.
239+
const req = dc.nextRequest(THREAD);
240+
const ended = await Promise.race([
241+
dc.waitForEvent('terminated').then(() => true),
242+
dc.waitForEvent('stopped').then(() => false),
243+
]);
244+
await req;
245+
assert.ok(ended, 'expected the default-granularity next past the last statement stop to terminate (S20)');
244246
});
245247

246248
it('instruction-granularity stepIn advances exactly one record', async () => {

0 commit comments

Comments
 (0)