[Lldb-commits] [lldb] [lldb] Flush delayed breakpoints before eagerly changing one (PR #195815)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

<details>
<summary>Changes</summary>

See the discussion in https://github.com/llvm/llvm-project/pull/192971

When LLDB makes the decision to eagerly send a breakpoint packet, it should first ensure the delayed breakpoints are flushed, as they may interfere with the eager breakpoint that is about to be changed.

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


2 Files Affected:

- (modified) lldb/source/Target/Process.cpp (+8) 
- (modified) lldb/test/API/functionalities/breakpoint/delayed_breakpoints/TestDelayedBreakpoint.py (+26) 


``````````diff
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 114bbd7355f0c..011617c2836c6 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1763,6 +1763,14 @@ Process::CreateBreakpointSite(const BreakpointLocationSP &constituent,
       BreakpointResolver::ResolverTy::AddressResolver;
   bool should_be_eager = use_hardware || bp_from_address;
 
+  // If this breakpoint must be eager, flush the breakpoint queue in case there
+  // is an interaction between the sites in the queue and this new site.
+  if (should_be_eager)
+    if (auto E = FlushDelayedBreakpoints())
+      LLDB_LOG_ERROR(
+          GetLog(LLDBLog::Breakpoints), std::move(E),
+          "eager breakpoint requested, but failed to flush breakpoints: {0}");
+
   auto error = should_be_eager ? EnableBreakpointSite(bp_site_sp.get())
                                : Status::FromError(ExecuteBreakpointSiteAction(
                                      *bp_site_sp, BreakpointAction::Enable));
diff --git a/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/TestDelayedBreakpoint.py b/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/TestDelayedBreakpoint.py
index 70a28bb935027..bd4619bfb663a 100644
--- a/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/TestDelayedBreakpoint.py
+++ b/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/TestDelayedBreakpoint.py
@@ -39,3 +39,29 @@ def test(self):
 
         log_after = log_text.split("AFTER_BPS", 1)[-1].split("AFTER_CONTINUE", 1)[0]
         self.assertIn("send packet: $Z", log_after)
+
+    def test_eager_breakpoints(self):
+        self.build()
+        logfile = os.path.join(self.getBuildDir(), "log.txt")
+        self.runCmd(f"log enable -f {logfile} gdb-remote packets")
+
+        target, process, _, _ = lldbutil.run_to_source_breakpoint(
+            self, "main", lldb.SBFileSpec("main.c")
+        )
+
+        bp1 = target.BreakpointCreateByLocation("main.c", 1)
+        self.runCmd("proc plugin packet send BEGIN_EAGER", check=False)
+        # Create an address breakpoint to trigger eager breakpoints.
+        fake_address = 0x1234567
+        target.BreakpointCreateByAddress(fake_address)
+        self.runCmd("proc plugin packet send END_EAGER", check=False)
+
+        self.assertTrue(os.path.exists(logfile))
+        log = open(logfile).read().split("BEGIN_EAGER")[1].split("END_EAGER")[0]
+        breakpoint_lines = [line for line in log if "send packet: $Z" in line]
+        breakpoint_lines = "".join(breakpoint_lines)
+
+        bp_addresses = [f"{loc.GetLoadAddress():x}" for loc in bp1.locations]
+        bp_addresses += [f"{fake_address:x}"]
+        for addr in bp_addresses:
+            self.assertIn(addr, breakpoint_lines)

``````````

</details>


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