From: Florian Krohm Date: Mon, 16 Sep 2013 17:08:50 +0000 (+0000) Subject: Intercept prctl(PR_SET_NAME, name) and store the thread name so it X-Git-Tag: svn/VALGRIND_3_9_0~138 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dbc0ecfa9fd0eaca44c162888972f78bc30f01aa;p=thirdparty%2Fvalgrind.git Intercept prctl(PR_SET_NAME, name) and store the thread name so it can be used in error messages. That should be helpful when debugging multithreaded applications. Patch by Matthias Schwarzott with some minor modifications. Fixes BZ 322254. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13553 --- diff --git a/NEWS b/NEWS index d489a78e83..4d0fbd88a5 100644 --- a/NEWS +++ b/NEWS @@ -464,6 +464,9 @@ FIXED r?? 321969 ppc32 and ppc64 don't support [lf]setxattr FIXED r13449 +322254 Show threadname together with tid if set by application + FIXED r13553 + 322368 Assertion failure in wqthread_hijack under OS X 10.8 FIXED 13523 diff --git a/coregrind/m_errormgr.c b/coregrind/m_errormgr.c index 1374cd421d..35b334f0db 100644 --- a/coregrind/m_errormgr.c +++ b/coregrind/m_errormgr.c @@ -591,6 +591,10 @@ static void pp_Error ( Error* err, Bool allow_db_attach, Bool xml ) VG_(printf_xml)("\n"); VG_(printf_xml)(" 0x%x\n", err->unique); VG_(printf_xml)(" %d\n", err->tid); + ThreadState* tst = VG_(get_ThreadState)(err->tid); + if (tst->thread_name) { + VG_(printf_xml)(" %s\n", tst->thread_name); + } /* actually print it */ VG_TDICT_CALL( tool_pp_Error, err ); @@ -608,7 +612,12 @@ static void pp_Error ( Error* err, Bool allow_db_attach, Bool xml ) if (VG_(tdict).tool_show_ThreadIDs_for_errors && err->tid > 0 && err->tid != last_tid_printed) { - VG_(umsg)("Thread %d:\n", err->tid ); + ThreadState* tst = VG_(get_ThreadState)(err->tid); + if (tst->thread_name) { + VG_(umsg)("Thread %d %s:\n", err->tid, tst->thread_name ); + } else { + VG_(umsg)("Thread %d:\n", err->tid ); + } last_tid_printed = err->tid; } diff --git a/coregrind/m_gdbserver/server.c b/coregrind/m_gdbserver/server.c index 46abb7cfa6..9feb0f61a5 100644 --- a/coregrind/m_gdbserver/server.c +++ b/coregrind/m_gdbserver/server.c @@ -588,10 +588,18 @@ void handle_query (char *arg_own_buf, int *new_packet_len_p) ti = gdb_id_to_thread (gdb_id); if (ti != NULL) { tst = (ThreadState *) inferior_target_data (ti); - /* Additional info is the tid and the thread status. */ - VG_(snprintf) (status, sizeof(status), "tid %d %s", - tst->tid, - VG_(name_of_ThreadStatus)(tst->status)); + /* Additional info is the tid, the thread status and the thread's + name, if any. */ + if (tst->thread_name) { + VG_(snprintf) (status, sizeof(status), "tid %d %s %s", + tst->tid, + VG_(name_of_ThreadStatus)(tst->status), + tst->thread_name); + } else { + VG_(snprintf) (status, sizeof(status), "tid %d %s", + tst->tid, + VG_(name_of_ThreadStatus)(tst->status)); + } hexify (arg_own_buf, status, strlen(status)); return; } else { diff --git a/coregrind/m_scheduler/scheduler.c b/coregrind/m_scheduler/scheduler.c index b20ab919ae..f0fb675239 100644 --- a/coregrind/m_scheduler/scheduler.c +++ b/coregrind/m_scheduler/scheduler.c @@ -238,6 +238,7 @@ ThreadId VG_(alloc_ThreadState) ( void ) if (VG_(threads)[i].status == VgTs_Empty) { VG_(threads)[i].status = VgTs_Init; VG_(threads)[i].exitreason = VgSrc_None; + VG_(threads)[i].thread_name = NULL; return i; } } @@ -616,6 +617,7 @@ ThreadId VG_(scheduler_init_phase1) ( void ) VG_(threads)[i].client_stack_szB = 0; VG_(threads)[i].client_stack_highest_word = (Addr)NULL; VG_(threads)[i].err_disablement_level = 0; + VG_(threads)[i].thread_name = NULL; } tid_main = VG_(alloc_ThreadState)(); diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c index 1db46e0ad8..e948bd555e 100644 --- a/coregrind/m_syswrap/syswrap-linux.c +++ b/coregrind/m_syswrap/syswrap-linux.c @@ -946,6 +946,18 @@ POST(sys_prctl) case VKI_PR_GET_ENDIAN: POST_MEM_WRITE(ARG2, sizeof(Int)); break; + case VKI_PR_SET_NAME: + { + const HChar* new_name = (const HChar*) ARG2; + if (new_name) { // Paranoia + ThreadState* tst = VG_(get_ThreadState)(tid); + + /* Don't bother reusing the memory. This is a rare event. */ + tst->thread_name = + VG_(arena_strdup)(VG_AR_CORE, "syswrap.prctl", new_name); + } + } + break; } } diff --git a/coregrind/pub_core_threadstate.h b/coregrind/pub_core_threadstate.h index 54dce4a588..cfe2121130 100644 --- a/coregrind/pub_core_threadstate.h +++ b/coregrind/pub_core_threadstate.h @@ -357,6 +357,9 @@ typedef struct { /* Per-thread jmp_buf to resume scheduler after a signal */ Bool sched_jmpbuf_valid; VG_MINIMAL_JMP_BUF(sched_jmpbuf); + + /* This thread's name. NULL, if no name. */ + HChar *thread_name; } ThreadState; diff --git a/docs/internals/xml-output-protocol4.txt b/docs/internals/xml-output-protocol4.txt index 861d8ca21b..a147eaa4cb 100644 --- a/docs/internals/xml-output-protocol4.txt +++ b/docs/internals/xml-output-protocol4.txt @@ -408,6 +408,7 @@ following: HEX64 INT + NAME if set KIND (either WHAT or XWHAT) @@ -428,6 +429,10 @@ following: is arbitrary but may be used to determine which threads produced which errors (at least, the first instance of each error). +* The tag identifies the name of the thread if it was + set by the client application. If no name was set, the tag is + omitted. + * The tag specifies one of a small number of fixed error types, so that GUIs may roughly categorise errors by type if they want. The tags themselves are tool-specific and are defined further diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index 293813323e..2951423dd8 100644 --- a/memcheck/tests/Makefile.am +++ b/memcheck/tests/Makefile.am @@ -260,7 +260,10 @@ EXTRA_DIST = \ wrap8.vgtest wrap8.stdout.exp wrap8.stderr.exp \ wrap8.stdout.exp2 wrap8.stderr.exp2 \ writev1.stderr.exp writev1.vgtest \ - xml1.stderr.exp xml1.stdout.exp xml1.vgtest xml1.stderr.exp-s390x-mvc + xml1.stderr.exp xml1.stdout.exp xml1.vgtest xml1.stderr.exp-s390x-mvc \ + threadname.vgtest threadname.stdout.exp threadname.stderr.exp \ + threadname_xml.vgtest threadname_xml.stdout.exp \ + threadname_xml.stderr.exp check_PROGRAMS = \ accounting \ @@ -335,7 +338,8 @@ check_PROGRAMS = \ wcs \ xml1 \ wrap1 wrap2 wrap3 wrap4 wrap5 wrap6 wrap7 wrap7so.so wrap8 \ - writev1 + writev1 \ + threadname if DWARF4 check_PROGRAMS += dw4 @@ -367,6 +371,7 @@ dw4_CFLAGS = $(AM_CFLAGS) -gdwarf-4 -fdebug-types-section err_disable3_LDADD = -lpthread err_disable4_LDADD = -lpthread thread_alloca_LDADD = -lpthread +threadname_LDADD = -lpthread error_counts_CFLAGS = $(AM_CFLAGS) @FLAG_W_NO_UNINITIALIZED@ diff --git a/memcheck/tests/threadname.c b/memcheck/tests/threadname.c new file mode 100644 index 0000000000..2eabfd5e70 --- /dev/null +++ b/memcheck/tests/threadname.c @@ -0,0 +1,80 @@ +//#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include + +static pthread_t children[3]; + +void bad_things(int offset) +{ + char* m = malloc(sizeof(char)*offset); + m[offset] = 0; + free(m); +} + +void* child_fn_2 ( void* arg ) +{ + const char* threadname = "012345678901234"; + + pthread_setname_np(pthread_self(), threadname); + + bad_things(4); + + return NULL; +} + +void* child_fn_1 ( void* arg ) +{ + const char* threadname = "try1"; + int r; + + pthread_setname_np(pthread_self(), threadname); + + bad_things(3); + + r = pthread_create(&children[2], NULL, child_fn_2, NULL); + assert(!r); + + r = pthread_join(children[2], NULL); + assert(!r); + + return NULL; +} + +void* child_fn_0 ( void* arg ) +{ + int r; + + bad_things(2); + + r = pthread_create(&children[1], NULL, child_fn_1, NULL); + assert(!r); + + r = pthread_join(children[1], NULL); + assert(!r); + + return NULL; +} + +int main(int argc, const char** argv) +{ + int r; + + bad_things(1); + + r = pthread_create(&children[0], NULL, child_fn_0, NULL); + assert(!r); + + r = pthread_join(children[0], NULL); + assert(!r); + + bad_things(5); + + return 0; +} + diff --git a/memcheck/tests/threadname.stderr.exp b/memcheck/tests/threadname.stderr.exp new file mode 100644 index 0000000000..8ee6752979 --- /dev/null +++ b/memcheck/tests/threadname.stderr.exp @@ -0,0 +1,50 @@ +Invalid write of size 1 + at 0x........: bad_things (threadname.c:16) + by 0x........: main (threadname.c:68) + Address 0x........ is 0 bytes after a block of size 1 alloc'd + at 0x........: malloc (vg_replace_malloc.c:...) + by 0x........: bad_things (threadname.c:15) + by 0x........: main (threadname.c:68) + +Thread 2: +Invalid write of size 1 + at 0x........: bad_things (threadname.c:16) + by 0x........: child_fn_0 (threadname.c:53) + ... + Address 0x........ is 0 bytes after a block of size 2 alloc'd + at 0x........: malloc (vg_replace_malloc.c:...) + by 0x........: bad_things (threadname.c:15) + by 0x........: child_fn_0 (threadname.c:53) + ... + +Thread 3 try1: +Invalid write of size 1 + at 0x........: bad_things (threadname.c:16) + by 0x........: child_fn_1 (threadname.c:38) + ... + Address 0x........ is 0 bytes after a block of size 3 alloc'd + at 0x........: malloc (vg_replace_malloc.c:...) + by 0x........: bad_things (threadname.c:15) + by 0x........: child_fn_1 (threadname.c:38) + ... + +Thread 4 012345678901234: +Invalid write of size 1 + at 0x........: bad_things (threadname.c:16) + by 0x........: child_fn_2 (threadname.c:26) + ... + Address 0x........ is 0 bytes after a block of size 4 alloc'd + at 0x........: malloc (vg_replace_malloc.c:...) + by 0x........: bad_things (threadname.c:15) + by 0x........: child_fn_2 (threadname.c:26) + ... + +Thread 1: +Invalid write of size 1 + at 0x........: bad_things (threadname.c:16) + by 0x........: main (threadname.c:76) + Address 0x........ is 0 bytes after a block of size 5 alloc'd + at 0x........: malloc (vg_replace_malloc.c:...) + by 0x........: bad_things (threadname.c:15) + by 0x........: main (threadname.c:76) + diff --git a/memcheck/tests/threadname.vgtest b/memcheck/tests/threadname.vgtest new file mode 100644 index 0000000000..4cffb8edd8 --- /dev/null +++ b/memcheck/tests/threadname.vgtest @@ -0,0 +1,2 @@ +prog: threadname +vgopts: -q diff --git a/memcheck/tests/threadname_xml.stderr.exp b/memcheck/tests/threadname_xml.stderr.exp new file mode 100644 index 0000000000..7ecc850861 --- /dev/null +++ b/memcheck/tests/threadname_xml.stderr.exp @@ -0,0 +1,373 @@ + + + + +4 +memcheck + + + ... + ... + ... + ... + + +... +... +memcheck + + + ... + + ./threadname + + + + + RUNNING + + + + + 0x........ + ... + InvalidWrite + Invalid write of size 1 + + + 0x........ + ... + bad_things + ... + threadname.c + ... + + + 0x........ + ... + main + ... + threadname.c + ... + + + Address 0x........ is 0 bytes after a block of size 1 alloc'd + + + 0x........ + ... + malloc + ... + vg_replace_malloc.c + ... + + + 0x........ + ... + bad_things + ... + threadname.c + ... + + + 0x........ + ... + main + ... + threadname.c + ... + + + + + + 0x........ + ... + InvalidWrite + Invalid write of size 1 + + + 0x........ + ... + bad_things + ... + threadname.c + ... + + + 0x........ + ... + child_fn_0 + ... + threadname.c + ... + + + 0x........ + ... + start_thread + ... + pthread_create.c + ... + + + Address 0x........ is 0 bytes after a block of size 2 alloc'd + + + 0x........ + ... + malloc + ... + vg_replace_malloc.c + ... + + + 0x........ + ... + bad_things + ... + threadname.c + ... + + + 0x........ + ... + child_fn_0 + ... + threadname.c + ... + + + 0x........ + ... + start_thread + ... + pthread_create.c + ... + + + + + + 0x........ + ... + try1 + InvalidWrite + Invalid write of size 1 + + + 0x........ + ... + bad_things + ... + threadname.c + ... + + + 0x........ + ... + child_fn_1 + ... + threadname.c + ... + + + 0x........ + ... + start_thread + ... + pthread_create.c + ... + + + Address 0x........ is 0 bytes after a block of size 3 alloc'd + + + 0x........ + ... + malloc + ... + vg_replace_malloc.c + ... + + + 0x........ + ... + bad_things + ... + threadname.c + ... + + + 0x........ + ... + child_fn_1 + ... + threadname.c + ... + + + 0x........ + ... + start_thread + ... + pthread_create.c + ... + + + + + + 0x........ + ... + 012345678901234 + InvalidWrite + Invalid write of size 1 + + + 0x........ + ... + bad_things + ... + threadname.c + ... + + + 0x........ + ... + child_fn_2 + ... + threadname.c + ... + + + 0x........ + ... + start_thread + ... + pthread_create.c + ... + + + Address 0x........ is 0 bytes after a block of size 4 alloc'd + + + 0x........ + ... + malloc + ... + vg_replace_malloc.c + ... + + + 0x........ + ... + bad_things + ... + threadname.c + ... + + + 0x........ + ... + child_fn_2 + ... + threadname.c + ... + + + 0x........ + ... + start_thread + ... + pthread_create.c + ... + + + + + + 0x........ + ... + InvalidWrite + Invalid write of size 1 + + + 0x........ + ... + bad_things + ... + threadname.c + ... + + + 0x........ + ... + main + ... + threadname.c + ... + + + Address 0x........ is 0 bytes after a block of size 5 alloc'd + + + 0x........ + ... + malloc + ... + vg_replace_malloc.c + ... + + + 0x........ + ... + bad_things + ... + threadname.c + ... + + + 0x........ + ... + main + ... + threadname.c + ... + + + + + + + FINISHED + + + + + + ... + 0x........ + + + ... + 0x........ + + + ... + 0x........ + + + ... + 0x........ + + + ... + 0x........ + + + +... + + + diff --git a/memcheck/tests/threadname_xml.vgtest b/memcheck/tests/threadname_xml.vgtest new file mode 100644 index 0000000000..730fe72a57 --- /dev/null +++ b/memcheck/tests/threadname_xml.vgtest @@ -0,0 +1,3 @@ +prog: threadname +vgopts: --xml=yes --xml-fd=2 --log-file=/dev/null +stderr_filter: filter_xml