[lldb-dap] Allow providing debug adapter arguments in the extension

[matthewbastien]

Added a new setting called lldb-dap.arguments and a debug configuration attribute called debugAdapterArgs that can be used to set the arguments used to launch the debug adapter. Right now this is mostly useful for debugging purposes to add the --wait-for-debugger option to lldb-dap.

Additionally, the extension will now check for a changed lldb-dap executable or arguments when launching a debug session in server mode. I had to add a new DebugConfigurationProvider to do this because VSCode will show an unhelpful error modal when the DebugAdapterDescriptorFactory returns undefined.

In order to facilitate this, I had to add two new properties to the launch configuration that are used by the DebugAdapterDescriptorFactory to tell VS Code how to launch the debug adapter:

• debugAdapterHostname - the hostname for an existing lldb-dap server
• debugAdapterPort - the port for an existing lldb-dap server

I've also removed the check for the executable argument in LLDBDapDescriptorFactory.createDebugAdapterDescriptor(). This argument is only set by VS Code when the debug adapter executable properties are set in the package.json. The LLDB DAP extension does not currently do this (and I don't think it ever will). So, this makes the debug adapter descriptor factory a little easier to read.

The check for whether or not lldb-dap exists has been moved into the new DebugConfigurationProvider as well. This way the extension won't get in the user's way unless they actually try to start a debugging session. The error will show up as a modal which will also make it more obvious when something goes wrong, rather than popping up as a warning at the bottom right of the screen.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Matthew Bastien (matthewbastien)

Changes

Added a new setting called lldb-dap.arguments and a debug configuration attribute called debugAdapterArgs that can be used to set the arguments used to launch the debug adapter. Right now this is mostly useful for debugging purposes to add the --wait-for-debugger option to lldb-dap.

I've also removed the check for the executable argument in LLDBDapDescriptorFactory.createDebugAdapterDescriptor(). This argument is only set by VS Code when the debug adapter executable properties are set in the package.json. The LLDB DAP extension does not currently do this (and I don't think it ever will). So, this makes the debug adapter descriptor factory a little easier to read.

---

Full diff: https://github.com/llvm/llvm-project/pull/129262.diff

2 Files Affected:

• (modified) lldb/tools/lldb-dap/package.json (+23)
• (modified) lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts (+27-21)

diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json
index 31d808eda4c35..0859b6e388a4e 100644
--- a/lldb/tools/lldb-dap/package.json
+++ b/lldb/tools/lldb-dap/package.json
@@ -75,6 +75,15 @@
           "type": "string",
           "description": "The path to the lldb-dap binary."
         },
+        "lldb-dap.arguments": {
+          "scope": "resource",
+          "type": "array",
+          "default": [],
+          "items": {
+            "type": "string"
+          },
+          "description": "The arguments provided to the lldb-dap process."
+        },
         "lldb-dap.log-path": {
           "scope": "resource",
           "type": "string",
@@ -156,6 +165,13 @@
                 "type": "string",
                 "markdownDescription": "The absolute path to the LLDB debug adapter executable to use."
               },
+              "debugAdapterArgs": {
+                "type": "array",
+                "items": {
+                  "type": "string"
+                },
+                "markdownDescription": "The list of arguments used to launch the debug adapter executable."
+              },
               "program": {
                 "type": "string",
                 "description": "Path to the program to debug."
@@ -346,6 +362,13 @@
                 "type": "string",
                 "markdownDescription": "The absolute path to the LLDB debug adapter executable to use."
               },
