[Lldb-commits] [PATCH] D149040: Refactor and generalize debugserver code for setting hardware watchpoints on AArch64

Sun Apr 23 23:17:15 PDT 2023

jasonmolenda created this revision.
jasonmolenda added a reviewer: JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

I am adding MASK (power of 2, with that alignment) watchpoint support to debugserver AArch64, where it currently only does Byte Address Select (BAS, 1-8 bytes aligned to doubleword) watchpoints.  This patch generalizes debugserver's watchpoint setting code so that it will be simple to drop in MASK watchpoints for larger memory regions in a later patch.

debugserver supports an unusual feature where a request to watch an unaligned range of bytes is handled.  If you have a 16 byte buffer at 0x1000 and ask to watch 4 bytes at 0x1006, lldb needs to watch 0x1006-0x1009 inclusive.  A single BAS watchpoint can only watch either 0x1000-0x1007 or 0x1008-0x100f.  Debugserver splits this unaligned watch request into two BAS watchpoints and uses two hardware registers to do it.  Most of this patch is generalizing that "align and find the correct place to split the watch request" code to work for any size watch request byte range, not just 8-byte watch requests.  I spent a bit of time to check that it was behaving correctly for a lot of BAS+MASK scenarios, e.g. watch 32 bytes at 0x1038 means an 8-byte BAS watchpoint for 0x1038-0x103f and a 32-byte MASK watchpoint from 0x1040-0x105f, ideally ignoring writes to the last 8 bytes which the user didn't ask to watch.

This is also  part of the processing I'll need to do for the WatchpointLocations I proposed a bit ago ( https://discourse.llvm.org/t/aarch64-watchpoints-reported-address-outside-watched-range-adopting-mask-style-watchpoints/67660/6 ).  If a target/stub only supports 8-byte watchpoints, and a user asks to watch a 32-byte object, lldb should use 4 watchpoint registers (if it can) to watch that object.  Or a single MASK watchpoint register.  Or two hardware registers if we have MASK+BAS to watch an unaligned memory range.

This is only doing the simple "find correct alignment, split memory request to aligned ranges", it doesn't try to use n watchpoints to watch a large object, only 2 for unaligned requests.  It's a starting point.

debugserver internally has a "HiLo" table to track these joined hardware registers corresponding to one user requested watchpoint.  There's no feedback given to lldb that this is how the object was watched; the Z packet doesn't have any way of communicating it.  I added a test for this specific debugserver feature where I watch 4 bytes unaligned, then detect that the watchpoints are triggered as loops in the binary run.  It's not a fancy test, but we didn't have anything for this previously so it's a simple start.

I expect this patch to result in no observable change in behavior to debugserver.  It is laying the groundwork for adding MASK watchpoints, which will be a little patch on top of it.  lldb itself is in no position to deal with larger watchpoints (and the false positives we can get when we watch a region larger than the user's requested one).  I also don't try to solve a user requesting multiple watchpoints that all overlap in a single hardware register.  e.g. user asks to watch 0x1000-0x1001 and 0x1005-0x1007 inclusive.  This can be represented by a single BAS watchpoint in AArch64 watching noncontiguous byte ranges within that doubleword, but debugserver isn't set up to combine a new watchpoint request with existing watchpoints that already cover part of that range.  Another thing I might need to think about when we start using power-of-2 MASK watchpoints.

Functionally, it's taking EnableHardwareWatchpoint, separating out the "align and split" logic to its own method, then once we have valid watchpoint specification(s), we call out to the new EnableBASWatchpoint to set the correct bits in the control register.  The next patch will be to add an EnableMASKWatchpoint for larger requests.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149040

Files:
  lldb/test/API/functionalities/watchpoint/unaligned-spanning-two-dwords/Makefile
  lldb/test/API/functionalities/watchpoint/unaligned-spanning-two-dwords/TestUnalignedSpanningDwords.py
  lldb/test/API/functionalities/watchpoint/unaligned-spanning-two-dwords/main.c
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149040.516276.patch
Type: text/x-patch
Size: 20512 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230424/1ad9dfea/attachment-0001.bin>