[Lldb-commits] [lldb] [lldb] Adopt LockedPtr in Module (NFC) (PR #199160)

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Fri May 22 09:30:52 PDT 2026


https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/199160

>From b231bb035eed0f1e140ae861719320dcdb0eaaaa Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Thu, 21 May 2026 20:38:37 -0700
Subject: [PATCH 1/2] [lldb] Adopt LockedPtr in Module (NFC)

Follow-up to #198941, which introduced Locked<T> and SharedLocked<T>.
Add GetObjectFileLocked, GetSymbolFileLocked, GetSymtabLocked, and
GetSectionListLocked alongside the existing accessors.

The locked variants cover two things:

1. They prevent the pointer from being swapped out from under the
   caller. The old getters take m_mutex only during lazy
   initialization and release it before returning. The unique_ptr or
   shared_ptr that owns the pointee can therefore be reassigned by
   another thread while the caller still holds the raw value.
   LockedPtr keeps the Module mutex held alongside the borrowed
   pointer, pinning the binding for the lifetime of the handle.

2. They serialize access to the pointee itself. This is not new, the
   classes in question were already relying on the Module mutex for
   synchronization.

Migrate the four call sites in Module where the existing patter maps to
a single LockedPtr.

The legacy raw-pointer getters remain so call sites can migrate
incrementally.
---
 lldb/include/lldb/Core/Module.h | 19 ++++++++++++++++
 lldb/source/Core/Module.cpp     | 39 +++++++++++++++++++++++----------
 2 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index d33d1b1938ef4..b605a9e3b4edc 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -20,6 +20,7 @@
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Locked.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/UUID.h"
 #include "lldb/Utility/XcodeSDK.h"
@@ -537,6 +538,11 @@ class Module : public std::enable_shared_from_this<Module>,
   ///     remains valid as long as the object is around.
   virtual ObjectFile *GetObjectFile();
 
+  /// Like GetObjectFile, but the returned handle holds the Module mutex for
+  /// its lifetime, serializing concurrent access to the ObjectFile against
+  /// other callers using the locked accessors.
+  LockedPtr<ObjectFile> GetObjectFileLocked();
+
   /// Get the unified section list for the module. This is the section list
   /// created by the module's object file and any debug info and symbol files
   /// created by the symbol vendor.
@@ -548,6 +554,10 @@ class Module : public std::enable_shared_from_this<Module>,
   ///     Unified module section list.
   virtual SectionList *GetSectionList();
 
+  /// Like GetSectionList, but the returned handle holds the Module mutex for
+  /// its lifetime.
+  LockedPtr<SectionList> GetSectionListLocked();
+
   /// Notify the module that the file addresses for the Sections have been
   /// updated.
   ///
@@ -598,12 +608,21 @@ class Module : public std::enable_shared_from_this<Module>,
   virtual SymbolFile *GetSymbolFile(bool can_create = true,
                                     Stream *feedback_strm = nullptr);
 
+  /// Like GetSymbolFile, but the returned handle holds the Module mutex for
+  /// its lifetime.
+  LockedPtr<SymbolFile> GetSymbolFileLocked(bool can_create = true,
+                                            Stream *feedback_strm = nullptr);
+
   /// Get the module's symbol table
   ///
   /// If the symbol table has already been loaded, this function returns it.
   /// Otherwise, it will only be loaded when can_create is true.
   Symtab *GetSymtab(bool can_create = true);
 
+  /// Like GetSymtab, but the returned handle holds the Module mutex for its
+  /// lifetime.
+  LockedPtr<Symtab> GetSymtabLocked(bool can_create = true);
+
   /// Get a reference to the UUID value contained in this object.
   ///
   /// If the executable image file doesn't not have a UUID value built into
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 9c06613818c12..2e6df8787262b 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -368,13 +368,11 @@ void Module::ForEachTypeSystem(
 }
 
 void Module::ParseAllDebugSymbols() {
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  size_t num_comp_units = GetNumCompileUnits();
+  LockedPtr<SymbolFile> symbols = GetSymbolFileLocked();
+  size_t num_comp_units = symbols ? symbols->GetNumCompileUnits() : 0;
   if (num_comp_units == 0)
     return;
 
-  SymbolFile *symbols = GetSymbolFile();
-
   for (size_t cu_idx = 0; cu_idx < num_comp_units; cu_idx++) {
     SymbolContext sc;
     sc.module_sp = shared_from_this();
@@ -411,8 +409,7 @@ void Module::DumpSymbolContext(Stream *s) {
 }
 
 size_t Module::GetNumCompileUnits() {
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  if (SymbolFile *symbols = GetSymbolFile())
+  if (LockedPtr<SymbolFile> symbols = GetSymbolFileLocked())
     return symbols->GetNumCompileUnits();
   return 0;
 }
@@ -430,10 +427,9 @@ CompUnitSP Module::GetCompileUnitAtIndex(size_t index) {
 }
 
 bool Module::ResolveFileAddress(lldb::addr_t vm_addr, Address &so_addr) {
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  SectionList *section_list = GetSectionList();
+  LockedPtr<SectionList> section_list = GetSectionListLocked();
   if (section_list)
-    return so_addr.ResolveAddressUsingFileSections(vm_addr, section_list);
+    return so_addr.ResolveAddressUsingFileSections(vm_addr, section_list.get());
   return false;
 }
 
@@ -1234,6 +1230,28 @@ SectionList *Module::GetSectionList() {
   return m_sections_up.get();
 }
 
+LockedPtr<ObjectFile> Module::GetObjectFileLocked() {
+  std::unique_lock<std::recursive_mutex> guard(m_mutex);
+  return LockedPtr<ObjectFile>(std::move(guard), GetObjectFile());
+}
+
+LockedPtr<SectionList> Module::GetSectionListLocked() {
+  std::unique_lock<std::recursive_mutex> guard(m_mutex);
+  return LockedPtr<SectionList>(std::move(guard), GetSectionList());
+}
+
+LockedPtr<SymbolFile> Module::GetSymbolFileLocked(bool can_create,
+                                                  Stream *feedback_strm) {
+  std::unique_lock<std::recursive_mutex> guard(m_mutex);
+  return LockedPtr<SymbolFile>(std::move(guard),
+                               GetSymbolFile(can_create, feedback_strm));
+}
+
+LockedPtr<Symtab> Module::GetSymtabLocked(bool can_create) {
+  std::unique_lock<std::recursive_mutex> guard(m_mutex);
+  return LockedPtr<Symtab>(std::move(guard), GetSymtab(can_create));
+}
+
 void Module::SectionFileAddressesChanged() {
   ObjectFile *obj_file = GetObjectFile();
   if (obj_file)
@@ -1320,8 +1338,7 @@ void Module::FindSymbolsMatchingRegExAndType(
 }
 
 void Module::PreloadSymbols() {
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  SymbolFile *sym_file = GetSymbolFile();
+  LockedPtr<SymbolFile> sym_file = GetSymbolFileLocked();
   if (!sym_file)
     return;
 

>From 79f209c07e98844bd84e7f2ae1d9671336aeceda Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 22 May 2026 09:30:35 -0700
Subject: [PATCH 2/2] Address Raphael's comments: pass mutex directly

---
 lldb/source/Core/Module.cpp | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 2e6df8787262b..d857fd500c662 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1231,25 +1231,21 @@ SectionList *Module::GetSectionList() {
 }
 
 LockedPtr<ObjectFile> Module::GetObjectFileLocked() {
-  std::unique_lock<std::recursive_mutex> guard(m_mutex);
-  return LockedPtr<ObjectFile>(std::move(guard), GetObjectFile());
+  return LockedPtr<ObjectFile>(m_mutex, GetObjectFile());
 }
 
 LockedPtr<SectionList> Module::GetSectionListLocked() {
-  std::unique_lock<std::recursive_mutex> guard(m_mutex);
-  return LockedPtr<SectionList>(std::move(guard), GetSectionList());
+  return LockedPtr<SectionList>(m_mutex, GetSectionList());
 }
 
 LockedPtr<SymbolFile> Module::GetSymbolFileLocked(bool can_create,
                                                   Stream *feedback_strm) {
-  std::unique_lock<std::recursive_mutex> guard(m_mutex);
-  return LockedPtr<SymbolFile>(std::move(guard),
+  return LockedPtr<SymbolFile>(m_mutex,
                                GetSymbolFile(can_create, feedback_strm));
 }
 
 LockedPtr<Symtab> Module::GetSymtabLocked(bool can_create) {
-  std::unique_lock<std::recursive_mutex> guard(m_mutex);
-  return LockedPtr<Symtab>(std::move(guard), GetSymtab(can_create));
+  return LockedPtr<Symtab>(m_mutex, GetSymtab(can_create));
 }
 
 void Module::SectionFileAddressesChanged() {



More information about the lldb-commits mailing list