[lldb][debugserver][NFC] Simplify macOS thread name fetching.

[ldm0]

Remove unnecessary proc_pidinfo calling.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Donough Liu (ldm0)

Changes

Remove unnecessary proc_pidinfo calling.

---

Full diff: https://github.com/llvm/llvm-project/pull/111684.diff

2 Files Affected:

• (modified) lldb/tools/debugserver/source/MacOSX/MachThread.cpp (+25-22)
• (modified) lldb/tools/debugserver/source/MacOSX/MachThread.h (+4-2)

diff --git a/lldb/tools/debugserver/source/MacOSX/MachThread.cpp b/lldb/tools/debugserver/source/MacOSX/MachThread.cpp
index d34914be802041..1568157286a52d 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachThread.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/MachThread.cpp
@@ -31,9 +31,8 @@ MachThread::MachThread(MachProcess *process, bool is_64_bit,
       m_state(eStateUnloaded), m_state_mutex(PTHREAD_MUTEX_RECURSIVE),
       m_suspend_count(0), m_stop_exception(),
       m_arch_up(DNBArchProtocol::Create(this)), m_reg_sets(NULL),
-      m_num_reg_sets(0), m_ident_info(), m_proc_threadinfo(),
-      m_dispatch_queue_name(), m_is_64_bit(is_64_bit),
-      m_pthread_qos_class_decode(nullptr) {
+      m_num_reg_sets(0), m_extended_info(), m_dispatch_queue_name(),
+      m_is_64_bit(is_64_bit), m_pthread_qos_class_decode(nullptr) {
   nub_size_t num_reg_sets = 0;
   m_reg_sets = m_arch_up->GetRegisterSetInfo(&num_reg_sets);
   m_num_reg_sets = num_reg_sets;
@@ -255,7 +254,7 @@ struct thread_basic_info *MachThread::GetBasicInfo() {
 bool MachThread::GetBasicInfo(thread_t thread,
                               struct thread_basic_info *basicInfoPtr) {
   if (MachPortNumberIsValid(thread)) {
-    unsigned int info_count = THREAD_BASIC_INFO_COUNT;
+    mach_msg_type_number_t info_count = THREAD_BASIC_INFO_COUNT;
     kern_return_t err = ::thread_info(thread, THREAD_BASIC_INFO,
                                       (thread_info_t)basicInfoPtr, &info_count);
     if (err == KERN_SUCCESS)
@@ -265,6 +264,25 @@ bool MachThread::GetBasicInfo(thread_t thread,
   return false;
 }
 
+struct thread_extended_info *MachThread::GetExtendedInfo() {
+  if (MachThread::GetExtendedInfo(m_mach_port_number, &m_extended_info))
+    return &m_extended_info;
+  return NULL;
+}
+
+bool MachThread::GetExtendedInfo(thread_t thread,
+                                 struct thread_extended_info *basicInfoPtr) {
+  if (MachPortNumberIsValid(thread)) {
+    mach_msg_type_number_t info_count = THREAD_EXTENDED_INFO_COUNT;
+    kern_return_t err = ::thread_info(thread, THREAD_EXTENDED_INFO,
+                                      (thread_info_t)basicInfoPtr, &info_count);
+    if (err == KERN_SUCCESS)
+      return true;
+  }
+  ::memset(basicInfoPtr, 0, sizeof(struct thread_extended_info));
+  return false;
+}
+
 bool MachThread::ThreadIDIsValid(uint64_t thread) { return thread != 0; }
 
 bool MachThread::MachPortNumberIsValid(thread_t thread) {
@@ -579,28 +597,13 @@ uint32_t MachThread::NumSupportedHardwareWatchpoints() const {
   return m_arch_up->NumSupportedHardwareWatchpoints();
 }
 
-bool MachThread::GetIdentifierInfo() {
+const char *MachThread::GetName() {
   // Don't try to get the thread info once and cache it for the life of the
   // thread.  It changes over time, for instance
   // if the thread name changes, then the thread_handle also changes...  So you
   // have to refetch it every time.
-  mach_msg_type_number_t count = THREAD_IDENTIFIER_INFO_COUNT;
-  kern_return_t kret = ::thread_info(m_mach_port_number, THREAD_IDENTIFIER_INFO,
-                                     (thread_info_t)&m_ident_info, &count);
-  return kret == KERN_SUCCESS;
-
-  return false;
-}
-
-const char *MachThread::GetName() {
-  if (GetIdentifierInfo()) {
-    int len = ::proc_pidinfo(m_process->ProcessID(), PROC_PIDTHREADINFO,
-                             m_ident_info.thread_handle, &m_proc_threadinfo,
-                             sizeof(m_proc_threadinfo));
-
-    if (len && m_proc_threadinfo.pth_name[0])
-      return m_proc_threadinfo.pth_name;
-  }
+  if (GetExtendedInfo() && m_extended_info.pth_name[0])
+    return m_extended_info.pth_name;
   return NULL;
 }
 
diff --git a/lldb/tools/debugserver/source/MacOSX/MachThread.h b/lldb/tools/debugserver/source/MacOSX/MachThread.h
index 5466c6f9f95095..cf9b4cbb61a681 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachThread.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachThread.h
@@ -108,6 +108,7 @@ class MachThread {
 
   bool IsUserReady();
   struct thread_basic_info *GetBasicInfo();
+  struct thread_extended_info *GetExtendedInfo();
   const char *GetBasicInfoAsString() const;
   const char *GetName();
 
@@ -126,6 +127,8 @@ class MachThread {
 protected:
   static bool GetBasicInfo(thread_t threadID,
                            struct thread_basic_info *basic_info);
+  static bool GetExtendedInfo(thread_t threadID,
+                              struct thread_extended_info *extended_info);
 
   bool GetIdentifierInfo();
 
@@ -152,8 +155,7 @@ class MachThread {
   const DNBRegisterSetInfo
       *m_reg_sets; // Register set information for this thread
   nub_size_t m_num_reg_sets;
-  thread_identifier_info_data_t m_ident_info;
-  struct proc_threadinfo m_proc_threadinfo;
+  thread_extended_info_data_t m_extended_info;
   std::string m_dispatch_queue_name;
   bool m_is_64_bit;
 

[JDevlieghere]

LGTM if this makes sense to @jasonmolenda

[jimingham]

I asked some folks who know and the proc_pidinfo call and the extended_thread_info read the same data, so this should be NFC...

[ldm0]

I asked some folks who know and the proc_pidinfo call and the extended_thread_info read the same data, so this should be NFC...

Yes, this PR does not intend to change the behavior: thread_info, proc_pidinfo. Just a drive-by optimization to reduce syscalls.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff e85fcb763173590fdcd5cb922b7ca1fc97cf170b aefa120446a4c31fe9569d76ae3950c29d8d35e9 --extensions cpp,h -- lldb/tools/debugserver/source/MacOSX/MachThread.cpp lldb/tools/debugserver/source/MacOSX/MachThread.h

View the diff from clang-format here.

diff --git a/lldb/tools/debugserver/source/MacOSX/MachThread.cpp b/lldb/tools/debugserver/source/MacOSX/MachThread.cpp
index bce60ed94e..de2bebfcec 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachThread.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/MachThread.cpp
@@ -274,8 +274,9 @@ bool MachThread::GetExtendedInfo(thread_t thread,
                                  struct thread_extended_info *extendedInfoPtr) {
   if (MachPortNumberIsValid(thread)) {
     mach_msg_type_number_t info_count = THREAD_EXTENDED_INFO_COUNT;
-    kern_return_t err = ::thread_info(thread, THREAD_EXTENDED_INFO,
-                                      (thread_info_t)extendedInfoPtr, &info_count);
+    kern_return_t err =
+        ::thread_info(thread, THREAD_EXTENDED_INFO,
+                      (thread_info_t)extendedInfoPtr, &info_count);
     if (err == KERN_SUCCESS)
       return true;
   }

[jasonmolenda]

Looks good to me, thanks for putting this PR together. I can merge the PR for you, if you don't have permissions to do that on llvm-project currently.

[github-actions]

@ldm0 Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!