diff --git a/README.md b/README.md index 8124322..29b0ba3 100644 --- a/README.md +++ b/README.md @@ -192,15 +192,30 @@ is not used, to stay within the TCF specification. Use **Simics Target Debug: Add Function Breakpoint in Context...** from the Command Palette, the Breakpoints toolbar, or a stack frame's context menu to -choose a core before entering the symbol. The selected call-stack context is -offered first; **All contexts** keeps the breakpoint global. Scoped entries -appear as `start_kernel @ armv8.cluster[0].core[0]` in the Breakpoints panel. -The ` @ ` suffix is reserved for the exact TCF context ID and is saved with -the breakpoint; selecting another frame does not change its scope. The same -syntax can be entered or edited using the built-in function-breakpoint `+`. +check one or more cores before entering the symbol. The selected call-stack +context is listed first and checked; accept with nothing checked to keep the +breakpoint global. Scoped entries appear as +`start_kernel @ armv8.cluster[0].core[0]` in the Breakpoints panel, or with a +comma-separated list such as +`start_kernel @ armv8.cluster[0].core[0], armv8.cluster[0].core[1]` when +several cores are checked. The ` @ ` suffix is reserved for the exact TCF +context IDs, which are sent as the breakpoint's `ContextIds` and saved with +the breakpoint; selecting another frame does not change its scope. Context IDs +that contain commas cannot be used. The same syntax can be entered or edited +using the built-in function-breakpoint `+` or **Edit Function Breakpoint...**. Scoped function breakpoints require the agent's `ContextIds` support and are never silently planted globally when it is unavailable. Their scope applies -to that context (and, for a process context, its threads), not other cores. +to those contexts (and, for a process context, its threads), not other cores. + +To change the contexts of an existing function breakpoint, use **Simics Target +Debug: Edit Function Breakpoint Contexts...** from the Command Palette or the +edit icon in the Breakpoints toolbar. VS Code does not tell extensions which +breakpoint is selected in the Breakpoints panel, so the command asks for the +function breakpoint when there is more than one. The checklist starts with the +breakpoint's current contexts checked; contexts that are not in the current +session stay listed so they are not dropped by accident. The breakpoint is +replaced by one with the new name and the same enabled state, condition, hit +count, and log message, so it moves to the end of the Breakpoints panel. In **Symbol Files**, right-click a configured symbol file with a context and select **Add Function Breakpoint...** to enter a symbol directly. The breakpoint diff --git a/package.json b/package.json index 7696ef7..c827584 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,12 @@ "category": "Simics Target Debug", "icon": "$(debug-breakpoint-function)" }, + { + "command": "simics-target-debug.editFunctionBreakpointContexts", + "title": "Edit Function Breakpoint Contexts...", + "category": "Simics Target Debug", + "icon": "$(edit)" + }, { "command": "simics-target-debug.addPathMapRule", "title": "Add Path Map Rule...", @@ -256,6 +262,10 @@ "command": "simics-target-debug.addFunctionBreakpointInContext", "when": "debugType == 'simics-target-debug'" }, + { + "command": "simics-target-debug.editFunctionBreakpointContexts", + "when": "debugType == 'simics-target-debug'" + }, { "command": "simics-target-debug.saveSymbolFiles", "when": "debugType == 'simics-target-debug'" @@ -405,8 +415,13 @@ "view/title": [ { "command": "simics-target-debug.addFunctionBreakpointInContext", - "when": "view == 'workbench.debug.viewlet.state' && debugType == 'simics-target-debug'", - "group": "navigation@1" + "when": "view == 'workbench.debug.breakPointsView' && debugType == 'simics-target-debug'", + "group": "navigation@10.1" + }, + { + "command": "simics-target-debug.editFunctionBreakpointContexts", + "when": "view == 'workbench.debug.breakPointsView' && debugType == 'simics-target-debug'", + "group": "navigation@10.2" }, { "command": "simics-target-debug.openConsole", diff --git a/src/breakpointManager.ts b/src/breakpointManager.ts index 4cca440..04ef449 100644 --- a/src/breakpointManager.ts +++ b/src/breakpointManager.ts @@ -219,19 +219,22 @@ export class BreakpointManager { const caps = await this.capabilities(); const specs = requested.map((r): BreakpointSpec => { const spec: BreakpointSpec = { kind: "function", key: r.name }; - const { location, contextId, hasContextDelimiter } = parseFunctionBreakpointName(r.name); + const { location, contextIds, hasContextDelimiter, hasEmptyContext } = parseFunctionBreakpointName(r.name); if (caps.Location === false) { return { ...spec, error: "The TCF agent does not support breakpoints on expressions" }; } - if (hasContextDelimiter && (!contextId || !location)) { - return { ...spec, error: "Use 'symbol @ context ID' for a context-scoped function breakpoint" }; + if (hasContextDelimiter && (hasEmptyContext || !location)) { + return { + ...spec, + error: "Use 'symbol @ context ID' or 'symbol @ ID1, ID2' for a context-scoped function breakpoint", + }; } - if (contextId && caps.ContextIds === false) { + if (contextIds.length > 0 && caps.ContextIds === false) { return { ...spec, error: "Context-scoped function breakpoints require breakpoint ContextIds support" }; } return this.withCommonProps( spec, - { Location: location, ...(contextId && { ContextIds: [contextId] }) }, + { Location: location, ...(contextIds.length > 0 && { ContextIds: contextIds }) }, r, caps, ); diff --git a/src/extension.ts b/src/extension.ts index ab5d0ce..207aad0 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -7,7 +7,13 @@ import { parseInstructionReference } from "./disassembly"; import { registerFunctionHeaderFileSystem } from "./functionHeaderFileSystem"; import { applyAndSave, registerLaunchSettings } from "./launchSettings/editorProvider"; import { setNamedConfigurationValue } from "./launchSettings/launchJson"; -import { formatFunctionBreakpointName } from "./functionBreakpoint"; +import { + FunctionBreakpointContext, + FunctionBreakpointContextChoice, + formatFunctionBreakpointName, + functionBreakpointContextChoices, + parseFunctionBreakpointName, +} from "./functionBreakpoint"; import { MissingSource, SourceLocator } from "./locateSource"; import { pathMapProblems } from "./pathMapper"; import { registerPathMapView } from "./pathMapView"; @@ -96,6 +102,10 @@ export function activate(context: vscode.ExtensionContext): void { "simics-target-debug.addFunctionBreakpointInContext", (_source: unknown, frameContext: unknown) => addFunctionBreakpointInContext(sessions, frameContext), ), + vscode.commands.registerCommand( + "simics-target-debug.editFunctionBreakpointContexts", + editFunctionBreakpointContexts, + ), vscode.commands.registerCommand( "simics-target-debug.addSymbolFileBreakpoint", (item?: SymbolFileViewItem) => item && addFunctionBreakpointInContext(sessions, undefined, item), @@ -182,51 +192,26 @@ async function addFunctionBreakpointInContext( } const threadId = frame?.threadId ?? (active?.session.id === session.id ? active.threadId : undefined); try { - const contexts = (await session.customRequest( - "getFunctionBreakpointContexts", - symbolFile ? { includeContainers: true } : undefined, - )) as { - id: string; - name: string; - threadId?: number; - }[]; + const contexts = await functionBreakpointContexts(session, symbolFile !== undefined); const selected = contexts.find((context) => threadId !== undefined && context.threadId === threadId); - const choices: SymbolContextChoice[] = [ - ...(selected - ? [ - { - label: selected.name, - description: "Selected call-stack context", - detail: selected.id, - id: selected.id, - }, - ] - : []), - { label: "All contexts", description: "No context filter", id: "" }, - ...contexts - .filter((context) => context !== selected) - .map((context) => ({ label: context.name, description: context.id, id: context.id })), - ]; - const context = symbolFile - ? { id: symbolFileBreakpointContext(symbolFile.file, contexts) } - : await vscode.window.showQuickPick(choices, { - title: "Function Breakpoint Context", - placeHolder: "Choose a core or all contexts", - matchOnDescription: true, - matchOnDetail: true, - }); - if (!context) { + const contextIds = symbolFile + ? [symbolFileBreakpointContext(symbolFile.file, contexts)] + : await pickFunctionBreakpointContexts( + "Function Breakpoint Contexts", + functionBreakpointContextChoices(contexts, selected ? [selected.id] : [], selected?.id), + ); + if (!contextIds) { return; } const symbol = await vscode.window.showInputBox({ title: "Add Function Breakpoint", - prompt: context.id ? `Symbol in ${context.id}` : "Symbol in all contexts", + prompt: contextIds.length > 0 ? `Symbol in ${contextIds.join(", ")}` : "Symbol in all contexts", validateInput: (value) => (!value.trim() ? "Enter a symbol or expression" : undefined), }); if (!symbol?.trim()) { return; } - const name = formatFunctionBreakpointName(symbol.trim(), context.id); + const name = formatFunctionBreakpointName(symbol.trim(), contextIds); if ( !vscode.debug.breakpoints.some((bp) => bp instanceof vscode.FunctionBreakpoint && bp.functionName === name) ) { @@ -239,6 +224,117 @@ async function addFunctionBreakpointInContext( } } +function functionBreakpointContexts( + session: vscode.DebugSession, + includeContainers: boolean, +): Promise<(FunctionBreakpointContext & { threadId?: number })[]> { + return session.customRequest( + "getFunctionBreakpointContexts", + includeContainers ? { includeContainers: true } : undefined, + ) as Promise<(FunctionBreakpointContext & { threadId?: number })[]>; +} + +/** Returns the checked context IDs, empty for all contexts, or undefined if cancelled. */ +async function pickFunctionBreakpointContexts( + title: string, + choices: FunctionBreakpointContextChoice[], +): Promise { + const picked = await vscode.window.showQuickPick(choices, { + title, + placeHolder: "Check the cores to stop in, or check none for all contexts", + canPickMany: true, + matchOnDescription: true, + matchOnDetail: true, + }); + return picked?.map((choice) => choice.id); +} + +/** + * Changes the contexts of a function breakpoint. VS Code cannot rename a + * breakpoint, so it is replaced by one with the same settings. + */ +async function editFunctionBreakpointContexts(): Promise { + const active = vscode.debug.activeStackItem; + const session = active?.session ?? vscode.debug.activeDebugSession; + if (session?.type !== DEBUG_TYPE) { + void vscode.window.showErrorMessage( + "Start or select an active Simics Target Debug session before editing function breakpoint contexts.", + ); + return; + } + const breakpoints = vscode.debug.breakpoints.filter( + (bp): bp is vscode.FunctionBreakpoint => bp instanceof vscode.FunctionBreakpoint, + ); + if (breakpoints.length === 0) { + void vscode.window.showInformationMessage("There are no function breakpoints to edit."); + return; + } + const breakpoint = + breakpoints.length === 1 + ? breakpoints[0] + : ( + await vscode.window.showQuickPick( + breakpoints.map((bp) => ({ + label: bp.functionName, + description: [ + !bp.enabled && "Disabled", + bp.condition && `Condition: ${bp.condition}`, + bp.hitCondition && `Hit count: ${bp.hitCondition}`, + bp.logMessage && `Log message: ${bp.logMessage}`, + ] + .filter(Boolean) + .join(", "), + breakpoint: bp, + })), + { title: "Edit Function Breakpoint Contexts", placeHolder: "Choose a function breakpoint" }, + ) + )?.breakpoint; + if (!breakpoint) { + return; + } + const { location, contextIds } = parseFunctionBreakpointName(breakpoint.functionName); + if (!location) { + void vscode.window.showErrorMessage(`'${breakpoint.functionName}' has no symbol to scope.`); + return; + } + try { + const contexts = await functionBreakpointContexts(session, false); + const threadId = active?.session.id === session.id ? active.threadId : undefined; + const selected = contexts.find((context) => threadId !== undefined && context.threadId === threadId); + const ids = await pickFunctionBreakpointContexts( + `Contexts for ${location}`, + functionBreakpointContextChoices(contexts, contextIds, selected?.id), + ); + if (!ids) { + return; + } + const name = formatFunctionBreakpointName(location, ids); + if (name === breakpoint.functionName) { + return; + } + if ( + vscode.debug.breakpoints.some((bp) => bp instanceof vscode.FunctionBreakpoint && bp.functionName === name) + ) { + void vscode.window.showErrorMessage(`The function breakpoint '${name}' already exists.`); + return; + } + vscode.debug.removeBreakpoints([breakpoint]); + vscode.debug.addBreakpoints([ + new vscode.FunctionBreakpoint( + name, + breakpoint.enabled, + breakpoint.condition, + breakpoint.hitCondition, + breakpoint.logMessage, + ), + ]); + } catch (err) { + void vscode.window.showErrorMessage( + `Could not edit function breakpoint contexts: ${err instanceof Error ? err.message : String(err)}`, + ); + } +} + async function addSymbolFile(symbolFiles: SymbolFilesProvider): Promise { const session = vscode.debug.activeDebugSession; if (session?.type !== DEBUG_TYPE) { diff --git a/src/functionBreakpoint.ts b/src/functionBreakpoint.ts index 7f42cf9..0c74757 100644 --- a/src/functionBreakpoint.ts +++ b/src/functionBreakpoint.ts @@ -1,23 +1,84 @@ export const FUNCTION_BREAKPOINT_CONTEXT_SEPARATOR = " @ "; +export const FUNCTION_BREAKPOINT_CONTEXT_LIST_SEPARATOR = ", "; export interface ParsedFunctionBreakpointName { location: string; - contextId?: string; + /** The distinct context IDs after the separator, empty for a global breakpoint. */ + contextIds: string[]; hasContextDelimiter: boolean; + /** True if the context list has an empty entry, for example "f @ " or "f @ a, , b". */ + hasEmptyContext: boolean; } -export function formatFunctionBreakpointName(location: string, contextId?: string): string { - return contextId ? `${location}${FUNCTION_BREAKPOINT_CONTEXT_SEPARATOR}${contextId}` : location; +export function formatFunctionBreakpointName(location: string, contextIds: readonly string[] = []): string { + return contextIds.length > 0 + ? `${location}${FUNCTION_BREAKPOINT_CONTEXT_SEPARATOR}${contextIds.join(FUNCTION_BREAKPOINT_CONTEXT_LIST_SEPARATOR)}` + : location; } export function parseFunctionBreakpointName(name: string): ParsedFunctionBreakpointName { const separator = name.lastIndexOf(FUNCTION_BREAKPOINT_CONTEXT_SEPARATOR); - const hasContextDelimiter = separator >= 0; + if (separator < 0) { + return { location: name, contextIds: [], hasContextDelimiter: false, hasEmptyContext: false }; + } + const entries = name + .slice(separator + FUNCTION_BREAKPOINT_CONTEXT_SEPARATOR.length) + .split(",") + .map((id) => id.trim()); + const contextIds = [...new Set(entries.filter((id) => id))]; return { - location: hasContextDelimiter ? name.slice(0, separator).trim() : name, - contextId: hasContextDelimiter - ? name.slice(separator + FUNCTION_BREAKPOINT_CONTEXT_SEPARATOR.length).trim() - : undefined, - hasContextDelimiter, + location: name.slice(0, separator).trim(), + contextIds, + hasContextDelimiter: true, + hasEmptyContext: entries.some((id) => !id), }; } + +export interface FunctionBreakpointContext { + id: string; + name: string; +} + +export interface FunctionBreakpointContextChoice { + label: string; + description?: string; + detail?: string; + id: string; + picked: boolean; +} + +/** + * The items of the context checklist: the selected call-stack context first, + * then the session's contexts, then checked IDs the session does not have, so + * that editing a breakpoint never drops part of its scope silently. + */ +export function functionBreakpointContextChoices( + contexts: readonly FunctionBreakpointContext[], + checkedIds: readonly string[], + selectedId?: string, +): FunctionBreakpointContextChoice[] { + const selected = contexts.find((context) => context.id === selectedId); + const ordered = selected ? [selected, ...contexts.filter((context) => context !== selected)] : contexts; + const known = new Set(contexts.map((context) => context.id)); + return [ + ...ordered.map((context) => + context === selected + ? { + label: context.name, + description: "Selected call-stack context", + detail: context.id, + id: context.id, + picked: checkedIds.includes(context.id), + } + : { + label: context.name, + description: context.id, + id: context.id, + picked: checkedIds.includes(context.id), + }, + ), + ...checkedIds + .filter((id) => !known.has(id)) + .map((id) => ({ label: id, description: "Not available in this session", id, picked: true })), + ]; +} diff --git a/src/test/breakpointManager.test.ts b/src/test/breakpointManager.test.ts index 1bf7b78..78fe523 100644 --- a/src/test/breakpointManager.test.ts +++ b/src/test/breakpointManager.test.ts @@ -74,22 +74,31 @@ function fakeHost(threads: string[]): BreakpointHost & { events: unknown[] } { } test("function breakpoint names preserve explicit context scopes", async () => { - assert.deepEqual(parseFunctionBreakpointName(formatFunctionBreakpointName("start_kernel", "cpu0")), { + assert.deepEqual(parseFunctionBreakpointName(formatFunctionBreakpointName("start_kernel", ["cpu0"])), { location: "start_kernel", - contextId: "cpu0", + contextIds: ["cpu0"], + hasContextDelimiter: true, + hasEmptyContext: false, + }); + assert.equal(formatFunctionBreakpointName("start_kernel", ["cpu0", "cpu1"]), "start_kernel @ cpu0, cpu1"); + assert.deepEqual(parseFunctionBreakpointName("start_kernel @ cpu0,cpu1 , cpu0"), { + location: "start_kernel", + contextIds: ["cpu0", "cpu1"], hasContextDelimiter: true, + hasEmptyContext: false, }); assert.deepEqual(parseFunctionBreakpointName(formatFunctionBreakpointName("start_kernel")), { location: "start_kernel", - contextId: undefined, + contextIds: [], hasContextDelimiter: false, + hasEmptyContext: false, }); const agent = fakeAgent(); const manager = new BreakpointManager(agent.service, fakeHost(["thread1"])); await manager.setFunctionBreakpoints([ { name: "start_kernel @ armv8.cluster[0].core[0]", condition: "ready", hitCondition: "3" }, { name: "start_kernel" }, - { name: "start_kernel @ armv8.cluster[1].core[0]" }, + { name: "start_kernel @ armv8.cluster[1].core[0], armv8.cluster[1].core[1]" }, ]); const properties = agent.commands.map(([, props]) => props as BreakpointProperties); assert.equal(properties[0].Location, "start_kernel"); @@ -98,7 +107,8 @@ test("function breakpoint names preserve explicit context scopes", async () => { assert.equal(properties[0].IgnoreCount, 2); assert.equal(properties[1].Location, "start_kernel"); assert.equal(properties[1].ContextIds, undefined); - assert.deepEqual(properties[2].ContextIds, ["armv8.cluster[1].core[0]"]); + assert.equal(properties[2].Location, "start_kernel"); + assert.deepEqual(properties[2].ContextIds, ["armv8.cluster[1].core[0]", "armv8.cluster[1].core[1]"]); await manager.setFunctionBreakpoints([{ name: "start_kernel @ armv8.cluster[0].core[0]", condition: "updated" }]); const changed = agent.commands.find(([command]) => command === "change")?.[1] as BreakpointProperties; assert.equal(changed.ID, properties[0].ID); @@ -109,15 +119,17 @@ test("scoped function breakpoints never fall back to global planting", async () const agent = fakeAgent(); agent.caps.ContextIds = false; const manager = new BreakpointManager(agent.service, fakeHost(["thread1"])); - const [unsupported, missingContext, missingSymbol] = await manager.setFunctionBreakpoints([ + const [unsupported, missingContext, missingSymbol, emptyEntry] = await manager.setFunctionBreakpoints([ { name: "start_kernel @ cpu0" }, { name: "start_kernel @ " }, { name: " @ cpu0" }, + { name: "start_kernel @ cpu0, , cpu1" }, ]); assert.equal(unsupported.verified, false); assert.match(unsupported.message ?? "", /ContextIds support/); assert.match(missingContext.message ?? "", /symbol @ context ID/); assert.match(missingSymbol.message ?? "", /symbol @ context ID/); + assert.match(emptyEntry.message ?? "", /symbol @ ID1, ID2/); assert.equal(agent.commands.length, 0); });