[all-commits] [llvm/llvm-project] 147d7a: [lldb] Add support for large watchpoints in lldb (...

[Jason Molenda via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 147d7a64f8493e78669581097a3ff06c985aa3a1
      https://github.com/llvm/llvm-project/commit/147d7a64f8493e78669581097a3ff06c985aa3a1
  Author: Jason Molenda <jason at molenda.com>
  Date:   2024-01-31 (Wed, 31 Jan 2024)

  Changed paths:
    A lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h
    M lldb/include/lldb/lldb-enumerations.h
    M lldb/packages/Python/lldbsuite/test/concurrent_base.py
    M lldb/source/Breakpoint/CMakeLists.txt
    M lldb/source/Breakpoint/Watchpoint.cpp
    A lldb/source/Breakpoint/WatchpointAlgorithms.cpp
    M lldb/source/Breakpoint/WatchpointResource.cpp
    M lldb/source/Commands/CommandObjectWatchpoint.cpp
    M lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
    M lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
    M lldb/source/Target/StopInfo.cpp
    M lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py
    A lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/Makefile
    A lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/TestUnalignedLargeWatchpoint.py
    A lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/main.c
    M lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
    M lldb/unittests/Breakpoint/CMakeLists.txt
    A lldb/unittests/Breakpoint/WatchpointAlgorithmsTests.cpp

  Log Message:
  -----------
  [lldb] Add support for large watchpoints in lldb (#79962)

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