Skip to content

Commit f22db1f

Browse files
committed
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
1 parent 3f60302 commit f22db1f

File tree

1 file changed

+18
-22
lines changed

1 file changed

+18
-22
lines changed

lldb/source/Target/StopInfo.cpp

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,15 @@ class StopInfoBreakpoint : public StopInfo {
8888
: StopInfo(thread, break_id), m_should_stop(false),
8989
m_should_stop_is_valid(false), m_should_perform_action(true),
9090
m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
91-
m_was_one_shot(false) {
91+
m_was_all_internal(false), m_was_one_shot(false) {
9292
StoreBPInfo();
9393
}
9494

9595
StopInfoBreakpoint(Thread &thread, break_id_t break_id, bool should_stop)
9696
: StopInfo(thread, break_id), m_should_stop(should_stop),
9797
m_should_stop_is_valid(true), m_should_perform_action(true),
9898
m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
99-
m_was_one_shot(false) {
99+
m_was_all_internal(false), m_was_one_shot(false) {
100100
StoreBPInfo();
101101
}
102102

@@ -108,11 +108,22 @@ class StopInfoBreakpoint : public StopInfo {
108108
BreakpointSiteSP bp_site_sp(
109109
thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
110110
if (bp_site_sp) {
111-
if (bp_site_sp->GetNumberOfOwners() == 1) {
111+
uint32_t num_owners = bp_site_sp->GetNumberOfOwners();
112+
if (num_owners == 1) {
112113
BreakpointLocationSP bp_loc_sp = bp_site_sp->GetOwnerAtIndex(0);
113114
if (bp_loc_sp) {
114-
m_break_id = bp_loc_sp->GetBreakpoint().GetID();
115-
m_was_one_shot = bp_loc_sp->GetBreakpoint().IsOneShot();
115+
Breakpoint & bkpt = bp_loc_sp->GetBreakpoint();
116+
m_break_id = bkpt.GetID();
117+
m_was_one_shot = bkpt.IsOneShot();
118+
m_was_all_internal = bkpt.IsInternal();
119+
}
120+
} else {
121+
m_was_all_internal = true;
122+
for (uint32_t i = 0; i < num_owners; i++) {
123+
if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) {
124+
m_was_all_internal = false;
125+
break;
126+
}
116127
}
117128
}
118129
m_address = bp_site_sp->GetLoadAddress();
@@ -163,23 +174,7 @@ class StopInfoBreakpoint : public StopInfo {
163174
}
164175

165176
bool DoShouldNotify(Event *event_ptr) override {
166-
ThreadSP thread_sp(m_thread_wp.lock());
167-
if (thread_sp) {
168-
BreakpointSiteSP bp_site_sp(
169-
thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
170-
if (bp_site_sp) {
171-
bool all_internal = true;
172-
173-
for (uint32_t i = 0; i < bp_site_sp->GetNumberOfOwners(); i++) {
174-
if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) {
175-
all_internal = false;
176-
break;
177-
}
178-
}
179-
return !all_internal;
180-
}
181-
}
182-
return true;
177+
return !m_was_all_internal;
183178
}
184179

185180
const char *GetDescription() override {
@@ -603,6 +598,7 @@ class StopInfoBreakpoint : public StopInfo {
603598
// in case somebody deletes it between the time the StopInfo is made and the
604599
// description is asked for.
605600
lldb::break_id_t m_break_id;
601+
bool m_was_all_internal;
606602
bool m_was_one_shot;
607603
};
608604

0 commit comments

Comments
 (0)