[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jason Molenda via lldb-commits]

================
@@ -1787,30 +1788,48 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
           // addresses should be provided as \a wp_addr.
           StringExtractor desc_extractor(description.c_str());
           addr_t wp_addr = desc_extractor.GetU64(LLDB_INVALID_ADDRESS);
-          uint32_t wp_index = desc_extractor.GetU32(LLDB_INVALID_INDEX32);
+          wp_resource_id_t wp_hw_index =
+              desc_extractor.GetU32(LLDB_INVALID_WATCHPOINT_RESOURCE_ID);
           addr_t wp_hit_addr = desc_extractor.GetU64(LLDB_INVALID_ADDRESS);
           watch_id_t watch_id = LLDB_INVALID_WATCH_ID;
           bool silently_continue = false;
-          WatchpointSP wp_sp;
+          WatchpointResourceSP wp_resource_sp;
+          if (wp_hw_index != LLDB_INVALID_WATCHPOINT_RESOURCE_ID) {
+            wp_resource_sp = m_watchpoint_resource_list.FindByID(wp_hw_index);
+            if (wp_resource_sp) {
+              // If we were given an access address, and the Resource we
+              // found by watchpoint register index does not contain that
+              // address, then the wp_resource_id's have not tracked the
+              // hardware watchpoint registers correctly, discard this
+              // Resource found by ID and look it up by access address.
+              if (wp_hit_addr != LLDB_INVALID_ADDRESS &&
+                  !wp_resource_sp->Contains(wp_hit_addr)) {
+                wp_resource_sp.reset();
+              }
+            }
+          }
           if (wp_hit_addr != LLDB_INVALID_ADDRESS) {
-            wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_hit_addr);
+            wp_resource_sp =
+                m_watchpoint_resource_list.FindByAddress(wp_hit_addr);
             // On MIPS, \a wp_hit_addr outside the range of a watched
             // region means we should silently continue, it is a false hit.
             ArchSpec::Core core = GetTarget().GetArchitecture().GetCore();
-            if (!wp_sp && core >= ArchSpec::kCore_mips_first &&
+            if (!wp_resource_sp && core >= ArchSpec::kCore_mips_first &&
                 core <= ArchSpec::kCore_mips_last)
               silently_continue = true;
           }
-          if (!wp_sp && wp_addr != LLDB_INVALID_ADDRESS)
-            wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_addr);
-          if (wp_sp) {
-            wp_sp->SetHardwareIndex(wp_index);
-            watch_id = wp_sp->GetID();
-          }
-          if (watch_id == LLDB_INVALID_WATCH_ID) {
+          if (!wp_resource_sp && wp_addr != LLDB_INVALID_ADDRESS)
+            wp_resource_sp = m_watchpoint_resource_list.FindByAddress(wp_addr);
+          if (!wp_resource_sp) {
             Log *log(GetLog(GDBRLog::Watchpoints));
             LLDB_LOGF(log, "failed to find watchpoint");
+            abort(); // LWP_TODO FIXME don't continue executing this block if
----------------
jasonmolenda wrote:

My refactored code added a new nullptr deref, when I saw what I'd done I threw in an abort and a FIXME comment to fix that.  But I see how to fix it, let me do that.

https://github.com/llvm/llvm-project/pull/68845