[Lldb-commits] [lldb] 28b869d - [NFC] Fix leak handling breakpoint names.

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2022-12-08T17:14:38-08:00
New Revision: 28b869d8724207bd7fd8b80f57f6c02abe4bc607

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

LOG: [NFC] Fix leak handling breakpoint names.

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`.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 9e016a970cfa6..09fbf45191d4c 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -757,8 +757,7 @@ class Target : public std::enable_shared_from_this<Target>,
                                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 @@ class Target : public std::enable_shared_from_this<Target>,
   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;

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index c4275db2c8a73..11a882e2e62b1 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -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);
 }