[NFC] Fix leak handling breakpoint names.
authorJordan Rupprecht <rupprecht@google.com>
Fri, 9 Dec 2022 01:14:38 +0000 (17:14 -0800)
committerJordan Rupprecht <rupprecht@google.com>
Fri, 9 Dec 2022 01:14:38 +0000 (17:14 -0800)
The breakpoint list is a list of raw pointers. When breakpoints are removed, the memory is not deleted. Switch to unique pointers. I did some minor cleanup while making this change.

Found by the LLDB command interpreter fuzzer. The input is `br m G`.

lldb/include/lldb/Target/Target.h
lldb/source/Target/Target.cpp

index 9e016a9..09fbf45 100644 (file)
@@ -757,8 +757,7 @@ public:
                                const BreakpointName::Permissions &permissions);
   void ApplyNameToBreakpoints(BreakpointName &bp_name);
 
-  // This takes ownership of the name obj passed in.
-  void AddBreakpointName(BreakpointName *bp_name);
+  void AddBreakpointName(std::unique_ptr<BreakpointName> bp_name);
 
   void GetBreakpointNames(std::vector<std::string> &names);
 
@@ -1512,7 +1511,8 @@ protected:
   SectionLoadHistory m_section_load_history;
   BreakpointList m_breakpoint_list;
   BreakpointList m_internal_breakpoint_list;
-  using BreakpointNameList = std::map<ConstString, BreakpointName *>;
+  using BreakpointNameList =
+      std::map<ConstString, std::unique_ptr<BreakpointName>>;
   BreakpointNameList m_breakpoint_names;
 
   lldb::BreakpointSP m_last_created_breakpoint;
index c4275db..11a882e 100644 (file)
@@ -141,10 +141,8 @@ void Target::PrimeFromDummyTarget(Target &target) {
     AddBreakpoint(std::move(new_bp), false);
   }
 
-  for (auto bp_name_entry : target.m_breakpoint_names) {
-
-    BreakpointName *new_bp_name = new BreakpointName(*bp_name_entry.second);
-    AddBreakpointName(new_bp_name);
+  for (const auto &bp_name_entry : target.m_breakpoint_names) {
+    AddBreakpointName(std::make_unique<BreakpointName>(*bp_name_entry.second));
   }
 
   m_frame_recognizer_manager_up = std::make_unique<StackFrameRecognizerManager>(
@@ -712,8 +710,9 @@ void Target::AddNameToBreakpoint(BreakpointSP &bp_sp, const char *name,
   bp_sp->AddName(name);
 }
 
-void Target::AddBreakpointName(BreakpointName *bp_name) {
-  m_breakpoint_names.insert(std::make_pair(bp_name->GetName(), bp_name));
+void Target::AddBreakpointName(std::unique_ptr<BreakpointName> bp_name) {
+  m_breakpoint_names.insert(
+      std::make_pair(bp_name->GetName(), std::move(bp_name)));
 }
 
 BreakpointName *Target::FindBreakpointName(ConstString name, bool can_create,
@@ -723,19 +722,20 @@ BreakpointName *Target::FindBreakpointName(ConstString name, bool can_create,
     return nullptr;
 
   BreakpointNameList::iterator iter = m_breakpoint_names.find(name);
-  if (iter == m_breakpoint_names.end()) {
-    if (!can_create) {
-      error.SetErrorStringWithFormat("Breakpoint name \"%s\" doesn't exist and "
-                                     "can_create is false.",
-                                     name.AsCString());
-      return nullptr;
-    }
+  if (iter != m_breakpoint_names.end()) {
+    return iter->second.get();
+  }
 
-    iter = m_breakpoint_names
-               .insert(std::make_pair(name, new BreakpointName(name)))
-               .first;
+  if (!can_create) {
+    error.SetErrorStringWithFormat("Breakpoint name \"%s\" doesn't exist and "
+                                   "can_create is false.",
+                                   name.AsCString());
+    return nullptr;
   }
-  return (iter->second);
+
+  return m_breakpoint_names
+      .insert(std::make_pair(name, std::make_unique<BreakpointName>(name)))
+      .first->second.get();
 }
 
 void Target::DeleteBreakpointName(ConstString name) {
@@ -778,8 +778,8 @@ void Target::ApplyNameToBreakpoints(BreakpointName &bp_name) {
 
 void Target::GetBreakpointNames(std::vector<std::string> &names) {
   names.clear();
-  for (auto bp_name : m_breakpoint_names) {
-    names.push_back(bp_name.first.AsCString());
+  for (const auto& bp_name_entry : m_breakpoint_names) {
+    names.push_back(bp_name_entry.first.AsCString());
   }
   llvm::sort(names);
 }