jingham created this revision.
jingham added reviewers: JDevlieghere, jasonmolenda.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
We noticed several test suite failures (e.g. TestStopAtEntry.py) with the new
MacOS dyld support that was recently added when run on the macOS Ventura beta.
The reason was that he was an unexpected breakpoint stop event coming from the
new three-step dance needed to follow dyld's loading into the process. At the
first two stops, we delete the instrumentation breakpoint we just hit in the
callback, and add another. Since these were internal breakpoints the
ThreadPlanBase::ShouldNotify should have returned eVoteNo. But that logic
requires knowing that all the owners of the breakpoint site were internal, and
by the time we got to asking the thread plan the breakpoint site had been
removed, so we no longer knew that.
The only time you know no one could have deleted the site you hit is when you
make the StopInfoBreakpoint for the stop. So this change adds an ivar to
record whether all the breakpoints at this site were internal at that point,
and uses that in the "ShouldNotify" calculation.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127997
Files:
lldb/source/Target/StopInfo.cpp
Index: lldb/source/Target/StopInfo.cpp
===================================================================
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -88,7 +88,7 @@
: 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 @@
: 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 @@
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 @@
}
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 @@
// 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;
};
Index: lldb/source/Target/StopInfo.cpp
===================================================================
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -88,7 +88,7 @@
: 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 @@
: 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 @@
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 @@
}
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 @@
// 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;
};
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
