[Lldb-commits] [lldb] [lldb][debugserver] Fix bugs in FindBreakpointsThatOverlapRange (PR #217837)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)

<details>
<summary>Changes</summary>

Fixes #<!-- -->217359

The first issue is obvious, when checking the previous breakpoint we should push back prev_pos->second, instead of pos->second.

The second problem is what happens when lower_bound returns end(). Before, `if (pos != end)` would stop us checking the previous breakpoint.

If lower_bound returned end() but there were breakpoints, the last breakpoint may start before addr and extend past it. This was being missed.

To fix that:
* Remove the `if (pos != end)` check.
* Return early if there are no breakpoints.
* If lower_bound does not find the first one, look at the previous one.
* Look at the rest from pos onwards.

The earlly return means we know that
m_breakpoints.begin() != m_breakpoints.end() and so even if lower_bound returns end(), we are safe to decrement that iterator.

I think this will fix the API tests in #<!-- -->217348, when those are enabled from debugserver.

---
Full diff: https://github.com/llvm/llvm-project/pull/217837.diff


1 Files Affected:

- (modified) lldb/tools/debugserver/source/DNBBreakpoint.cpp (+25-21) 


``````````diff
diff --git a/lldb/tools/debugserver/source/DNBBreakpoint.cpp b/lldb/tools/debugserver/source/DNBBreakpoint.cpp
index 74f0fb17129f3..eecce96d2d2bd 100644
--- a/lldb/tools/debugserver/source/DNBBreakpoint.cpp
+++ b/lldb/tools/debugserver/source/DNBBreakpoint.cpp
@@ -125,34 +125,38 @@ DNBBreakpointList::FindNearestWatchpoint(nub_addr_t addr) const {
 size_t DNBBreakpointList::FindBreakpointsThatOverlapRange(
     nub_addr_t addr, nub_addr_t size, std::vector<DNBBreakpoint *> &bps) {
   bps.clear();
+
+  if (m_breakpoints.empty())
+    return bps.size();
+
   iterator end = m_breakpoints.end();
   // Find the first breakpoint with an address >= to "addr"
   iterator pos = m_breakpoints.lower_bound(addr);
-  if (pos != end) {
-    if (pos != m_breakpoints.begin()) {
-      // Watch out for a breakpoint at an address less than "addr" that might
-      // still overlap
-      iterator prev_pos = pos;
-      --prev_pos;
-      if (prev_pos->second.IntersectsRange(addr, size, NULL, NULL, NULL))
-        bps.push_back(&pos->second);
-    }
 
-    while (pos != end) {
-      // When we hit a breakpoint whose start address is greater than "addr +
-      // size" we are done.
-      // Do the math in a way that doesn't risk unsigned overflow with bad
-      // input.
-      if ((pos->second.Address() - addr) >= size)
-        break;
+  if (pos != m_breakpoints.begin()) {
+    // Watch out for a breakpoint at an address less than "addr" that might
+    // still overlap
+    iterator prev_pos = pos;
+    --prev_pos;
+    if (prev_pos->second.IntersectsRange(addr, size, NULL, NULL, NULL))
+      bps.push_back(&prev_pos->second);
+  }
 
-      // Check if this breakpoint overlaps, and if it does, add it to the list
-      if (pos->second.IntersectsRange(addr, size, NULL, NULL, NULL))
-        bps.push_back(&pos->second);
+  while (pos != end) {
+    // When we hit a breakpoint whose start address is greater than "addr +
+    // size" we are done.
+    // Do the math in a way that doesn't risk unsigned overflow with bad
+    // input.
+    if ((pos->second.Address() - addr) >= size)
+      break;
 
-      ++pos;
-    }
+    // Check if this breakpoint overlaps, and if it does, add it to the list
+    if (pos->second.IntersectsRange(addr, size, NULL, NULL, NULL))
+      bps.push_back(&pos->second);
+
+    ++pos;
   }
+
   return bps.size();
 }
 

``````````

</details>


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