[lldb] Use weak pointers instead of shared pointers in DynamicLoader

DynamicLoaderWindowsDYLD uses pointers to Modules to maintain a map
from modules to their addresses, but it does not need to keep "strong"
references to them. Weak pointers should be enough, and would allow
modules to be released elsewhere.

Other DynamicLoader classes do not use shared pointers as well. For
example, DynamicLoaderPOSIXDYLD has a similar map with weak pointers.

Actually testing for modules being completely released can be
tricky. The test here is just to illustrate the case where shared
pointers kept modules in DynamicLoaderWindowsDYLD and prevented them
from being released. The test executes the following sequence:

  1. Create a target, load an executable and run it.

  2. Remove one module from the target. The target should be the last
     actual use of the module, but we have another reference to it
     in the shared module cache.

  3. Call MemoryPressureDetected to remove this last reference from
     the cache.

  4. Replace the corresponding DLL file.

LLDB memory maps DLLs, and this makes the files read-only on
Windows. Unless the modules are completely released (and therefore
unmapped), (4) is going to fail with "access denied".

However, the test does not trigger the bug completely - it passes with
and without the change.

Author: asavonic

lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h

@@ -45,7 +45,8 @@ class DynamicLoaderWindowsDYLD : public DynamicLoader {
   lldb::addr_t GetLoadAddress(lldb::ModuleSP executable);
 
 private:
-  std::map<lldb::ModuleSP, lldb::addr_t> m_loaded_modules;
+  std::map<lldb::ModuleWP, lldb::addr_t, std::owner_less<lldb::ModuleWP>>
+      m_loaded_modules;
 };
 
 } // namespace lldb_private

lldb/test/API/windows/launch/replace-dll/Makefile

@@ -0,0 +1 @@
+include Makefile.rules

lldb/test/API/windows/launch/replace-dll/TestReplaceDLL.py

@@ -0,0 +1,62 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import gc
+import os
+
+
+class ReplaceDllTestCase(TestBase):
+    @skipUnlessWindows
+    def test(self):
+        """
+        Test that LLDB unlocks module files once all references are released.
+        """
+
+        exe = self.getBuildArtifact("a.out")
+        foo = self.getBuildArtifact("foo.dll")
+        bar = self.getBuildArtifact("bar.dll")
+
+        self.build(
+            dictionary={
+                "DYLIB_NAME": "foo",
+                "DYLIB_C_SOURCES": "foo.c",
+                "C_SOURCES": "test.c",
+            }
+        )
+        self.build(
+            dictionary={
+                "DYLIB_ONLY": "YES",
+                "DYLIB_NAME": "bar",
+                "DYLIB_C_SOURCES": "bar.c",
+            }
+        )
+
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        shlib_names = ["foo"]
+        environment = self.registerSharedLibrariesWithTarget(target, shlib_names)
+        process = target.LaunchSimple(
+            None, environment, self.get_process_working_directory()
+        )
+        self.assertEqual(process.GetExitStatus(), 42)
+
+        module = next((m for m in target.modules if "foo" in m.file.basename), None)
+        self.assertIsNotNone(module)
+        self.assertEqual(module.file.fullpath, foo)
+
+        target.RemoveModule(module)
+        del module
+        gc.collect()
+
+        self.dbg.MemoryPressureDetected()
+
+        os.remove(foo)
+        os.rename(bar, foo)
+
+        process = target.LaunchSimple(
+            None, environment, self.get_process_working_directory()
+        )
+        self.assertEqual(process.GetExitStatus(), 43)

lldb/test/API/windows/launch/replace-dll/bar.c

@@ -0,0 +1 @@
+__declspec(dllexport) int foo() { return 43; }

lldb/test/API/windows/launch/replace-dll/foo.c

@@ -0,0 +1 @@
+__declspec(dllexport) int foo() { return 42; }

lldb/test/API/windows/launch/replace-dll/test.c

@@ -0,0 +1,3 @@
+int foo(void);
+
+int main() { return foo(); }