https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/131404
>From ab4700b007becba1dfe63310bbb2a627214b6a60 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Wed, 19 Mar 2025 10:39:50 -0700 Subject: [PATCH 1/2] [lldb] Expose the Target API lock through the SB API --- lldb/bindings/interface/SBLockExtensions.i | 11 ++++ lldb/bindings/interfaces.swig | 4 +- lldb/include/lldb/API/LLDB.h | 1 + lldb/include/lldb/API/SBDefines.h | 1 + lldb/include/lldb/API/SBLock.h | 46 +++++++++++++++ lldb/include/lldb/API/SBTarget.h | 2 + lldb/include/lldb/Target/Target.h | 15 +++++ lldb/source/API/CMakeLists.txt | 1 + lldb/source/API/SBLock.cpp | 52 +++++++++++++++++ lldb/source/API/SBTarget.cpp | 16 ++++-- .../API/python_api/target/TestTargetAPI.py | 28 +++++++++ lldb/unittests/API/CMakeLists.txt | 1 + lldb/unittests/API/SBLockTest.cpp | 57 +++++++++++++++++++ 13 files changed, 230 insertions(+), 5 deletions(-) create mode 100644 lldb/bindings/interface/SBLockExtensions.i create mode 100644 lldb/include/lldb/API/SBLock.h create mode 100644 lldb/source/API/SBLock.cpp create mode 100644 lldb/unittests/API/SBLockTest.cpp diff --git a/lldb/bindings/interface/SBLockExtensions.i b/lldb/bindings/interface/SBLockExtensions.i new file mode 100644 index 0000000000000..27736c3f287b7 --- /dev/null +++ b/lldb/bindings/interface/SBLockExtensions.i @@ -0,0 +1,11 @@ +%extend lldb::SBLock { +#ifdef SWIGPYTHON + %pythoncode %{ + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + self.Unlock() + %} +#endif +} diff --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig index 08df9a1a8d539..3dffc23d1a4f7 100644 --- a/lldb/bindings/interfaces.swig +++ b/lldb/bindings/interfaces.swig @@ -47,6 +47,7 @@ %include "./interface/SBLaunchInfoDocstrings.i" %include "./interface/SBLineEntryDocstrings.i" %include "./interface/SBListenerDocstrings.i" +%include "./interface/SBLockExtensions.i" %include "./interface/SBMemoryRegionInfoDocstrings.i" %include "./interface/SBMemoryRegionInfoListDocstrings.i" %include "./interface/SBModuleDocstrings.i" @@ -121,11 +122,12 @@ %include "lldb/API/SBHostOS.h" %include "lldb/API/SBInstruction.h" %include "lldb/API/SBInstructionList.h" -%include "lldb/API/SBLanguages.h" %include "lldb/API/SBLanguageRuntime.h" +%include "lldb/API/SBLanguages.h" %include "lldb/API/SBLaunchInfo.h" %include "lldb/API/SBLineEntry.h" %include "lldb/API/SBListener.h" +%include "lldb/API/SBLock.h" %include "lldb/API/SBMemoryRegionInfo.h" %include "lldb/API/SBMemoryRegionInfoList.h" %include "lldb/API/SBModule.h" diff --git a/lldb/include/lldb/API/LLDB.h b/lldb/include/lldb/API/LLDB.h index 126fcef31b416..47474d9ead1bf 100644 --- a/lldb/include/lldb/API/LLDB.h +++ b/lldb/include/lldb/API/LLDB.h @@ -46,6 +46,7 @@ #include "lldb/API/SBLaunchInfo.h" #include "lldb/API/SBLineEntry.h" #include "lldb/API/SBListener.h" +#include "lldb/API/SBLock.h" #include "lldb/API/SBMemoryRegionInfo.h" #include "lldb/API/SBMemoryRegionInfoList.h" #include "lldb/API/SBModule.h" diff --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h index ed5a80da117a5..7fa5150d69d8a 100644 --- a/lldb/include/lldb/API/SBDefines.h +++ b/lldb/include/lldb/API/SBDefines.h @@ -84,6 +84,7 @@ class LLDB_API SBLanguageRuntime; class LLDB_API SBLaunchInfo; class LLDB_API SBLineEntry; class LLDB_API SBListener; +class LLDB_API SBLock; class LLDB_API SBMemoryRegionInfo; class LLDB_API SBMemoryRegionInfoList; class LLDB_API SBModule; diff --git a/lldb/include/lldb/API/SBLock.h b/lldb/include/lldb/API/SBLock.h new file mode 100644 index 0000000000000..93503d051edaa --- /dev/null +++ b/lldb/include/lldb/API/SBLock.h @@ -0,0 +1,46 @@ +//===-- SBLock.h ----------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_API_SBLOCK_H +#define LLDB_API_SBLOCK_H + +#include "lldb/API/SBDefines.h" +#include "lldb/lldb-forward.h" +#include <mutex> + +namespace lldb_private { +class APILock; +} + +namespace lldb { + +class LLDB_API SBLock { +public: + SBLock(); + SBLock(SBLock &&rhs); + SBLock &operator=(SBLock &&rhs); + ~SBLock(); + + bool IsValid() const; + + void Unlock() const; + +private: + // Private constructor used by SBTarget to create the Target API lock. + // Requires a friend declaration. + SBLock(lldb::TargetSP target_sp); + friend class SBTarget; + + SBLock(const SBLock &rhs) = delete; + const SBLock &operator=(const SBLock &rhs) = delete; + + std::unique_ptr<lldb_private::APILock> m_opaque_up; +}; +#endif + +} // namespace lldb diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index bb912ab41d0fe..75bf1ccd9e935 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -946,6 +946,8 @@ class LLDB_API SBTarget { /// An error if a Trace already exists or the trace couldn't be created. lldb::SBTrace CreateTrace(SBError &error); + lldb::SBLock AcquireAPILock() const; + protected: friend class SBAddress; friend class SBAddressRange; diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 80ce5f013344c..3b99593292209 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1692,6 +1692,21 @@ class Target : public std::enable_shared_from_this<Target>, } }; +/// The private implementation backing SBLock. +class APILock { +public: + APILock(std::shared_ptr<std::recursive_mutex> mutex_sp) + : m_mutex(std::move(mutex_sp)), m_lock(*m_mutex) {} + + void Unlock() { m_lock.unlock(); } + + operator bool() const { return static_cast<bool>(m_lock); } + +private: + std::shared_ptr<std::recursive_mutex> m_mutex; + std::unique_lock<std::recursive_mutex> m_lock; +}; + } // namespace lldb_private #endif // LLDB_TARGET_TARGET_H diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt index 48d5cde5bf592..c9a9433b2329d 100644 --- a/lldb/source/API/CMakeLists.txt +++ b/lldb/source/API/CMakeLists.txt @@ -77,6 +77,7 @@ add_lldb_library(liblldb SHARED ${option_framework} SBLaunchInfo.cpp SBLineEntry.cpp SBListener.cpp + SBLock.cpp SBMemoryRegionInfo.cpp SBMemoryRegionInfoList.cpp SBModule.cpp diff --git a/lldb/source/API/SBLock.cpp b/lldb/source/API/SBLock.cpp new file mode 100644 index 0000000000000..598fd26720763 --- /dev/null +++ b/lldb/source/API/SBLock.cpp @@ -0,0 +1,52 @@ +//===-- SBLock.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 "lldb/API/SBLock.h" +#include "lldb/Target/Target.h" +#include "lldb/Utility/Instrumentation.h" +#include "lldb/lldb-forward.h" +#include <memory> +#include <mutex> + +using namespace lldb; +using namespace lldb_private; + +SBLock::SBLock() { LLDB_INSTRUMENT_VA(this); } + +SBLock::SBLock(SBLock &&rhs) : m_opaque_up(std::move(rhs.m_opaque_up)) { + LLDB_INSTRUMENT_VA(this); +} + +SBLock &SBLock::operator=(SBLock &&rhs) { + LLDB_INSTRUMENT_VA(this); + + m_opaque_up = std::move(rhs.m_opaque_up); + return *this; +} + +SBLock::SBLock(lldb::TargetSP target_sp) + : m_opaque_up( + std::make_unique<APILock>(std::shared_ptr<std::recursive_mutex>( + target_sp, &target_sp->GetAPIMutex()))) { + LLDB_INSTRUMENT_VA(this, target_sp); +} + +SBLock::~SBLock() { LLDB_INSTRUMENT_VA(this); } + +bool SBLock::IsValid() const { + LLDB_INSTRUMENT_VA(this); + + return static_cast<bool>(m_opaque_up) && static_cast<bool>(*m_opaque_up); +} + +void SBLock::Unlock() const { + LLDB_INSTRUMENT_VA(this); + + if (m_opaque_up) + m_opaque_up->Unlock(); +} diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index dd9caa724ea36..da9b51e56b27b 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -7,10 +7,6 @@ //===----------------------------------------------------------------------===// #include "lldb/API/SBTarget.h" -#include "lldb/Utility/Instrumentation.h" -#include "lldb/Utility/LLDBLog.h" -#include "lldb/lldb-public.h" - #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBEnvironment.h" @@ -18,6 +14,7 @@ #include "lldb/API/SBExpressionOptions.h" #include "lldb/API/SBFileSpec.h" #include "lldb/API/SBListener.h" +#include "lldb/API/SBLock.h" #include "lldb/API/SBModule.h" #include "lldb/API/SBModuleSpec.h" #include "lldb/API/SBProcess.h" @@ -58,11 +55,14 @@ #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/Args.h" #include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/Instrumentation.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/ProcessInfo.h" #include "lldb/Utility/RegularExpression.h" #include "lldb/ValueObject/ValueObjectConstResult.h" #include "lldb/ValueObject/ValueObjectList.h" #include "lldb/ValueObject/ValueObjectVariable.h" +#include "lldb/lldb-public.h" #include "Commands/CommandObjectBreakpoint.h" #include "lldb/Interpreter/CommandReturnObject.h" @@ -2439,3 +2439,11 @@ lldb::SBTrace SBTarget::CreateTrace(lldb::SBError &error) { } return SBTrace(); } + +lldb::SBLock SBTarget::AcquireAPILock() const { + LLDB_INSTRUMENT_VA(this); + + if (TargetSP target_sp = GetSP()) + return lldb::SBLock(target_sp); + return lldb::SBLock(); +} diff --git a/lldb/test/API/python_api/target/TestTargetAPI.py b/lldb/test/API/python_api/target/TestTargetAPI.py index 155a25b576b03..b529c6381fc61 100644 --- a/lldb/test/API/python_api/target/TestTargetAPI.py +++ b/lldb/test/API/python_api/target/TestTargetAPI.py @@ -537,3 +537,31 @@ def test_setting_selected_target_with_invalid_target(self): """Make sure we don't crash when trying to select invalid target.""" target = lldb.SBTarget() self.dbg.SetSelectedTarget(target) + + @no_debug_info_test + def test_acquire_sblock(self): + """Make sure we can acquire the API lock from Python.""" + target = self.dbg.GetDummyTarget() + + lock = target.AcquireAPILock() + self.assertTrue(lock.IsValid()) + # The API call below doesn't actually matter, it's just there to + # confirm we don't block on the API lock. + target.BreakpointCreateByName("foo", "bar") + lock.Unlock() + self.assertFalse(lock.IsValid()) + + @no_debug_info_test + def test_acquire_sblock_with_statement(self): + """Make sure we can acquire the API lock using a with-statement from Python.""" + target = self.dbg.GetDummyTarget() + + lock_copy = None + with target.AcquireAPILock() as lock: + self.assertTrue(lock.IsValid()) + # The API call below doesn't actually matter, it's just there to + # confirm we don't block on the API lock. + target.BreakpointCreateByName("foo", "bar") + lock_copy = lock + self.assertTrue(lock.IsValid()) + self.assertFalse(lock_copy.IsValid()) diff --git a/lldb/unittests/API/CMakeLists.txt b/lldb/unittests/API/CMakeLists.txt index fe2ff684a5d92..5a88d06b601a3 100644 --- a/lldb/unittests/API/CMakeLists.txt +++ b/lldb/unittests/API/CMakeLists.txt @@ -1,6 +1,7 @@ add_lldb_unittest(APITests SBCommandInterpreterTest.cpp SBLineEntryTest.cpp + SBLockTest.cpp LINK_LIBS liblldb diff --git a/lldb/unittests/API/SBLockTest.cpp b/lldb/unittests/API/SBLockTest.cpp new file mode 100644 index 0000000000000..57705c2cb722c --- /dev/null +++ b/lldb/unittests/API/SBLockTest.cpp @@ -0,0 +1,57 @@ +//===-- SBLockTest.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 +// +//===----------------------------------------------------------------------===/ + +// Use the umbrella header for -Wdocumentation. +#include "lldb/API/LLDB.h" + +#include "TestingSupport/SubsystemRAII.h" +#include "lldb/API/SBDebugger.h" +#include "lldb/API/SBTarget.h" +#include "gtest/gtest.h" +#include <atomic> +#include <chrono> +#include <future> + +using namespace lldb; +using namespace lldb_private; + +class SBLockTest : public testing::Test { +protected: + void SetUp() override { debugger = SBDebugger::Create(); } + void TearDown() override { SBDebugger::Destroy(debugger); } + + SubsystemRAII<lldb::SBDebugger> subsystems; + SBDebugger debugger; +}; + +TEST_F(SBLockTest, LockTest) { + lldb::SBTarget target = debugger.GetDummyTarget(); + + std::future<void> f; + { + std::atomic<bool> locked = false; + lldb::SBLock lock = target.AcquireAPILock(); + ASSERT_FALSE(locked.exchange(true)); + + f = std::async(std::launch::async, [&]() { + { + ASSERT_TRUE(locked); + target.BreakpointCreateByName("foo", "bar"); + ASSERT_FALSE(locked); + } + }); + ASSERT_TRUE(f.valid()); + + // Wait 500ms to confirm the thread is blocked. + auto status = f.wait_for(std::chrono::milliseconds(500)); + ASSERT_EQ(status, std::future_status::timeout); + + ASSERT_TRUE(locked.exchange(false)); + } + f.wait(); +} >From 97d512cc380833d26387d1a5be83411f7dc24909 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Thu, 20 Mar 2025 08:54:20 -0700 Subject: [PATCH 2/2] Make multiple with-statements work --- lldb/bindings/interface/SBLockExtensions.i | 2 ++ lldb/include/lldb/API/SBLock.h | 8 ++++++++ lldb/include/lldb/Target/Target.h | 1 + lldb/source/API/SBLock.cpp | 7 +++++++ lldb/test/API/python_api/target/TestTargetAPI.py | 15 +++++++++++++++ 5 files changed, 33 insertions(+) diff --git a/lldb/bindings/interface/SBLockExtensions.i b/lldb/bindings/interface/SBLockExtensions.i index 27736c3f287b7..4b2b05c02099d 100644 --- a/lldb/bindings/interface/SBLockExtensions.i +++ b/lldb/bindings/interface/SBLockExtensions.i @@ -2,6 +2,8 @@ #ifdef SWIGPYTHON %pythoncode %{ def __enter__(self): + if not self.IsValid(): + self.Lock() return self def __exit__(self, exc_type, exc_value, traceback): diff --git a/lldb/include/lldb/API/SBLock.h b/lldb/include/lldb/API/SBLock.h index 93503d051edaa..9e77d305e87d5 100644 --- a/lldb/include/lldb/API/SBLock.h +++ b/lldb/include/lldb/API/SBLock.h @@ -19,6 +19,9 @@ class APILock; namespace lldb { +/// A general-purpose lock in the SB API. The lock can be locked and unlocked. +/// The default constructed lock is unlocked, but generally the lock is locked +/// when it is returned from a class. class LLDB_API SBLock { public: SBLock(); @@ -26,8 +29,13 @@ class LLDB_API SBLock { SBLock &operator=(SBLock &&rhs); ~SBLock(); + /// Returns true if this lock has ownership of the underlying mutex. bool IsValid() const; + /// Blocking operation that takes ownership of this lock. + void Lock() const; + + /// Releases ownership of this lock. void Unlock() const; private: diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 3b99593292209..b36bf2f0e3653 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1698,6 +1698,7 @@ class APILock { APILock(std::shared_ptr<std::recursive_mutex> mutex_sp) : m_mutex(std::move(mutex_sp)), m_lock(*m_mutex) {} + void Lock() { m_lock.lock(); } void Unlock() { m_lock.unlock(); } operator bool() const { return static_cast<bool>(m_lock); } diff --git a/lldb/source/API/SBLock.cpp b/lldb/source/API/SBLock.cpp index 598fd26720763..7f918dc3fd214 100644 --- a/lldb/source/API/SBLock.cpp +++ b/lldb/source/API/SBLock.cpp @@ -44,6 +44,13 @@ bool SBLock::IsValid() const { return static_cast<bool>(m_opaque_up) && static_cast<bool>(*m_opaque_up); } +void SBLock::Lock() const { + LLDB_INSTRUMENT_VA(this); + + if (m_opaque_up) + m_opaque_up->Lock(); +} + void SBLock::Unlock() const { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/test/API/python_api/target/TestTargetAPI.py b/lldb/test/API/python_api/target/TestTargetAPI.py index b529c6381fc61..9a3f38c3b8b12 100644 --- a/lldb/test/API/python_api/target/TestTargetAPI.py +++ b/lldb/test/API/python_api/target/TestTargetAPI.py @@ -565,3 +565,18 @@ def test_acquire_sblock_with_statement(self): lock_copy = lock self.assertTrue(lock.IsValid()) self.assertFalse(lock_copy.IsValid()) + + lock = target.AcquireAPILock() + self.assertTrue(lock.IsValid()) + lock.Unlock() + self.assertFalse(lock.IsValid()) + + with lock: + self.assertTrue(lock.IsValid()) + + self.assertFalse(lock.IsValid()) + + with lock: + self.assertTrue(lock.IsValid()) + + self.assertFalse(lock.IsValid()) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits