Summary:
The request "evaluate" supports a "context" attribute, which is sent by VSCode. The attribute is defined here https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Evaluate
The "clipboard" context is not yet supported by lldb-vscode, so we can forget about it for now. The 'repl' (i.e. Debug Console) and 'watch' (i.e. Watch Expression) contexts must use the expression parser in case the frame's variable path is not enough, as the user expects these expressions to never fail. On the other hand, the 'hover' expression is invoked whenever the user hovers on any keyword on the UI and the user is fine with the expression not being fully resolved, as they know that the 'repl' case is the fallback they can rely on.
Given that the 'hover' expression is invoked many many times without the user noticing it due to it being triggered by the mouse, I'm making it use only the frame's variable path functionality and not the expression parser. This should speed up tremendously the responsiveness of a debug session when the user only sets source breakpoints and inspect local variables, as the entire debug info is not needed to be parsed.
Regarding tests, I've tried to be as comprehensive as possible considering a multi-file project. Fortunately, the results from the "hover" case are enough most of the times.
Differential Revision: https://reviews.llvm.org/D98656
VSCode was not being informed whenever a location had been resolved (after being initated as non-resolved), so even though it was actually resolved, the IDE would show a hollow dot (instead of a red dot) because it didn't know about the change.
Differential Revision: https://reviews.llvm.org/D96680
@mstorsjo found a mistake that I made when trying to fix some Windows
compilation errors encountered by @stella.stamenova.
I was incorrectly using the LLVM_ON_UNIX macro. In any case, proper use
of
#if defined(_WIN32)
should be the actual fix.
Differential Revision: https://reviews.llvm.org/D96060
@stella.stamenova found out that lldb-vscode's Win32 macros were failing
when building on windows targetings POSIX platforms.
I'm changing these macros for LLVM_ON_UNIX, which should be more
accurate.
stella.stemenova mentioned in https://reviews.llvm.org/D93951 failures on Windows for this test.
I'm fixing the macro definitions and disabling the tests for python
versions lower than 3.7. I'll figure out that actual issue with
python3.6 after the buildbots are fine again.
Depends on D93874.
runInTerminal was using --wait-for, but it was some problems because it uses process polling looking for a single instance of the debuggee:
- it gets to know of the target late, which renders breakpoints in the main function almost impossible
- polling might fail if there are already other processes with the same name
- polling might also fail on some linux machine, as it's implemented with the ps command, and the ps command's args and output are not standard everywhere
As a better way to implement this so that it works well on Darwin and Linux, I'm using now the following process:
- lldb-vscode notices the runInTerminal, so it spawns lldb-vscode with a special flag --launch-target <target>. This flags tells lldb-vscode to wait to be attached and then it execs the target program. I'm using lldb-vscode itself to do this, because it makes finding the launcher program easier. Also no CMAKE INSTALL scripts are needed.
- Besides this, the debugger creates a temporary FIFO file where the launcher program will write its pid to. That way the debugger will be sure of which program to attach.
- Once attach happend, the debugger creates a second temporary file to notify the launcher program that it has been attached, so that it can then exec. I'm using this instead of using a signal or a similar mechanism because I don't want the launcher program to wait indefinitely to be attached in case the debugger crashed. That would pollute the process list with a lot of hanging processes. Instead, I'm setting a 20 seconds timeout (that's an overkill) and the launcher program seeks in intervals the second tepmorary file.
Some notes:
- I preferred not to use sockets because it requires a lot of code and I only need a pid. It would also require a lot of code when windows support is implemented.
- I didn't add Windows support, as I don't have a windows machine, but adding support for it should be easy, as the FIFO file can be implemented with a named pipe, which is standard on Windows and works pretty much the same way.
The existing test which didn't pass on Linux, now passes.
Differential Revision: https://reviews.llvm.org/D93951
lldb-vsdode was communicating the list of modules to the IDE with events, which in practice ended up having some drawbacks
- when debugging large targets, the number of these events were easily 10k, which polluted the messages being transmitted, which caused the following: a harder time debugging the messages, a lag after terminated the process because of these messages being processes (this could easily take several seconds). The latter was specially bad, as users were complaining about it even when they didn't check the modules view.
- these events were rarely used, as users only check the modules view when something is wrong and they try to debug things.
After getting some feedback from users, we realized that it's better to not used events but make this simply a request and is triggered by users whenever they needed.
This diff achieves that and does some small clean up in the existing code.
Differential Revision: https://reviews.llvm.org/D94033
Per the DAP spec for SetBreakpoints [1], the way to clear breakpoints is: `To clear all breakpoint for a source, specify an empty array.`
However, leaving the breakpoints field unset is also a well formed request (note the `breakpoints?:` in the `SetBreakpointsArguments` definition). If it's unset, we have a couple choices:
1. Crash (current behavior)
2. Clear breakpoints
3. Return an error response that the breakpoints field is missing.
I propose we do (2) instead of (1), and treat an unset breakpoints field the same as an empty breakpoints field.
[1] https://microsoft.github.io/debug-adapter-protocol/specification#Requests_SetBreakpoints
Reviewed By: wallace, labath
Differential Revision: https://reviews.llvm.org/D88513
The Symbol Status in modules view is simplified so that only when the module has debug info and its size is non-zero, will the status message be displayed. The symbol status message is renamed to debug info size and flag message like "Symbols not found" and "Symbols loaded" is deleted.
Differential Revision: https://reviews.llvm.org/D86662
When lldb cannot find source file thus IDE renders a disassembly view, add syntax highlighting for constants, registers and final line comments for better debugging experience.
The original plain disassembly view looks like:
{F12401687}
An ideal view is like the screenshot attached.
{F12401515}
In this diff, the mimeType is a kind of media type for formatting the content in the response to a source request. Elements in the disassembly view, like constants, registers and final line comments are colored for highlighting.
A built-in support in the VSCode IDE for syntax highlighting will identify the which mimeType to apply and render the disassembly view as expected.
Reviewed By: wallace, clayborg
Differential Revision: https://reviews.llvm.org/D84555
If a module has debug info, the size of debug symbol will be displayed after the Symbols Loaded Message for each module in the VScode modules view.{F12335461}
Reviewed By: wallace, clayborg
Differential Revision: https://reviews.llvm.org/D83731
Summary: If a module has debug info, the size of debug symbol will be displayed after the Symbols Loaded Message for each module in the VScode modules view.{F12335461}
Reviewers: wallace, clayborg
Reviewed By: wallace, clayborg
Subscribers: cfe-commits, aprantl, lldb-commits
Tags: #lldb, #clang
Differential Revision: https://reviews.llvm.org/D83731
It was failing because some module events had empty UUID, and that was not handled correctly.
The diff that added that logic is https://reviews.llvm.org/D82477
Original commit c60216db15.
The test can only run on Darwin because of how it was setup, so I'm
enforcing that.
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Summary: User can expand and check compile unit list for the modules that have debug info.
Reviewers: wallace, clayborg
Reviewed By: clayborg
Subscribers: aprantl, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D83072
Summary:
Whenever a module is created, removed or changed, lldb-vscode is now sending an event that can be interpreted by the IDE so that modules can be rendered in the IDE, like the tree view in this screenshot
{F12229758}
Reviewers: wallace, clayborg, kusmour, aadsm
Reviewed By: clayborg
Subscribers: cfe-commits, labath, lldb-commits
Tags: #lldb, #clang
Differential Revision: https://reviews.llvm.org/D82477
Summary:
This redoes https://reviews.llvm.org/D79726 and fixes two things.
- The logic that determines whether to automatically disconnect during the tear down is not very dumb compared to the original implementation. Each test will determine whether to do that or not.
- The terminate commands and terminate event were being sent after the disconnect response was sent to the IDE. That was not good, as VSCode stops the debug session as soon as it receives a disconnect response. Now, the terminate event and terminateEvents are being executed before the disconnect response is sent. This ensures that any connection between the IDE and lldb-vscode is alive while the terminate commands are executed. Besides, it also allows displaying the output of the terminate commands on the debug console, as it's still alive.
Reviewers: clayborg, aadsm, kusmour, labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D81978
Summary:
Recently I've noticed that VSCode sometimes doesn't send the terminateDebuggee flag within the disconnectRequest,
even though lldb-vscode sets the terminateDebuggee capability correctly.
This has been causing that inferiors don't die after the debug session ends, and many users have reported issues because of this.
An easy way to mitigate this is to set better default values for the terminateDebuggee field in the disconnect request.
I'm assuming that for a launch request, the default will be true, and for attach it'll be false.
Reviewers: clayborg, labath, aadsm
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D81200
Summary:
Running `vsce package` to package lldb-vscode as an installable .vsix file errors with:
```
ERROR Invalid publisher name 'llvm.org'. Expected the identifier of a publisher, not its human-friendly name.
```
This patch fixes the publisher name and bumps a required dependency so that `vsce package` succeeds.
Reviewers: clayborg
Reviewed By: clayborg
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D80569
Summary:
It turns out that the order in which we provide completions for expressions is
nondeterministic. This leads to confusing user experience and also breaks the
reproducer tests (as two LLDB tests can go out of sync due to the
non-determinism in the completion lists)
The reason for the non-determinism is that the CompletionConsumer informs us
about decls in the order in which it finds declarations in the lookup store of
the DeclContexts it visits (mainly this snippet in SemaLookup.cpp):
``` lang=c++
// Enumerate all of the results in this context.
for (DeclContextLookupResult R :
Load ? Ctx->lookups()
: Ctx->noload_lookups(/*PreserveInternalState=*/false)) {
[...]
```
This storage of the lookup is sorted by pointer values (see the hash of
`DeclarationName`) and can therefore be non-deterministic. The LLDB code
completion consumer that receives these calls originally expected that the order
of declarations is defined by Clang, but it seems the API expects the client to
provide an order to the completions.
This patch fixes the issue as follows:
* We sort the completions we get from Clang alphabetically and also by the
priority value we get from Clang (with priority value sorting having precedence
over the alphabetical sorting)
* We make all the functions/variables that touch a completion before the sorting
const-qualified. The idea is that this should prevent that we never have
observable side-effect from touching these declarations in a non-deterministic
order (e.g., we don't try to complete the type by accident).
This way we behave like the other parts of Clang which also sort the results by
some deterministic value (usually the name or something computed from a name,
e.g., edit distance to a given string).
We most likely also need to fix the Clang code to make the loop I listed above
deterministic to prevent these issues in the future (tracked in rdar://63442513
). This wouldn't replace the functionality provided in this patch though as we
would still need the priority and overall alphabetical sorting.
Note: I had to increase the lldb-vscode completion limit to 100 as the tests
look for strings that aren't in the first 50 results anymore due to variable
names starting with letters like 'v' (which are now always shown much further
down in the list due to the alphabetical sorting).
Fixes rdar://63200995
Reviewers: JDevlieghere, clayborg
Reviewed By: JDevlieghere
Subscribers: mgrang, abidh
Differential Revision: https://reviews.llvm.org/D80292
Summary: Adding this in line with "stopCommands" and "exitCommands" so that we can run commands at the end of the debugging session.
Reviewers: clayborg, wallace, labath
Reviewed By: clayborg, labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D79726
Summary:
Currently loading core files on lldb-vscode is broken because there's a check in the attach workflow that asserts that the PID is valid, which of course fails for this case.
Hence, I'm adding a "coreFile" argument for the attach request, which does the work correctly.
I don't know how to test it effectively so that it runs on the buildbots and the debugger can in fact makes sense of it. Anyway, the change has been relatively simple.
Reviewers: labath, clayborg
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D78839
to make the code conform to llvm style better:
- avoid use of auto where the type is not obivous
- avoid StringRef::data where it is not needed
No functional change intended.
Summary:
When using source maps for a breakpoint, in order to find the actual source that breakpoint has resolved, we
need to use a query similar to what CommandObjectSource::DumpLinesInSymbolContexts does, which is the logic
used by the CLI to display the source line at a given breakpoint. That's necessary because from a breakpoint
location you only have an address, which points to the original source location, not the source mapped one.
in the setBreakpoints request handler, we haven't been doing such query and we were returning the original
source location, which was breaking the UX of VSCode, as many breakpoints were being set but not displayed
in the source file next to each line. Besides, clicking the source path of a breakpoint in the breakpoints
view in the debug tab was not working for this case, as VSCode was trying to open a non-existent file, thus
showing an error to the user.
Ideally, we should do the query mentioned above to find the actual source location to respond to the IDE,
however, that query is expensive and users can have an arbitrary number of breakpoints set. As a simpler fix,
the request sent by VSCode already contains the full source path, which exists because the user set it from
the IDE itself, so we can simply reuse it instead of querying from the SB API.
I wrote a test for this, and found out that I had to move SetSourceMapFromArguments after RunInitCommands in
lldb-vscode.cpp, because an init command used in all tests is `settings clear -all`, which would clear the
source map unless specified after initCommands. And in any case, I think it makes sense to have initCommands
run before anything the debugger would do.
Reviewers: clayborg, kusmour, labath, aadsm
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76968
Summary: The IDE has no packets that are sent to lldb-vscode that say which thread and frame are selected. The only way we know is we get a request for variables for a stack frame via a "scopes" request. When we receive this packet we make that thread and frame the selected thread and frame in lldb. This way when people execute lldb commands in the debug console by prefixing the expression with the backtick character, we will have the right thread and frame selected. Previously this was not updated as new stack frames were selected.
Reviewers: labath, aadsm, wallace, JDevlieghere
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77347
Summary:
The DAP specifies the following for the SetBreakpoints request:
The breakpoints returned are in the same order as the elements of the 'breakpoints' arguments
This was not followed, as lldb-vscode was returning the breakpoints in a different order, because they were first stored into a map, and then traversed. Of course, maps normally don't preserve ordering.
See this log I captured:
-->
{"command":"setBreakpoints",
"arguments":{
"source":{
"name":"main.cpp",
"path":"/Users/wallace/fbsource/xplat/sand/test-projects/buck-cpp/main.cpp"
},
"lines":[6,10,11],
"breakpoints":[{"line":6},{"line":10},{"line":11}],
"sourceModified":false
},
"type":"request",
"seq":3
}
<--
{"body":{
"breakpoints":[
{"id":1, "line":11,"source":{"name":"main.cpp","path":"xplat/sand/test-projects/buck-cpp/main.cpp"},"verified":true},
{"id":2,"line":6,"source":{"name":"main.cpp","path":"xplat/sand/test-projects/buck-cpp/main.cpp"},"verified":true},
{"id":3,"line":10,"source":{"name":"main.cpp","path":"xplat/sand/test-projects/buck-cpp/main.cpp"},"verified":true}]},
"command":"setBreakpoints",
"request_seq":3,
"seq":0,
"success":true,
"type":"response"
}
As you can see, the order was not respected. This was causing the IDE not to be able to disable/enable breakpoints by clicking on them in the breakpoint view in the lower corner of the Debug tab.
This diff fixes the ordering problem. The traversal + querying was done very fast in O(nlogn) time. I'm keeping the same complexity.
I also updated a couple of tests to account for the ordering.
Reviewers: clayborg, aadsm, kusmour, labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76891
Those fields inside of the global variable can be local variables because
they are used in only inside of one function: request_launch for launch_info
and request_attach for attach_info.
To avoid confusion an already existing local variable attach_info of
request_attach has been renamed to better reflect its purpose.
Differential Revision: https://reviews.llvm.org/D76593
Summary:
If no custom launching is used, lldb-vscode launches a program with an empty environment by default. In some scenarios, the user might want to simply use the same environment as the IDE to have a set of working environment variables (e.g. PATH wouldn't be empty). In fact, most DAPs in VSCode have this behavior by default. In other cases the user definitely needs to set their custom environment, which is already supported. To make the first case easier for the user (e.g. not having to copy the PATH to the launch.json every time they want to debug simple programs that rely on PATH), a new option is now offered. inheritEnvironment will launch the program copying its own environment, and it's just a boolean flag.
{F11347695}
Reviewers: clayborg, aadsm, diazhector98, kusmour
Subscribers: labath, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74636
Summary:
https://reviews.llvm.org/D65363 introduced the launchCommands argument. However, it did not add
a corresponding definition in the package.json
Reviewers: clayborg, labath, kusmour, aadsm
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76529
Summary:
On Linux, when executing lldb-vscode on a remote machine, lldb-vscode doesn't die after the debug session ends. It keeps trying to read JSON input to no avail.
This diff indicates lldb-vscode to stop reading after a termination event has been processed.
Reviewers: clayborg, aadsm, kusmour
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76314
Walter Erquinigo