+              "debugAdapterArgs": {
+                "type": "array",
+                "items": {
+                  "type": "string"
+                },
+                "markdownDescription": "The list of arguments used to launch the debug adapter executable."
+              },
               "program": {
                 "type": "string",
                 "description": "Path to the program to attach to."
diff --git a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts
index 36107336ebc4d..ea7b4ce97ac1d 100644
--- a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts
+++ b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts
@@ -92,6 +92,21 @@ async function getDAPExecutable(
   return undefined;
 }
 
+function getDAPArguments(session: vscode.DebugSession): string[] {
+  // Check the debug configuration for arguments first
+  const debugConfigArgs = session.configuration.debugAdapterArgs;
+  if (
+    Array.isArray(debugConfigArgs) &&
+    debugConfigArgs.findIndex((entry) => typeof entry !== "string") === -1
+  ) {
+    return debugConfigArgs;
+  }
+  // Fall back on the workspace configuration
+  return vscode.workspace
+    .getConfiguration("lldb-dap")
+    .get<string[]>("arguments", []);
+}
+
 /**
  * This class defines a factory used to find the lldb-dap binary to use
  * depending on the session configuration.
@@ -101,7 +116,7 @@ export class LLDBDapDescriptorFactory
 {
   async createDebugAdapterDescriptor(
     session: vscode.DebugSession,
-    executable: vscode.DebugAdapterExecutable | undefined,
+    _executable: vscode.DebugAdapterExecutable | undefined,
   ): Promise<vscode.DebugAdapterDescriptor | undefined> {
     const config = vscode.workspace.getConfiguration(
       "lldb-dap",
@@ -116,40 +131,31 @@ export class LLDBDapDescriptorFactory
     const configEnvironment =
       config.get<{ [key: string]: string }>("environment") || {};
     const dapPath = await getDAPExecutable(session);
+    const dapArgs = getDAPArguments(session);
     const dbgOptions = {
       env: {
-        ...executable?.options?.env,
         ...configEnvironment,
         ...env,
       },
     };
-    if (dapPath) {
-      if (!(await isExecutable(dapPath))) {
-        LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(dapPath);
-        return undefined;
-      }
-      return new vscode.DebugAdapterExecutable(dapPath, [], dbgOptions);
-    } else if (executable) {
-      if (!(await isExecutable(executable.command))) {
-        LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(executable.command);
-        return undefined;
-      }
-      return new vscode.DebugAdapterExecutable(
-        executable.command,
-        executable.args,
-        dbgOptions,
-      );
+    if (dapPath === undefined || !(await isExecutable(dapPath))) {
+      LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(dapPath);
+      return undefined;
     }
-    return undefined;
+    return new vscode.DebugAdapterExecutable(dapPath, dapArgs, dbgOptions);
   }
 
   /**
    * Shows a message box when the debug adapter's path is not found
    */
-  static async showLLDBDapNotFoundMessage(path: string) {
+  static async showLLDBDapNotFoundMessage(path: string | undefined) {
     const openSettingsAction = "Open Settings";
+    const message =
+      path === undefined
+        ? "Unable to find the LLDB debug adapter executable."
+        : `Debug adapter path: ${path} is not a valid file`;
     const callbackValue = await vscode.window.showErrorMessage(
-      `Debug adapter path: ${path} is not a valid file`,
+      message,
       openSettingsAction,
     );
 

[JDevlieghere]

Thanks, this LGTM!

[matthewbastien]

I rebased and ended up refactoring the UI logic so that we don't have to pass userInteractive everywhere and can share common buttons with different parts of the extension. I also added where the lldb-dap path comes from to the error message.

Probably worth another review before merging.

CC @JDevlieghere @walter-erquinigo

[JDevlieghere]

I'd like @vogelsgesang and @ashgti to have a look before we merge this, but the new experience seems really nice.

I left a few nits about comments missing a period. Event though this is TypeScript, we should follow the LLVM Coding Standards unless there's a good reason to diverge.

[ashgti]

Overall looks great, a few questions / follow items

[ashgti]

I wonder if we should have our own type + validator for the debugConfiguration

Something along the lines of:

interface LLDBDebugConfiguration extends vscode.DebugConfiguration {
  type: 'lldb-dap';
  request: 'launch' | 'attach';
  program: string;
  args?: string[];
  env?: Record<string, string>;
  debugAdapterHostname?: string;
  initCommands?: string[];
  stopOnEntry?: boolean;
}
... 

async resolveDebugConfiguration(folder, config) {
  assert(config, isLLDBDebugConfiguration);
  // config is of type LLDBDebugConfiguration

TypeScript Playground example

This would help with the inline type validation being performed here.

I could also follow up this CL with some of these validations. I think they could also help the URL launching service as well.

[matthewbastien]

I really like this idea!

Playing around with your code locally, I found that we would also need some more complex validators for things like making sure debugAdapterPort and debugAdapterExecutable aren't set together. Would also be nice to have some of the validators coerce strings to numbers for things like the port information which accept both.

I think this definitely warrants a separate PR that consolidates the validation logic.