[lldb-dev] PATCH for REVIEW: Implement Host::SetThreadName on Linux

[Mike Sartain]

This is my first submission, so any feedback/comments on anything I'm messing up or should do differently are greatly appreciated. I also don't have a Mac yet (on order) - so I haven't tested on that platform just yet...

This patch:
- Host::SetThreadName was declared void, but the comments in Host.h said it returns a bool. It now returns true on success, false on failure.
- Host::SetThreadName is now implemented on Linux.
- I added code to set the lldb thread name in ThreadCreateTrampoline. I've found this helps me when debugging and looking at the output of htop or top as I get something other than 'lldb' for each thread. As an example, ps now looks like this:

mikesart at mikesart-rad:~/data/src/llvm/llvm/tools/lldb$ ps -Leo pid,tid,class,wchan:14,comm | grep `pidof lldb`
18162 18162 TS  futex_wait_que lldb
18162 18163 TS  poll_schedule_ input
18162 18164 TS  poll_schedule_ editline
18162 18165 TS  poll_schedule_ editline_output
18162 18166 TS  n_tty_read     commandline_io
18162 18348 TS  poll_schedule_ operation
18162 18350 TS  wait           wait4(pid=18349
18162 18351 TS  poll_schedule_ stdio
18162 18352 TS  futex_wait_que internal-state

If anyone has any complaints against naming the lldb threads for any reason, this part could be pulled out or possibly wrapped in a command line argument?

Patch is down below. Also on https://gist.github.com/mikesartain/5550857

Thanks much!
 -Mike


Index: include/lldb/Host/Host.h
===================================================================
--- include/lldb/Host/Host.h	(revision 181496)
+++ include/lldb/Host/Host.h	(working copy)
@@ -299,7 +299,7 @@
     ///     \b true if the thread name was able to be set, \b false
     ///     otherwise.
     //------------------------------------------------------------------
-    static void
+    static bool
     SetThreadName (lldb::pid_t pid, lldb::tid_t tid, const char *name);
 
     //------------------------------------------------------------------
Index: source/Host/common/Host.cpp
===================================================================
--- source/Host/common/Host.cpp	(revision 181496)
+++ source/Host/common/Host.cpp	(working copy)
@@ -558,7 +558,38 @@
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD));
     if (log)
         log->Printf("thread created");
-    
+
+    if (!Host::SetThreadName (LLDB_INVALID_PROCESS_ID, LLDB_INVALID_THREAD_ID, info->thread_name.c_str()))
+    {
+        // pthread_setname_np_func can fail if the thread name is longer than
+        //  the supported limit on Linux. When this occurs, the error ERANGE is returned
+        // and SetThreadName will fail. Let's drop it down to 16 characters and try again.
+        char namebuf[16];
+        const char *name = info->thread_name.c_str();
+
+        // Thread names are coming in like '<lldb.comm.debugger.edit>' and '<lldb.comm.debugger.editline>'
+        // So just chopping the end of the string off leads to a lot of similar named threads.
+        // Go through the thread name and search for the last dot and use that.
+        const char *lastdot = ::strrchr( name, '.' );
+
+        if (lastdot && lastdot != name)
+            name = lastdot + 1;
+        ::strncpy (namebuf, name, sizeof(namebuf));
+        namebuf[ sizeof(namebuf) - 1 ] = 0;
+
+        int namebuflen = strlen(namebuf);
+        if (namebuflen > 0)
+        {
+            if (namebuf[namebuflen - 1] == '(' || namebuf[namebuflen - 1] == '>')
+            {
+                // Trim off trailing '(' and '>' characters for a bit more cleanup.
+                namebuflen--;
+                namebuf[namebuflen] = 0;
+            }
+            Host::SetThreadName (LLDB_INVALID_PROCESS_ID, LLDB_INVALID_THREAD_ID, namebuf);
+        }
+    }
+
     delete info;
     return thread_fptr (thread_arg);
 }
@@ -652,7 +683,7 @@
     return thread_name;
 }
 
-void
+bool
 Host::SetThreadName (lldb::pid_t pid, lldb::tid_t tid, const char *name)
 {
 #if defined(__APPLE__) && MAC_OS_X_VERSION_MAX_ALLOWED > MAC_OS_X_VERSION_10_5
@@ -667,8 +698,33 @@
     // Set the pthread name if possible
     if (pid == curr_pid && tid == curr_tid)
     {
-        ::pthread_setname_np (name);
+        if (::pthread_setname_np (name) == 0)
+            return true;
     }
+    return false;
+#elif defined (__linux__)
+    void *fn = dlsym (RTLD_DEFAULT, "pthread_setname_np");
+    if (fn)
+    {
+        int (*pthread_setname_np_func)(pthread_t thread, const char *name);
+        *reinterpret_cast<void **> (&pthread_setname_np_func) = fn;
+
+        lldb::pid_t curr_pid = Host::GetCurrentProcessID();
+        lldb::tid_t curr_tid = Host::GetCurrentThreadID();
+
+        if (pid == LLDB_INVALID_PROCESS_ID)
+            pid = curr_pid;
+
+        if (tid == LLDB_INVALID_THREAD_ID)
+            tid = curr_tid;
+
+        if (pid == curr_pid)
+        {
+            if (pthread_setname_np_func (tid, name) == 0)
+                return true;
+        }
+    }
+    return false;
 #endif
 }