[lldb] Expose the Target API lock through the SB API

Author: JDevlieghere

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;

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

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;

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

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

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

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

lldb/unittests/API/CMakeLists.txt

@@ -1,6 +1,7 @@
 add_lldb_unittest(APITests
   SBCommandInterpreterTest.cpp
   SBLineEntryTest.cpp
+  SBLockTest.cpp
 
   LINK_LIBS
     liblldb

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();
+}