[Lldb-commits] [lldb] [lldb] Fix missing overloads in ThreadMemory (PR #132734)
Felipe de Azevedo Piovezan via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 24 06:26:47 PDT 2025
https://github.com/felipepiovezan created https://github.com/llvm/llvm-project/pull/132734
Please read the two commit messages individually.
>From 00da78461321b019303ddffcec5620859829cd40 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Thu, 20 Mar 2025 14:59:33 -0300
Subject: [PATCH 1/2] [lldb][NFC] Break ThreadMemory into smaller abstractions
ThreadMemory attempts to be a class abstracting the notion of a "fake"
Thread that is backed by a "real" thread. According to its
documentation, it is meant to be a class forwarding most methods to the
backing thread, but it does so only for a handful of methods.
Along the way, it also tries to represent a Thread that may or may not
have a different name, and may or may not have a different queue from
the backing thread. This can be problematic for a couple of reasons:
1. It forces all users into this optional behavior.
2. The forwarding behavior is incomplete: not all methods are currently
being forwarded properly. Some of them involve queues and seem to
have been intentionally left unimplemented.
This commit creates the following separation:
ThreadMemory <- NamedThreadMemory <- NamedThreadMemoryWithQueue
ThreadMemory captures the notion of a backed thread that _really_
forwards all methods to the backing thread. (Missing methods should be
implemented in a later commit, and allowing them to be implemented
without changing behavior of other derived classes is the main purpose
of this refactor).
NamedThreadMemory is a ThreadMemory that allows users to override the
thread name. If a name is present, it is used; otherwise the forwarding
behavior is used.
NamedThreadMemoryWithQueue is a NamedThreadMemory that allows users to
override queue information. If queue information is present, it is used;
otherwise, the forwarding behavior is used.
With this separation, we can more explicitly implement missing methods
of the base class and override them, if needed, in
ThreadMemoryWithQueue. But this commit really is NFC, no new methods are
implemented and no method implementation is changed.
---
.../Python/OperatingSystemPython.cpp | 4 +-
.../Plugins/Process/Utility/ThreadMemory.cpp | 23 +++---
.../Plugins/Process/Utility/ThreadMemory.h | 72 ++++++++++++++-----
3 files changed, 66 insertions(+), 33 deletions(-)
diff --git a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
index aff521890858c..ef2291f5c1b3d 100644
--- a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
+++ b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
@@ -259,8 +259,8 @@ ThreadSP OperatingSystemPython::CreateThreadFromThreadInfo(
if (!thread_sp) {
if (did_create_ptr)
*did_create_ptr = true;
- thread_sp = std::make_shared<ThreadMemory>(*m_process, tid, name, queue,
- reg_data_addr);
+ thread_sp = std::make_shared<NamedThreadMemoryWithQueue>(
+ *m_process, tid, name, queue, reg_data_addr);
}
if (core_number < core_thread_list.GetSize(false)) {
diff --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
index 550b53688fd39..2824a943f757b 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
@@ -20,18 +20,17 @@
using namespace lldb;
using namespace lldb_private;
-ThreadMemory::ThreadMemory(Process &process, lldb::tid_t tid,
- const ValueObjectSP &thread_info_valobj_sp)
- : Thread(process, tid), m_backing_thread_sp(),
- m_thread_info_valobj_sp(thread_info_valobj_sp), m_name(), m_queue(),
- m_register_data_addr(LLDB_INVALID_ADDRESS) {}
-
-ThreadMemory::ThreadMemory(Process &process, lldb::tid_t tid,
- llvm::StringRef name, llvm::StringRef queue,
- lldb::addr_t register_data_addr)
- : Thread(process, tid), m_backing_thread_sp(), m_thread_info_valobj_sp(),
- m_name(std::string(name)), m_queue(std::string(queue)),
- m_register_data_addr(register_data_addr) {}
+NamedThreadMemoryWithQueue::NamedThreadMemoryWithQueue(
+ Process &process, lldb::tid_t tid,
+ const ValueObjectSP &thread_info_valobj_sp)
+ : NamedThreadMemory(process, tid, LLDB_INVALID_ADDRESS, ""),
+ m_thread_info_valobj_sp(thread_info_valobj_sp), m_queue() {}
+
+NamedThreadMemoryWithQueue::NamedThreadMemoryWithQueue(
+ Process &process, lldb::tid_t tid, llvm::StringRef name,
+ llvm::StringRef queue, lldb::addr_t register_data_addr)
+ : NamedThreadMemory(process, tid, register_data_addr, name),
+ m_thread_info_valobj_sp(), m_queue(std::string(queue)) {}
ThreadMemory::~ThreadMemory() { DestroyThread(); }
diff --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.h b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
index cebb31538eaf2..d7a73e63964fc 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.h
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
@@ -13,14 +13,14 @@
#include "lldb/Target/Thread.h"
+/// A memory thread optionally backed by a real thread.
+/// All methods of this class dispatch to the real thread, if it is not null.
+/// Otherwise the methods are no-op.
class ThreadMemory : public lldb_private::Thread {
public:
ThreadMemory(lldb_private::Process &process, lldb::tid_t tid,
- const lldb::ValueObjectSP &thread_info_valobj_sp);
-
- ThreadMemory(lldb_private::Process &process, lldb::tid_t tid,
- llvm::StringRef name, llvm::StringRef queue,
- lldb::addr_t register_data_addr);
+ lldb::addr_t register_data_addr)
+ : Thread(process, tid), m_register_data_addr(register_data_addr) {}
~ThreadMemory() override;
@@ -38,16 +38,12 @@ class ThreadMemory : public lldb_private::Thread {
}
const char *GetName() override {
- if (!m_name.empty())
- return m_name.c_str();
if (m_backing_thread_sp)
return m_backing_thread_sp->GetName();
return nullptr;
}
const char *GetQueueName() override {
- if (!m_queue.empty())
- return m_queue.c_str();
if (m_backing_thread_sp)
return m_backing_thread_sp->GetQueueName();
return nullptr;
@@ -68,8 +64,6 @@ class ThreadMemory : public lldb_private::Thread {
void RefreshStateAfterStop() override;
- lldb::ValueObjectSP &GetValueObject() { return m_thread_info_valobj_sp; }
-
void ClearStackFrames() override;
void ClearBackingThread() override {
@@ -79,34 +73,74 @@ class ThreadMemory : public lldb_private::Thread {
}
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;
+ return thread_sp.get();
}
lldb::ThreadSP GetBackingThread() const override {
return m_backing_thread_sp;
}
-protected:
bool IsOperatingSystemPluginThread() const override { return true; }
+private:
+ lldb::addr_t m_register_data_addr;
// If this memory thread is actually represented by a thread from the
// lldb_private::Process subclass, then fill in the thread here and
// all APIs will be routed through this thread object. If m_backing_thread_sp
// is empty, then this thread is simply in memory with no representation
// through the process plug-in.
lldb::ThreadSP m_backing_thread_sp;
- lldb::ValueObjectSP m_thread_info_valobj_sp;
+
+ ThreadMemory(const ThreadMemory &) = delete;
+ const ThreadMemory &operator=(const ThreadMemory &) = delete;
+};
+
+class NamedThreadMemory : public ThreadMemory {
+public:
+ NamedThreadMemory(lldb_private::Process &process, lldb::tid_t tid,
+ lldb::addr_t register_data_addr, llvm::StringRef name)
+ : ThreadMemory(process, tid, register_data_addr), m_name(name) {}
+
+ const char *GetName() override {
+ if (!m_name.empty())
+ return m_name.c_str();
+ return ThreadMemory::GetName();
+ }
+
+private:
std::string m_name;
+};
+
+/// A NamedThreadMemory that has optional queue information.
+class NamedThreadMemoryWithQueue : public NamedThreadMemory {
+public:
+ NamedThreadMemoryWithQueue(lldb_private::Process &process, lldb::tid_t tid,
+ const lldb::ValueObjectSP &thread_info_valobj_sp);
+
+ NamedThreadMemoryWithQueue(lldb_private::Process &process, lldb::tid_t tid,
+ llvm::StringRef name, llvm::StringRef queue,
+ lldb::addr_t register_data_addr);
+
+ ~NamedThreadMemoryWithQueue() override = default;
+
+ const char *GetQueueName() override {
+ if (!m_queue.empty())
+ return m_queue.c_str();
+ return ThreadMemory::GetQueueName();
+ }
+
+ lldb::ValueObjectSP &GetValueObject() { return m_thread_info_valobj_sp; }
+
+protected:
+ lldb::ValueObjectSP m_thread_info_valobj_sp;
std::string m_queue;
- lldb::addr_t m_register_data_addr;
private:
- ThreadMemory(const ThreadMemory &) = delete;
- const ThreadMemory &operator=(const ThreadMemory &) = delete;
+ NamedThreadMemoryWithQueue(const NamedThreadMemoryWithQueue &) = delete;
+ const NamedThreadMemoryWithQueue &
+ operator=(const NamedThreadMemoryWithQueue &) = delete;
};
#endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_THREADMEMORY_H
>From 36c9b8d37ccdaf8e1934cf56b5f9c2f6bf2232ec Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Fri, 21 Mar 2025 14:59:05 -0300
Subject: [PATCH 2/2] [lldb] Implement missing queue overloads from
ThreadMemory
This commit makes ThreadMemory a real "forwarder" class by implementing
the missing queue methods: they will just call the corresponding backing
thread method.
To make this patch NFC(*) and not change the behavior of the Python OS
plugin, NamedThreadMemoryWithQueue also overrides these methods to
simply call the `Thread` method, just as it was doing before. This also
makes it obvious that there are missing pieces of this class if it were
to provide full queue support.
(*) This patch is NFC in the sense that all llvm.org plugins will not have
any behavior change, but downstream consumers of ThreadMemory will
benefit from the newly implemented forwarding methods.
---
.../Plugins/Process/Utility/ThreadMemory.h | 112 ++++++++++++++++++
1 file changed, 112 insertions(+)
diff --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.h b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
index d7a73e63964fc..2d196c33d475d 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.h
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
@@ -51,6 +51,69 @@ class ThreadMemory : public lldb_private::Thread {
void WillResume(lldb::StateType resume_state) override;
+ void SetQueueName(const char *name) override {
+ if (m_backing_thread_sp)
+ m_backing_thread_sp->SetQueueName(name);
+ }
+
+ lldb::queue_id_t GetQueueID() override {
+ if (m_backing_thread_sp)
+ return m_backing_thread_sp->GetQueueID();
+ return LLDB_INVALID_QUEUE_ID;
+ }
+
+ void SetQueueID(lldb::queue_id_t new_val) override {
+ if (m_backing_thread_sp)
+ m_backing_thread_sp->GetQueueID();
+ }
+
+ lldb::QueueKind GetQueueKind() override {
+ if (m_backing_thread_sp)
+ return m_backing_thread_sp->GetQueueKind();
+ return lldb::eQueueKindUnknown;
+ }
+
+ void SetQueueKind(lldb::QueueKind kind) override {
+ if (m_backing_thread_sp)
+ m_backing_thread_sp->SetQueueKind(kind);
+ }
+
+ lldb::QueueSP GetQueue() override {
+ if (m_backing_thread_sp)
+ return m_backing_thread_sp->GetQueue();
+ return lldb::QueueSP();
+ }
+
+ lldb::addr_t GetQueueLibdispatchQueueAddress() override {
+ if (m_backing_thread_sp)
+ return m_backing_thread_sp->GetQueueLibdispatchQueueAddress();
+ return LLDB_INVALID_ADDRESS;
+ }
+
+ void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t) override {
+ if (m_backing_thread_sp)
+ m_backing_thread_sp->SetQueueLibdispatchQueueAddress(dispatch_queue_t);
+ }
+
+ lldb_private::LazyBool GetAssociatedWithLibdispatchQueue() override {
+ if (m_backing_thread_sp)
+ return m_backing_thread_sp->GetAssociatedWithLibdispatchQueue();
+ return lldb_private::eLazyBoolNo;
+ }
+
+ void SetAssociatedWithLibdispatchQueue(
+ lldb_private::LazyBool associated_with_libdispatch_queue) override {
+ if (m_backing_thread_sp)
+ m_backing_thread_sp->SetAssociatedWithLibdispatchQueue(
+ associated_with_libdispatch_queue);
+ }
+
+ bool ThreadHasQueueInformation() const override {
+ if (m_backing_thread_sp)
+ return m_backing_thread_sp->ThreadHasQueueInformation();
+ return false;
+ }
+
void DidResume() override {
if (m_backing_thread_sp)
m_backing_thread_sp->DidResume();
@@ -131,6 +194,55 @@ class NamedThreadMemoryWithQueue : public NamedThreadMemory {
return ThreadMemory::GetQueueName();
}
+ /// This method has not yet been specialized.
+ void SetQueueName(const char *name) override { Thread::SetQueueName(name); }
+
+ /// This method has not yet been specialized.
+ lldb::queue_id_t GetQueueID() override { return Thread::GetQueueID(); }
+
+ /// This method has not yet been specialized.
+ void SetQueueID(lldb::queue_id_t new_val) override {
+ Thread::SetQueueID(new_val);
+ }
+
+ /// This method has not yet been specialized.
+ lldb::QueueKind GetQueueKind() override { return Thread::GetQueueKind(); }
+
+ /// This method has not yet been specialized.
+ void SetQueueKind(lldb::QueueKind kind) override {
+ Thread::SetQueueKind(kind);
+ }
+
+ /// This method has not yet been specialized.
+ lldb::QueueSP GetQueue() override { return Thread::GetQueue(); }
+
+ /// This method has not yet been specialized.
+ lldb::addr_t GetQueueLibdispatchQueueAddress() override {
+ return Thread::GetQueueLibdispatchQueueAddress();
+ }
+
+ /// This method has not yet been specialized.
+ void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t) override {
+ Thread::SetQueueLibdispatchQueueAddress(dispatch_queue_t);
+ }
+
+ /// This method has not yet been specialized.
+ bool ThreadHasQueueInformation() const override {
+ return Thread::ThreadHasQueueInformation();
+ }
+
+ /// This method has not yet been specialized.
+ lldb_private::LazyBool GetAssociatedWithLibdispatchQueue() override {
+ return Thread::GetAssociatedWithLibdispatchQueue();
+ }
+
+ /// This method has not yet been specialized.
+ void SetAssociatedWithLibdispatchQueue(
+ lldb_private::LazyBool associated_with_libdispatch_queue) override {
+ Thread::SetAssociatedWithLibdispatchQueue(
+ associated_with_libdispatch_queue);
+ }
+
lldb::ValueObjectSP &GetValueObject() { return m_thread_info_valobj_sp; }
protected:
More information about the lldb-commits
mailing list