llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Andrew Savonichev (asavonic) <details> <summary>Changes</summary> Shared modules are stored in a global `ModuleList` cache, and it is intentionally leaked to avoid doing cleanup when lldb exits. However, when lldb is used as a library, we need a way to manage opened modules to avoid problems with file locks (on some systems) for modules that we no longer need. It should be possible to record all loaded modules and use `ModuleList::RemoveSharedModule` and `RemoveOrphanSharedModules` functions to clear the cache, but these functions are not available in the API. This approach is also way too complicated when we just need to cleanup the library. The patch adds the following: - `ModuleList::ClearSharedModules` function to clear all shared modules. - `SBDebugger::SetClearSharedModules(bool)` API to enable clearing during `SBDebugger::Clear()`. - `settings set clear-shared-modules true` command line option that is equivalent to `SetClearSharedModules(true)`. This new behaviour is turned off by default: the debugger does not release these shared modules until the process exits. --- Full diff: https://github.com/llvm/llvm-project/pull/147289.diff 10 Files Affected: - (modified) lldb/include/lldb/API/SBDebugger.h (+2) - (modified) lldb/include/lldb/Core/Debugger.h (+4) - (modified) lldb/include/lldb/Core/ModuleList.h (+3) - (modified) lldb/source/API/SBDebugger.cpp (+6) - (modified) lldb/source/Core/CoreProperties.td (+4) - (modified) lldb/source/Core/Debugger.cpp (+13) - (modified) lldb/source/Core/ModuleList.cpp (+7) - (modified) lldb/test/API/python_api/debugger/TestDebuggerAPI.py (+31) - (modified) lldb/unittests/Target/CMakeLists.txt (+1) - (added) lldb/unittests/Target/SharedModuleListTest.cpp (+209) ``````````diff diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h index 192fbee9c0c6d..6a2f76f2d5685 100644 --- a/lldb/include/lldb/API/SBDebugger.h +++ b/lldb/include/lldb/API/SBDebugger.h @@ -319,6 +319,8 @@ class LLDB_API SBDebugger { bool SetShowInlineDiagnostics(bool); + bool SetClearSharedModules(bool); + bool SetUseSourceCache(bool use_source_cache); bool GetUseSourceCache() const; diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 504f936fe317a..4cf7fc75bafd4 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -373,6 +373,10 @@ class Debugger : public std::enable_shared_from_this<Debugger>, bool SetShowInlineDiagnostics(bool); + bool GetClearSharedModules() const; + + bool SetClearSharedModules(bool); + bool LoadPlugin(const FileSpec &spec, Status &error); void RunIOHandlers(); diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h index 909ee08f9ba62..587843dd05a4d 100644 --- a/lldb/include/lldb/Core/ModuleList.h +++ b/lldb/include/lldb/Core/ModuleList.h @@ -482,6 +482,9 @@ class ModuleList { static bool RemoveSharedModuleIfOrphaned(const Module *module_ptr); + /// Empty global cache of modules to release memory, file locks, etc. + static void ClearSharedModules(); + /// Applies 'callback' to each module in this ModuleList. /// If 'callback' returns false, iteration terminates. /// The 'module_sp' passed to 'callback' is guaranteed to diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index 603e306497841..221c02cfe66ed 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -1466,6 +1466,12 @@ bool SBDebugger::SetShowInlineDiagnostics(bool value) { return (m_opaque_sp ? m_opaque_sp->SetShowInlineDiagnostics(value) : false); } +bool SBDebugger::SetClearSharedModules(bool value) { + LLDB_INSTRUMENT_VA(this, value); + + return (m_opaque_sp ? m_opaque_sp->SetClearSharedModules(value) : false); +} + bool SBDebugger::SetUseSourceCache(bool value) { LLDB_INSTRUMENT_VA(this, value); diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td index 53dd333f045c9..1a6ba1a9af84e 100644 --- a/lldb/source/Core/CoreProperties.td +++ b/lldb/source/Core/CoreProperties.td @@ -268,4 +268,8 @@ let Definition = "debugger" in { Global, DefaultFalse, Desc<"Controls whether diagnostics can refer directly to the command input, drawing arrows to it. If false, diagnostics will echo the input.">; + def ClearSharedModules: Property<"clear-shared-modules", "Boolean">, + Global, + DefaultFalse, + Desc<"Controls whether the debugger clears internal shared modules as it exits.">; } diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index ed674ee1275c7..fbd2f37960e19 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -700,6 +700,17 @@ bool Debugger::SetShowInlineDiagnostics(bool b) { return SetPropertyAtIndex(idx, b); } +bool Debugger::GetClearSharedModules() const { + const uint32_t idx = ePropertyClearSharedModules; + return GetPropertyAtIndexAs<bool>( + idx, g_debugger_properties[idx].default_uint_value); +} + +bool Debugger::SetClearSharedModules(bool b) { + const uint32_t idx = ePropertyClearSharedModules; + return SetPropertyAtIndex(idx, b); +} + #pragma mark Debugger // const DebuggerPropertiesSP & @@ -1092,6 +1103,8 @@ void Debugger::Clear() { StopIOHandlerThread(); StopEventHandlerThread(); m_listener_sp->Clear(); + if (GetClearSharedModules()) + ModuleList::ClearSharedModules(); for (TargetSP target_sp : m_target_list.Targets()) { if (target_sp) { if (ProcessSP process_sp = target_sp->GetProcessSP()) diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp index d5ddf6e846112..81a63b7ec541e 100644 --- a/lldb/source/Core/ModuleList.cpp +++ b/lldb/source/Core/ModuleList.cpp @@ -1040,6 +1040,13 @@ bool ModuleList::RemoveSharedModuleIfOrphaned(const Module *module_ptr) { return GetSharedModuleList().RemoveIfOrphaned(module_ptr); } +void ModuleList::ClearSharedModules() { + GetSharedModuleList().Clear(); + Log *log = GetLog(LLDBLog::Modules); + if (log != nullptr) + LLDB_LOGF(log, "cleared shared modules"); +} + bool ModuleList::LoadScriptingResourcesInTarget(Target *target, std::list<Status> &errors, Stream &feedback_stream, diff --git a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py index 43f45f330ee2a..7ff88eb252967 100644 --- a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py +++ b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py @@ -294,3 +294,34 @@ def test_version(self): self.assertEqual(instance_str, class_str) self.assertEqual(class_str, property_str) + + def test_default_SetClearSharedModules(self): + """Check that that SBDebugger does not clear shared modules by + default. + """ + messages = list() + self.dbg.SetLoggingCallback(messages.append) + self.runCmd("log enable lldb module") + self.dbg.Destroy(self.dbg) + self.assertFalse(any("cleared shared modules cache" in msg for msg in messages)) + + def test_enable_SetClearSharedModules(self): + """Check that SetClearSharedModule(true) clears shared module cache. + """ + messages = list() + self.dbg.SetLoggingCallback(messages.append) + self.dbg.SetClearSharedModules(True) + self.runCmd("log enable lldb module") + self.dbg.Destroy(self.dbg) + self.assertTrue(any("cleared shared modules" in msg for msg in messages)) + + def test_enable_clear_shared_modules(self): + """Check that clear-shared-module setting is equivalent + to SetClearSharedModules(true). + """ + messages = list() + self.dbg.SetLoggingCallback(messages.append) + self.runCmd("settings set clear-shared-modules true") + self.runCmd("log enable lldb module") + self.dbg.Destroy(self.dbg) + self.assertTrue(any("cleared shared modules" in msg for msg in messages)) diff --git a/lldb/unittests/Target/CMakeLists.txt b/lldb/unittests/Target/CMakeLists.txt index 3169339ec699f..aeb552e22ac3f 100644 --- a/lldb/unittests/Target/CMakeLists.txt +++ b/lldb/unittests/Target/CMakeLists.txt @@ -12,6 +12,7 @@ add_lldb_unittest(TargetTests RemoteAwarePlatformTest.cpp StackFrameRecognizerTest.cpp SummaryStatisticsTest.cpp + SharedModuleListTest.cpp FindFileTest.cpp LINK_COMPONENTS diff --git a/lldb/unittests/Target/SharedModuleListTest.cpp b/lldb/unittests/Target/SharedModuleListTest.cpp new file mode 100644 index 0000000000000..837f934f439c3 --- /dev/null +++ b/lldb/unittests/Target/SharedModuleListTest.cpp @@ -0,0 +1,209 @@ +//===---------- SharedModuleList.cpp --------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h" +#include "Plugins/ObjectFile/ELF/ObjectFileELF.h" +#include "Plugins/Platform/Android/PlatformAndroid.h" +#include "Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h" +#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h" +#include "TestingSupport/SubsystemRAII.h" +#include "TestingSupport/TestUtilities.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Core/Module.h" +#include "lldb/Core/PluginManager.h" +#include "lldb/Host/HostInfo.h" +#include "lldb/Target/Target.h" + +using namespace lldb; +using namespace lldb_private; +using namespace lldb_private::platform_android; +using namespace lldb_private::platform_linux; +using namespace lldb_private::breakpad; +using namespace testing; + +namespace { + +constexpr llvm::StringLiteral k_process_plugin("mock-process-plugin"); +constexpr llvm::StringLiteral k_platform_dir("remote-android"); +constexpr llvm::StringLiteral k_cache_dir(".cache"); +constexpr llvm::StringLiteral k_module_file("AndroidModule.so"); +constexpr llvm::StringLiteral k_arch("aarch64-none-linux"); +constexpr llvm::StringLiteral + k_module_uuid("80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC"); +const size_t k_module_size = 3784; + +FileSpec GetTestDir() { + const auto *info = UnitTest::GetInstance()->current_test_info(); + FileSpec test_dir = HostInfo::GetProcessTempDir(); + test_dir.AppendPathComponent(std::string(info->test_case_name()) + "-" + + info->name()); + std::error_code ec = llvm::sys::fs::create_directory(test_dir.GetPath()); + EXPECT_FALSE(ec); + return test_dir; +} + +FileSpec GetRemotePath() { + FileSpec fs("/", FileSpec::Style::posix); + fs.AppendPathComponent("bin"); + fs.AppendPathComponent(k_module_file); + return fs; +} + +FileSpec GetUuidView(FileSpec spec) { + spec.AppendPathComponent(k_platform_dir); + spec.AppendPathComponent(k_cache_dir); + spec.AppendPathComponent(k_module_uuid); + spec.AppendPathComponent(k_module_file); + return spec; +} + +FileSpec BuildCacheDir(const FileSpec &test_dir) { + FileSpec uuid_view = GetUuidView(test_dir); + std::error_code ec = + llvm::sys::fs::create_directories(uuid_view.GetDirectory().GetCString()); + EXPECT_FALSE(ec); + ec = llvm::sys::fs::copy_file(GetInputFilePath(k_module_file), + uuid_view.GetPath().c_str()); + EXPECT_FALSE(ec); + return uuid_view; +} + +ModuleSpec GetTestModuleSpec() { + ModuleSpec module_spec(GetRemotePath(), ArchSpec(k_arch)); + module_spec.GetUUID().SetFromStringRef(k_module_uuid); + module_spec.SetObjectSize(k_module_size); + return module_spec; +} + +void CheckModule(const ModuleSP &module_sp) { + ASSERT_TRUE(module_sp); + ASSERT_EQ(module_sp->GetUUID().GetAsString(), k_module_uuid); + ASSERT_EQ(module_sp->GetObjectOffset(), 0U); + ASSERT_EQ(module_sp->GetPlatformFileSpec(), GetRemotePath()); +} + +class MockProcess : public Process { +public: + MockProcess(TargetSP target_sp, ListenerSP listener_sp) + : Process(target_sp, listener_sp) {} + + llvm::StringRef GetPluginName() override { return k_process_plugin; }; + + bool CanDebug(TargetSP target, bool plugin_specified_by_name) override { + return true; + } + + Status DoDestroy() override { return Status(); } + + void RefreshStateAfterStop() override {} + + bool DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) override { + return false; + } + + size_t DoReadMemory(addr_t vm_addr, void *buf, size_t size, + Status &error) override { + return 0; + } + + bool GetModuleSpec(const FileSpec &module_file_spec, const ArchSpec &arch, + ModuleSpec &module_spec) override { + module_spec = GetTestModuleSpec(); + return true; + } +}; + +ProcessSP MockProcessCreateInstance(TargetSP target_sp, ListenerSP listener_sp, + const FileSpec *crash_file_path, + bool can_connect) { + return std::make_shared<MockProcess>(target_sp, listener_sp); +} + +class SharedModuleListTest : public testing::Test { + SubsystemRAII<FileSystem, HostInfo, ObjectFileBreakpad, ObjectFileELF, + PlatformAndroid, PlatformLinux, SymbolFileBreakpad, + SymbolFileSymtab> + subsystems; + +public: + void SetUp() override { + m_test_dir = GetTestDir(); + + // Set module cache directory for PlatformAndroid. + PlatformAndroid::GetGlobalPlatformProperties().SetModuleCacheDirectory( + m_test_dir); + + // Create Debugger. + ArchSpec host_arch("i386-pc-linux"); + Platform::SetHostPlatform( + platform_linux::PlatformLinux::CreateInstance(true, &host_arch)); + m_debugger_sp = Debugger::CreateInstance(); + EXPECT_TRUE(m_debugger_sp); + + // Create PlatformAndroid. + ArchSpec arch(k_arch); + m_platform_sp = PlatformAndroid::CreateInstance(true, &arch); + EXPECT_TRUE(m_platform_sp); + + // Create Target. + m_debugger_sp->GetTargetList().CreateTarget(*m_debugger_sp, "", arch, + eLoadDependentsNo, + m_platform_sp, m_target_sp); + EXPECT_TRUE(m_target_sp); + + // Create MockProcess. + PluginManager::RegisterPlugin(k_process_plugin, "", + MockProcessCreateInstance); + 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(); + m_module_spec_without_uuid = ModuleSpec(GetRemotePath(), ArchSpec(k_arch)); + } + + void TearDown() override { + if (m_module_sp) + ModuleList::RemoveSharedModule(m_module_sp); + } + +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; + ModuleSpec m_module_spec_without_uuid; + ModuleSP m_module_sp; + int m_callback_call_count = 0; +}; + +} // namespace + +TEST_F(SharedModuleListTest, TestClear) { + FileSpec uuid_view = BuildCacheDir(m_test_dir); + + 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()); + + UUID uuid = m_module_sp->GetUUID(); + + // Check if the module is cached + ASSERT_TRUE(ModuleList::FindSharedModule(uuid)); + + // Clear cache and check that it is gone + ModuleList::ClearSharedModules(); + ASSERT_FALSE(ModuleList::FindSharedModule(uuid)); + m_module_sp = nullptr; +} `````````` </details> https://github.com/llvm/llvm-project/pull/147289 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits