[lldb-dap] Add process picker command to VS Code extension #128943
Conversation
|
@llvm/pr-subscribers-lldb Author: Matthew Bastien (matthewbastien) ChangesAdds a process picker command to the LLDB DAP extension that will prompt the user to select a process running on their machine. It is hidden from the command palette, but can be used in an {
"type": "lldb-dap",
"request": "attach",
"name": "Attach to Process",
"pid": "${command:PickProcess}"
}The logic is largely the same as the process picker in the
I manually tested this on a MacBook Pro running macOS Sequoia, a Windows 11 VM, and an Ubuntu 22.04 VM. Fixes #96279 Full diff: https://github.com/llvm/llvm-project/pull/128943.diff 9 Files Affected:
diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json
index 31d808eda4c35..1bbdbf045dd1b 100644
--- a/lldb/tools/lldb-dap/package.json
+++ b/lldb/tools/lldb-dap/package.json
@@ -146,6 +146,9 @@
"windows": {
"program": "./bin/lldb-dap.exe"
},
+ "variables": {
+ "PickProcess": "lldb-dap.pickProcess"
+ },
"configurationAttributes": {
"launch": {
"required": [
@@ -517,6 +520,16 @@
"cwd": "^\"\\${workspaceRoot}\""
}
},
+ {
+ "label": "LLDB: Attach to Process",
+ "description": "",
+ "body": {
+ "type": "lldb-dap",
+ "request": "attach",
+ "name": "${1:Attach}",
+ "pid": "^\"\\${command:PickProcess}\""
+ }
+ },
{
"label": "LLDB: Attach",
"description": "",
@@ -541,6 +554,21 @@
}
]
}
- ]
+ ],
+ "commands": [
+ {
+ "command": "lldb-dap.pickProcess",
+ "title": "Pick Process",
+ "category": "LLDB DAP"
+ }
+ ],
+ "menus": {
+ "commandPalette": [
+ {
+ "command": "lldb-dap.pickProcess",
+ "when": "false"
+ }
+ ]
+ }
}
}
diff --git a/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts b/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts
new file mode 100644
index 0000000000000..355d508075080
--- /dev/null
+++ b/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts
@@ -0,0 +1,41 @@
+import * as path from "path";
+import * as vscode from "vscode";
+import { createProcessTree } from "../process-tree";
+
+interface ProcessQuickPick extends vscode.QuickPickItem {
+ processId: number;
+}
+
+/**
+ * Prompts the user to select a running process.
+ *
+ * The return value must be a string so that it is compatible with VS Code's
+ * string substitution infrastructure. The value will eventually be converted
+ * to a number by the debug configuration provider.
+ *
+ * @returns The pid of the process as a string or undefined if cancelled.
+ */
+export async function pickProcess(): Promise<string | undefined> {
+ const processTree = createProcessTree();
+ const selectedProcess = await vscode.window.showQuickPick<ProcessQuickPick>(
+ processTree.listAllProcesses().then((processes): ProcessQuickPick[] => {
+ return processes
+ .sort((a, b) => b.start - a.start) // Sort by start date in descending order
+ .map((proc) => {
+ return {
+ processId: proc.id,
+ label: path.basename(proc.command),
+ description: proc.id.toString(),
+ detail: proc.arguments,
+ } satisfies ProcessQuickPick;
+ });
+ }),
+ {
+ placeHolder: "Select a process to attach the debugger to",
+ },
+ );
+ if (!selectedProcess) {
+ return;
+ }
+ return selectedProcess.processId.toString();
+}
diff --git a/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts b/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts
new file mode 100644
index 0000000000000..02b8bd5aa8147
--- /dev/null
+++ b/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts
@@ -0,0 +1,54 @@
+import * as vscode from "vscode";
+
+/**
+ * Converts the given value to an integer if it isn't already.
+ *
+ * If the value cannot be converted then this function will return undefined.
+ *
+ * @param value the value to to be converted
+ * @returns the integer value or undefined if unable to convert
+ */
+function convertToInteger(value: any): number | undefined {
+ let result: number | undefined;
+ switch (typeof value) {
+ case "number":
+ result = value;
+ break;
+ case "string":
+ result = Number(value);
+ break;
+ default:
+ return undefined;
+ }
+ if (!Number.isInteger(result)) {
+ return undefined;
+ }
+ return result;
+}
+
+/**
+ * A {@link vscode.DebugConfigurationProvider} used to resolve LLDB DAP debug configurations.
+ *
+ * Performs checks on the debug configuration before launching a debug session.
+ */
+export class LLDBDapConfigurationProvider
+ implements vscode.DebugConfigurationProvider
+{
+ resolveDebugConfigurationWithSubstitutedVariables(
+ _folder: vscode.WorkspaceFolder | undefined,
+ debugConfiguration: vscode.DebugConfiguration,
+ ): vscode.ProviderResult<vscode.DebugConfiguration> {
+ // Convert the "pid" option to a number if it is a string
+ if ("pid" in debugConfiguration) {
+ const pid = convertToInteger(debugConfiguration.pid);
+ if (pid === undefined) {
+ vscode.window.showErrorMessage(
+ "Invalid debug configuration: property 'pid' must either be an integer or a string containing an integer value.",
+ );
+ return null;
+ }
+ debugConfiguration.pid = pid;
+ }
+ return debugConfiguration;
+ }
+}
diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts b/lldb/tools/lldb-dap/src-ts/extension.ts
index 71fd48298f8f5..74815022d468a 100644
--- a/lldb/tools/lldb-dap/src-ts/extension.ts
+++ b/lldb/tools/lldb-dap/src-ts/extension.ts
@@ -1,12 +1,12 @@
-import * as path from "path";
-import * as util from "util";
import * as vscode from "vscode";
+import { pickProcess } from "./commands/pick-process";
import {
LLDBDapDescriptorFactory,
isExecutable,
} from "./debug-adapter-factory";
import { DisposableContext } from "./disposable-context";
+import { LLDBDapConfigurationProvider } from "./debug-configuration-provider";
/**
* This class represents the extension and manages its life cycle. Other extensions
@@ -15,6 +15,12 @@ import { DisposableContext } from "./disposable-context";
export class LLDBDapExtension extends DisposableContext {
constructor() {
super();
+ this.pushSubscription(
+ vscode.debug.registerDebugConfigurationProvider(
+ "lldb-dap",
+ new LLDBDapConfigurationProvider(),
+ ),
+ );
this.pushSubscription(
vscode.debug.registerDebugAdapterDescriptorFactory(
"lldb-dap",
@@ -38,6 +44,10 @@ export class LLDBDapExtension extends DisposableContext {
}
}),
);
+
+ this.pushSubscription(
+ vscode.commands.registerCommand("lldb-dap.pickProcess", pickProcess),
+ );
}
}
diff --git a/lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts b/lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts
new file mode 100644
index 0000000000000..3c08f49035b35
--- /dev/null
+++ b/lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts
@@ -0,0 +1,102 @@
+import { ChildProcessWithoutNullStreams } from "child_process";
+import { Process, ProcessTree } from ".";
+import { Transform } from "stream";
+
+/** Parses process information from a given line of process output. */
+export type ProcessTreeParser = (line: string) => Process | undefined;
+
+/**
+ * Implements common behavior between the different {@link ProcessTree} implementations.
+ */
+export abstract class BaseProcessTree implements ProcessTree {
+ /**
+ * Spawn the process responsible for collecting all processes on the system.
+ */
+ protected abstract spawnProcess(): ChildProcessWithoutNullStreams;
+
+ /**
+ * Create a new parser that can read the process information from stdout of the process
+ * spawned by {@link spawnProcess spawnProcess()}.
+ */
+ protected abstract createParser(): ProcessTreeParser;
+
+ listAllProcesses(): Promise<Process[]> {
+ return new Promise<Process[]>((resolve, reject) => {
+ const proc = this.spawnProcess();
+ const parser = this.createParser();
+
+ // Capture processes from stdout
+ const processes: Process[] = [];
+ proc.stdout.pipe(new LineBasedStream()).on("data", (line) => {
+ const process = parser(line.toString());
+ if (process && process.id !== proc.pid) {
+ processes.push(process);
+ }
+ });
+
+ // Resolve or reject the promise based on exit code/signal/error
+ proc.on("error", reject);
+ proc.on("exit", (code, signal) => {
+ if (code === 0) {
+ resolve(processes);
+ } else if (signal) {
+ reject(
+ new Error(
+ `Unable to list processes: process exited due to signal ${signal}`,
+ ),
+ );
+ } else {
+ reject(
+ new Error(
+ `Unable to list processes: process exited with code ${code}`,
+ ),
+ );
+ }
+ });
+ });
+ }
+}
+
+/**
+ * A stream that emits each line as a single chunk of data. The end of a line is denoted
+ * by the newline character '\n'.
+ */
+export class LineBasedStream extends Transform {
+ private readonly newline: number = "\n".charCodeAt(0);
+ private buffer: Buffer = Buffer.alloc(0);
+
+ override _transform(
+ chunk: Buffer,
+ _encoding: string,
+ callback: () => void,
+ ): void {
+ let currentIndex = 0;
+ while (currentIndex < chunk.length) {
+ const newlineIndex = chunk.indexOf(this.newline, currentIndex);
+ if (newlineIndex === -1) {
+ this.buffer = Buffer.concat([
+ this.buffer,
+ chunk.subarray(currentIndex),
+ ]);
+ break;
+ }
+
+ const newlineChunk = chunk.subarray(currentIndex, newlineIndex);
+ const line = Buffer.concat([this.buffer, newlineChunk]);
+ this.push(line);
+ this.buffer = Buffer.alloc(0);
+
+ currentIndex = newlineIndex + 1;
+ }
+
+ callback();
+ }
+
+ override _flush(callback: () => void): void {
+ if (this.buffer.length) {
+ this.push(this.buffer);
+ }
+
+ callback();
+ }
+}
diff --git a/lldb/tools/lldb-dap/src-ts/process-tree/index.ts b/lldb/tools/lldb-dap/src-ts/process-tree/index.ts
new file mode 100644
index 0000000000000..9c46bc92d8548
--- /dev/null
+++ b/lldb/tools/lldb-dap/src-ts/process-tree/index.ts
@@ -0,0 +1,36 @@
+import { DarwinProcessTree } from "./platforms/darwin-process-tree";
+import { LinuxProcessTree } from "./platforms/linux-process-tree";
+import { WindowsProcessTree } from "./platforms/windows-process-tree";
+
+/**
+ * Represents a single process running on the system.
+ */
+export interface Process {
+ /** Process ID */
+ id: number;
+
+ /** Command that was used to start the process */
+ command: string;
+
+ /** The full command including arguments that was used to start the process */
+ arguments: string;
+
+ /** The date when the process was started */
+ start: number;
+}
+
+export interface ProcessTree {
+ listAllProcesses(): Promise<Process[]>;
+}
+
+/** Returns a {@link ProcessTree} based on the current platform. */
+export function createProcessTree(): ProcessTree {
+ switch (process.platform) {
+ case "darwin":
+ return new DarwinProcessTree();
+ case "win32":
+ return new WindowsProcessTree();
+ default:
+ return new LinuxProcessTree();
+ }
+}
diff --git a/lldb/tools/lldb-dap/src-ts/process-tree/platforms/darwin-process-tree.ts b/lldb/tools/lldb-dap/src-ts/process-tree/platforms/darwin-process-tree.ts
new file mode 100644
index 0000000000000..954644288869e
--- /dev/null
+++ b/lldb/tools/lldb-dap/src-ts/process-tree/platforms/darwin-process-tree.ts
@@ -0,0 +1,16 @@
+import { ChildProcessWithoutNullStreams, spawn } from "child_process";
+import { LinuxProcessTree } from "./linux-process-tree";
+
+function fill(prefix: string, suffix: string, length: number): string {
+ return prefix + suffix.repeat(length - prefix.length);
+}
+
+export class DarwinProcessTree extends LinuxProcessTree {
+ protected override spawnProcess(): ChildProcessWithoutNullStreams {
+ return spawn("ps", [
+ "-xo",
+ // The length of comm must be large enough or data will be truncated.
+ `pid=PID,lstart=START,comm=${fill("COMMAND", "-", 256)},command=ARGUMENTS`,
+ ]);
+ }
+}
diff --git a/lldb/tools/lldb-dap/src-ts/process-tree/platforms/linux-process-tree.ts b/lldb/tools/lldb-dap/src-ts/process-tree/platforms/linux-process-tree.ts
new file mode 100644
index 0000000000000..65733f6c547b3
--- /dev/null
+++ b/lldb/tools/lldb-dap/src-ts/process-tree/platforms/linux-process-tree.ts
@@ -0,0 +1,38 @@
+import { ChildProcessWithoutNullStreams, spawn } from "child_process";
+import { BaseProcessTree, ProcessTreeParser } from "../base-process-tree";
+
+export class LinuxProcessTree extends BaseProcessTree {
+ protected override spawnProcess(): ChildProcessWithoutNullStreams {
+ return spawn(
+ "ps",
+ ["-axo", `pid=PID,lstart=START,comm:128=COMMAND,command=ARGUMENTS`],
+ {
+ stdio: "pipe",
+ },
+ );
+ }
+
+ protected override createParser(): ProcessTreeParser {
+ let commandOffset: number | undefined;
+ let argumentsOffset: number | undefined;
+ return (line) => {
+ if (!commandOffset || !argumentsOffset) {
+ commandOffset = line.indexOf("COMMAND");
+ argumentsOffset = line.indexOf("ARGUMENTS");
+ return;
+ }
+
+ const pid = /^\s*([0-9]+)\s*/.exec(line);
+ if (!pid) {
+ return;
+ }
+
+ return {
+ id: Number(pid[1]),
+ command: line.slice(commandOffset, argumentsOffset).trim(),
+ arguments: line.slice(argumentsOffset).trim(),
+ start: Date.parse(line.slice(pid[0].length, commandOffset).trim()),
+ };
+ };
+ }
+}
diff --git a/lldb/tools/lldb-dap/src-ts/process-tree/platforms/windows-process-tree.ts b/lldb/tools/lldb-dap/src-ts/process-tree/platforms/windows-process-tree.ts
new file mode 100644
index 0000000000000..9cfbfa29ab5d3
--- /dev/null
+++ b/lldb/tools/lldb-dap/src-ts/process-tree/platforms/windows-process-tree.ts
@@ -0,0 +1,52 @@
+import * as path from "path";
+import { BaseProcessTree, ProcessTreeParser } from "../base-process-tree";
+import { ChildProcessWithoutNullStreams, spawn } from "child_process";
+
+export class WindowsProcessTree extends BaseProcessTree {
+ protected override spawnProcess(): ChildProcessWithoutNullStreams {
+ const wmic = path.join(
+ process.env["WINDIR"] || "C:\\Windows",
+ "System32",
+ "wbem",
+ "WMIC.exe",
+ );
+ return spawn(
+ wmic,
+ ["process", "get", "CommandLine,CreationDate,ProcessId"],
+ { stdio: "pipe" },
+ );
+ }
+
+ protected override createParser(): ProcessTreeParser {
+ const lineRegex = /^(.*)\s+([0-9]+)\.[0-9]+[+-][0-9]+\s+([0-9]+)$/;
+
+ return (line) => {
+ const matches = lineRegex.exec(line.trim());
+ if (!matches || matches.length !== 4) {
+ return;
+ }
+
+ const id = Number(matches[3]);
+ const start = Number(matches[2]);
+ let fullCommandLine = matches[1].trim();
+ if (isNaN(id) || !fullCommandLine) {
+ return;
+ }
+ // Extract the command from the full command line
+ let command = fullCommandLine;
+ if (fullCommandLine[0] === '"') {
+ const end = fullCommandLine.indexOf('"', 1);
+ if (end > 0) {
+ command = fullCommandLine.slice(1, end - 1);
+ }
+ } else {
+ const end = fullCommandLine.indexOf(" ");
+ if (end > 0) {
+ command = fullCommandLine.slice(0, end);
+ }
+ }
+
+ return { id, command, arguments: fullCommandLine, start };
+ };
+ }
+}
|
|
It's a little unfortunate that listing processes is not part of the Debug Adapter Protocol. LLDB already knows how to do this through the host platform. I guess even more unfortunate is that neither node nor VSCode provides a cross platform API to do this. I was expecting the implementation to be more convoluted but @matthewbastien did a great job abstracting the different platforms. This is definitely better than relying on an external package that's outside of our control. One possible alternative would be to add a flag to the |
|
Yeah, the CodeLLDB extension uses |
|
Though now that I think about it further: we could take a hybrid approach where this PR is merged and then the functionality to list processes is added to Just thinking out loud. |
|
And more out loud thinking: the other issue to overcome with using |
|
Thanks for the additional perspective. I was on the fence when I mentioned it and it sounds like sticking with the TS approach avoids some complications, plus it matches what other VSCode plugins are doing. |
|
Adding Adrian and Walter as reviewers and tagging @da-viper as they might want to take a look. To me this looks good. |
lldb/tools/lldb-dap/src-ts/process-tree/platforms/linux-process-tree.ts
Outdated
Show resolved
Hide resolved
matthewbastien
force-pushed
the
lldb-dap-process-picker
branch
2 times, most recently
from
c015ed9 to
2e68d89
Compare
matthewbastien
force-pushed
the
lldb-dap-process-picker
branch
from
2e68d89 to
f4407b2
Compare
lldb/tools/lldb-dap/src-ts/process-tree/platforms/darwin-process-tree.ts
Outdated
Show resolved
Hide resolved
lldb/tools/lldb-dap/src-ts/process-tree/platforms/darwin-process-tree.ts
Outdated
Show resolved
Hide resolved
lldb/tools/lldb-dap/src-ts/process-tree/platforms/windows-process-tree.ts
Outdated
Show resolved
Hide resolved
lldb/tools/lldb-dap/src-ts/process-tree/platforms/linux-process-tree.ts
Outdated
Show resolved
Hide resolved
lldb/tools/lldb-dap/src-ts/process-tree/platforms/darwin-process-tree.ts
Outdated
Show resolved
Hide resolved
|
you could also validate the pid input in package.json from llvm-project/lldb/tools/lldb-dap/package.json Lines 359 to 364 in 038731c to "pid": {
"anyOf": [
{
"type": "number",
"description": "System process ID to attach to."
},
{
"const": "${command:pickProcess}",
"description": "Picks a process ID using the process ID picker"
}
]
}, |
There was a problem hiding this comment.
You can go one level beyond this and provide a way to attach to programs without needing launch.json configs at all!
- Create a new pallete command "attach to process"
- Reuse the same process picker and once the user has selected one item, issue a vscode.debug.StartDebugging() call passing a minimal debug config with the selected process as PID.
matthewbastien
force-pushed
the
lldb-dap-process-picker
branch
from
4137b52 to
0a615f2
Compare
See discussion further up in the thread (first comment and follow-up messages). It seems the final blocker was that we didn't have access to the That being said, I am not sure if it's worth using the lldb layer for that...
When you SSH-connect into a remote host, the complete However, if the user does connect remotely from lldb via gdbserver to some other host, then we would indeed be running the Does currently even work? Also, does the gdb-remote protocol even offer capabilities for process-picking? (Not sure, I seldomly used gdb-remote functionality so far, except for connect to |
| !("pid" in debugConfiguration) && | ||
| !("program" in debugConfiguration) | ||
| ) { | ||
| debugConfiguration.pid = "${command:pickProcess}"; |
There was a problem hiding this comment.
should we mark pid as optional in the the package.json and the README.md?
There was a problem hiding this comment.
I've updated both of these to better reflect my changes. I did a pretty major overhaul of the README section on attaching and would appreciate another thorough review there to make sure the wording sounds good.
lldb/tools/lldb-dap/src-ts/process-tree/platforms/windows-process-tree.ts
Show resolved
Hide resolved
Co-authored-by: Da-Viper <[email protected]>
…e pid will be ignored
Yes it works. LLDB actually always uses GDB remote for all local debug sessions by spawning a lldb-server to debug the binary, so yes remote debugging is very well supported in lldb.
Our lldb-server can be run in two modes: debug or platform. When running in platform mode it does give us the connection we need to the remote server. So yes this can and does work. You can spawn a GDB remote platform on a remote linux machine with |
|
I agree that it would be cleaner to just use LLDB's process listing functionality and not have all this extra code. My major concern about the lack of access to the launch configuration ended up not actually being an issue. However, I still have a few questions about how this would work.
The fallback would be to launch
edit: there is an explanation in the README about attaching to a remote debug server. |
|
I have an idea for using lldb's process list logic, although it's a bit of code.
We shouldn't invoke lldb directly because we just don't know where it is, and it might be error-prone to try to find it. Instead, it might be better to simply add a flag to lldb-dap for this, which seems to be kind of appropriate for this kind of flow. Let me know what you think. |
Why would we not use
Note that we would also need to pass through parameters like All in all, this sounds like a good long-term direction to me.
@matthewbastien I am not sure I understand that concern. Looking at the commit log of swiftlang/llvm-project, it seems that they are picking up changes to |
Swift uses specific branches of the name That being said, I still agree that adding the process picking functionality to Once this gets into Swift's |
|
One more thing that crossed my mind: given that the LLDB DAP extension will automatically be updated to the latest version by VS Code, should we also have feature flags for things like this in lldb-dap? (or is there some way already to detect which features are available?) The version of |
IMO, The VS-Code extension should be prepared to handle this exit code accordingly. For the process picker, it should probably display an error "Picking a process for attaching is not yet supported by this lldb-dap binary. Please upgrade to a newer lldb-dap binary.". The error message is still slightly misleading, because at least for the next 6 months, i.e. until lldb-dap 21, there will not be any release of lldb-dap which is recent enough. But I think this is good still good enough, given that we still have relatively few users (as evident by comparing the download numbers of lldb dap against vscode-lldb and Microsofts C++ debugger). Long-term we might need a better compatibility story, though...
We probably want to update the Typescript code to check if |
Thats a good catch, I added a note to #129283 that we should validate the binary supports server mode. |
|
Created #130855 to validate the binary before we try to use server mode. |
Looks great, thanks! The approach in that commit (using |
Adds a process picker command to the LLDB DAP extension that will prompt the user to select a process running on their machine. It is hidden from the command palette, but can be used in an
"attach"debug configuration to select a process at the start of a debug session. I've also added a debug configuration snippet for this called"LLDB: Attach to Process"that will fill in the appropriate variable substitution. e.g:{ "type": "lldb-dap", "request": "attach", "name": "Attach to Process", "pid": "${command:pickProcess}" }The logic is largely the same as the process picker in the
vscode-js-debugextension created by Microsoft. It will use available executables based on the current platform to find the list of available processes:psexecutable to list processespsare differentGet-CimInstanceto query for processesI manually tested this on a MacBook Pro running macOS Sequoia, a Windows 11 VM, and an Ubuntu 22.04 VM.
Fixes #96279