A synthetic child provider might need to do considerable amount of work
to compute the number of children. lldb-dap is currently calling that
for all synthethic variables, but it's only actually using the value for
values which it deems to be "indexed" (which is determined by looking at
the name of the first child). This patch reverses the logic so that
GetNumChildren is only called for variables with a suitable first child.
Update the folder titles for targets in the monorepository that have not
seen taken care of for some time. These are the folders that targets are
organized in Visual Studio and XCode
(`set_property(TARGET <target> PROPERTY FOLDER "<title>")`)
when using the respective CMake's IDE generator.
* Ensure that every target is in a folder
* Use a folder hierarchy with each LLVM subproject as a top-level folder
* Use consistent folder names between subprojects
* When using target-creating functions from AddLLVM.cmake, automatically
deduce the folder. This reduces the number of
`set_property`/`set_target_property`, but are still necessary when
`add_custom_target`, `add_executable`, `add_library`, etc. are used. A
LLVM_SUBPROJECT_TITLE definition is used for that in each subproject's
root CMakeLists.txt.
VSCode will automatically ask for the children (in structured form) so
there's no point in sending the textual representation. This can make
displaying hover popups for complex variables with complicated data
formatters much faster. See discussion on #77026 for context.
ResolveCommand will not succeed for an alias command with arguments, and
the code wasn't providing any. Replace that with explicit query(ies) for
the existence of a command with the given name.
The README.md is what users see when they look for the extension in the
Marketplace [1]. Right now, it's a mix of developer documentation (for
us) and user documentation. This commit moves the developer docs into
`docs` and the lldb website and refocuses the README on using the
extension.
[1] https://marketplace.visualstudio.com/items?itemName=llvm-vs-code-extensions.lldb-dap
The `disconnect` response contains the `error` message with invalid
characters (a junk data). To reproduce this issue it is enough to run
the `TestDAP_commands` test on Windows host and Linux target. The test
will fail to run ELF file on Windows and dap_server will be disconnected
unexpectedly.
Note dap_server hangs if read_packet() cannot decode JSON with invalid
characters. read_packet() must return None in this case instead of an
exception. But dap_server does not require any fix after this patch.
lldb-dap was setting a flag which was meant to shut it down as soon as
it sent a terminated event. The problem with this flag is two-fold:
- as far as I can tell (definitely not an expert here), there's no
justification for this in the protocol spec. The only way I found to
shut the server down was to send it a disconnect request.
- the flag did not actually work most of the time, because it's only
checked between requests so nothing will happen if the server starts
listening for a new request before a different thread manages to send
the terminated event. And since the next request is usually the
disconnect request, everything will operate normally.
The combination of these two things meant that the issue was largely
unnoticable, except for rare flaky test failures, which happened when
the handler thread was too slow, and checked the flag after it has
already been said. This caused the test suite to complain as it did not
get a response to the disconnect request. This situation could be
s(t)imulated by adding a sleep to the and of the main loop, which
delayed the flag check, and caused the DAP tests to fail reliably.
This patch changes the shutdown condition to only trigger when the
disconnect request has been received. Since the flag can now only be set
from the handler thread, it no longer needs to be atomic.
On Darwin platforms, the system will generate a crash report in
~/Library/Logs/DiagnosticReports/ when a process crashes.
These reports are much more useful than the "pretty backtraces" printed
by LLVM and are preferred when filing bug reports on Darwin.
Fixes#47549
`lldb-server`'s platform mode seems to have an issue with its
`--min-gdbserver-port` `--max-gdbserver-port` flags (and probably the
`--gdbserver-port` flag, but I didn't test it).
How the platform code seems to work is that it listens on a port, and
whenever there's an incoming connection, it forks the process to handle
the connection. To handle the port flags, the main process uses an
instance of the helper class
`GDBRemoteCommunicationServerPlatform::PortMap`, that can be configured
and track usages of ports. The child process handling the platform
connection, can then use the port map to allocate a port for the
gdb-server connection it will make (this is another process it spawns).
However, in the current code, this works only once. After the first
connection is handled by forking a child process, the main platform
listener code loops around, and then 'forgets' about the port map. This
is because this code:
```cpp
GDBRemoteCommunicationServerPlatform platform(
acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
if (!gdbserver_portmap.empty()) {
platform.SetPortMap(std::move(gdbserver_portmap));
}
```
is within the connection listening loop. This results in the
`gdbserver_portmap` being moved into the platform object at the
beginning of the first iteration of the loop, but on the second
iteration, after the first fork, the next instance of the platform
object will not have its platform port mapped.
The result of this bug is that subsequent connections to the platform,
when spawning the gdb-remote connection, will be supplied a random port
- which isn't bounded by the `--min-gdbserver-port` and
`--max-gdbserver--port` parameters passed in by the user.
This PR fixes this issue by having the port map be maintained by the
parent platform listener process. On connection, the listener allocates
a single available port from the port map, associates the child process
pid with the port, and lets the connection handling child use that
single port number.
Additionally, when cleaning up child processes, the main listener
process tracks the child that exited to deallocate the previously
associated port, so it can be reused for a new connection.
`SBProcess::GetMemoryRegionInfo` uses `qMemoryRegionInfo` packet to get
memory region info, but this is not supported in gdb-server and causing
downstream lldb test failures. This change ignores the the error from
`SBProcess::GetMemoryRegionInfo` .
Reported by @tedwoodward @jerinphilip.
While adding a UI feature in VSCode to toggle hex/dec in variables view
window. I noticed that it does not work after second toggle. Then I
noticed that there is a bug that we only explicitly set hex format not
reset back to default during further toggle. The new test demonstrates
the bug.
This PR resets the format back to default if not using hex. One
complexity is that, we explicitly set registers value format to
AddressInfo, which shouldn't be overridden by default or hex settings.
---------
Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
In DNBArchImplARM64.cpp I'm doing
```
#if __has_feature(ptrauth_calls) && defined(__LP64__)
```
And the preprocessor warns that this is not defined behavior. This
checks if ptrauth_calls is available and if this is being compiled
64-bit (i.e. arm64e), and defines a single DEBUGSERVER_IS_ARM64E when
those are both true.
I did have to duplicate one DNBLogThreaded() call which itself is a
macro, and using an ifdef in the middle of macro arguments also got me a
warning from the preprocessor.
While testing this for all the different targets, I found a DNBError
initialization that accepts a c-string but I'm passing in a printf-style
formatter c-string and an argument. Create the string before the call
and pass in the constructed string.
rdar://127129242
https://github.com/llvm/llvm-project/pull/87409 removed the broadcast
bits from SBDebugger and placed them in `lldb-enumerations.h`. This is
API-breaking so this commits places the enum back into `SBDebugger.h`
and references the bits from `lldb-enumerations.h`.
rdar://127128536
This patch provides the initial implementation for the "Step Into
Specific/Step In Targets" feature in VSCode DAP.
The implementation disassembles all the call instructions in step range
and try to resolve operand name (assuming one operand) using debug info.
Later, the call target function name is chosen by end user and specified
in the StepInto() API call.
It is v1 because of using the existing step in target function name API.
This implementation has several limitations:
* Won't for indirect/virtual function call -- in most cases, our
disassembler won't be able to solve the indirect call target
address/name.
* Won't work for target function without debug info -- if the target
function has symbol but not debug info, the existing
ThreadPlanStepInRange won't stop.
* Relying on function names can be fragile -- if there is some middle
glue/thunk code, our disassembler can only resolve the glue/thunk code's
name not the real target function name. It can be fragile to depend
compiler/linker emits the same names for both.
* Does not support step into raw address call sites -- it is a valid
scenario that in Visual Studio debugger, user can explicitly choose a
raw address to step into which land in the function without debug
info/symbol, then choose UI to load the debug info on-demand for that
module/frame to continue exploring.
A more reliable design could be extending the ThreadPlanStepInRange to
support step in based on call-site instruction offset/PC which I will
propose in next iteration.
---------
Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
Summary:
When the target inferior process that is being debugged exits in lldb
command line, it emits following message:
`Process 4049526 exited with status = -1 (0xffffffff) debugserver died
with signal SIGTERM`
lldb-dap on the other hand does not emit a similar message. This PR adds
the same status message to lldb-dap.
Test Plan:
In VSCode debug any target and hit stop mode, kill lldb-server and
observe an exit status message similar to the following: Process 2167677
exited with status = -1 (0xffffffff) debugserver died with signal
SIGTERM
Reviewers:
@jeffreytan81,@clayborg,@kusmour,
Subscribers:
Tasks:
lldb-dap
Tags:
The `column` field is mandatory in StackTraceResponse, otherwise the
debugger client may raise error (e.g. VSCode can't correctly open an
editor without the column field)
---------
Signed-off-by: Xu Jun <693788454@qq.com>
…db-enumerations.h" (#88324)"
This reverts commit 9f6d08f2566a26144ea1753f80aebb1f2ecfdc63. This broke
the build because of a usage of one of the original SBDebugger broadcast
bits that wasn't updated in the original commit.
The first half of this patch is a long-standing annoyance, if I attach
to debugserver with lldb while it is waiting for an lldb connection, the
syscall is interrupted and it doesn't retry, debugserver exits
immediately.
The second half is a request from another tool that is communicating
with debugserver, that we retry reads on our sockets in the same way. I
haven't dug in to the details of how they're communicating that this is
necessary, but the best I've been able to find reading the POSIX API
docs, this is fine.
rdar://117113298
When terminating the debugger, we wait for all background tasks to
complete. Given that there's no way to interrupt those treads, this can
take a while. When that happens, the debugger appears to hang at exit.
The above situation is unfortunately not uncommon when background
downloading of dSYMs is enabled (`symbols.auto-download background`).
Even when calling dsymForUUID with a reasonable timeout, it can take a
while to complete.
This patch improves the user experience by printing a message from the
driver when it takes more than one (1) second to terminate the debugger.
Pavel added an extension to lldb's gdb remote serial protocol that
allows the debug stub to append an error message (ascii hex encoded)
after an error response packet Exx. This was added in 2017 in
https://reviews.llvm.org/D34945 . lldb sends the
QErrorStringInPacketSupported packet and then the remote stub may add
these error strings.
debugserver has two bugs in its use of extended error messages: the
vAttach family would send the extended error string without checking if
the mode had been enabled. And qLaunchSuccess would not properly format
its error response packet (missing the hex digits, did not asciihex
encode the string).
There is also a bug in the HandlePacket_D (detach) packet where the
error packets did not include hex digits, but this one does not append
an error string.
I'm adding a new RNBRemote::SendErrorPacket() and routing all error
packet returns though this one method. It takes an optional second
string which is the longer error message; it now handles appending it to
the Exx response or not, depending on the QErrorStringInPacketSupported
state. I updated all packets to send their errors via this method.
The -d(ebug) option broke 5 years ago when I migrated the driver to
libOption. Since then, we were never check if the option is set. We were
incorrectly toggling the internal variable (m_debug_mode) based on
OPT_no_use_colors instead.
Given that the functionality doesn't seem particularly useful and nobody
noticed it has been broken for 5 years, I'm just removing the flag.
If a SetDataBreakpointsRequest contains a list data breakpoints which
have duplicate starting addresses, the current behaviour is returning
`{verified: true}` to both watchpoints with duplicated starting
addresses. This confuses the client and what actually happens in lldb is
the second one overwrite the first one.
This fixes it by letting the last watchpoint at given address have
`{verified: true}` and all previous watchpoints at the same address
should have `{verfied: false}` at response.
The `EventThreadFunction` can end up calling `HandleCommand`
concurrently with the main request processing thread. The underlying API
does not appear to be thread safe, so add a narrowly scoped mutex lock
to prevent calling it in this place from more than one thread.
Fixes#81686. Prior to this, TestDAP_launch.py is 4% flaky. After, it
passes in 1000 runs.
This implements functionality to handle DataBreakpointInfo request and
SetDataBreakpoints request.
Previous commit
8c56e78ec5
was reverted because setting 1 byte watchpoint failed in the new test on
ARM64. So, I changed the test to setting 4 byte watchpoint instead, and
hope this won't break it again. It also adds the fixes from
https://github.com/llvm/llvm-project/pull/81680.
This adds a layer between `SounceBreakpoint`/`FunctionBreakpoint` and
`BreakpointBase` to have better separation and encapsulation so we are
not directly operating on `SBBreakpoint`.
I basically moved the `SBBreakpoint` and the methods that requires it
from `BreakpointBase` to `Breakpoint`. This allows adding support for
data watchpoint easier by sharing the logic inside `BreakpointBase`.
debugserver will not compress small packets; the overhead of the
compression header makes this useful for larger packets. I default to
384 bytes in debugserver, and added an option for lldb to request a
different cutoff. This option has never been used in lldb in the past
nine years, so I don't think there's any point to keeping it around.
debugserver on arm64 devices can manage both Byte Address Select
watchpoints (1-8 bytes) and MASK watchpoints (8 bytes-2 gigabytes). This
adds a SupportedWatchpointTypes key to the QSupported response from
debugserver with a list of these, so lldb can take full advantage of
them when creating larger regions with a single hardware watchpoint.
Also add documentation for this, and two other lldb extensions, to the
lldb-gdb-remote.txt documentation.
Re-enable TestLargeWatchpoint.py on Darwin systems when testing with the
in-tree built debugserver. I can remove the "in-tree built debugserver"
in the future when this new key is handled by an Xcode debugserver.
This patch is the next piece of work in my Large Watchpoint proposal,
https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116
This patch breaks a user's watchpoint into one or more
WatchpointResources which reflect what the hardware registers can cover.
This means we can watch objects larger than 8 bytes, and we can watched
unaligned address ranges. On a typical 64-bit target with 4 watchpoint
registers you can watch 32 bytes of memory if the start address is
doubleword aligned.
Additionally, if the remote stub implements AArch64 MASK style
watchpoints (e.g. debugserver on Darwin), we can watch any power-of-2
size region of memory up to 2GB, aligned to that same size.
I updated the Watchpoint constructor and CommandObjectWatchpoint to
create a CompilerType of Array<UInt8> when the size of the watched
region is greater than pointer-size and we don't have a variable type to
use. For pointer-size and smaller, we can display the watched granule as
an integer value; for larger-than-pointer-size we will display as an
array of bytes.
I have `watchpoint list` now print the WatchpointResources used to
implement the watchpoint.
I added a WatchpointAlgorithm class which has a top-level static method
that takes an enum flag mask WatchpointHardwareFeature and a user
address and size, and returns a vector of WatchpointResources covering
the request. It does not take into account the number of watchpoint
registers the target has, or the number still available for use. Right
now there is only one algorithm, which monitors power-of-2 regions of
memory. For up to pointer-size, this is what Intel hardware supports.
AArch64 Byte Address Select watchpoints can watch any number of
contiguous bytes in a pointer-size memory granule, that is not currently
supported so if you ask to watch bytes 3-5, the algorithm will watch the
entire doubleword (8 bytes). The newly default "modify" style means we
will silently ignore modifications to bytes outside the watched range.
I've temporarily skipped TestLargeWatchpoint.py for all targets. It was
only run on Darwin when using the in-tree debugserver, which was a proxy
for "debugserver supports MASK watchpoints". I'll be adding the
aforementioned feature flag from the stub and enabling full mask
watchpoints when a debugserver with that feature is enabled, and
re-enable this test.
I added a new TestUnalignedLargeWatchpoint.py which only has one test
but it's a great one, watching a 22-byte range that is unaligned and
requires four 8-byte watchpoints to cover.
I also added a unit test, WatchpointAlgorithmsTests, which has a number
of simple tests against WatchpointAlgorithms::PowerOf2Watchpoints. I
think there's interesting possible different approaches to how we cover
these; I note in the unit test that a user requesting a watch on address
0x12e0 of 120 bytes will be covered by two watchpoints today, a
128-bytes at 0x1280 and at 0x1300. But it could be done with a 16-byte
watchpoint at 0x12e0 and a 128-byte at 0x1300, which would have fewer
false positives/private stops. As we try refining this one, it's helpful
to have a collection of tests to make sure things don't regress.
I tested this on arm64 macOS, (genuine) x86_64 macOS, and AArch64
Ubuntu. I have not modifed the Windows process plugins yet, I might try
that as a standalone patch, I'd be making the change blind, but the
necessary changes (see ProcessGDBRemote::EnableWatchpoint) are pretty
small so it might be obvious enough that I can change it and see what
the Windows CI thinks.
There isn't yet a packet (or a qSupported feature query) for the gdb
remote serial protocol stub to communicate its watchpoint capabilities
to lldb. I'll be doing that in a patch right after this is landed,
having debugserver advertise its capability of AArch64 MASK watchpoints,
and have ProcessGDBRemote add eWatchpointHardwareArmMASK to
WatchpointAlgorithms so we can watch larger than 32-byte requests on
Darwin.
I haven't yet tackled WatchpointResource *sharing* by multiple
Watchpoints. This is all part of the goal, especially when we may be
watching a larger memory range than the user requested, if they then add
another watchpoint next to their first request, it may be covered by the
same WatchpointResource (hardware watchpoint register). Also one "read"
watchpoint and one "write" watchpoint on the same memory granule need to
be handled, making the WatchpointResource cover all requests.
As WatchpointResources aren't shared among multiple Watchpoints yet,
there's no handling of running the conditions/commands/etc on multiple
Watchpoints when their shared WatchpointResource is hit. The goal beyond
"large watchpoint" is to unify (much more) the Watchpoint and Breakpoint
behavior and commands. I have a feeling I may be slowly chipping away at
this for a while.
Re-landing this patch after fixing two undefined behaviors in
WatchpointAlgorithms found by UBSan and by failures on different
CI bots.
rdar://108234227
lldb-dap instances managed by other extensions benefit from having a
welcome message with, for example, a basic user guide or a
troubleshooting message.
This PR adds a cmake variable for defining such message in a simple way.
This message appears upon initialization but before initCommands are
executed, as they might cause a failure and prevent the message from
being displayed.
The previous logic for determining if an expression was a command or
variable expression in the repl would incorrectly identify the context
in many common cases where a local variable name partially overlaps with
the repl input.
For example:
```
int foo() {
int var = 1; // break point, evaluating "p var", previously emitted a warning
}
```
Instead of checking potentially multiple conflicting values against the
expression input, I updated the heuristic to only consider the first
term. This is much more reliable at eliminating false positives when the
input does not actually hide a local variable.
Additionally, I updated the warning on conflicts to occur anytime the
conflict is detected since the specific conflict can change based on the
current input. This also includes additional details on how users can
change the behavior.
Example Debug Console output from
lldb/test/API/tools/lldb-dap/evaluate/main.cpp:11 breakpoint 3.
```
lldb-dap> var + 3
Warning: Expression 'var' is both an LLDB command and variable. It will be evaluated as a variable. To evaluate the expression as an LLDB command, use '`' as a prefix.
45
lldb-dap> var + 1
Warning: Expression 'var' is both an LLDB command and variable. It will be evaluated as a variable. To evaluate the expression as an LLDB command, use '`' as a prefix.
43
```
There's a bad interaction between the macOS 14 dyld and the "dyld_sim"
shim that comes from older (iOS 15) simulator downloads that results in
dyld reporting some modules twice in the return from the dyld callback
to list modules. The records were identical, but lldb wasn't happy with
seeing the duplicates...
Since it's not possible to load two different modules at the same
address, this change just picks the first instance of any entries that
have the same load address.
There really isn't a good way to test this patch.
