[lldb] Expose the Target API lock through the SB API by JDevlieghere · Pull Request #131404 · llvm/llvm-project (original) (raw)

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

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 --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 + +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, 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, } };

+/// The private implementation backing SBLock. +struct APILock {

• APILock(std::recursive_mutex &mutex) : lock(mutex) {}

• std::lock_guardstd::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 +#include + +using namespace lldb; +using namespace lldb_private; + +#ifndef SWIG + +SBLock::SBLock(std::recursive_mutex &mutex)

• : m_opaque_up(std::make_unique(mutex)) {
• LLDB_INSTRUMENT_VA(this); +}
• +SBLock::SBLock(std::recursive_mutex &mutex, lldb::TargetSP target_sp)
• : m_opaque_up(std::make_unique(mutex, target_sp)) {
• LLDB_INSTRUMENT_VA(this); +}
• +SBLock::~SBLock() { LLDB_INSTRUMENT_VA(this); }
• +bool SBLock::IsValid() const {
• LLDB_INSTRUMENT_VA(this);
• return static_cast(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_guardstd::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_guardstd::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 +#include +#include + +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 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(); +}