|
From: Matthias S. <zz...@ge...> - 2012-11-07 15:54:48
|
Hi! I wonder if it would be good to have valgrind remember the names of the threads. Our application sets the name of the threads using pthread_setname_np and that is implemented by calling the syscall prctl(PR_SET_NAME). Using plain gdb, info threads also lists these user-defined names. But valgrind and gdb+vgdb only show the thread-ids. The only place in valgrind that already handles thread names is a helgrind client request. I think it would be a good idea if valgrind could also show the thread name. My first attempt is to add more code to POST(sys_prctl) and in case of VKI_PR_SET_NAME is called, store the set name into a new field of ThreadState. Then in every user visible place where a threadid is printed, also print the name. How could this look like? ==123== Thread 5 (MyThread1): ==123== Invalid read of size 1: ... I will send a patch once this is done. The other option would be to call prctl(PR_GET_NAME) at every place that needs a thread name. Regards Matthias |
|
From: Philippe W. <phi...@sk...> - 2012-11-07 18:48:35
|
On Wed, 2012-11-07 at 16:54 +0100, Matthias Schwarzott wrote: Printing thread names is not a bad idea (not too sure that a lot of applications are using pthread_setname or prctl but never mind). In any case, I see several subtilities to look at. > Using plain gdb, info threads also lists these user-defined names. > But valgrind and gdb+vgdb only show the thread-ids. I tried with plain gdb 7.5 on fedora 12 (kernel 2.6.32.26-175.fc12.i686.PAE). The prctl get/set name syscalls are working, but I could not persuade (gdb) info threads or ps command to show these names. With which versions have you succeeded to have info threads showing the thread names ? > The only place in valgrind that already handles thread names is a > helgrind client request. Which is unimplemented in helgrind, but implemented by drd. There will be a need to decide which one(s) to show if both prctl and the DRD client request were used to give a name to a thread. > > > I think it would be a good idea if valgrind could also show the thread name. > My first attempt is to add more code to POST(sys_prctl) and in case of > VKI_PR_SET_NAME is called, store the set name into a new field of > ThreadState. > Then in every user visible place where a threadid is printed, also print > the name. > How could this look like? I think that in some cases, an error can reference a terminated thread (e.g. for helgrind or drd errors). For such a case, you cannot recuperate the thread name in the ThreadState as this ThreadState could have been emptied or re-used by another thread. I think that helgrind maintains a unique thread data structure (even for terminated thread). So, you will have to copy the thread name to this unique helgrind data structure. (in other words, it looks like printing the thread name cannot be done "centrally" in coregrind error mgr, at least e.g. for helgrind). Maybe/probably drd does a similar thing. > The other option would be to call prctl(PR_GET_NAME) at every place that > needs a thread name. This would not work when a thread prints an error which references another thread. Otherwise, if you can retrieve somewhere the name of a non terminated thread (i.e. the "ThreadState solution"), then it is straightforward to have Valgrind gdbsrv giving back this info to GDB. See handling of packet "qThreadExtraInfo," in file server.c. Philippe |
|
From: Tom T. <tr...@re...> - 2012-11-09 15:20:04
|
>>>>> "Matthias" == Matthias Schwarzott <zz...@ge...> writes:
Matthias> pthread_setname_np and that is implemented by calling the syscall
Matthias> prctl(PR_SET_NAME).
Matthias> Using plain gdb, info threads also lists these user-defined names.
Matthias> But valgrind and gdb+vgdb only show the thread-ids.
Currently I don't think there is a way for vgdb to report this back to
gdb. I filed a bug for this:
http://sourceware.org/bugzilla/show_bug.cgi?id=14817
Fixing this is part of our project for native/remote parity:
http://sourceware.org/gdb/wiki/LocalRemoteFeatureParity
Tom
|
|
From: Philippe W. <phi...@sk...> - 2012-11-10 17:24:30
|
On Fri, 2012-11-09 at 07:52 -0700, Tom Tromey wrote: > Matthias> Using plain gdb, info threads also lists these user-defined names. > Matthias> But valgrind and gdb+vgdb only show the thread-ids. > > Currently I don't think there is a way for vgdb to report this back to > gdb. The thread name could be reported via packet "qThreadExtraInfo," shown in 'info threads' (this is how Valgrind gdbsrv shows e.g. the Valgrind thread state and id). This extra info is just a string, and so GDB will not be able to understand it contains a thread name. Philippe |
|
From: Matthias S. <zz...@ge...> - 2012-11-11 13:15:41
|
On 10.11.2012 18:24, Philippe Waroquiers wrote: > On Fri, 2012-11-09 at 07:52 -0700, Tom Tromey wrote: > >> Matthias> Using plain gdb, info threads also lists these user-defined names. >> Matthias> But valgrind and gdb+vgdb only show the thread-ids. >> >> Currently I don't think there is a way for vgdb to report this back to >> gdb. > The thread name could be reported via packet "qThreadExtraInfo," > shown in 'info threads' (this is how Valgrind gdbsrv shows e.g. > the Valgrind thread state and id). > This extra info is just a string, and so GDB will not be able to > understand it contains a thread name. Yes, this is the way I implemented my proof of concept. A bit of googling shows that kgdb (the kernel gdbserver) also sends the comm field (the thread name) in qThreadExtraInfo. I will send a patch tomorrow. Matthias |
|
From: Matthias S. <zz...@ge...> - 2012-11-15 05:49:08
Attachments:
threadname.patch
|
On 07.11.2012 16:54, Matthias Schwarzott wrote: > Hi! > > I wonder if it would be good to have valgrind remember the names of the > threads. > Here is a first implementation of this change. It just enhances the struct ThreadState by a char array thread_name. The handler of prctl writes its argument there. For the first thread it is initialized to "main" (without valgrind there is the name of the executable by default, how could I get this). pp_Error prints Errors like this (if thread name is there): Thread 2 abcdxyz01234567: gdbserver appends it at the end for the qThreadExtraInfo query. (gdb) info threads Id Target Id Frame * 2 Thread 30698 (tid 2 VgTs_Runnable abcdxyz01234567) 0x00000000004009aa in child_fn (arg=0x0) at threadname.c:32 1 Thread 30679 (tid 1 VgTs_Yielding main) 0x00000034cfae88d1 in clone () from /lib64/libc.so.6 The testcase I started to write is also included. Regards Matthias |
|
From: Philippe W. <phi...@sk...> - 2012-11-15 19:38:11
|
On Thu, 2012-11-15 at 06:48 +0100, Matthias Schwarzott wrote: > On 07.11.2012 16:54, Matthias Schwarzott wrote: > > Hi! > > > > I wonder if it would be good to have valgrind remember the names of the > > threads. > > > Here is a first implementation of this change. Thanks for looking at this. > It just enhances the struct ThreadState by a char array thread_name. > The handler of prctl writes its argument there. According to the (maybe too quick?) discussion in http://sourceforge.net/mailarchive/message.php?msg_id=30069236 this implementation will give wrong information for some kinds of errors (typically when showing an error after a thread has terminated). Or do you think these cases can in fact not happen ? Or maybe this is just making an existing bug more visible ? => would be worth digging more in depth in that. > For the first thread it is initialized to "main" (without valgrind there > is the name of the executable by default, how could I get this). If you do call prctl(PR_GET_NAME, ...) just at startup, it returns the name of the executable (truncated to 15 characters). > The testcase I started to write is also included. Some first little comments about the (early) patch: + if (tst->thread_name[0]) { + VG_(umsg)("Thread %d %s:\n", err->tid, VG_(get_ThreadState)(err->tid)->thread_name); Should (re-)use tst rather than (re-)get the thread state. Indentation is not correct at a few places or lines are longer than 80 chars. For the test case: is pthread_setname_np available in all the supported platforms ? Otherwise I guess that some autoconf magic or a #ifdef or another The .exp expected files are still expected :). Philippe |
|
From: Matthias S. <zz...@ge...> - 2012-11-19 20:15:19
Attachments:
valgrind-threadname-v2.patch
|
On 15.11.2012 20:38, Philippe Waroquiers wrote: > On Thu, 2012-11-15 at 06:48 +0100, Matthias Schwarzott wrote: >> On 07.11.2012 16:54, Matthias Schwarzott wrote: >>> Hi! >>> >>> I wonder if it would be good to have valgrind remember the names of the >>> threads. >>> >> Here is a first implementation of this change. > Thanks for looking at this. > >> It just enhances the struct ThreadState by a char array thread_name. >> The handler of prctl writes its argument there. > According to the (maybe too quick?) discussion in > http://sourceforge.net/mailarchive/message.php?msg_id=30069236 > this implementation will give wrong information for some kinds > of errors (typically when showing an error after a thread has > terminated). > Or do you think these cases can in fact not happen ? I think for memcheck at least this cannot happen (or does the leak list shows thread names?). For helgrind it may be possible to copy the threadname if a thread finishes (for later reports). But I observed, that memcheck does not notice this sequence: create a thread. error end a thread create a thread with same tid. error For the second error, there is no "Thread %d" line printed, as the tid is unchanged. > Or maybe this is just making an existing bug more visible ? > => would be worth digging more in depth in that. > > >> For the first thread it is initialized to "main" (without valgrind there >> is the name of the executable by default, how could I get this). > If you do call prctl(PR_GET_NAME, ...) just at startup, > it returns the name of the executable (truncated to 15 characters). > Sadly for a process under valgrind, this returns just "memcheck-amd64-". Maybe it can be fetched from VG_(args_the_exename). >> The testcase I started to write is also included. > Some first little comments about the (early) patch: > + if (tst->thread_name[0]) { > + VG_(umsg)("Thread %d %s:\n", err->tid, VG_(get_ThreadState)(err->tid)->thread_name); > Should (re-)use tst rather than (re-)get the thread state. > > Indentation is not correct at a few places or lines are longer than 80 chars. > > For the test case: is pthread_setname_np available in all the > supported platforms ? Otherwise I guess that some autoconf magic or a #ifdef or another > The .exp expected files are still expected :). No idea, but as this function is part of glibc I guess at least on every linux-based platform it should be available. Maybe it need to be checked at what glibc version this call was introduced. Matthias |
|
From: Matthias S. <zz...@ge...> - 2013-07-25 09:44:10
|
On 07.11.2012 16:54, Matthias Schwarzott wrote: > Hi! > > I wonder if it would be good to have valgrind remember the names of the > threads. > Our application sets the name of the threads using I finally managed to create a ticket with the current version of the patch attached: https://bugs.kde.org/show_bug.cgi?id=322254 Remaining open issues: * The thread info tid+name is not printed if it is still the same thread but the name changed. * The thread info is not printed if it is a different thread, but the tid is the same (see https://bugs.kde.org/show_bug.cgi?id=322258). * The first thread is called "main", but it could get the name of the real executable (as without valgrind). * The gdb test stdout filter does not like the changed output of "info threads". * For drd/helgrind, copy the name over to the thread structures of the tool (to also keep it after a thread has terminated). Regards Matthias |