https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/131404
>From cd65c4cf07b5b2f03fd1f2ebe8d586b4d1bbc062 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Fri, 14 Mar 2025 15:19:58 -0700 Subject: [PATCH 1/4] [lldb] Expose the Target API lock through the SB API --- lldb/include/lldb/API/SBDefines.h | 1 + lldb/include/lldb/API/SBLock.h | 45 ++++++++++++++++++++++ lldb/include/lldb/API/SBTarget.h | 6 ++- lldb/include/lldb/Target/Target.h | 16 +++++++- lldb/source/API/CMakeLists.txt | 1 + lldb/source/API/SBLock.cpp | 40 +++++++++++++++++++ lldb/source/API/SBTarget.cpp | 36 +++++++++-------- lldb/unittests/API/CMakeLists.txt | 1 + lldb/unittests/API/SBLockTest.cpp | 64 +++++++++++++++++++++++++++++++ 9 files changed, 193 insertions(+), 17 deletions(-) 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/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..8d6fdfb959dfb --- /dev/null +++ b/lldb/include/lldb/API/SBLock.h @@ -0,0 +1,45 @@ +//===-- 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 { +struct APILock; +} + +namespace lldb { + +#ifndef SWIG +class LLDB_API SBLock { +public: + ~SBLock(); + + bool IsValid() const; + +private: + friend class SBTarget; + + SBLock() = default; + SBLock(std::recursive_mutex &mutex); + SBLock(std::recursive_mutex &mutex, lldb::TargetSP target_sp); + SBLock(const SBLock &rhs) = delete; + const SBLock &operator=(const SBLock &rhs) = delete; + + std::unique_ptr<lldb_private::APILock> m_opaque_up; +}; +#endif + +} // namespace lldb + +#endif diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index bb912ab41d0fe..6120c289743e3 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -342,7 +342,7 @@ class LLDB_API SBTarget { uint32_t GetAddressByteSize(); const char *GetTriple(); - + const char *GetABIName(); const char *GetLabel() const; @@ -946,6 +946,10 @@ class LLDB_API SBTarget { /// An error if a Trace already exists or the trace couldn't be created. lldb::SBTrace CreateTrace(SBError &error); +#ifndef SWIG + lldb::SBLock GetAPILock() const; +#endif + 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..8321a963d4c5b 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1327,7 +1327,7 @@ class Target : public std::enable_shared_from_this<Target>, StopHook(const StopHook &rhs); virtual ~StopHook() = default; - enum class StopHookKind : uint32_t { CommandBased = 0, ScriptBased }; + enum class StopHookKind : uint32_t { CommandBased = 0, ScriptBased }; enum class StopHookResult : uint32_t { KeepStopped = 0, RequestContinue, @@ -1692,6 +1692,20 @@ class Target : public std::enable_shared_from_this<Target>, } }; +/// The private implementation backing SBLock. +struct APILock { + APILock(std::recursive_mutex &mutex) : lock(mutex) {} + std::lock_guard<std::recursive_mutex> lock; +}; + +/// The private implementation used by SBLock to hand out the target API mutex. +/// It has a TargetSP to ensure the lock cannot outlive the target. +struct TargetAPILock : public APILock { + TargetAPILock(std::recursive_mutex &mutex, lldb::TargetSP target_sp) + : APILock(mutex), target_sp(target_sp) {} + lldb::TargetSP target_sp; +}; + } // 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..448c8ccd46747 --- /dev/null +++ b/lldb/source/API/SBLock.cpp @@ -0,0 +1,40 @@ +//===-- 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; + +#ifndef SWIG + +SBLock::SBLock(std::recursive_mutex &mutex) + : m_opaque_up(std::make_unique<APILock>(mutex)) { + LLDB_INSTRUMENT_VA(this); +} + +SBLock::SBLock(std::recursive_mutex &mutex, lldb::TargetSP target_sp) + : m_opaque_up(std::make_unique<TargetAPILock>(mutex, target_sp)) { + LLDB_INSTRUMENT_VA(this); +} + +SBLock::~SBLock() { LLDB_INSTRUMENT_VA(this); } + +bool SBLock::IsValid() const { + LLDB_INSTRUMENT_VA(this); + + return static_cast<bool>(m_opaque_up); +} + +#endif diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index dd9caa724ea36..a2761de8bfc5e 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "lldb/API/SBTarget.h" +#include "lldb/API/SBLock.h" #include "lldb/Utility/Instrumentation.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/lldb-public.h" @@ -18,6 +19,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" @@ -544,9 +546,8 @@ lldb::SBProcess SBTarget::ConnectRemote(SBListener &listener, const char *url, if (target_sp) { std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex()); if (listener.IsValid()) - process_sp = - target_sp->CreateProcess(listener.m_opaque_sp, plugin_name, nullptr, - true); + process_sp = target_sp->CreateProcess(listener.m_opaque_sp, plugin_name, + nullptr, true); else process_sp = target_sp->CreateProcess( target_sp->GetDebugger().GetListener(), plugin_name, nullptr, true); @@ -1040,7 +1041,7 @@ SBTarget::BreakpointCreateForException(lldb::LanguageType language, std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex()); const bool hardware = false; sb_bp = target_sp->CreateExceptionBreakpoint(language, catch_bp, throw_bp, - hardware); + hardware); } return sb_bp; @@ -1060,14 +1061,9 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateFromScript( Status error; StructuredData::ObjectSP obj_sp = extra_args.m_impl_up->GetObjectSP(); - sb_bp = - target_sp->CreateScriptedBreakpoint(class_name, - module_list.get(), - file_list.get(), - false, /* internal */ - request_hardware, - obj_sp, - &error); + sb_bp = target_sp->CreateScriptedBreakpoint( + class_name, module_list.get(), file_list.get(), false, /* internal */ + request_hardware, obj_sp, &error); } return sb_bp; @@ -1692,8 +1688,8 @@ uint32_t SBTarget::GetMaximumNumberOfChildrenToDisplay() const { LLDB_INSTRUMENT_VA(this); TargetSP target_sp(GetSP()); - if(target_sp){ - return target_sp->GetMaximumNumberOfChildrenToDisplay(); + if (target_sp) { + return target_sp->GetMaximumNumberOfChildrenToDisplay(); } return 0; } @@ -2193,7 +2189,7 @@ SBError SBTarget::SetModuleLoadAddress(lldb::SBModule module, } SBError SBTarget::SetModuleLoadAddress(lldb::SBModule module, - uint64_t slide_offset) { + uint64_t slide_offset) { SBError sb_error; @@ -2439,3 +2435,13 @@ lldb::SBTrace SBTarget::CreateTrace(lldb::SBError &error) { } return SBTrace(); } + +#ifndef SWIG +lldb::SBLock SBTarget::GetAPILock() const { + LLDB_INSTRUMENT_VA(this); + + if (TargetSP target_sp = GetSP()) + return lldb::SBLock(target_sp->GetAPIMutex(), target_sp); + return lldb::SBLock(); +} +#endif 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..1fa9c184bf79b --- /dev/null +++ b/lldb/unittests/API/SBLockTest.cpp @@ -0,0 +1,64 @@ +//===-- 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 +// +//===----------------------------------------------------------------------===/ + +#include "lldb/API/SBLock.h" +#include "lldb/API/SBDebugger.h" +#include "lldb/API/SBTarget.h" +#include "gtest/gtest.h" +#include <atomic> +#include <chrono> +#include <thread> + +TEST(SBLockTest, LockTest) { + lldb::SBDebugger debugger = lldb::SBDebugger::Create(); + lldb::SBTarget target = debugger.GetDummyTarget(); + + std::mutex m; + std::condition_variable cv; + bool wakeup = false; + std::atomic<bool> locked = false; + + std::thread test_thread([&]() { + { + { + std::unique_lock lk(m); + cv.wait(lk, [&] { return wakeup; }); + } + + ASSERT_TRUE(locked); + target.BreakpointCreateByName("foo", "bar"); + ASSERT_FALSE(locked); + + cv.notify_one(); + } + }); + + // Take the API lock. + { + lldb::SBLock lock = target.GetAPILock(); + ASSERT_FALSE(locked.exchange(true)); + + // Wake up the test thread. + { + std::lock_guard lk(m); + wakeup = true; + } + cv.notify_one(); + + // Wait 500ms to confirm the thread is blocked. + { + std::unique_lock<std::mutex> lk(m); + auto result = cv.wait_for(lk, std::chrono::milliseconds(500)); + ASSERT_EQ(result, std::cv_status::timeout); + } + + ASSERT_TRUE(locked.exchange(false)); + } + + test_thread.join(); +} >From 42aa712e1b7b30bd4fecfa5803d198e4b596e49b Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Fri, 14 Mar 2025 16:20:35 -0700 Subject: [PATCH 2/4] Address code review feedback --- lldb/include/lldb/API/SBLock.h | 8 +++++--- lldb/include/lldb/API/SBTarget.h | 2 +- lldb/include/lldb/Target/Target.h | 2 ++ lldb/source/API/SBLock.cpp | 5 ----- lldb/source/API/SBTarget.cpp | 10 ++++------ lldb/unittests/API/SBLockTest.cpp | 2 +- 6 files changed, 13 insertions(+), 16 deletions(-) diff --git a/lldb/include/lldb/API/SBLock.h b/lldb/include/lldb/API/SBLock.h index 8d6fdfb959dfb..d13fec989990b 100644 --- a/lldb/include/lldb/API/SBLock.h +++ b/lldb/include/lldb/API/SBLock.h @@ -28,11 +28,13 @@ class LLDB_API SBLock { bool IsValid() const; private: - friend class SBTarget; - SBLock() = default; - SBLock(std::recursive_mutex &mutex); + + // Private constructor used by SBTarget to create the Target API lock. + // Requires a friend declaration. SBLock(std::recursive_mutex &mutex, lldb::TargetSP target_sp); + friend class SBTarget; + SBLock(const SBLock &rhs) = delete; const SBLock &operator=(const SBLock &rhs) = delete; diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 6120c289743e3..03d6b623fd425 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -947,7 +947,7 @@ class LLDB_API SBTarget { lldb::SBTrace CreateTrace(SBError &error); #ifndef SWIG - lldb::SBLock GetAPILock() const; + lldb::SBLock AcquireAPILock() const; #endif protected: diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 8321a963d4c5b..4379f73936d0c 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1695,6 +1695,7 @@ class Target : public std::enable_shared_from_this<Target>, /// The private implementation backing SBLock. struct APILock { APILock(std::recursive_mutex &mutex) : lock(mutex) {} + virtual ~APILock() = default; std::lock_guard<std::recursive_mutex> lock; }; @@ -1703,6 +1704,7 @@ struct APILock { struct TargetAPILock : public APILock { TargetAPILock(std::recursive_mutex &mutex, lldb::TargetSP target_sp) : APILock(mutex), target_sp(target_sp) {} + ~TargetAPILock() override = default; lldb::TargetSP target_sp; }; diff --git a/lldb/source/API/SBLock.cpp b/lldb/source/API/SBLock.cpp index 448c8ccd46747..4338049cb1689 100644 --- a/lldb/source/API/SBLock.cpp +++ b/lldb/source/API/SBLock.cpp @@ -19,11 +19,6 @@ using namespace lldb_private; #ifndef SWIG -SBLock::SBLock(std::recursive_mutex &mutex) - : m_opaque_up(std::make_unique<APILock>(mutex)) { - LLDB_INSTRUMENT_VA(this); -} - SBLock::SBLock(std::recursive_mutex &mutex, lldb::TargetSP target_sp) : m_opaque_up(std::make_unique<TargetAPILock>(mutex, target_sp)) { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index a2761de8bfc5e..96d5fd1cbf31a 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -7,11 +7,6 @@ //===----------------------------------------------------------------------===// #include "lldb/API/SBTarget.h" -#include "lldb/API/SBLock.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" @@ -60,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" @@ -2437,7 +2435,7 @@ lldb::SBTrace SBTarget::CreateTrace(lldb::SBError &error) { } #ifndef SWIG -lldb::SBLock SBTarget::GetAPILock() const { +lldb::SBLock SBTarget::AcquireAPILock() const { LLDB_INSTRUMENT_VA(this); if (TargetSP target_sp = GetSP()) diff --git a/lldb/unittests/API/SBLockTest.cpp b/lldb/unittests/API/SBLockTest.cpp index 1fa9c184bf79b..33bf94ead1dd3 100644 --- a/lldb/unittests/API/SBLockTest.cpp +++ b/lldb/unittests/API/SBLockTest.cpp @@ -40,7 +40,7 @@ TEST(SBLockTest, LockTest) { // Take the API lock. { - lldb::SBLock lock = target.GetAPILock(); + lldb::SBLock lock = target.AcquireAPILock(); ASSERT_FALSE(locked.exchange(true)); // Wake up the test thread. >From 10a175541370f920c3aa8fb1a745cfe5a093dad7 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Mon, 17 Mar 2025 11:29:41 -0700 Subject: [PATCH 3/4] Address Pavel's feedback --- lldb/include/lldb/API/LLDB.h | 1 + lldb/include/lldb/API/SBLock.h | 3 +- lldb/include/lldb/Target/Target.h | 4 +- lldb/source/API/SBLock.cpp | 9 +---- lldb/source/API/SBTarget.cpp | 2 +- lldb/unittests/API/SBLockTest.cpp | 63 +++++++++++++------------------ 6 files changed, 34 insertions(+), 48 deletions(-) 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/SBLock.h b/lldb/include/lldb/API/SBLock.h index d13fec989990b..b19071e63413f 100644 --- a/lldb/include/lldb/API/SBLock.h +++ b/lldb/include/lldb/API/SBLock.h @@ -11,7 +11,6 @@ #include "lldb/API/SBDefines.h" #include "lldb/lldb-forward.h" - #include <mutex> namespace lldb_private { @@ -32,7 +31,7 @@ class LLDB_API SBLock { // Private constructor used by SBTarget to create the Target API lock. // Requires a friend declaration. - SBLock(std::recursive_mutex &mutex, lldb::TargetSP target_sp); + SBLock(lldb::TargetSP target_sp); friend class SBTarget; SBLock(const SBLock &rhs) = delete; diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 4379f73936d0c..f873ddd03a96e 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1702,8 +1702,8 @@ struct APILock { /// The private implementation used by SBLock to hand out the target API mutex. /// It has a TargetSP to ensure the lock cannot outlive the target. struct TargetAPILock : public APILock { - TargetAPILock(std::recursive_mutex &mutex, lldb::TargetSP target_sp) - : APILock(mutex), target_sp(target_sp) {} + TargetAPILock(lldb::TargetSP target_sp) + : APILock(target_sp->GetAPIMutex()), target_sp(target_sp) {} ~TargetAPILock() override = default; lldb::TargetSP target_sp; }; diff --git a/lldb/source/API/SBLock.cpp b/lldb/source/API/SBLock.cpp index 4338049cb1689..5a3df015196d5 100644 --- a/lldb/source/API/SBLock.cpp +++ b/lldb/source/API/SBLock.cpp @@ -10,17 +10,14 @@ #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; -#ifndef SWIG - -SBLock::SBLock(std::recursive_mutex &mutex, lldb::TargetSP target_sp) - : m_opaque_up(std::make_unique<TargetAPILock>(mutex, target_sp)) { +SBLock::SBLock(lldb::TargetSP target_sp) + : m_opaque_up(std::make_unique<TargetAPILock>(target_sp)) { LLDB_INSTRUMENT_VA(this); } @@ -31,5 +28,3 @@ bool SBLock::IsValid() const { return static_cast<bool>(m_opaque_up); } - -#endif diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 96d5fd1cbf31a..2af3b206d5d6a 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -2439,7 +2439,7 @@ lldb::SBLock SBTarget::AcquireAPILock() const { LLDB_INSTRUMENT_VA(this); if (TargetSP target_sp = GetSP()) - return lldb::SBLock(target_sp->GetAPIMutex(), target_sp); + return lldb::SBLock(target_sp); return lldb::SBLock(); } #endif diff --git a/lldb/unittests/API/SBLockTest.cpp b/lldb/unittests/API/SBLockTest.cpp index 33bf94ead1dd3..265c5964f687b 100644 --- a/lldb/unittests/API/SBLockTest.cpp +++ b/lldb/unittests/API/SBLockTest.cpp @@ -6,59 +6,50 @@ // //===----------------------------------------------------------------------===/ -#include "lldb/API/SBLock.h" +// 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 <thread> - -TEST(SBLockTest, LockTest) { - lldb::SBDebugger debugger = lldb::SBDebugger::Create(); - lldb::SBTarget target = debugger.GetDummyTarget(); +#include <future> - std::mutex m; - std::condition_variable cv; - bool wakeup = false; - std::atomic<bool> locked = false; +using namespace lldb; +using namespace lldb_private; - std::thread test_thread([&]() { - { - { - std::unique_lock lk(m); - cv.wait(lk, [&] { return wakeup; }); - } +class SBLockTest : public testing::Test { + SubsystemRAII<lldb::SBDebugger> subsystems; +}; - ASSERT_TRUE(locked); - target.BreakpointCreateByName("foo", "bar"); - ASSERT_FALSE(locked); - - cv.notify_one(); - } - }); +TEST_F(SBLockTest, LockTest) { + lldb::SBDebugger debugger = lldb::SBDebugger::Create(); + lldb::SBTarget target = debugger.GetDummyTarget(); - // Take the API lock. + std::future<void> f; { + std::atomic<bool> locked = false; lldb::SBLock lock = target.AcquireAPILock(); ASSERT_FALSE(locked.exchange(true)); - // Wake up the test thread. - { - std::lock_guard lk(m); - wakeup = true; - } - cv.notify_one(); + 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. - { - std::unique_lock<std::mutex> lk(m); - auto result = cv.wait_for(lk, std::chrono::milliseconds(500)); - ASSERT_EQ(result, std::cv_status::timeout); - } + auto status = f.wait_for(std::chrono::milliseconds(500)); + ASSERT_EQ(status, std::future_status::timeout); ASSERT_TRUE(locked.exchange(false)); } + f.wait(); - test_thread.join(); + lldb::SBDebugger::Destroy(debugger); } >From 896e53ad90639a74b7d66060855a04c51ceac987 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Mon, 17 Mar 2025 15:20:02 -0700 Subject: [PATCH 4/4] Fix SWIG --- lldb/bindings/headers.swig | 3 ++- lldb/bindings/interfaces.swig | 3 ++- lldb/include/lldb/API/SBLock.h | 8 +++----- lldb/include/lldb/API/SBTarget.h | 2 -- lldb/source/API/SBLock.cpp | 9 ++++++++- lldb/source/API/SBTarget.cpp | 2 -- lldb/unittests/API/SBLockTest.cpp | 8 +++++--- 7 files changed, 20 insertions(+), 15 deletions(-) diff --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig index 5e7c54d1eb839..3e2d10966eb27 100644 --- a/lldb/bindings/headers.swig +++ b/lldb/bindings/headers.swig @@ -39,11 +39,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/bindings/interfaces.swig b/lldb/bindings/interfaces.swig index 08df9a1a8d539..7eff6531e0630 100644 --- a/lldb/bindings/interfaces.swig +++ b/lldb/bindings/interfaces.swig @@ -121,11 +121,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/SBLock.h b/lldb/include/lldb/API/SBLock.h index b19071e63413f..5c00ad4e10f60 100644 --- a/lldb/include/lldb/API/SBLock.h +++ b/lldb/include/lldb/API/SBLock.h @@ -19,16 +19,16 @@ struct APILock; namespace lldb { -#ifndef SWIG class LLDB_API SBLock { public: + SBLock(); + SBLock(SBLock &&rhs); + SBLock &operator=(SBLock &&rhs); ~SBLock(); bool IsValid() const; private: - SBLock() = default; - // Private constructor used by SBTarget to create the Target API lock. // Requires a friend declaration. SBLock(lldb::TargetSP target_sp); @@ -42,5 +42,3 @@ class LLDB_API SBLock { #endif } // namespace lldb - -#endif diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 03d6b623fd425..4c3850bad8df9 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -946,9 +946,7 @@ class LLDB_API SBTarget { /// An error if a Trace already exists or the trace couldn't be created. lldb::SBTrace CreateTrace(SBError &error); -#ifndef SWIG lldb::SBLock AcquireAPILock() const; -#endif protected: friend class SBAddress; diff --git a/lldb/source/API/SBLock.cpp b/lldb/source/API/SBLock.cpp index 5a3df015196d5..e51a2c76fddfd 100644 --- a/lldb/source/API/SBLock.cpp +++ b/lldb/source/API/SBLock.cpp @@ -11,11 +11,18 @@ #include "lldb/Utility/Instrumentation.h" #include "lldb/lldb-forward.h" #include <memory> -#include <mutex> using namespace lldb; using namespace lldb_private; +SBLock::SBLock() {} +SBLock::SBLock(SBLock &&rhs) : m_opaque_up(std::move(rhs.m_opaque_up)) {} + +SBLock &SBLock::operator=(SBLock &&rhs) { + m_opaque_up = std::move(rhs.m_opaque_up); + return *this; +} + SBLock::SBLock(lldb::TargetSP target_sp) : m_opaque_up(std::make_unique<TargetAPILock>(target_sp)) { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 2af3b206d5d6a..b41cc8911b362 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -2434,7 +2434,6 @@ lldb::SBTrace SBTarget::CreateTrace(lldb::SBError &error) { return SBTrace(); } -#ifndef SWIG lldb::SBLock SBTarget::AcquireAPILock() const { LLDB_INSTRUMENT_VA(this); @@ -2442,4 +2441,3 @@ lldb::SBLock SBTarget::AcquireAPILock() const { return lldb::SBLock(target_sp); return lldb::SBLock(); } -#endif diff --git a/lldb/unittests/API/SBLockTest.cpp b/lldb/unittests/API/SBLockTest.cpp index 265c5964f687b..57705c2cb722c 100644 --- a/lldb/unittests/API/SBLockTest.cpp +++ b/lldb/unittests/API/SBLockTest.cpp @@ -21,11 +21,15 @@ 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::SBDebugger debugger = lldb::SBDebugger::Create(); lldb::SBTarget target = debugger.GetDummyTarget(); std::future<void> f; @@ -50,6 +54,4 @@ TEST_F(SBLockTest, LockTest) { ASSERT_TRUE(locked.exchange(false)); } f.wait(); - - lldb::SBDebugger::Destroy(debugger); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits