From: Bart Van Assche Date: Thu, 28 Jul 2011 09:54:37 +0000 (+0000) Subject: drd: Delay deletion of memory access information of joined threads in order not X-Git-Tag: svn/VALGRIND_3_7_0~311 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6cf2bc2c3409b286ba9dd55b2399de898c7d81f7;p=thirdparty%2Fvalgrind.git drd: Delay deletion of memory access information of joined threads in order not to miss any races caused by these threads. To do: refine handling of pthread_once() again. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11926 --- diff --git a/drd/drd_main.c b/drd/drd_main.c index 070fb5be84..e282c3a8a9 100644 --- a/drd/drd_main.c +++ b/drd/drd_main.c @@ -69,6 +69,7 @@ static Bool s_trace_alloc; static Bool DRD_(process_cmd_line_option)(Char* arg) { int check_stack_accesses = -1; + int join_list_vol = -1; int exclusive_threshold_ms = -1; int first_race_only = -1; int report_signal_unlocked = -1; @@ -92,6 +93,7 @@ static Bool DRD_(process_cmd_line_option)(Char* arg) Char* trace_address = 0; if VG_BOOL_CLO(arg, "--check-stack-var", check_stack_accesses) {} + else if VG_INT_CLO (arg, "--join-list-vol", join_list_vol) {} else if VG_BOOL_CLO(arg, "--drd-stats", s_print_stats) {} else if VG_BOOL_CLO(arg, "--first-race-only", first_race_only) {} else if VG_BOOL_CLO(arg, "--free-is-write", DRD_(g_free_is_write)) {} @@ -134,6 +136,8 @@ static Bool DRD_(process_cmd_line_option)(Char* arg) { DRD_(set_first_race_only)(first_race_only); } + if (join_list_vol != -1) + DRD_(thread_set_join_list_vol)(join_list_vol); if (report_signal_unlocked != -1) { DRD_(cond_set_report_signal_unlocked)(report_signal_unlocked); diff --git a/drd/drd_pthread_intercepts.c b/drd/drd_pthread_intercepts.c index 670659cee9..ebc2cc8559 100644 --- a/drd/drd_pthread_intercepts.c +++ b/drd/drd_pthread_intercepts.c @@ -530,7 +530,9 @@ int pthread_once_intercept(pthread_once_t *once_control, * any known adverse effects. */ DRD_IGNORE_VAR(*once_control); + ANNOTATE_IGNORE_READS_AND_WRITES_BEGIN(); CALL_FN_W_WW(ret, fn, once_control, init_routine); + ANNOTATE_IGNORE_READS_AND_WRITES_END(); DRD_STOP_IGNORING_VAR(*once_control); return ret; } diff --git a/drd/drd_thread.c b/drd/drd_thread.c index a681ba0bfd..2b0edab19d 100644 --- a/drd/drd_thread.c +++ b/drd/drd_thread.c @@ -76,6 +76,9 @@ static Bool s_trace_fork_join = False; static Bool s_segment_merging = True; static Bool s_new_segments_since_last_merge; static int s_segment_merge_interval = 10; +static unsigned s_join_list_vol = 10; +static unsigned s_deletion_head; +static unsigned s_deletion_tail; /* Function definitions. */ @@ -133,6 +136,11 @@ void DRD_(thread_set_segment_merge_interval)(const int i) s_segment_merge_interval = i; } +void DRD_(thread_set_join_list_vol)(const int jlv) +{ + s_join_list_vol = jlv; +} + /** * Convert Valgrind's ThreadId into a DrdThreadId. * @@ -167,12 +175,11 @@ static DrdThreadId DRD_(VgThreadIdToNewDrdThreadId)(const ThreadId tid) for (i = 1; i < DRD_N_THREADS; i++) { - if (DRD_(g_threadinfo)[i].vg_thread_exists == False - && DRD_(g_threadinfo)[i].posix_thread_exists == False - && DRD_(g_threadinfo)[i].detached_posix_thread == False) + if (!DRD_(g_threadinfo)[i].valid) { tl_assert(! DRD_(IsValidDrdThreadId)(i)); + DRD_(g_threadinfo)[i].valid = True; DRD_(g_threadinfo)[i].vg_thread_exists = True; DRD_(g_threadinfo)[i].vg_threadid = tid; DRD_(g_threadinfo)[i].pt_threadid = INVALID_POSIX_THREADID; @@ -186,6 +193,7 @@ static DrdThreadId DRD_(VgThreadIdToNewDrdThreadId)(const ThreadId tid) DRD_(g_threadinfo)[i].is_recording_stores = True; DRD_(g_threadinfo)[i].pthread_create_nesting_level = 0; DRD_(g_threadinfo)[i].synchr_nesting = 0; + DRD_(g_threadinfo)[i].deletion_seq = s_deletion_tail - 1; tl_assert(DRD_(g_threadinfo)[i].first == 0); tl_assert(DRD_(g_threadinfo)[i].last == 0); @@ -323,6 +331,32 @@ DrdThreadId DRD_(thread_post_create)(const ThreadId vg_created) return created; } +static void DRD_(thread_delayed_delete)(const DrdThreadId tid) +{ + int j; + + DRD_(g_threadinfo)[tid].vg_thread_exists = False; + DRD_(g_threadinfo)[tid].posix_thread_exists = False; + DRD_(g_threadinfo)[tid].deletion_seq = s_deletion_head++; +#if 0 + VG_(message)(Vg_DebugMsg, "Adding thread %d to the deletion list\n", tid); +#endif + if (s_deletion_head - s_deletion_tail >= s_join_list_vol) { + for (j = 0; j < DRD_N_THREADS; ++j) { + if (DRD_(IsValidDrdThreadId)(j) + && DRD_(g_threadinfo)[j].deletion_seq == s_deletion_tail) + { + s_deletion_tail++; +#if 0 + VG_(message)(Vg_DebugMsg, "Delayed delete of thread %d\n", j); +#endif + DRD_(thread_delete)(j, False); + break; + } + } + } +} + /** * Process VG_USERREQ__POST_THREAD_JOIN. This client request is invoked just * after thread drd_joiner joined thread drd_joinee. @@ -367,7 +401,7 @@ void DRD_(thread_post_join)(DrdThreadId drd_joiner, DrdThreadId drd_joinee) DRD_(thread_get_stack_max)(drd_joinee)); } DRD_(clientobj_delete_thread)(drd_joinee); - DRD_(thread_delete)(drd_joinee, False); + DRD_(thread_delayed_delete)(drd_joinee); } /** @@ -447,8 +481,7 @@ Int DRD_(thread_get_threads_on_alt_stack)(void) } /** - * Clean up thread-specific data structures. Call this just after - * pthread_join(). + * Clean up thread-specific data structures. */ void DRD_(thread_delete)(const DrdThreadId tid, const Bool detached) { @@ -465,6 +498,7 @@ void DRD_(thread_delete)(const DrdThreadId tid, const Bool detached) sg->next = 0; DRD_(sg_put)(sg); } + DRD_(g_threadinfo)[tid].valid = False; DRD_(g_threadinfo)[tid].vg_thread_exists = False; DRD_(g_threadinfo)[tid].posix_thread_exists = False; if (detached) @@ -1217,9 +1251,10 @@ void DRD_(thread_print_all)(void) if (DRD_(g_threadinfo)[i].first) { VG_(printf)("**************\n" - "* thread %3d (%d/%d/%d/0x%lx/%d) *\n" + "* thread %3d (%d/%d/%d/%d/0x%lx/%d) *\n" "**************\n", i, + DRD_(g_threadinfo)[i].valid, DRD_(g_threadinfo)[i].vg_thread_exists, DRD_(g_threadinfo)[i].vg_threadid, DRD_(g_threadinfo)[i].posix_thread_exists, diff --git a/drd/drd_thread.h b/drd/drd_thread.h index a110b48e2e..b5f2843320 100644 --- a/drd/drd_thread.h +++ b/drd/drd_thread.h @@ -78,6 +78,8 @@ typedef struct SizeT stack_size; /**< Maximum size of stack. */ char name[64]; /**< User-assigned thread name. */ Bool on_alt_stack; + /** Whether this structure contains valid information. */ + Bool valid; /** Indicates whether the Valgrind core knows about this thread. */ Bool vg_thread_exists; /** Indicates whether there is an associated POSIX thread ID. */ @@ -95,6 +97,8 @@ typedef struct Int pthread_create_nesting_level; /** Nesting level of synchronization functions called by the client. */ Int synchr_nesting; + /** Delayed thread deletion sequence number. */ + unsigned deletion_seq; } ThreadInfo; @@ -125,6 +129,7 @@ void DRD_(thread_set_trace_fork_join)(const Bool t); void DRD_(thread_set_segment_merging)(const Bool m); int DRD_(thread_get_segment_merge_interval)(void); void DRD_(thread_set_segment_merge_interval)(const int i); +void DRD_(thread_set_join_list_vol)(const int jlv); DrdThreadId DRD_(VgThreadIdToDrdThreadId)(const ThreadId tid); DrdThreadId DRD_(NewVgThreadIdToDrdThreadId)(const ThreadId tid); @@ -210,9 +215,7 @@ static __inline__ Bool DRD_(IsValidDrdThreadId)(const DrdThreadId tid) { return (0 <= (int)tid && tid < DRD_N_THREADS && tid != DRD_INVALID_THREADID - && (DRD_(g_threadinfo)[tid].vg_thread_exists - || DRD_(g_threadinfo)[tid].posix_thread_exists - || DRD_(g_threadinfo)[tid].detached_posix_thread)); + && (DRD_(g_threadinfo)[tid].valid)); } /** Returns the DRD thread ID of the currently running thread. */ diff --git a/drd/tests/tc21_pthonce.stderr.exp b/drd/tests/tc21_pthonce.stderr.exp index d18786f806..b6458280f1 100644 --- a/drd/tests/tc21_pthonce.stderr.exp +++ b/drd/tests/tc21_pthonce.stderr.exp @@ -1,3 +1,24 @@ +Thread 3: +Conflicting load by thread 3 at 0x........ size 4 + at 0x........: child (tc21_pthonce.c:74) + by 0x........: vgDrd_thread_wrapper (drd_pthread_intercepts.c:?) + by 0x........: (within libpthread-?.?.so) +Allocation context: BSS section of tc21_pthonce +Other segment start (thread 2) + (thread finished, call stack no longer available) +Other segment end (thread 2) + (thread finished, call stack no longer available) -ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) +Conflicting store by thread 3 at 0x........ size 4 + at 0x........: child (tc21_pthonce.c:74) + by 0x........: vgDrd_thread_wrapper (drd_pthread_intercepts.c:?) + by 0x........: (within libpthread-?.?.so) +Allocation context: BSS section of tc21_pthonce +Other segment start (thread 2) + (thread finished, call stack no longer available) +Other segment end (thread 2) + (thread finished, call stack no longer available) + + +ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)