[Lldb-commits] [PATCH] D147816: Clarify how watchpoint description in stop packets work, fix AArch64 unintended behavior

Jason Molenda via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 11 15:37:40 PDT 2023

jasonmolenda added inline comments.

Comment at: lldb/docs/lldb-gdb-remote.txt:1609
+//           There may be false-positive watchpoint hits on AArch64 as well,
+//           in the SVE Streaming Mode, but that is less common (v. ESR 
+//           register flag "WPF", "Watchpoint might be False-Positive") and
DavidSpickett wrote:
> What does this "v." mean? Is it short for "see here " or some kind of citation?
Yeah, maybe not widely known.  "v." short for "vide" latin, see. cf https://latinlexicon.org/LNS_abbreviations.php ;)

Comment at: lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h:62
+  //   \a address is 0x1000
+  //   size is 8
+  //   If a one-byte write to 0x1006 is the most recent watchpoint trap,
DavidSpickett wrote:
> Worth noting why here? (minimum watch size and alignment I guess)
I was taking notes as I read the source to understand how arm linux behaved, and left those notes in the header, maybe I should just remove them. The DREG struct has three addresses in it, and it wasn't really clear to me what was stored in them.

Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2232-2235
+        // Rewrite this to mimic the "reason:watchpoint" and
+        // "description:ADDR" entries in the
+        // stop-reply packet, handled in
+        // ProcessGDBRemote::SetThreadStopInfo
DavidSpickett wrote:
> Is this a note to self or is this code now actually doing that?
Sorry that wasn't clear.  We can receive watchpoint notifications one of three ways:  `watch`/`awatch`/`rwatch` (standard gdb RSP), `reason:watchpoint` with a `description` asciihex string of 1-to-3 numbers, and on Darwin systems, a mach exception.  ProcessGDBRemote rewrites watch/awatch/rwatch to `reason:watchpoint` + `description` with only the one address, and that's the only thing decoded into a StopInfo object.

Comment at: lldb/source/Target/StopInfo.cpp:1019
   bool m_should_stop_is_valid = false;
-  lldb::addr_t m_watch_hit_addr;
+  bool m_silently_skip = false;
   bool m_step_over_plan_complete = false;
DavidSpickett wrote:
> Please document this here as well.
> I am not 100% whether silently skip true means always silently skip this watchpoint, or silently skip out of range hits attributed to this watchpoint.
Good point.

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list