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

Jason Molenda via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri Apr 28 09:26:19 PDT 2023


jasonmolenda added a comment.

Thanks for all the helpful comments, I'll update the patch.



================
Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:833-835
+std::vector<DNBArchMachARM64::watchpoint_spec>
+DNBArchMachARM64::AlignRequestedWatchpoint(nub_addr_t user_addr,
+                                           nub_size_t user_size) {
----------------
JDevlieghere wrote:
> Do you ever expect this to return more than two watchpoints? Seems like this could be a `struct` that holds two optional `WatchpointSpec`s. I don't feel strongly about it, but it looks like you never iterate over the list and the way you have to check after the recursive call is a bit awkward. 
> 
> edit: nvm, it looks like you do actually iterate over them in `EnableHardwareWatchpoint`
I was thinking about how this current scheme only ever splits 1 watchpoint into 2, but a future design that could expand a user requested watch into a larger number of hardware watchpoints would expand it further.  If a user asks to watch a 32 byte object, and the target only supports 8 byte watchpoints, and the object is doubleword aligned, we could watch it with 4 hardware watchpoints.  The older code in debugserver was written in terms of "one or two" but I switched to a vector of hardware implementable watchpoints that may expand as we evaluate the hardware capabilities of the target/stub.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149040/new/

https://reviews.llvm.org/D149040



More information about the lldb-commits mailing list