From 03f4a7ed93ef31eb06cad9debcfd14d1e45d2ffc Mon Sep 17 00:00:00 2001 From: Claes Lillieskold Date: Thu, 5 Nov 2026 21:44:39 +0100 Subject: [PATCH] Disable breakpoints instead of removing them Unchecking a breakpoint in VS Code removed it from the TCF agent, and checking it again added it back with a new ID. The Breakpoints service has enable and disable commands for this, so use them and keep the breakpoint in the agent. DAP has no enabled flag. VS Code leaves a disabled breakpoint out of the next set*Breakpoints request, the same as a deleted one. The extension API still lists disabled breakpoints, so the adapter checks that list: breakpoints still in it are disabled, the others removed. VS Code does not show instruction breakpoints to extensions, so these are still removed when unchecked. Deactivating all breakpoints in VS Code now disables them in the agent, as VS Code still lists them. Signed-off-by: Claes Lillieskold --- src/breakpointManager.ts | 101 ++++++++++++++++++++++++++--- src/extension.ts | 49 +++++++++++++- src/tcf/services/breakpoints.ts | 10 +++ src/tcfDebugSession.ts | 9 ++- src/test/breakpointManager.test.ts | 90 ++++++++++++++++++++++++- 5 files changed, 245 insertions(+), 14 deletions(-) diff --git a/src/breakpointManager.ts b/src/breakpointManager.ts index 4cca440..a279941 100644 --- a/src/breakpointManager.ts +++ b/src/breakpointManager.ts @@ -52,8 +52,18 @@ interface ManagedBreakpoint extends BreakpointSpec { reported: boolean; /** The last DAP breakpoint sent to VS Code, to avoid sending unchanged ones. */ lastReported?: string; + /** The line last sent to VS Code, which VS Code uses as the breakpoint's line. */ + reportedLine?: number; + /** Disabled in the agent: VS Code left it out of the last request, but still has it. */ + disabled?: boolean; } +/** A breakpoint that VS Code left out of a set*Breakpoints request. */ +export type OmittedBreakpoint = + | { kind: "source"; path: string; lines: number[]; column?: number } + | { kind: "function"; name: string } + | { kind: "data"; address: bigint }; + export interface BreakpointHost { sendEvent(event: DebugProtocol.Event): void; /** Maps an address in a context to a source location on the host, if known. */ @@ -69,6 +79,11 @@ export interface BreakpointHost { processContexts(contextId: string): string[]; /** The context of instruction references without one. */ defaultContext(): string | undefined; + /** + * True if VS Code still has an omitted breakpoint, which is then disabled + * (or all breakpoints deactivated) rather than deleted. + */ + isDisabled?(bp: OmittedBreakpoint): boolean; } export interface BreakpointHits { @@ -89,9 +104,9 @@ const ACCESS_TYPES: Record = { /** * Keeps the TCF agent's breakpoint table in sync with the breakpoints in VS - * Code. VS Code always sends the full list of breakpoints of one kind (or one - * source file), which is diffed against what is planted to send Breakpoints - * add/change/remove commands. + * Code. VS Code always sends the full list of enabled breakpoints of one kind + * (or one source file), which is diffed against what is planted to send + * Breakpoints add/change/enable/disable/remove commands. */ export class BreakpointManager { private readonly idPrefix = `vscode-tcf-${randomUUID()}`; @@ -454,18 +469,23 @@ export class BreakpointManager { const toChange: ManagedBreakpoint[] = []; const toRemove: string[] = []; + const toEnable: ManagedBreakpoint[] = []; + const toDisable: ManagedBreakpoint[] = []; + for (const spec of specs) { const old = unused.get(spec.key); if (old) { unused.delete(spec.key); const propsChanged = JSON.stringify(old.props) !== JSON.stringify(spec.props); + const wasDisabled = old.disabled === true; Object.assign(old, spec); old.error = spec.error; old.logMessage = spec.logMessage; old.scopeContext = spec.scopeContext; - if (propsChanged) { + old.disabled = false; + if (propsChanged || wasDisabled) { if (old.tcfId && spec.props) { - toChange.push(old); + (propsChanged ? toChange : toEnable).push(old); } else if (old.tcfId) { toRemove.push(this.forget(old)); } else if (spec.props) { @@ -481,8 +501,21 @@ export class BreakpointManager { result.push(bp); } } + // Kept after the requested ones, which report() returns in request order. for (const bp of unused.values()) { - if (bp.tcfId) { + if (!bp.tcfId) { + continue; + } + const left = omitted(bp); + if (left && this.host.isDisabled?.(left)) { + if (!bp.disabled) { + bp.disabled = true; + toDisable.push(bp); + } + // VS Code no longer knows the DAP ID. + bp.reported = false; + result.push(bp); + } else { toRemove.push(this.forget(bp)); } } @@ -497,10 +530,36 @@ export class BreakpointManager { if (toRemove.length > 0) { await this.service.remove(toRemove).catch(() => undefined); } - await Promise.all([...toAdd.map((bp) => this.plant(bp, false)), ...toChange.map((bp) => this.plant(bp, true))]); + await Promise.all([ + this.setEnabled(toDisable, false), + this.setEnabled(toEnable, true), + ...toAdd.map((bp) => this.plant(bp, false)), + ...toChange.map((bp) => this.plant(bp, true)), + ]); return result; } + private async setEnabled(bps: ManagedBreakpoint[], enabled: boolean): Promise { + if (bps.length === 0) { + return; + } + const service = this.service!; + const ids = bps.map((bp) => bp.tcfId!); + try { + await (enabled ? service.enable(ids) : service.disable(ids)); + } catch { + await Promise.all(bps.map((bp) => this.plant(bp, true))); + return; + } + if (enabled) { + await Promise.all( + bps.map(async (bp) => { + bp.status = await service.getStatus(bp.tcfId!).catch(() => bp.status); + }), + ); + } + } + private async plant(bp: ManagedBreakpoint, change: boolean): Promise { const service = this.service!; if (!change) { @@ -509,11 +568,12 @@ export class BreakpointManager { } const id = bp.tcfId!; bp.status = undefined; + const props: BreakpointProperties = { ...bp.props, ID: id, ...(bp.disabled && { Enabled: false }) }; try { if (change) { - await service.change({ ...bp.props, ID: id }); + await service.change(props); } else { - await service.add({ ...bp.props, ID: id }); + await service.add(props); } } catch (err) { this.forget(bp); @@ -537,11 +597,13 @@ export class BreakpointManager { return id; } - private async report(bps: ManagedBreakpoint[]): Promise { + private async report(all: ManagedBreakpoint[]): Promise { + const bps = all.filter((bp) => !bp.disabled); const result = await Promise.all(bps.map((bp) => this.toDap(bp))); bps.forEach((bp, i) => { bp.reported = true; bp.lastReported = JSON.stringify(result[i]); + bp.reportedLine = result[i].line; }); return result; } @@ -552,6 +614,7 @@ export class BreakpointManager { const json = JSON.stringify(dap); if (bp.reported && json !== bp.lastReported) { bp.lastReported = json; + bp.reportedLine = dap.line; this.host.sendEvent(new BreakpointEvent("changed", dap)); } } @@ -646,6 +709,24 @@ export function parseHitCondition(hitCondition: string): number | undefined { return Math.max(Number(m[1]) - 1, 0); } +function omitted(bp: ManagedBreakpoint): OmittedBreakpoint | undefined { + switch (bp.kind) { + case "source": { + const lines = [bp.line, bp.reportedLine].filter((l): l is number => l !== undefined); + return { kind: "source", path: bp.source?.path ?? bp.source?.name ?? "", lines, column: bp.column }; + } + case "function": + return { kind: "function", name: bp.key }; + case "data": { + const target = parseDataId(bp.key.slice(0, bp.key.lastIndexOf("|"))); + return target && { kind: "data", address: target.address }; + } + default: + // VS Code does not show instruction breakpoints to extensions. + return undefined; + } +} + function parseDataId(dataId: string): { address: bigint; size: number } | undefined { const m = /^(0x[0-9a-f]+):(\d+)$/i.exec(dataId); return m ? { address: BigInt(m[1]), size: Number(m[2]) } : undefined; diff --git a/src/extension.ts b/src/extension.ts index ab5d0ce..8a1dcf8 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -2,6 +2,7 @@ import * as fs from "fs"; import * as path from "path"; import * as vscode from "vscode"; import { DebugProtocol } from "@vscode/debugprotocol"; +import { OmittedBreakpoint } from "./breakpointManager"; import { parseCallStackContext } from "./callStackFrame"; import { parseInstructionReference } from "./disassembly"; import { registerFunctionHeaderFileSystem } from "./functionHeaderFileSystem"; @@ -45,7 +46,7 @@ export function activate(context: vscode.ExtensionContext): void { vscode.debug.registerDebugConfigurationProvider(DEBUG_TYPE, new TcfConfigurationProvider()), vscode.debug.registerDebugAdapterDescriptorFactory(DEBUG_TYPE, { createDebugAdapterDescriptor: (session) => { - const adapter = new TcfDebugSession(sessionLog(tcfLog, session.name)); + const adapter = new TcfDebugSession(sessionLog(tcfLog, session.name), hasBreakpoint); adapters.set(session.id, adapter); return new vscode.DebugAdapterInlineImplementation(adapter); }, @@ -125,6 +126,52 @@ export function deactivate(): void { // Nothing to clean up, sessions are disposed by VS Code. } +/** + * True if VS Code still has a breakpoint, enabled or not, that it left out of + * a set*Breakpoints request. VS Code updates this list before it sends the + * request. + */ +function hasBreakpoint(omitted: OmittedBreakpoint): boolean { + return vscode.debug.breakpoints.some((bp) => { + switch (omitted.kind) { + case "source": { + if (!(bp instanceof vscode.SourceBreakpoint)) { + return false; + } + const { uri, range } = bp.location; + const file = uri.scheme === "file" ? uri.fsPath : uri.toString(); + const column = range.start.character + 1; + return ( + path.normalize(file) === path.normalize(omitted.path) && + omitted.lines.includes(range.start.line + 1) && + (omitted.column === undefined || column === 1 || column === omitted.column) + ); + } + case "function": + return bp instanceof vscode.FunctionBreakpoint && bp.functionName === omitted.name; + case "data": + return dataBreakpointAddress(bp) === omitted.address; + } + }); +} + +/** + * The address of a data breakpoint. vscode.DataBreakpoint is newer than the + * API version used here; its dataId is ours ("
:") or the + * address entered in VS Code. + */ +function dataBreakpointAddress(bp: vscode.Breakpoint): bigint | undefined { + const dataId: unknown = (bp as { dataId?: unknown }).dataId; + if (typeof dataId !== "string") { + return undefined; + } + try { + return BigInt(dataId.split(":")[0].trim()); + } catch { + return undefined; + } +} + /** * Copies the address of a Call Stack frame: the PC for the top frame, the * return address for callers. diff --git a/src/tcf/services/breakpoints.ts b/src/tcf/services/breakpoints.ts index 7d1da04..fe8780b 100644 --- a/src/tcf/services/breakpoints.ts +++ b/src/tcf/services/breakpoints.ts @@ -76,6 +76,16 @@ export class BreakpointsService { await this.channel.command(BreakpointsService.NAME, "change", bp); } + /** Sets only the Enabled property of the breakpoints to true. */ + async enable(ids: string[]): Promise { + await this.channel.command(BreakpointsService.NAME, "enable", ids); + } + + /** Sets only the Enabled property of the breakpoints to false. */ + async disable(ids: string[]): Promise { + await this.channel.command(BreakpointsService.NAME, "disable", ids); + } + async remove(ids: string[]): Promise { await this.channel.command(BreakpointsService.NAME, "remove", ids); } diff --git a/src/tcfDebugSession.ts b/src/tcfDebugSession.ts index b152e81..7b39777 100644 --- a/src/tcfDebugSession.ts +++ b/src/tcfDebugSession.ts @@ -36,7 +36,7 @@ import { } from "./disassembly"; import { FunctionInfo, FunctionRanges } from "./functionHeaders"; import { MemoryAccessMode, MemoryService } from "./tcf/services/memory"; -import { BreakpointHits, BreakpointManager } from "./breakpointManager"; +import { BreakpointHits, BreakpointManager, OmittedBreakpoint } from "./breakpointManager"; import { VariableNode, VariablesProvider } from "./variables"; import { PathMapper, PathMapRule, parsePathMapConfig } from "./pathMapper"; import { PathMapService } from "./tcf/services/pathMap"; @@ -220,7 +220,11 @@ export class TcfDebugSession extends DebugSession { private readonly regContextCache = new Map>(); private readonly regChildrenCache = new Map>(); - constructor(private readonly tcfLog?: TcfLog) { + constructor( + private readonly tcfLog?: TcfLog, + /** See BreakpointHost.isDisabled. */ + private readonly isBreakpointDisabled?: (bp: OmittedBreakpoint) => boolean, + ) { super(); this.setDebuggerLinesStartAt1(true); this.setDebuggerColumnsStartAt1(true); @@ -397,6 +401,7 @@ export class TcfDebugSession extends DebugSession { toTargetPath: (hostPath, contextId) => pathMapper.toTargetLocation(hostPath, contextId), processContexts: (ctxId) => this.processContexts(ctxId), defaultContext: () => this.suspendedThreadContext(), + isDisabled: (bp) => this.isBreakpointDisabled?.(bp) ?? false, }, ); diff --git a/src/test/breakpointManager.test.ts b/src/test/breakpointManager.test.ts index 1bf7b78..f3d993a 100644 --- a/src/test/breakpointManager.test.ts +++ b/src/test/breakpointManager.test.ts @@ -1,6 +1,6 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { BreakpointHost, BreakpointManager, parseHitCondition } from "../breakpointManager"; +import { BreakpointHost, BreakpointManager, OmittedBreakpoint, parseHitCondition } from "../breakpointManager"; import { instructionReference } from "../disassembly"; import { formatFunctionBreakpointName, parseFunctionBreakpointName } from "../functionBreakpoint"; import { PathMapper } from "../pathMapper"; @@ -51,6 +51,12 @@ function fakeAgent(): FakeAgent { remove: async (ids: string[]) => { agent.commands.push(["remove", ids]); }, + enable: async (ids: string[]) => { + agent.commands.push(["enable", ids]); + }, + disable: async (ids: string[]) => { + agent.commands.push(["disable", ids]); + }, getStatus: async (): Promise => ({ Instances: [{ LocationContext: "thread1", Address: 0x1010 }], }), @@ -121,6 +127,88 @@ test("scoped function breakpoints never fall back to global planting", async () assert.equal(agent.commands.length, 0); }); +// VS Code leaves disabled breakpoints out of setBreakpoints, like deleted +// ones; only the host can tell them apart. +test("breakpoints that VS Code still has are disabled, not removed", async () => { + const agent = fakeAgent(); + const host = fakeHost(["thread1"]); + let kept = true; + const asked: OmittedBreakpoint[] = []; + host.isDisabled = (bp) => { + asked.push(bp); + return kept; + }; + const manager = new BreakpointManager(agent.service, host); + const source = { path: "/host/main.c" }; + const [before] = await manager.setSourceBreakpoints(source, [{ line: 12 }]); + const id = (agent.commands[0][1] as BreakpointProperties).ID; + + assert.deepEqual(await manager.setSourceBreakpoints(source, []), []); + assert.deepEqual(agent.commands[1], ["disable", [id]]); + assert.deepEqual(asked, [{ kind: "source", path: "/host/main.c", lines: [12, 12], column: undefined }]); + + const [after] = await manager.setSourceBreakpoints(source, [{ line: 12 }]); + assert.deepEqual(agent.commands[2], ["enable", [id]]); + assert.equal(after.id, before.id); + assert.equal(after.verified, true); + + await manager.setSourceBreakpoints(source, []); + kept = false; + await manager.setSourceBreakpoints(source, []); + assert.deepEqual(agent.commands.slice(3), [ + ["disable", [id]], + ["remove", [id]], + ]); +}); + +test("a disabled breakpoint stays disabled when it is changed", async () => { + const agent = fakeAgent(); + const host = fakeHost(["thread1"]); + let prefix = "/old"; + host.toTargetPath = async () => `${prefix}/main.c`; + host.isDisabled = () => true; + const manager = new BreakpointManager(agent.service, host); + const source = { path: "/host/main.c" }; + await manager.setSourceBreakpoints(source, [{ line: 12 }]); + await manager.setSourceBreakpoints(source, []); + prefix = "/new"; + await manager.updateSourcePaths(); + const [command, props] = agent.commands[2] as [string, BreakpointProperties]; + assert.equal(command, "change"); + assert.equal(props.File, "/new/main.c"); + assert.equal(props.Enabled, false); + assert.equal(host.events.length, 0); + + await manager.setSourceBreakpoints(source, [{ line: 12, condition: "x" }]); + const [again, enabled] = agent.commands[3] as [string, BreakpointProperties]; + assert.equal(again, "change"); + assert.equal(enabled.Enabled, true); + assert.equal(enabled.Condition, "x"); +}); + +test("function and data breakpoints are disabled too", async () => { + const agent = fakeAgent(); + const host = fakeHost(["thread1"]); + const asked: OmittedBreakpoint[] = []; + host.isDisabled = (bp) => { + asked.push(bp); + return true; + }; + const manager = new BreakpointManager(agent.service, host); + await manager.setFunctionBreakpoints([{ name: "main" }]); + await manager.setFunctionBreakpoints([]); + await manager.setDataBreakpoints([{ dataId: "0x1000:4", accessType: "write" }]); + await manager.setDataBreakpoints([]); + assert.deepEqual(asked, [ + { kind: "function", name: "main" }, + { kind: "data", address: 0x1000n }, + ]); + assert.deepEqual( + agent.commands.map(([c]) => c), + ["add", "disable", "add", "disable"], + ); +}); + test("path map edits remap planted source breakpoints without changing IDs", async () => { const agent = fakeAgent(); const host = fakeHost(["thread1"]);