[Lldb-commits] [PATCH] D131900: [LLDB][NFC] Fix memory leak in IntstumentationRuntimeTSan.cpp

[Slava Gurevich via Phabricator via lldb-commits]

fixathon created this revision.
fixathon added reviewers: clayborg, JDevlieghere, DavidSpickett, jasonmolenda.
Herald added a project: All.
fixathon published this revision for review.
fixathon added inline comments.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.


================
Comment at: lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp:227
 
 static StructuredData::Array *ConvertToStructuredArray(
     ValueObjectSP return_value_sp, const std::string &items_name,
----------------
Function creating heap-based allocation


================
Comment at: lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp:232
         &callback) {
   StructuredData::Array *array = new StructuredData::Array();
   unsigned int count =
----------------
Allocation of the leaked entity


================
Comment at: lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp:264
                        std::map<uint64_t, user_id_t> &thread_id_map) {
-  ConvertToStructuredArray(
+  std::unique_ptr<StructuredData::Array> cleanup(ConvertToStructuredArray(
       data, ".threads", ".thread_count",
----------------
Delete the heap-allocated resource via RAII on function exit.


================
Comment at: lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp:287
 
         thread_id_map[thread_id] = lldb_user_id;
+      }));
----------------
Result is set here. Note, at this point, there's no continued dependency on the StructuredData::Array object returned by the call to ConvertToStructuredArray(), and that object does not need to persist.


ConvertToStructuredArray() relies on its caller to deallocate the heap-allocated object pointer it returns. One of its call-sites, in GetRenumberedThreadIds(), fails to deallocate causing a memory/resource leak. This fix aims to fix this issue via RAII-style clean up.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131900

Files:
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp


Index: lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
===================================================================
--- lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -261,7 +261,7 @@
 static void
 GetRenumberedThreadIds(ProcessSP process_sp, ValueObjectSP data,
                        std::map<uint64_t, user_id_t> &thread_id_map) {
-  ConvertToStructuredArray(
+  std::unique_ptr<StructuredData::Array> cleanup(ConvertToStructuredArray(
       data, ".threads", ".thread_count",
       [process_sp, &thread_id_map](ValueObjectSP o,
                                    StructuredData::Dictionary *dict) {
@@ -285,7 +285,7 @@
         }
 
         thread_id_map[thread_id] = lldb_user_id;
-      });
+      }));
 }
 
 static user_id_t Renumber(uint64_t id,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D131900.452701.patch
Type: text/x-patch
Size: 914 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220815/f63d7a88/attachment.bin>