https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/125300
>From 0c9d9ed5b1aa78f397e95c894def54ee627bea62 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan <fpiove...@apple.com> Date: Fri, 31 Jan 2025 12:07:45 -0800 Subject: [PATCH 1/4] [lldb] Implement bidirectional access for backing<->backed thread relationship This enables finding the backed thread from the backing thread without going through the thread list, and it will be useful for subsequent commits. --- lldb/include/lldb/Target/Thread.h | 15 +++++++++++++++ lldb/include/lldb/Target/ThreadList.h | 2 -- .../source/Plugins/Process/Utility/ThreadMemory.h | 7 ++++++- .../Process/gdb-remote/ProcessGDBRemote.cpp | 2 +- lldb/source/Target/ThreadList.cpp | 14 -------------- 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index ef66fa11574db9..d5fe33586fa3cf 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -470,6 +470,18 @@ class Thread : public std::enable_shared_from_this<Thread>, virtual void ClearStackFrames(); + /// Derived classes implementing SetBackingThread should use this to provide + /// bidirectional access to the Backing-Backed relationship. + void SetBackedThread(Thread &backed_thread) { + assert(backed_thread.GetBackingThread().get() == this); + m_backed_thread = backed_thread.shared_from_this(); + } + + void ClearBackedThread() { m_backed_thread.reset(); } + + /// Returns the thread that is backed by this thread, if any. + lldb::ThreadSP GetBackedThread() const { return m_backed_thread; } + virtual bool SetBackingThread(const lldb::ThreadSP &thread_sp) { return false; } @@ -1349,6 +1361,9 @@ class Thread : public std::enable_shared_from_this<Thread>, LazyBool m_override_should_notify; mutable std::unique_ptr<ThreadPlanStack> m_null_plan_stack_up; + /// The Thread backed by this thread, if any. + lldb::ThreadSP m_backed_thread; + private: bool m_extended_info_fetched; // Have we tried to retrieve the m_extended_info // for this thread? diff --git a/lldb/include/lldb/Target/ThreadList.h b/lldb/include/lldb/Target/ThreadList.h index f931bb83a8ceaf..bca01f5fe2083e 100644 --- a/lldb/include/lldb/Target/ThreadList.h +++ b/lldb/include/lldb/Target/ThreadList.h @@ -101,8 +101,6 @@ class ThreadList : public ThreadCollection { lldb::ThreadSP GetThreadSPForThreadPtr(Thread *thread_ptr); - lldb::ThreadSP GetBackingThread(const lldb::ThreadSP &real_thread); - bool ShouldStop(Event *event_ptr); Vote ShouldReportStop(Event *event_ptr); diff --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.h b/lldb/source/Plugins/Process/Utility/ThreadMemory.h index d124f5780ea9bd..1e309671e85c65 100644 --- a/lldb/source/Plugins/Process/Utility/ThreadMemory.h +++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.h @@ -72,12 +72,17 @@ class ThreadMemory : public lldb_private::Thread { void ClearStackFrames() override; - void ClearBackingThread() override { m_backing_thread_sp.reset(); } + void ClearBackingThread() override { + if (m_backing_thread_sp) + m_backing_thread_sp->ClearBackedThread(); + m_backing_thread_sp.reset(); + } bool SetBackingThread(const lldb::ThreadSP &thread_sp) override { // printf ("Thread 0x%llx is being backed by thread 0x%llx\n", GetID(), // thread_sp->GetID()); m_backing_thread_sp = thread_sp; + thread_sp->SetBackedThread(*this); return (bool)thread_sp; } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 21a0fa283644d6..3f34ea2930a66a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1728,7 +1728,7 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo( thread_sp->SetStopInfo(StopInfoSP()); // If there's a memory thread backed by this thread, we need to use it to // calculate StopInfo. - if (ThreadSP memory_thread_sp = m_thread_list.GetBackingThread(thread_sp)) + if (ThreadSP memory_thread_sp = thread_sp->GetBackedThread()) thread_sp = memory_thread_sp; if (exc_type != 0) { diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index 6cbef330bf4888..c0440d82fd1ffc 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -191,20 +191,6 @@ ThreadSP ThreadList::GetThreadSPForThreadPtr(Thread *thread_ptr) { return thread_sp; } -ThreadSP ThreadList::GetBackingThread(const ThreadSP &real_thread) { - std::lock_guard<std::recursive_mutex> guard(GetMutex()); - - ThreadSP thread_sp; - const uint32_t num_threads = m_threads.size(); - for (uint32_t idx = 0; idx < num_threads; ++idx) { - if (m_threads[idx]->GetBackingThread() == real_thread) { - thread_sp = m_threads[idx]; - break; - } - } - return thread_sp; -} - ThreadSP ThreadList::FindThreadByIndexID(uint32_t index_id, bool can_update) { std::lock_guard<std::recursive_mutex> guard(GetMutex()); >From b19eb1c1e438f12785775e5e063c926a8c863843 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan <fpiove...@apple.com> Date: Fri, 31 Jan 2025 16:54:10 -0800 Subject: [PATCH 2/4] fixup! Address review comments --- lldb/include/lldb/Target/Thread.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index d5fe33586fa3cf..35a71b3761ebe1 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -470,11 +470,19 @@ class Thread : public std::enable_shared_from_this<Thread>, virtual void ClearStackFrames(); - /// Derived classes implementing SetBackingThread should use this to provide - /// bidirectional access to the Backing-Backed relationship. + /// Sets the thread that is backed by this thread. + /// If backed_thread.GetBackedThread() is null, this method also calls + /// backed_thread.SetBackedThread(this). + /// If backed_thread.GetBackedThread() is non-null, asserts that it is equal + /// to `this`. void SetBackedThread(Thread &backed_thread) { - assert(backed_thread.GetBackingThread().get() == this); m_backed_thread = backed_thread.shared_from_this(); + + // Ensure the bidrectional relationship is preserved. + Thread *backing_thread = backed_thread.GetBackingThread().get(); + assert(backing_thread == nullptr || backing_thread == this); + if (backing_thread == nullptr) + backed_thread.SetBackingThread(shared_from_this()); } void ClearBackedThread() { m_backed_thread.reset(); } >From c54af403582377bf4749683323d0228059a86622 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan <fpiove...@apple.com> Date: Fri, 31 Jan 2025 17:06:41 -0800 Subject: [PATCH 3/4] fixup! Fix documentation typo --- lldb/include/lldb/Target/Thread.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index 35a71b3761ebe1..3af8a516918a39 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -472,7 +472,7 @@ class Thread : public std::enable_shared_from_this<Thread>, /// Sets the thread that is backed by this thread. /// If backed_thread.GetBackedThread() is null, this method also calls - /// backed_thread.SetBackedThread(this). + /// backed_thread.SetBackingThread(this). /// If backed_thread.GetBackedThread() is non-null, asserts that it is equal /// to `this`. void SetBackedThread(Thread &backed_thread) { >From 1882d04ac380b9ac36527679c9c550269d13b4f8 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan <fpiove...@apple.com> Date: Mon, 3 Feb 2025 08:03:14 -0800 Subject: [PATCH 4/4] fixup! Break circular shared_ptr references --- lldb/include/lldb/Target/Thread.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index 3af8a516918a39..9749fd8d575a1c 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -488,7 +488,7 @@ class Thread : public std::enable_shared_from_this<Thread>, void ClearBackedThread() { m_backed_thread.reset(); } /// Returns the thread that is backed by this thread, if any. - lldb::ThreadSP GetBackedThread() const { return m_backed_thread; } + lldb::ThreadSP GetBackedThread() const { return m_backed_thread.lock(); } virtual bool SetBackingThread(const lldb::ThreadSP &thread_sp) { return false; @@ -1370,7 +1370,7 @@ class Thread : public std::enable_shared_from_this<Thread>, mutable std::unique_ptr<ThreadPlanStack> m_null_plan_stack_up; /// The Thread backed by this thread, if any. - lldb::ThreadSP m_backed_thread; + lldb::ThreadWP m_backed_thread; private: bool m_extended_info_fetched; // Have we tried to retrieve the m_extended_info _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits