[lldb][LocateModuleCallback] Fix LocateModuleCallbackTest

[splhack]

Summary

ModuleList unexpectedly caches module beyond test.
Tear it down.

• https://green.lab.llvm.org/green/view/LLDB/job/as-lldb-cmake/2260/testReport/junit/lldb-unit/Target___TargetTests_LocateModuleCallbackTest/GetOrCreateModuleWithCachedModule/
• https://green.lab.llvm.org/green/view/LLDB/job/as-lldb-cmake/2260/testReport/junit/lldb-unit/Target___TargetTests_LocateModuleCallbackTest/GetOrCreateModuleWithCachedModuleAndSymbol/

[JDevlieghere]

ASSERT_EQ should call operator== under the hood. Why would that be flakey on arm64? Can we achieve this by changing the implementation to reimplement this with FileSpec::Compare?

[jasonmolenda]

I ran the TargetTests under lldb to see (this test doesn't fail on my arm64 mac desktop ftr). ASSERT_EQ calls template <typename T1, typename T2> AssertionResult CmpHelperEQ() and that calls FileSpec::operator== on my desktop. Jonas suggested that we get the binary data dump of the FileSpec objects because there wasn't a method implemented in FileSpec that gtest knew to call to print the objects. I originally thought ASSERT_EQ might be doing a binary comparison of the object bytes when I saw the failing msg on the arm64 macos bot.

Would it be better to pass the GetPath() method on both of the FileSpec's we send to ASSERT_EQ so it is working with std::string's which it will know how to print if there is a comparison mismatch?

[splhack]

thanks @JDevlieghere @jasonmolenda, yeah GetPath() makes sense it could show the mismatch in human readable form.

[splhack]

Use GetPath() instead of Compare().

[jasonmolenda]

My first thought was, but what about all the other unit tests doing this, and git grepping around, I think there's one in Interpreter/TestOptionValue.cpp and one in Interpreter/TestOptionValueFileColonLine.cpp. I'd like to see what @JDevlieghere thinks, this patch is fine to me but it is an unfortunate - surely there is some method we could implement in FileSpec to get the gtest stuff to print the path when there's a failure, but I didn't see it when I tried to read through the code.

[jasonmolenda]

Like reading third-party/unittest/googletest/include/gtest/gtest-printers.h, I thought I could add a static PrintTo method? but

index 6eb5b805d9d..93d98baa5be 100644
--- a/lldb/include/lldb/Utility/FileSpec.h
+++ b/lldb/include/lldb/Utility/FileSpec.h
@@ -12,4 +12,5 @@
 #include <functional>
 #include <optional>
+#include <ostream>
 #include <string>
 
@@ -198,4 +199,8 @@ public:
   static std::optional<Style> GuessPathStyle(llvm::StringRef absolute_path);
 
+  /// Print the file path in a FileSpec to the ostream, for gtest-printers unit
+  /// test failure printing.
+  static void PrintTo(const FileSpec &fspec, std::ostream *ostr);
+
   /// Case sensitivity of path.
   ///
diff --git a/lldb/source/Utility/FileSpec.cpp b/lldb/source/Utility/FileSpec.cpp
index eb34ef97cea..6f09c8448df 100644
--- a/lldb/source/Utility/FileSpec.cpp
+++ b/lldb/source/Utility/FileSpec.cpp
@@ -320,4 +320,8 @@ FileSpec::GuessPathStyle(llvm::StringRef absolute_path) {
 }
 
+void FileSpec::PrintTo(const FileSpec &fspec, std::ostream *ostr) {
+  *ostr << fspec.GetPath();
+}
+
 // Dump the object to the supplied stream. If the object contains a valid
 // directory name, it will be displayed followed by a directory delimiter, and

and making one of the FileSpecs not match, I still get the "24-byte object" printed for the ASSERT_EQ mismatch. I must have misunderstood the header.

[jasonmolenda]

FWIW when I was testing this locally, I was seeing paths like

(lldb) p uuid_view
(lldb_private::FileSpec) $0 = file: "AndroidModule.so" dir: "/var/folders/1b/976z5_5513l64wvrj15vkt800000gn/T/lldb/49966/LocateModuleCallbackTest-GetOrCreateModuleWithCachedModuleAndSymbol/remote-android/.cache/80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC"

(lldb) p module_sp->GetFileSpec()
(const lldb_private::FileSpec) $1 = file: "AndroidModule.so" dir: "/var/folders/1b/976z5_5513l64wvrj15vkt800000gn/T/lldb/49966/LocateModuleCallbackTest-GetOrCreateModuleWithCachedModuleAndSymbol/remote-android/.cache/80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC"

and I'm betting the real issue is that the arm64 bot account has something funky about TMPRDIR, so one of these is realpath'ed to a different filepath and the comparison failed. It would be a lot easier to understand what was actually happening on that bot if we could see the paths, but I don't think we're addressing the actual issue here. (we need to fix the printing first, then we can see what is going on with the bot)

[splhack]

Tear down the module cache.

[splhack]

@jasonmolenda thank you for looking into this!
I was finally able to repro the test failure on arm64 macOS with this diff (the version with ASSERT_EQ(a_file_spec.GetPath(), b_file_spec.GetPath()) and this CMake config.

cmake \
    ../llvm \
    -G Ninja \
    -DCMAKE_BUILD_TYPE=RelWithDebInfo \
    -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
    -DLLVM_TARGETS_TO_BUILD=X86;ARM;AArch64 \
    -DLLDB_TEST_USER_ARGS="--build-dir;$(pwd);-t;--env;TERM=vt100" \
    -DLLDB_ENFORCE_STRICT_TEST_REQUIREMENTS=Off \
    -DLLDB_ENABLE_PYTHON=On \
    -DLLVM_ENABLE_ASSERTIONS:BOOL=TRUE \
    -DLLVM_ENABLE_MODULES=On \
    -DLLVM_ENABLE_PROJECTS="clang;lld;lldb;cross-project-tests" \
    -DLLVM_LIT_ARGS="-v --time-tests --shuffle --xunit-xml-output=$(pwd)/results.xml -v -j 8" \
    -DLLVM_VERSION_PATCH=99 \
    -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;compiler-rt" \
    -DPython3_EXECUTABLE=/usr/bin/python3 \
    -DSWIG_EXECUTABLE=/opt/brew/Cellar/swig@3/3.0.12/bin/swig

******************** TEST 'lldb-unit :: Target/./TargetTests/6/11' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/Users/user/Sources/llvm-project/build/tools/lldb/unittests/Target/./TargetTests-lldb-unit-45687-6-11.json GTEST_SHUFFLE=1 GTEST_TOTAL_SHARDS=11 GTEST_SHARD_INDEX=6 GTEST_RANDOM_SEED=20766 /Users/user/Sources/llvm-project/build/tools/lldb/unittests/Target/./TargetTests
--

Script:
--
/Users/user/Sources/llvm-project/build/tools/lldb/unittests/Target/./TargetTests --gtest_filter=LocateModuleCallbackTest.GetOrCreateModuleWithCachedModule
--
/Users/user/Sources/llvm-project/lldb/unittests/Target/LocateModuleCallbackTest.cpp:152: Failure
Expected equality of these values:
  a_file_spec.GetPath()
    Which is: "/Users/user/Sources/llvm-project/build/tools/lldb/unittests/Target/Inputs/AndroidModule.unstripped.so"
  b_file_spec.GetPath()
    Which is: "/var/folders/hv/pgk33lbd2snf__0bd0v30yyc0000gn/T/lit-tmp-qrocj9y_/lldb/45872/LocateModuleCallbackTest-GetOrCreateModuleWithCachedModule/remote-android/.cache/80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC/AndroidModule.so"
/Users/user/Sources/llvm-project/lldb/unittests/Target/LocateModuleCallbackTest.cpp:280: Failure
Value of: module_sp->GetSymbolFileFileSpec()
  Actual: true
Expected: false

/Users/user/Sources/llvm-project/lldb/unittests/Target/LocateModuleCallbackTest.cpp:152
Expected equality of these values:
  a_file_spec.GetPath()
    Which is: "/Users/user/Sources/llvm-project/build/tools/lldb/unittests/Target/Inputs/AndroidModule.unstripped.so"
  b_file_spec.GetPath()
    Which is: "/var/folders/hv/pgk33lbd2snf__0bd0v30yyc0000gn/T/lit-tmp-qrocj9y_/lldb/45872/LocateModuleCallbackTest-GetOrCreateModuleWithCachedModule/remote-android/.cache/80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC/AndroidModule.so"
/Users/user/Sources/llvm-project/lldb/unittests/Target/LocateModuleCallbackTest.cpp:280
Value of: module_sp->GetSymbolFileFileSpec()
  Actual: true
Expected: false

The root cause is that the module is cached in this specific test, GetOrCreateModuleWithCachedModule.
Because llvm-lit runs multiple tests in a same process by this CMake config.
Most runtime data is surely initialized by gtest SetUp but ModuleList global instance lives beyond the test lifespan.
Fixed it by tear down the cached module.

[jasonmolenda]

Great catch, thanks for digging in on this!

[JonChesterfield]

edit: local builds are failing for me, but on double checking I don't think it's this patch. CI is mostly green.

lldb/unittests/Target/LocateModuleCallbackTest.cpp

m_process_sp =
         m_target_sp->CreateProcess(Listener::MakeListener("test-listener"),
                                    k_process_plugin, /*crash_file=*/nullptr,
                                    /*can_connect=*/true);
     EXPECT_TRUE(m_process_sp);
 
     m_module_spec = GetTestModuleSpec();
   }
 
+  void TearDown() override {
+    if (m_module_sp)
+      ModuleList::RemoveSharedModule(m_module_sp);
+  }
+
   void CheckNoCallback() {
     EXPECT_FALSE(m_platform_sp->GetLocateModuleCallback());
     EXPECT_EQ(m_callback_call_count, 0);
   }
 
   void CheckCallbackArgs(const ModuleSpec &module_spec,
                          FileSpec &module_file_spec,
                          FileSpec &symbol_file_spec) {
     EXPECT_TRUE(m_module_spec.Matches(module_spec,
                                       /*exact_arch_match=*/true));
     EXPECT_FALSE(module_file_spec);
     EXPECT_FALSE(symbol_file_spec);
 
     EXPECT_EQ(m_callback_call_count, 0);
     m_callback_call_count++;
   }
 
 protected:
   FileSpec m_test_dir;
   DebuggerSP m_debugger_sp;
   PlatformSP m_platform_sp;
   TargetSP m_target_sp;
   ProcessSP m_process_sp;
   ModuleSpec m_module_spec;
+  ModuleSP m_module_sp;
   int m_callback_call_count = 0;
 };
 
 } // namespace
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleWithCachedModule) {
-  // Disable test on arm64 because of failures in the lldb incremental arm64
-  // bot.
-#if defined(__arm64__) || defined(__aarch64__) || defined(_M_ARM64)
-  GTEST_SKIP() << "broken on arm64.";
-#endif
   // The module file is cached, and the locate module callback is not set.
   // GetOrCreateModule should succeed to return the module from the cache.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
   CheckNoCallback();
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
-  CheckStrippedSymbol(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
+  CheckStrippedSymbol(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleWithCachedModuleAndSymbol) {
   // The module and symbol files are cached, and the locate module callback is
   // not set. GetOrCreateModule should succeed to return the module from the
   // cache with the symbol.
-
-  // Disable test on arm64 because of failures in the lldb incremental arm64
-  // bot.
-#if defined(__arm64__) || defined(__aarch64__) || defined(_M_ARM64)
-  GTEST_SKIP() << "broken on arm64.";
-#endif
   FileSpec uuid_view = BuildCacheDirWithSymbol(m_test_dir);
 
   CheckNoCallback();
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
-  CheckUnstrippedSymbol(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
+  CheckUnstrippedSymbol(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest,
        GetOrCreateModuleWithCachedModuleAndBreakpadSymbol) {
   // The module file and breakpad symbol file are cached, and the locate module
   // callback is not set. GetOrCreateModule should succeed to return the module
   // from the cache with the symbol.
   FileSpec uuid_view = BuildCacheDirWithBreakpadSymbol(m_test_dir);
 
   CheckNoCallback();
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
-  CheckUnstrippedSymbol(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
+  CheckUnstrippedSymbol(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleFailure) {
   // The cache dir is empty, and the locate module callback is not set.
   // GetOrCreateModule should fail because PlatformAndroid tries to download the
   // module and fails.
   BuildEmptyCacheDir(m_test_dir);
 
   CheckNoCallback();
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  ASSERT_FALSE(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_FALSE(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackFailureNoCache) {
   // The cache dir is empty, also the locate module callback fails for some
   // reason. GetOrCreateModule should fail because PlatformAndroid tries to
   // download the module and fails.
   BuildEmptyCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     return Status("The locate module callback failed");
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  ASSERT_FALSE(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_FALSE(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackFailureCached) {
   // The module file is cached, so GetOrCreateModule should succeed to return
   // the module from the cache even though the locate module callback fails for
   // some reason.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     return Status("The locate module callback failed");
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
-  CheckStrippedSymbol(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
+  CheckStrippedSymbol(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackNoFiles) {
   // The module file is cached, so GetOrCreateModule should succeed to return
   // the module from the cache even though the locate module callback returns
   // no files.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     // The locate module callback succeeds but it does not set
     // module_file_spec nor symbol_file_spec.
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
-  CheckStrippedSymbol(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
+  CheckStrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackNonExistentModule) {
   // The module file is cached, so GetOrCreateModule should succeed to return
   // the module from the cache even though the locate module callback returns
   // non-existent module file.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     module_file_spec.SetPath("/this path does not exist");
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
-  CheckStrippedSymbol(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
+  CheckStrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackNonExistentSymbol) {
   // The module file is cached, so GetOrCreateModule should succeed to return
   // the module from the cache even though the locate module callback returns
   // non-existent symbol file.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     // The locate module callback returns a right module file.
     module_file_spec.SetPath(GetInputFilePath(k_module_file));
     // But it returns non-existent symbols file.
     symbol_file_spec.SetPath("/this path does not exist");
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_TRUE(module_sp->GetSymbolFileFileSpec().GetPath().empty());
-  CheckStrippedSymbol(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_TRUE(m_module_sp->GetSymbolFileFileSpec().GetPath().empty());
+  CheckStrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackSuccessWithModule) {
   // The locate module callback returns a module file, GetOrCreateModule should
   // succeed to return the module from the Inputs directory.
   BuildEmptyCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     module_file_spec.SetPath(GetInputFilePath(k_module_file));
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(),
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(),
             FileSpec(GetInputFilePath(k_module_file)));
-  ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
-  CheckStrippedSymbol(module_sp);
+  ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
+  CheckStrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest,
        GetOrCreateModuleCallbackSuccessWithSymbolAsModule) {
   // The locate module callback returns the symbol file as a module file. It
   // should work since the sections and UUID of the symbol file are the exact
   // same with the module file, GetOrCreateModule should succeed to return the
   // module with the symbol file from Inputs directory.
   BuildEmptyCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     module_file_spec.SetPath(GetInputFilePath(k_symbol_file));
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(),
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(),
             FileSpec(GetInputFilePath(k_symbol_file)));
-  ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
-  CheckUnstrippedSymbol(module_sp);
+  ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
+  CheckUnstrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest,
        GetOrCreateModuleCallbackSuccessWithSymbolAsModuleAndSymbol) {
   // The locate module callback returns a symbol file as both a module file and
   // a symbol file. It should work since the sections and UUID of the symbol
   // file are the exact same with the module file, GetOrCreateModule should
   // succeed to return the module with the symbol file from Inputs directory.
   BuildEmptyCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     module_file_spec.SetPath(GetInputFilePath(k_symbol_file));
     symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(),
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(),
             FileSpec(GetInputFilePath(k_symbol_file)));
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(),
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
             FileSpec(GetInputFilePath(k_symbol_file)));
-  CheckUnstrippedSymbol(module_sp);
+  CheckUnstrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest,
        GetOrCreateModuleCallbackSuccessWithModuleAndSymbol) {
   // The locate module callback returns a module file and a symbol file,
   // GetOrCreateModule should succeed to return the module from Inputs
   // directory, along with the symbol file.
   BuildEmptyCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     module_file_spec.SetPath(GetInputFilePath(k_module_file));
     symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(),
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(),
             FileSpec(GetInputFilePath(k_module_file)));
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(),
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
             FileSpec(GetInputFilePath(k_symbol_file)));
-  CheckUnstrippedSymbol(module_sp);
+  CheckUnstrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest,
        GetOrCreateModuleCallbackSuccessWithModuleAndBreakpadSymbol) {
   // The locate module callback returns a module file and a breakpad symbol
   // file, GetOrCreateModule should succeed to return the module with the symbol
   // file from Inputs directory.
   BuildEmptyCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     module_file_spec.SetPath(GetInputFilePath(k_module_file));
     symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file));
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(),
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(),
             FileSpec(GetInputFilePath(k_module_file)));
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(),
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
             FileSpec(GetInputFilePath(k_breakpad_symbol_file)));
-  CheckUnstrippedSymbol(module_sp);
+  CheckUnstrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest,
        GetOrCreateModuleCallbackSuccessWithOnlySymbol) {
   // The get callback returns only a symbol file, and the module is cached,
   // GetOrCreateModule should succeed to return the module from the cache
   // along with the symbol file from the Inputs directory.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(),
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
             FileSpec(GetInputFilePath(k_symbol_file)));
-  CheckUnstrippedSymbol(module_sp);
+  CheckUnstrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest,
        GetOrCreateModuleCallbackSuccessWithOnlyBreakpadSymbol) {
   // The get callback returns only a breakpad symbol file, and the module is
   // cached, GetOrCreateModule should succeed to return the module from the
   // cache along with the symbol file from the Inputs directory.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file));
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(),
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
             FileSpec(GetInputFilePath(k_breakpad_symbol_file)));
-  CheckUnstrippedSymbol(module_sp);
+  CheckUnstrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest,
        GetOrCreateModuleNoCacheWithCallbackOnlySymbol) {
   // The get callback returns only a symbol file, but the module is not
   // cached, GetOrCreateModule should fail because of the missing module.
   BuildEmptyCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  ASSERT_FALSE(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_FALSE(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest,
        GetOrCreateModuleNoCacheWithCallbackOnlyBreakpadSymbol) {
   // The get callback returns only a breakpad symbol file, but the module is not
   // cached, GetOrCreateModule should fail because of the missing module.
   BuildEmptyCacheDir(m_test_dir);
 
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
     CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
     symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file));
     return Status();
   });
 
-  ModuleSP module_sp =
-      m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  ASSERT_FALSE(module_sp);
-}
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_FALSE(m_module_sp);
+}