[Lldb-commits] [lldb] 1267506 - [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Mon Jan 10 14:33:26 PST 2022


Author: Lirong Yuan
Date: 2022-01-10T14:33:09-08:00
New Revision: 1267506ea54a62e0c728215c033b256ce856db30

URL: https://github.com/llvm/llvm-project/commit/1267506ea54a62e0c728215c033b256ce856db30
DIFF: https://github.com/llvm/llvm-project/commit/1267506ea54a62e0c728215c033b256ce856db30.diff

LOG: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

While running heap checker on a test that uses LLDB API, the following memory leak is found:

RAW: HeapChecker started...
RAW: Leak check _main_ detected leaks of 34 bytes in 4 objects
RAW: The 2 largest leaks:
RAW: Leak of 17 bytes in 2 objects allocated from:
@ 0x7fb93bd20166 NewHook()
@ 0x7fb929372a73 absl::base_internal::MallocHook::InvokeNewHookSlow()
@ 0x5600d1046093 libc_malloc
@ 0x7fb974529c03 xmlStrdup
@ 0x7fb9744c2a0b xmlGetProp
@ 0x7fb9749d9ed6 lldb_private::XMLNode::GetAttributeValue()
@ 0x7fb979043001 std::u::function::policy_invoker<>::__call_impl<>()
@ 0x7fb9749da06d lldb_private::XMLNode::ForEachChildElement()
@ 0x7fb97903c54d lldb_private::process_gdb_remote::ProcessGDBRemote::GetGDBServerRegisterInfoXMLAndProcess()
@ 0x7fb97902cfe4 lldb_private::process_gdb_remote::ProcessGDBRemote::GetGDBServerRegisterInfo()
@ 0x7fb97902c1d0 lldb_private::process_gdb_remote::ProcessGDBRemote::BuildDynamicRegisterInfo()
@ 0x7fb97902e92a lldb_private::process_gdb_remote::ProcessGDBRemote::SetThreadStopInfo()
@ 0x7fb97902db18 lldb_private::process_gdb_remote::ProcessGDBRemote::DoConnectRemote()
@ 0x7fb97584965e lldb_private::Process::ConnectRemote()
@ 0x7fb975839fa6 lldb_private::Platform::DoConnectProcess()
@ 0x7fb97583a39e lldb_private::Platform::ConnectProcessSynchronous()
@ 0x7fb97545b28b CommandObjectProcessConnect::DoExecute()
@ 0x7fb9755a70c9 lldb_private::CommandObjectParsed::Execute()
@ 0x7fb97559c0e9 lldb_private::CommandInterpreter::HandleCommand()
@ 0x7fb975460145 lldb_private::CommandObjectRegexCommand::DoExecute()
@ 0x7fb9755a72d2 lldb_private::CommandObjectRaw::Execute()
@ 0x7fb97559c0e9 lldb_private::CommandInterpreter::HandleCommand()
@ 0x7fb997a5f22e lldb::SBCommandInterpreter::HandleCommand()
@ 0x7fb997a5ef9b lldb::SBCommandInterpreter::HandleCommand()

This change fixes the memory leaks by freeing memory after it is no
longer in use. Tested with "ninja check-lldb".

Differential revision: https://reviews.llvm.org/D116707

Added: 
    

Modified: 
    lldb/include/lldb/Host/XML.h
    lldb/source/Host/common/XML.cpp
    lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Host/XML.h b/lldb/include/lldb/Host/XML.h
index 9edf46bf09df5..da0f9cd7aa8c0 100644
--- a/lldb/include/lldb/Host/XML.h
+++ b/lldb/include/lldb/Host/XML.h
@@ -76,8 +76,8 @@ class XMLNode {
 
   XMLNode GetChild() const;
 
-  llvm::StringRef GetAttributeValue(const char *name,
-                                    const char *fail_value = nullptr) const;
+  std::string GetAttributeValue(const char *name,
+                                const char *fail_value = nullptr) const;
 
   bool GetAttributeValueAsUnsigned(const char *name, uint64_t &value,
                                    uint64_t fail_value = 0, int base = 0) const;

diff  --git a/lldb/source/Host/common/XML.cpp b/lldb/source/Host/common/XML.cpp
index 79128b98dc38d..2d48175a82346 100644
--- a/lldb/source/Host/common/XML.cpp
+++ b/lldb/source/Host/common/XML.cpp
@@ -130,22 +130,25 @@ XMLNode XMLNode::GetChild() const {
 #endif
 }
 
-llvm::StringRef XMLNode::GetAttributeValue(const char *name,
-                                           const char *fail_value) const {
-  const char *attr_value = nullptr;
+std::string XMLNode::GetAttributeValue(const char *name,
+                                       const char *fail_value) const {
+  std::string attr_value;
 #if LLDB_ENABLE_LIBXML2
-
-  if (IsValid())
-    attr_value = (const char *)xmlGetProp(m_node, (const xmlChar *)name);
-  else
-    attr_value = fail_value;
+  if (IsValid()) {
+    xmlChar *value = xmlGetProp(m_node, (const xmlChar *)name);
+    if (value) {
+      attr_value = (const char *)value;
+      xmlFree(value);
+    }
+  } else {
+    if (fail_value)
+      attr_value = fail_value;
+  }
 #else
-  attr_value = fail_value;
+  if (fail_value)
+    attr_value = fail_value;
 #endif
-  if (attr_value)
-    return llvm::StringRef(attr_value);
-  else
-    return llvm::StringRef();
+  return attr_value;
 }
 
 bool XMLNode::GetAttributeValueAsUnsigned(const char *name, uint64_t &value,

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index cb5ec7f18d190..dfc1c4a200689 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4351,9 +4351,9 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfoXMLAndProcess(
         } else if (name == "osabi") {
           node.GetElementText(target_info.osabi);
         } else if (name == "xi:include" || name == "include") {
-          llvm::StringRef href = node.GetAttributeValue("href");
+          std::string href = node.GetAttributeValue("href");
           if (!href.empty())
-            target_info.includes.push_back(href.str());
+            target_info.includes.push_back(href);
         } else if (name == "feature") {
           feature_nodes.push_back(node);
         } else if (name == "groups") {
@@ -4392,9 +4392,9 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfoXMLAndProcess(
                                         const XMLNode &node) -> bool {
           llvm::StringRef name = node.GetName();
           if (name == "xi:include" || name == "include") {
-            llvm::StringRef href = node.GetAttributeValue("href");
+            std::string href = node.GetAttributeValue("href");
             if (!href.empty())
-              target_info.includes.push_back(href.str());
+              target_info.includes.push_back(href);
             }
             return true;
           });
@@ -4530,7 +4530,7 @@ llvm::Expected<LoadedModuleInfoList> ProcessGDBRemote::GetLoadedModuleList() {
           "Error finding library-list-svr4 xml element");
 
     // main link map structure
-    llvm::StringRef main_lm = root_element.GetAttributeValue("main-lm");
+    std::string main_lm = root_element.GetAttributeValue("main-lm");
     // FIXME: we're silently ignoring invalid data here
     if (!main_lm.empty())
       llvm::to_integer(main_lm, list.m_link_map);
@@ -4618,15 +4618,15 @@ llvm::Expected<LoadedModuleInfoList> ProcessGDBRemote::GetLoadedModuleList() {
         "library", [log, &list](const XMLNode &library) -> bool {
           LoadedModuleInfoList::LoadedModuleInfo module;
 
-          llvm::StringRef name = library.GetAttributeValue("name");
-          module.set_name(name.str());
+          std::string name = library.GetAttributeValue("name");
+          module.set_name(name);
 
           // The base address of a given library will be the address of its
           // first section. Most remotes send only one section for Windows
           // targets for example.
           const XMLNode &section =
               library.FindFirstChildElementWithName("section");
-          llvm::StringRef address = section.GetAttributeValue("address");
+          std::string address = section.GetAttributeValue("address");
           uint64_t address_value = LLDB_INVALID_ADDRESS;
           llvm::to_integer(address, address_value);
           module.set_base(address_value);


        


More information about the lldb-commits mailing list