https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/205226
>From dd0a168e3f798913c22b62cc52715b4100dd5ee2 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <[email protected]> Date: Mon, 22 Jun 2026 17:14:05 -0700 Subject: [PATCH] [lldb] Fix data race in Module::GetSectionList Module::GetSectionList populated m_sections_up without a lock, so loading modules in parallel (e.g. crashlog.py's thread pool) let two threads build the section list at once and race on the unique_ptr and the SectionList vector, crashing in AppleObjCRuntime::GetObjCVersion. Serialize the build with a dedicated m_sections_mutex, not the module mutex: Module::PreloadSymbols holds the module mutex across the parallel DWARF index, whose worker threads re-enter GetSectionList (via the SymbolFileDWARFDwo and SymbolFileDWARF constructors), so guarding the build with the module mutex would deadlock the preload thread against its own workers. rdar://180308581 --- lldb/include/lldb/Core/Module.h | 3 + lldb/source/Core/Module.cpp | 8 ++- lldb/unittests/Core/ModuleTest.cpp | 93 ++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 3 deletions(-) diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h index eb09e8b602da7..33904ef7be5d8 100644 --- a/lldb/include/lldb/Core/Module.h +++ b/lldb/include/lldb/Core/Module.h @@ -1116,6 +1116,9 @@ class Module : public std::enable_shared_from_this<Module>, /// is used by the ObjectFile and /// ObjectFile instances for the debug info + /// Guards the lazy construction of m_sections_up. + mutable std::recursive_mutex m_sections_mutex; + std::atomic<bool> m_did_load_objfile{false}; std::atomic<bool> m_did_load_symfile{false}; std::atomic<bool> m_did_set_uuid{false}; diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index e232d322d762c..2bc8fd138427d 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -1221,10 +1221,12 @@ ObjectFile *Module::GetObjectFile() { } SectionList *Module::GetSectionList() { - // Populate m_sections_up with sections from objfile. + // Guard the lazy build with m_sections_mutex rather than m_mutex: + // Module::PreloadSymbols holds m_mutex across the parallel DWARF index, whose + // worker threads re-enter GetSectionList, so taking m_mutex here deadlocks. + std::lock_guard<std::recursive_mutex> guard(m_sections_mutex); if (!m_sections_up) { - ObjectFile *obj_file = GetObjectFile(); - if (obj_file != nullptr) + if (ObjectFile *obj_file = GetObjectFile()) obj_file->CreateSections(*GetUnifiedSectionList()); } return m_sections_up.get(); diff --git a/lldb/unittests/Core/ModuleTest.cpp b/lldb/unittests/Core/ModuleTest.cpp index bcaeede367bdd..62cc2c025c863 100644 --- a/lldb/unittests/Core/ModuleTest.cpp +++ b/lldb/unittests/Core/ModuleTest.cpp @@ -13,11 +13,17 @@ #include "TestingSupport/SubsystemRAII.h" #include "TestingSupport/TestUtilities.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Core/Section.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" #include "lldb/Target/Language.h" +#include "lldb/Utility/ConstString.h" #include "gtest/gtest.h" +#include <condition_variable> +#include <mutex> #include <optional> +#include <thread> +#include <vector> using namespace lldb; using namespace lldb_private; @@ -170,3 +176,90 @@ TEST(ModuleTest, ResolveSymbolContextForAddressExactMatch) { ASSERT_NE(sc.symbol, nullptr); EXPECT_STREQ(sc.symbol->GetName().GetCString(), "inner_function"); } + +// Module::GetSectionList builds the module's section list lazily. Concurrent +// first-time callers (e.g. AppleObjCRuntime::GetObjCVersion during parallel +// SBTarget module loading) must not race on m_sections_up. This hammers +// GetSectionList from several threads on a fresh module so a sanitizer flags an +// unsynchronized build. +TEST(ModuleTest, GetSectionListConcurrent) { + SubsystemRAII<FileSystem, HostInfo, ObjectFileELF, SymbolFileSymtab> + subsystems; + + // Several sections widen the window during which CreateSections is appending + // to the SectionList vector while another thread iterates it. + const char *yaml = R"( +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_DYN + Machine: EM_X86_64 +Sections: + - Name: .text + Type: SHT_PROGBITS + Flags: [ SHF_ALLOC, SHF_EXECINSTR ] + Address: 0x1000 + AddressAlign: 0x10 + Size: 0x100 + - Name: .data + Type: SHT_PROGBITS + Flags: [ SHF_ALLOC, SHF_WRITE ] + Address: 0x2000 + AddressAlign: 0x10 + Size: 0x100 + - Name: .rodata + Type: SHT_PROGBITS + Flags: [ SHF_ALLOC ] + Address: 0x3000 + AddressAlign: 0x10 + Size: 0x100 + - Name: .bss + Type: SHT_NOBITS + Flags: [ SHF_ALLOC, SHF_WRITE ] + Address: 0x4000 + AddressAlign: 0x10 + Size: 0x100 +... +)"; + + const ConstString text_name(".text"); + constexpr int kThreads = 8; + // Each iteration uses a fresh module so the lazy build (and its race) is + // re-triggered every time. + for (int iter = 0; iter < 100; ++iter) { + auto ExpectedFile = TestFile::fromYaml(yaml); + ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded()); + auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec()); + + // Release the threads together with a blocking gate. A busy-wait would peg + // every core and starve the workers when many test binaries run at once. + std::mutex mutex; + std::condition_variable cv; + bool go = false; + std::vector<std::thread> threads; + threads.reserve(kThreads); + for (int t = 0; t < kThreads; ++t) { + threads.emplace_back([&] { + { + std::unique_lock<std::mutex> lock(mutex); + cv.wait(lock, [&] { return go; }); + } + if (SectionList *sections = module_sp->GetSectionList()) + sections->FindSectionByName(text_name); + }); + } + { + std::lock_guard<std::mutex> lock(mutex); + go = true; + } + cv.notify_all(); + for (auto &th : threads) + th.join(); + + // The concurrently-built list must be intact and complete. + SectionList *sections = module_sp->GetSectionList(); + ASSERT_NE(sections, nullptr); + EXPECT_TRUE(sections->FindSectionByName(text_name)); + } +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
