See #144777 for details.
See #142196 and https://github.com/llvm/llvm-zorg/pull/452 for details.
The WatchAddress API includes a flag to indicate if watchpoint should be for read, modify or both. This API uses 2 booleans, but the 'modify' flag was ignored and WatchAddress unconditionally watched write (actually modify). We now only watch for modify when the modify flag is true. --- The included test fails prior to this patch and succeeds after. That is previously specifying `False` for `modify` would still stop on _write_, but after the patch correctly only stops on _read_
This uses [teyit](https://pypi.org/project/teyit/) to modernize asserts, as recommended by the [unittest release notes](https://docs.python.org/3.12/whatsnew/3.12.html#id3). For example, `assertTrue(a == b)` is replaced with `assertEqual(a, b)`. This produces better error messages, e.g. `error: unexpectedly found 1 and 2 to be different` instead of `error: False`.
This patch is rearranging code a bit to add WatchpointResources to
Process. A WatchpointResource is meant to represent a hardware
watchpoint register in the inferior process. It has an address, a size,
a type, and a list of Watchpoints that are using this
WatchpointResource.
This current patch doesn't add any of the features of
WatchpointResources that make them interesting -- a user asking to watch
a 24 byte object could watch this with three 8 byte WatchpointResources.
Or a Watchpoint on 1 byte at 0x1002 and a second watchpoint on 1 byte at
0x1003, these must both be served by a single WatchpointResource on that
doubleword at 0x1000 on a 64-bit target, if two hardware watchpoint
registers were used to track these separately, one of them may not be
hit. Or if you have one Watchpoint on a variable with a condition set,
and another Watchpoint on that same variable with a command defined or
different condition, or ignorecount, both of those Watchpoints need to
evaluate their criteria/commands when their WatchpointResource has been
hit.
There's a bit of code movement to rearrange things in the direction I'll
need for implementing this feature, so I want to start with reviewing &
landing this mostly NFC patch and we can focus on the algorithmic
choices about how WatchpointResources are shared and handled as they're
triggeed, separately.
This patch also stops printing "Watchpoint <n> hit: old value: <x>, new
vlaue: <y>" for Read watchpoints. I could make an argument for print
"Watchpoint <n> hit: current value <x>" but the current output doesn't
make any sense, and the user can print the value if they are
particularly interested. Read watchpoints are used primarily to
understand what code is reading a variable.
This patch adds more fallbacks for how to print the objects being
watched if we have types, instead of assuming they are all integral
values, so a struct will print its elements. As large watchpoints are
added, we'll be doing a lot more of those.
To track the WatchpointSP in the WatchpointResources, I changed the
internal API which took a WatchpointSP and devolved it to a Watchpoint*,
which meant touching several different Process files. I removed the
watchpoint code in ProcessKDP which only reported that watchpoints
aren't supported, the base class does that already.
I haven't yet changed how we receive a watchpoint to identify the
WatchpointResource responsible for the trigger, and identify all
Watchpoints that are using this Resource to evaluate their conditions
etc. This is the same work that a BreakpointSite needs to do when it has
been tiggered, where multiple Breakpoints may be at the same address.
There is not yet any printing of the Resources that a Watchpoint is
implemented in terms of ("watchpoint list", or
SBWatchpoint::GetDescription).
"watchpoint set var" and "watchpoint set expression" take a size
argument which was previously 1, 2, 4, or 8 (an enum). I've changed this
to an unsigned int. Most hardware implementations can only watch 1, 2,
4, 8 byte ranges, but with Resources we'll allow a user to ask for
different sized watchpoints and set them in hardware-expressble terms
soon.
I've annotated areas where I know there is work still needed with
LWP_TODO that I'll be working on once this is landed.
I've tested this on aarch64 macOS, aarch64 Linux, and Intel macOS.
https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116
(cherry picked from commit fc6b72523f3d73b921690a713e97a433c96066c6)
...and follow ups. As it has caused test failures on Linux Arm and AArch64: https://lab.llvm.org/buildbot/#/builders/96/builds/49126 https://lab.llvm.org/buildbot/#/builders/17/builds/45824 ``` lldb-shell :: Subprocess/clone-follow-child-wp.test lldb-shell :: Subprocess/fork-follow-child-wp.test lldb-shell :: Subprocess/vfork-follow-child-wp.test ``` This reverts commit a6c62bf1a4717accc852463b664cd1012237d334, commit a0a1ff3ab40e347589b4e27d8fd350c600526735 and commit fc6b72523f3d73b921690a713e97a433c96066c6.
This patch is rearranging code a bit to add WatchpointResources to
Process. A WatchpointResource is meant to represent a hardware
watchpoint register in the inferior process. It has an address, a size,
a type, and a list of Watchpoints that are using this
WatchpointResource.
This current patch doesn't add any of the features of
WatchpointResources that make them interesting -- a user asking to watch
a 24 byte object could watch this with three 8 byte WatchpointResources.
Or a Watchpoint on 1 byte at 0x1002 and a second watchpoint on 1 byte at
0x1003, these must both be served by a single WatchpointResource on that
doubleword at 0x1000 on a 64-bit target, if two hardware watchpoint
registers were used to track these separately, one of them may not be
hit. Or if you have one Watchpoint on a variable with a condition set,
and another Watchpoint on that same variable with a command defined or
different condition, or ignorecount, both of those Watchpoints need to
evaluate their criteria/commands when their WatchpointResource has been
hit.
There's a bit of code movement to rearrange things in the direction I'll
need for implementing this feature, so I want to start with reviewing &
landing this mostly NFC patch and we can focus on the algorithmic
choices about how WatchpointResources are shared and handled as they're
triggeed, separately.
This patch also stops printing "Watchpoint <n> hit: old value: <x>, new
vlaue: <y>" for Read watchpoints. I could make an argument for print
"Watchpoint <n> hit: current value <x>" but the current output doesn't
make any sense, and the user can print the value if they are
particularly interested. Read watchpoints are used primarily to
understand what code is reading a variable.
This patch adds more fallbacks for how to print the objects being
watched if we have types, instead of assuming they are all integral
values, so a struct will print its elements. As large watchpoints are
added, we'll be doing a lot more of those.
To track the WatchpointSP in the WatchpointResources, I changed the
internal API which took a WatchpointSP and devolved it to a Watchpoint*,
which meant touching several different Process files. I removed the
watchpoint code in ProcessKDP which only reported that watchpoints
aren't supported, the base class does that already.
I haven't yet changed how we receive a watchpoint to identify the
WatchpointResource responsible for the trigger, and identify all
Watchpoints that are using this Resource to evaluate their conditions
etc. This is the same work that a BreakpointSite needs to do when it has
been tiggered, where multiple Breakpoints may be at the same address.
There is not yet any printing of the Resources that a Watchpoint is
implemented in terms of ("watchpoint list", or
SBWatchpoint::GetDescription).
"watchpoint set var" and "watchpoint set expression" take a size
argument which was previously 1, 2, 4, or 8 (an enum). I've changed this
to an unsigned int. Most hardware implementations can only watch 1, 2,
4, 8 byte ranges, but with Resources we'll allow a user to ask for
different sized watchpoints and set them in hardware-expressble terms
soon.
I've annotated areas where I know there is work still needed with
LWP_TODO that I'll be working on once this is landed.
I've tested this on aarch64 macOS, aarch64 Linux, and Intel macOS.
https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116
The Watchpoint and Breakpoint objects try to track the hardware index that was used for them, if they are hardware wp/bp's. The majority of our debugging goes over the gdb remote serial protocol, and when we set the watchpoint/breakpoint, there is no (standard) way for the remote stub to communicate to lldb which hardware index was used. We have an lldb-extension packet to query the total number of watchpoint registers. When a watchpoint is hit, there is an lldb extension to the stop reply packet (documented in lldb-gdb-remote.txt) to describe the watchpoint including its actual hardware index, <addr within wp range> <wp hw index> <actual accessed address> (the third field is specifically needed for MIPS). At this point, if the stub reported these three fields (the stub is only required to provide the first), we can know the actual hardware index for this watchpoint. Breakpoints are worse; there's never any way for us to be notified about which hardware index was used. Breakpoints got this as a side effect of inherting from StoppointSite with Watchpoints. We expose the watchpoint hardware index through "watchpoint list -v" and through SBWatchpoint::GetHardwareIndex. With my large watchpoint support, there is no *single* hardware index that may be used for a watchpoint, it may need multiple resources. Also I don't see what a user is supposed to do with this information, or an IDE. Knowing the total number of watchpoint registers on the target, and knowing how many Watchpoint Resources are currently in use, is helpful. Knowing how many Watchpoint Resources a single user-specified watchpoint needed to be implemented is useful. But knowing which registers were used is an implementation detail and not available until we hit the watchpoint when using gdb remote serial protocol. So given all that, I'm removing watchpoint hardware index numbers. I'm changing the SB API to always return -1.
This reverts commit a7b78cac9a77e3ef6bbbd8ab1a559891dc693401. With updates to the tests. TestWatchTaggedAddress.py: Updated the expected watchpoint types, though I'm not sure there should be a differnt default for the two ways of setting them, that needs to be confirmed. TestStepOverWatchpoint.py: Skipped this everywhere because I think what used to happen is you couldn't put 2 watchpoints on the same address (after alignment). I guess that this is now allowed because modify watchpoints aren't accounted for, but likely should be. Needs investigating.
This reverts commit 933ad5c897ee366759a54869b35b2d7285a92137. This caused 1 test failure and an unexpected pass on AArch64 Linux: https://lab.llvm.org/buildbot/#/builders/96/builds/45765 Wasn't reported because the bot was already red at the time.