https://github.com/Teemperor created https://github.com/llvm/llvm-project/pull/219425
This replaces the mutex + variable pair with a `Guarded` variable that enforces the locking. >From d5fc2112dc99bc54f2e27388eb079a0e5535f61d Mon Sep 17 00:00:00 2001 From: Raphael Isemann <[email protected]> Date: Thu, 27 Aug 2026 15:28:32 +0100 Subject: [PATCH 1/2] [lldb] Add Guarded<T, Mutex> to Locked.h LLDB's code base has many variables that have an associated mutex that needs to be locked to safely access that variable from multiple threads. However, this locking scheme is currently not enforced by the compiler and code sometimes accesses these variables without aquiring the respective mutex first. This patch introduces a `Guarded` class that strictly enforces that some memory is only accessed after the respective mutex has been aquired. This class hands out `Locked` objects for every access which guarentee that the mutex is held as long as the variable is in scope. --- lldb/include/lldb/Utility/Locked.h | 28 ++++++++++++++++++++ lldb/unittests/Utility/LockedTest.cpp | 37 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/lldb/include/lldb/Utility/Locked.h b/lldb/include/lldb/Utility/Locked.h index acd34e454a918..f5d7b1aa8e4d9 100644 --- a/lldb/include/lldb/Utility/Locked.h +++ b/lldb/include/lldb/Utility/Locked.h @@ -168,6 +168,34 @@ template <typename T, typename Mutex = llvm::sys::RWMutex> using SharedLockedUP = SharedLocked<std::unique_ptr<const T>, Mutex>; /// @} +/// Bundles a value of type `T` with the `Mutex` that guards it. +/// +/// This class prevents accidential use of a value without aquiring the +/// lock and should be preferred over a member + mutex pair. +/// +/// `Mutex` must satisfy `Lockable` when calling `Lock()` and `SharedLockable` +/// when calling `LockShared()`. +template <typename T, typename Mutex = llvm::sys::RWMutex> class Guarded { +public: + Guarded() = default; + explicit Guarded(T value) : m_value(std::move(value)) {} + + Guarded(const Guarded &) = delete; + Guarded &operator=(const Guarded &) = delete; + + /// Exclusive (read/write) access to the value. + Locked<T *, Mutex> Lock() { return Locked<T *, Mutex>(m_mutex, &m_value); } + + /// Shared (read-only) access to the value. + SharedLocked<const T *, Mutex> LockShared() const { + return SharedLocked<const T *, Mutex>(m_mutex, &m_value); + } + +private: + mutable Mutex m_mutex; + T m_value{}; +}; + } // namespace lldb_private #endif // LLDB_UTILITY_LOCKED_H diff --git a/lldb/unittests/Utility/LockedTest.cpp b/lldb/unittests/Utility/LockedTest.cpp index cae24293dee2e..f898f21b56291 100644 --- a/lldb/unittests/Utility/LockedTest.cpp +++ b/lldb/unittests/Utility/LockedTest.cpp @@ -225,3 +225,40 @@ TEST(LockedTest, ExclusiveAccessOnRWMutex) { writer->value = 11; EXPECT_EQ(widget.value, 11); } + +// Guarded is neither copyable nor movable. +static_assert(!std::is_copy_constructible_v<Guarded<Widget>>); +static_assert(!std::is_move_constructible_v<Guarded<Widget>>); + +TEST(LockedTest, GuardedDefaultConstructed) { + Guarded<Widget> guarded; + EXPECT_EQ(guarded.Lock()->value, 0); +} + +TEST(LockedTest, GuardedValueConstructed) { + Guarded<Widget> guarded(Widget{42}); + EXPECT_EQ(guarded.Lock()->value, 42); +} + +TEST(LockedTest, GuardedExclusiveAccessMutatesValue) { + Guarded<Widget> guarded; + guarded.Lock()->value = 7; + EXPECT_EQ(guarded.Lock()->value, 7); +} + +TEST(LockedTest, GuardedSharedAccessIsReadOnly) { + Guarded<Widget> guarded(Widget{5}); + SharedLocked<const Widget *, llvm::sys::RWMutex> reader = + guarded.LockShared(); + EXPECT_EQ(reader->value, 5); + static_assert(std::is_same_v<decltype(reader.get()), const Widget *>, + "shared access borrows a const-qualified pointer"); +} + +// std::shared_mutex satisfies SharedLockable too, so Guarded works with it +// as a drop-in replacement for llvm::sys::RWMutex. +TEST(LockedTest, GuardedWorksWithStdSharedMutex) { + Guarded<Widget, std::shared_mutex> guarded; + guarded.Lock()->value = 3; + EXPECT_EQ(guarded.LockShared()->value, 3); +} >From 8730987f3e0b985142aabe6610c56c7f2bedf61c Mon Sep 17 00:00:00 2001 From: Raphael Isemann <[email protected]> Date: Thu, 27 Aug 2026 15:46:17 +0100 Subject: [PATCH 2/2] [lldb] Use Guarded in MemoryRegionInfoCache This replaces the mutex + variable pair with a `Guarded` variable that enforces the locking. --- .../include/lldb/Target/MemoryRegionInfoCache.h | 5 +++-- lldb/source/Target/MemoryRegionInfoCache.cpp | 17 ++++++----------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/lldb/include/lldb/Target/MemoryRegionInfoCache.h b/lldb/include/lldb/Target/MemoryRegionInfoCache.h index 5117f1f598e55..7daea43671646 100644 --- a/lldb/include/lldb/Target/MemoryRegionInfoCache.h +++ b/lldb/include/lldb/Target/MemoryRegionInfoCache.h @@ -10,6 +10,7 @@ #define LLDB_TARGET_MEMORYREGIONINFOCACHE_H #include "lldb/Target/MemoryRegionInfo.h" +#include "lldb/Utility/Locked.h" #include <map> #include <mutex> @@ -34,8 +35,8 @@ class MemoryRegionInfoCache { size_t GetSize(); private: - std::map<lldb::addr_t, MemoryRegionInfo> m_region_infos; - std::mutex m_mutex; + Guarded<std::map<lldb::addr_t, MemoryRegionInfo>, std::mutex> + m_region_infos; }; } // namespace lldb_private diff --git a/lldb/source/Target/MemoryRegionInfoCache.cpp b/lldb/source/Target/MemoryRegionInfoCache.cpp index 47694b3ad58fc..e84e56fbcc71e 100644 --- a/lldb/source/Target/MemoryRegionInfoCache.cpp +++ b/lldb/source/Target/MemoryRegionInfoCache.cpp @@ -12,21 +12,17 @@ using namespace lldb; using namespace lldb_private; -void MemoryRegionInfoCache::Clear() { - std::lock_guard<std::mutex> guard(m_mutex); - m_region_infos.clear(); -} +void MemoryRegionInfoCache::Clear() { m_region_infos.Lock()->clear(); } size_t MemoryRegionInfoCache::GetSize() { - std::lock_guard<std::mutex> guard(m_mutex); - return m_region_infos.size(); + return m_region_infos.Lock()->size(); } std::optional<MemoryRegionInfo> MemoryRegionInfoCache::GetMemoryRegion(addr_t load_addr) { - std::lock_guard<std::mutex> guard(m_mutex); - auto it = m_region_infos.upper_bound(load_addr); - if (it == m_region_infos.begin()) + auto region_infos = m_region_infos.Lock(); + auto it = region_infos->upper_bound(load_addr); + if (it == region_infos->begin()) return std::nullopt; --it; if (load_addr < it->second.GetRange().GetRangeEnd()) @@ -36,6 +32,5 @@ MemoryRegionInfoCache::GetMemoryRegion(addr_t load_addr) { } void MemoryRegionInfoCache::AddRegion(const MemoryRegionInfo &ri) { - std::lock_guard<std::mutex> guard(m_mutex); - m_region_infos.insert_or_assign(ri.GetRange().GetRangeBase(), ri); + m_region_infos.Lock()->insert_or_assign(ri.GetRange().GetRangeBase(), ri); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
