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

[Kopec, Matt]

Hi Mike,

Thanks for contributing the patch! This will help many of us in debugging lldb. Good to know there are other Linux lldb developers out there. :)

Some comments about the patch...

> +#elif defined (__linux__)
> +    void *fn = dlsym (RTLD_DEFAULT, "pthread_setname_np");
> +    if (fn)
> +    {


I believe this pthread function can be added by including pthread.h for Linux. That way you don't need dlsym and can just re-use most of the code that is already there for apple.


> +        const char *lastdot = ::strrchr( thread_name, '.' );
> +
> +        if (lastdot && lastdot != thread_name)
> +            thread_name = lastdot + 1;
> +        ::strncpy (namebuf, thread_name, sizeof(namebuf));
> +        namebuf[ sizeof(namebuf) - 1 ] = 0;
> +
> +        int namebuflen = strlen(namebuf);

It would have been nice to use std::string and its' facilities. It would make this easier to write and understand but what you have is fine.


Let me know what you think. I'll be able to commit the patch for you afterwards.

Thanks,
Matt


On 2013-05-09, at 6:44 PM, Mike Sartain <mikesart at valvesoftware.com> wrote:

>> This is my first submission, so any feedback/comments on anything I'm messing up ...
> 
> And, of course, I did screw it up. I just ran across ThreadCreated and noticed that's where thread names are being set for OSX.
> 
> Updated 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.
> - Added code to set the lldb thread name in Host::ThreadCreated() in linux/Host.cpp
> 
> Path down below, and also: https://gist.github.com/mikesartain/5550857
> 
> Thanks!
> -Mike
> 
> Index: source/Host/common/Host.cpp
> ===================================================================
> --- source/Host/common/Host.cpp	(revision 181496)
> +++ source/Host/common/Host.cpp	(working copy)
> @@ -512,7 +512,8 @@
> {
> }
> 
> -#if !defined (__APPLE__) && !defined (__FreeBSD__) // see macosx/Host.mm
> +#if !defined (__APPLE__) && !defined (__FreeBSD__) && !defined (__linux__) // see macosx/Host.mm
> +
> void
> Host::ThreadCreated (const char *thread_name)
> {
> @@ -531,7 +532,7 @@
>     return 0;
> }
> 
> -#endif
> +#endif // #if !defined (__APPLE__) && !defined (__FreeBSD__) && !defined (__linux__)
> 
> struct HostThreadCreateInfo
> {
> @@ -652,7 +653,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 +668,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
> }
> 
> Index: source/Host/linux/Host.cpp
> ===================================================================
> --- source/Host/linux/Host.cpp	(revision 181496)
> +++ source/Host/linux/Host.cpp	(working copy)
> @@ -169,4 +169,51 @@
>     // FIXME: Parse /proc/<pid>/status to get uid, gid, euid, egid and parent_pid
> 
>     return true;
> -}
> \ No newline at end of file
> +}
> +
> +void
> +Host::ThreadCreated (const char *thread_name)
> +{
> +    if (!Host::SetThreadName (LLDB_INVALID_PROCESS_ID, LLDB_INVALID_THREAD_ID, thread_name))
> +    {
> +        // 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];
> +
> +        // 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( thread_name, '.' );
> +
> +        if (lastdot && lastdot != thread_name)
> +            thread_name = lastdot + 1;
> +        ::strncpy (namebuf, thread_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);
> +        }
> +    }
> +}
> +
> +void
> +Host::Backtrace (Stream &strm, uint32_t max_frames)
> +{
> +    // TODO: Is there a way to backtrace the current process on linux?
> +}
> +
> +size_t
> +Host::GetEnvironment (StringList &env)
> +{
> +    // TODO: Is there a way to the host environment for this process on linux?
> +    return 0;
> +}
> 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);
> 
>     //------------------------------------------------------------------
> 
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev