[Lldb-commits] [lldb] [lldb][NFC] Sink eager breakpoint logic into ExecuteBreakpointSiteAction (PR #198741)

[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>

A future patch will want to control whether all breakpoints are eager or not based on whether the process is running. Sinking the logic allows for a smaller diff.

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


2 Files Affected:

- (modified) lldb/include/lldb/Target/Process.h (+2-1) 
- (modified) lldb/source/Target/Process.cpp (+11-13) 


``````````diff
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index e2d3d54b8bb0f..db004fcde8e29 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2308,7 +2308,8 @@ class Process : public std::enable_shared_from_this<Process>,
 
 public:
   llvm::Error ExecuteBreakpointSiteAction(BreakpointSite &site,
-                                          Process::BreakpointAction action);
+                                          Process::BreakpointAction action,
+                                          bool do_it_now = false);
 
   // This is implemented completely using the lldb::Process API. Subclasses
   // don't need to implement this function unless the standard flow of read
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index ae0f4fbcbb683..d0711bb36ebff 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1599,7 +1599,14 @@ Status Process::DisableBreakpointSiteByID(lldb::user_id_t break_id) {
 }
 
 llvm::Error Process::ExecuteBreakpointSiteAction(BreakpointSite &site,
-                                                 BreakpointAction action) {
+                                                 BreakpointAction action,
+                                                 bool do_it_now) {
+  if (do_it_now)
+    if (llvm::Error E = FlushDelayedBreakpoints())
+      LLDB_LOG_ERROR(
+          GetLog(LLDBLog::Breakpoints), std::move(E),
+          "eager breakpoint requested, but failed to flush breakpoints: {0}");
+
   auto site_sp = site.shared_from_this();
   std::unique_lock<std::recursive_mutex> guard(m_delayed_breakpoints_mutex);
 
@@ -1607,7 +1614,7 @@ llvm::Error Process::ExecuteBreakpointSiteAction(BreakpointSite &site,
   if (IsBreakpointSiteEnabled(*site_sp) == (action == BreakpointAction::Enable))
     return llvm::Error::success();
 
-  if (ShouldUseDelayedBreakpoints()) {
+  if (!do_it_now && ShouldUseDelayedBreakpoints()) {
     m_delayed_breakpoints.Enqueue(site_sp, action);
     return llvm::Error::success();
   }
@@ -1769,17 +1776,8 @@ 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 (llvm::Error 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));
+  auto error = Status::FromError(ExecuteBreakpointSiteAction(
+      *bp_site_sp, BreakpointAction::Enable, /*do_it_now=*/should_be_eager));
   if (error.Success()) {
     constituent->SetBreakpointSite(bp_site_sp);
     return m_breakpoint_site_list.Add(bp_site_sp);

``````````

</details>


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