Skip to content

Commit da4b8ab

Browse files
authored
[lldb] Stop creating BreakpointEventData raw pointers (llvm#78508)
The lifetime of these BreakpointEventData objects is difficult to reason about. These BreakpointEventData pointers are created and passed along to `Event` which takes the raw pointer and sticks them in a shared pointer. Instead of manually managing the lifetime and memory, it would be simpler to have them be shared pointers from the start.
1 parent 8f1d94a commit da4b8ab

File tree

2 files changed

+24
-31
lines changed

2 files changed

+24
-31
lines changed

lldb/include/lldb/Breakpoint/Breakpoint.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,7 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>,
672672

673673
void SendBreakpointChangedEvent(lldb::BreakpointEventType eventKind);
674674

675-
void SendBreakpointChangedEvent(BreakpointEventData *data);
675+
void SendBreakpointChangedEvent(const lldb::EventDataSP &breakpoint_data_sp);
676676

677677
Breakpoint(const Breakpoint &) = delete;
678678
const Breakpoint &operator=(const Breakpoint &) = delete;

lldb/source/Breakpoint/Breakpoint.cpp

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -460,17 +460,13 @@ void Breakpoint::ResolveBreakpointInModules(ModuleList &module_list,
460460
// If this is not an internal breakpoint, set up to record the new
461461
// locations, then dispatch an event with the new locations.
462462
if (!IsInternal() && send_event) {
463-
BreakpointEventData *new_locations_event = new BreakpointEventData(
464-
eBreakpointEventTypeLocationsAdded, shared_from_this());
465-
463+
std::shared_ptr<BreakpointEventData> new_locations_event =
464+
std::make_shared<BreakpointEventData>(
465+
eBreakpointEventTypeLocationsAdded, shared_from_this());
466466
ResolveBreakpointInModules(
467467
module_list, new_locations_event->GetBreakpointLocationCollection());
468-
469-
if (new_locations_event->GetBreakpointLocationCollection().GetSize() !=
470-
0) {
468+
if (new_locations_event->GetBreakpointLocationCollection().GetSize() != 0)
471469
SendBreakpointChangedEvent(new_locations_event);
472-
} else
473-
delete new_locations_event;
474470
} else {
475471
ElapsedTime elapsed(m_resolve_time);
476472
m_resolver_sp->ResolveBreakpointInModules(*m_filter_sp, module_list);
@@ -565,12 +561,10 @@ void Breakpoint::ModulesChanged(ModuleList &module_list, bool load,
565561
// the module list, then remove their breakpoint sites, and their locations
566562
// if asked to.
567563

568-
BreakpointEventData *removed_locations_event;
564+
std::shared_ptr<BreakpointEventData> removed_locations_event;
569565
if (!IsInternal())
570-
removed_locations_event = new BreakpointEventData(
566+
removed_locations_event = std::make_shared<BreakpointEventData>(
571567
eBreakpointEventTypeLocationsRemoved, shared_from_this());
572-
else
573-
removed_locations_event = nullptr;
574568

575569
for (ModuleSP module_sp : module_list.Modules()) {
576570
if (m_filter_sp->ModulePasses(module_sp)) {
@@ -795,31 +789,30 @@ void Breakpoint::ModuleReplaced(ModuleSP old_module_sp,
795789
// about telling the world about removing a location we didn't tell them
796790
// about adding.
797791

798-
BreakpointEventData *locations_event;
792+
std::shared_ptr<BreakpointEventData> removed_locations_event;
799793
if (!IsInternal())
800-
locations_event = new BreakpointEventData(
794+
removed_locations_event = std::make_shared<BreakpointEventData>(
801795
eBreakpointEventTypeLocationsRemoved, shared_from_this());
802-
else
803-
locations_event = nullptr;
804796

805797
for (BreakpointLocationSP loc_sp :
806798
locations_to_remove.BreakpointLocations()) {
807799
m_locations.RemoveLocation(loc_sp);
808-
if (locations_event)
809-
locations_event->GetBreakpointLocationCollection().Add(loc_sp);
800+
if (removed_locations_event)
801+
removed_locations_event->GetBreakpointLocationCollection().Add(loc_sp);
810802
}
811-
SendBreakpointChangedEvent(locations_event);
803+
SendBreakpointChangedEvent(removed_locations_event);
812804

813805
// And announce the new ones.
814806

815807
if (!IsInternal()) {
816-
locations_event = new BreakpointEventData(
817-
eBreakpointEventTypeLocationsAdded, shared_from_this());
808+
std::shared_ptr<BreakpointEventData> added_locations_event =
809+
std::make_shared<BreakpointEventData>(
810+
eBreakpointEventTypeLocationsAdded, shared_from_this());
818811
for (BreakpointLocationSP loc_sp :
819812
locations_to_announce.BreakpointLocations())
820-
locations_event->GetBreakpointLocationCollection().Add(loc_sp);
813+
added_locations_event->GetBreakpointLocationCollection().Add(loc_sp);
821814

822-
SendBreakpointChangedEvent(locations_event);
815+
SendBreakpointChangedEvent(added_locations_event);
823816
}
824817
m_locations.Compact();
825818
}
@@ -989,22 +982,22 @@ void Breakpoint::SendBreakpointChangedEvent(
989982
if (!m_being_created && !IsInternal() &&
990983
GetTarget().EventTypeHasListeners(
991984
Target::eBroadcastBitBreakpointChanged)) {
992-
BreakpointEventData *data =
993-
new Breakpoint::BreakpointEventData(eventKind, shared_from_this());
985+
std::shared_ptr<BreakpointEventData> data =
986+
std::make_shared<BreakpointEventData>(eventKind, shared_from_this());
994987

995988
GetTarget().BroadcastEvent(Target::eBroadcastBitBreakpointChanged, data);
996989
}
997990
}
998991

999-
void Breakpoint::SendBreakpointChangedEvent(BreakpointEventData *data) {
1000-
if (data == nullptr)
992+
void Breakpoint::SendBreakpointChangedEvent(
993+
const lldb::EventDataSP &breakpoint_data_sp) {
994+
if (!breakpoint_data_sp)
1001995
return;
1002996

1003997
if (!m_being_created && !IsInternal() &&
1004998
GetTarget().EventTypeHasListeners(Target::eBroadcastBitBreakpointChanged))
1005-
GetTarget().BroadcastEvent(Target::eBroadcastBitBreakpointChanged, data);
1006-
else
1007-
delete data;
999+
GetTarget().BroadcastEvent(Target::eBroadcastBitBreakpointChanged,
1000+
breakpoint_data_sp);
10081001
}
10091002

10101003
const char *Breakpoint::BreakpointEventTypeAsCString(BreakpointEventType type) {

0 commit comments

Comments
 (0)