[Lldb-commits] [PATCH] D147820: debugserver: move AArch64 watchpoint traps within a watchpointed region, parse ESR flags and send them to lldb

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

I didn't double check the spec so I'm assuming those bitfield indexes are correct. Everything else makes sense to me. LGTM.



================
Comment at: lldb/tools/debugserver/source/DNBBreakpoint.cpp:114-125
+      if (addr < start_addr) {
+        uint32_t delta = start_addr - addr;
+        if (delta < best_match) {
+          closest = &pos.second;
+          best_match = delta;
+        } else {
+          uint32_t delta = addr - end_addr;
----------------
You could simplify this with the ternary operator: 

```
uint32_t delta = addr < start_addr ? start_addr - addr : addr - end_addr;
if (delta < best_match) {
  closest = &pos.second;
  best_match = delta;
}
```


================
Comment at: lldb/tools/debugserver/source/DNBDefs.h:284-297
+      bool esr_fields_set;
+      struct {
+        uint32_t
+            iss; // "ISS encoding for an exception from a Watchpoint exception"
+        uint32_t wpt;  // Watchpoint number
+        bool wptv;     // Watchpoint number Valid
+        bool wpf;      // Watchpoint might be false-positive
----------------
Instead of a boolean and a field, you could make it a `std::optional<esr_field>`. 


================
Comment at: lldb/tools/debugserver/source/MacOSX/MachException.cpp:162-169
+  if (exc_type == EXC_BREAKPOINT && exc_data[0] == EXC_ARM_DA_DEBUG &&
+      exc_data.size() > 1) {
+    stop_info->reason = eStopTypeWatchpoint;
+    stop_info->details.watchpoint.mach_exception_addr = exc_data[1];
+    stop_info->details.watchpoint.addr = INVALID_NUB_ADDRESS;
+    if (exc_data.size() >= 3) {
+      stop_info->details.watchpoint.hw_idx = exc_data[2];
----------------
Interesting that you picked `> 1` on line 163 and `>= 3` on line 168. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147820