-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Fix lldb-dap pickProcess command for selecting process for debugger attachment #75342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
@llvm/pr-subscribers-lldb Author: None (Dodzey) ChangesHi, I see that the documentation for lldb-dap refers to a ${command.pickMyProcess} which can be used for interactive process selection within the VS Code IDE. It appears that this functionality no longer works, perhaps due to internal changes in VS Code. I can get interactive process selection if I add the following snippet to the lldp-dap extension package.json:
However, referencing extension.pickNativeProcess here is only valid if the Microsoft VSCode CPPTools extension is installed, as that is the extension that registers a native process picker under the name 'extension.pickNativeProcess' I'm not sure if it's desirable behaviour for the lldb-dap extension to be dependent on the presence of the Microsoft CPPTools extension? It appears (although I am not familiar with VSCode extension development) that any solution to this that was fully integrated in the lldp-dap extension would require typescript source to be added to the extension in order to register a private native process picker under a custom name - something like In the PR I have included the minimal changes required to seemingly make the process picker work when the Microsoft VSCode CPPTools extension is also installed. What are the recommendations on how we could proceed here? Thanks! Full diff: https://github.com/llvm/llvm-project/pull/75342.diff 2 Files Affected:
diff --git a/lldb/tools/lldb-dap/README.md b/lldb/tools/lldb-dap/README.md
index 00ceb0bedc40a4..d341a136293a87 100644
--- a/lldb/tools/lldb-dap/README.md
+++ b/lldb/tools/lldb-dap/README.md
@@ -125,7 +125,7 @@ The JSON configuration file can contain the following `lldb-dap` specific launch
|**type** |string |Y| Must be "lldb-dap".
|**request** |string |Y| Must be "attach".
|**program** |string | | Path to the executable to attach to. This value is optional but can help to resolve breakpoints prior the attaching to the program.
-|**pid** |number | | The process id of the process you wish to attach to. If **pid** is omitted, the debugger will attempt to attach to the program by finding a process whose file name matches the file name from **porgram**. Setting this value to `${command:pickMyProcess}` will allow interactive process selection in the IDE.
+|**pid** |number | | The process id of the process you wish to attach to. If **pid** is omitted, the debugger will attempt to attach to the program by finding a process whose file name matches the file name from **program**. Setting this value to `${command:pickProcess}` will allow interactive process selection in the IDE.
|**stopOnEntry** |boolean| | Whether to stop program immediately after launching.
|**waitFor** |boolean | | Wait for the process to launch.
|**initCommands** |[string]| | LLDB commands executed upon debugger startup prior to creating the LLDB target. Commands and command output will be sent to the debugger console when they are executed.
diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json
index ebb1103d695e17..7b77b32f890a6f 100644
--- a/lldb/tools/lldb-dap/package.json
+++ b/lldb/tools/lldb-dap/package.json
@@ -124,6 +124,9 @@
"swift"
]
},
+ "variables": {
+ "pickProcess": "extension.pickNativeProcess"
+ },
"program": "./bin/lldb-dap",
"windows": {
"program": "./bin/lldb-dap.exe"
|
@clayborg and @walter-erquinigo are lldb-dap code owners and should be able to advise. |
@Dodzey, thanks for submitting this PR. |
@walter-erquinigo Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me for now. We can coordinate on adding a native TypeScript layer after this, but no need to hold this up if this work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Since @Dodzey is a first-time contributor one of us needs to click the merge button. I'll leave that to @clayborg or @walter-erquinigo as the owners.
@clayborg Yes, it appears to. For context my workflow is C++ development, using recent versions of Clang and GCC. In VS Code I use the Microsoft CPPTools extension to allow for GDB debugging of GCC builds, with intellisense disabled. I am using clangd for auto-completion, inlay hints, automatic compiler errors/warning squiggles etc. That combination means that I have the Microsoft CPPTools extension enabled, which is enough for the 'workaround' here to be functional. I've tested launching the debugger twice within VSCode and using the process selection dropdown picker attaching to two copies of the same executable running at the same time. Both debuggers seem to be working ok and operating concurrently as you would expect. Before this modification it wasn't possible to select two instances of the same executable, as there was no way of specifying the pids, other than hard coding in the launch.json. This made debugging MPI processes etc difficult. I have another work in progress that adds fall back support for parsing the "pid" value as a string if the unsigned parsing fails. This allows use of the textual input boxes in VS Code - which allow you to input the pid manually - but it gets passed to the attach API as a string in the JSON object, not as unsigned, so it requires some small code changes to lldb-dap.cpp to enable that. I can put in a separate pull request for that once it is ready to get some opinions. I'm happy to make an attempt at adding some minimal typescript elements to the extension in a future PR if I can get something working. |
I've just checked one possible failure mode that came to mind. Do you get an error if you don't have the MS CPPTools extension enabled and you attempt to use a launch profile with a hardcoded pid (eg |
It should be part of the lldb-dap folder/project and it should also be the default way to use lldb-dap. Using it directly as a binary would be discouraged but left as a possibility. The main idea behind this approach is that several companies have built their own TS wrappers for lldb-dap (at least Meta, Google and Modular), which includes features they haven't contributed back to the community. This is something I'd like to get fixed. An additional architectural point for this extension is that it should be possible to copy/paste the TS files into another existing vscode extensions, and integrate it with possibly just one single call |
@Dodzey , I think I'll add the minimal TS project tonight and you can review it. Then we can add a few additional features there are as follow ups |
Here it is #75515! |
@Dodzey, the typescript extension has been merged and, if you want, you can implement an integrated process picker for lldb-dap. |
Hi,
I see that the documentation for lldb-dap refers to a ${command.pickMyProcess} which can be used for interactive process selection within the VS Code IDE. It appears that this functionality no longer works, perhaps due to internal changes in VS Code.
I can get interactive process selection if I add the following snippet to the lldp-dap extension package.json:
and then use
within my launch.json configuration for lldb-dap.
However, referencing extension.pickNativeProcess here is only valid if the Microsoft VSCode CPPTools extension is installed & enabled, as that is the extension that registers a native process picker under the name 'extension.pickNativeProcess'.
I'm not sure if it's desirable behaviour for the lldb-dap extension to be dependent on the presence of the Microsoft CPPTools extension?
It appears (although I am not familiar with VSCode extension development) that any solution to this that was fully integrated in the lldp-dap extension would require typescript source to be added to the extension in order to register a private native process picker under a custom name - something like
extension.lldb-dap.pickNativeProcess
. I would assume that this implementation would look very similar, if not identical, to the command currently exposed by the Microsoft CPPTools extension.In the PR I have included the minimal changes required to make the process picker work when the Microsoft VSCode CPPTools extension is also installed.
What are the recommendations on how we could proceed here?
Thanks!