[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