[Lldb-commits] [lldb] f22db1f - Fix StopInfoBreakpoint::ShouldNotify when a callback deletes the site we hit.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-06-16T11:54:11-07:00
New Revision: f22db1fabfa165e9b18b1b49c78566d15f22bda1

URL: https://github.com/llvm/llvm-project/commit/f22db1fabfa165e9b18b1b49c78566d15f22bda1
DIFF: https://github.com/llvm/llvm-project/commit/f22db1fabfa165e9b18b1b49c78566d15f22bda1.diff

LOG: Fix StopInfoBreakpoint::ShouldNotify when a callback deletes the site we hit.

When we hit a breakpoint site all of whose owners are internal, we don't
broadcast that event to the public event queue.  However, we were checking
whether that was true in the ShouldNotify method, which gets run after the
breakpoint callbacks get run.  If the breakpoint callback deletes the site
we just hit, we no longer have the information to make that determination.

This patch just gathers the "was all internal" fact when the StopInfoBreakpoint
gets made, which happens before anyone has a chance to delete the site, and then
uses that cached value.

This bug was causing a couple of tests (including TestStopAtEntry.py) to fail
when using new the macOS Ventura dyld support.

Differential Revision: https://reviews.llvm.org/D127997

Added: 
    

Modified: 
    lldb/source/Target/StopInfo.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 08c50ea5e62c..1ab2ad0d34f2 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -88,7 +88,7 @@ class StopInfoBreakpoint : public StopInfo {
       : StopInfo(thread, break_id), m_should_stop(false),
         m_should_stop_is_valid(false), m_should_perform_action(true),
         m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-        m_was_one_shot(false) {
+        m_was_all_internal(false), m_was_one_shot(false) {
     StoreBPInfo();
   }
 
@@ -96,7 +96,7 @@ class StopInfoBreakpoint : public StopInfo {
       : StopInfo(thread, break_id), m_should_stop(should_stop),
         m_should_stop_is_valid(true), m_should_perform_action(true),
         m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-        m_was_one_shot(false) {
+        m_was_all_internal(false), m_was_one_shot(false) {
     StoreBPInfo();
   }
 
@@ -108,11 +108,22 @@ class StopInfoBreakpoint : public StopInfo {
       BreakpointSiteSP bp_site_sp(
           thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
       if (bp_site_sp) {
-        if (bp_site_sp->GetNumberOfOwners() == 1) {
+        uint32_t num_owners = bp_site_sp->GetNumberOfOwners();
+        if (num_owners == 1) {
           BreakpointLocationSP bp_loc_sp = bp_site_sp->GetOwnerAtIndex(0);
           if (bp_loc_sp) {
-            m_break_id = bp_loc_sp->GetBreakpoint().GetID();
-            m_was_one_shot = bp_loc_sp->GetBreakpoint().IsOneShot();
+            Breakpoint & bkpt = bp_loc_sp->GetBreakpoint();
+            m_break_id = bkpt.GetID();
+            m_was_one_shot = bkpt.IsOneShot();
+            m_was_all_internal = bkpt.IsInternal();
+          }
+        } else {
+          m_was_all_internal = true;
+          for (uint32_t i = 0; i < num_owners; i++) {
+            if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) {
+              m_was_all_internal = false;
+              break;
+            }
           }
         }
         m_address = bp_site_sp->GetLoadAddress();
@@ -163,23 +174,7 @@ class StopInfoBreakpoint : public StopInfo {
   }
 
   bool DoShouldNotify(Event *event_ptr) override {
-    ThreadSP thread_sp(m_thread_wp.lock());
-    if (thread_sp) {
-      BreakpointSiteSP bp_site_sp(
-          thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
-      if (bp_site_sp) {
-        bool all_internal = true;
-
-        for (uint32_t i = 0; i < bp_site_sp->GetNumberOfOwners(); i++) {
-          if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) {
-            all_internal = false;
-            break;
-          }
-        }
-        return !all_internal;
-      }
-    }
-    return true;
+    return !m_was_all_internal;
   }
 
   const char *GetDescription() override {
@@ -603,6 +598,7 @@ class StopInfoBreakpoint : public StopInfo {
   // in case somebody deletes it between the time the StopInfo is made and the
   // description is asked for.
   lldb::break_id_t m_break_id;
+  bool m_was_all_internal;
   bool m_was_one_shot;
 };