[Lldb-commits] [lldb] [lldb] Fix use after free on ModuleList::RemoveSharedModuleIfOrphaned (PR #155331)

[Jonas Devlieghere via lldb-commits]

================
@@ -843,10 +846,10 @@ class SharedModuleList {
     ReplaceEquivalentInMap(module_sp);
   }
 
-  bool RemoveIfOrphaned(const Module *module_ptr) {
+  bool RemoveIfOrphaned(const ModuleWP module_wp) {
     std::lock_guard<std::recursive_mutex> guard(GetMutex());
-    RemoveFromMap(*module_ptr, /*if_orphaned=*/true);
-    return m_list.RemoveIfOrphaned(module_ptr);
+    RemoveFromMap(module_wp, /*if_orphaned=*/true);
----------------
JDevlieghere wrote:

Both `RemoveFromMap` and `RemoveIfOrphaned` lock the WP before removing it, which may beg the question why not do it here. The answer is because you call these functions from elsewhere so we don't want to account for that in the ref-count and complicate things more, but might be worth a comment for the future. 

Also it's safe, in the sense that the ref-count can't have gone to zero between these two calls, because both check that the count is at least one more than when passed it, which means that neither of these functions by themselves can have reduce the count to zero and get the pointer deallocated. 

https://github.com/llvm/llvm-project/pull/155331