[Lldb-commits] [lldb] [lldb] Expose the Target API lock through the SB API (PR #131404)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Mar 14 15:37:10 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
<details>
<summary>Changes</summary>
Expose the target API lock through the SB API. This is motivated by `lldb-dap`, which is built on top of the SB API and needs a way to execute a series of SB API calls in an atomic manner (see #<!-- -->131242). We can solve this problem by either introducing an additional layer of locking at the DAP level or by exposing the existing locking at the SB API level. This patch implements the second approach.
---
Full diff: https://github.com/llvm/llvm-project/pull/131404.diff
9 Files Affected:
- (modified) lldb/include/lldb/API/SBDefines.h (+1)
- (added) lldb/include/lldb/API/SBLock.h (+45)
- (modified) lldb/include/lldb/API/SBTarget.h (+5-1)
- (modified) lldb/include/lldb/Target/Target.h (+15-1)
- (modified) lldb/source/API/CMakeLists.txt (+1)
- (added) lldb/source/API/SBLock.cpp (+40)
- (modified) lldb/source/API/SBTarget.cpp (+21-15)
- (modified) lldb/unittests/API/CMakeLists.txt (+1)
- (added) lldb/unittests/API/SBLockTest.cpp (+64)
``````````diff
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();
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/131404
More information about the lldb-commits
mailing list