From 806d5abe6df3e96d4dac521cc4349e89d89c36c5 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sat, 28 Jun 2008 13:01:30 +0000 Subject: [PATCH] Some time ago reporting sending POSIX signals where the mutex associated with the signal via pthread_cond_wait()/pthread_cond_timedwait() was disabled. Reenabled this report, made it configurable, and added a regression test for the new command line option. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@8295 --- exp-drd/drd_cond.c | 28 ++++++++------- exp-drd/drd_cond.h | 17 +++++---- exp-drd/drd_error.c | 2 +- exp-drd/drd_main.c | 7 +++- exp-drd/tests/Makefile.am | 2 ++ exp-drd/tests/pth_cond_race.stderr.exp | 9 ++++- exp-drd/tests/pth_cond_race3.stderr.exp | 3 ++ exp-drd/tests/pth_cond_race3.vgtest | 3 ++ exp-drd/tests/tc23_bogus_condwait.stderr.exp | 37 +++++++++++++++++++- 9 files changed, 86 insertions(+), 22 deletions(-) create mode 100644 exp-drd/tests/pth_cond_race3.stderr.exp create mode 100644 exp-drd/tests/pth_cond_race3.vgtest diff --git a/exp-drd/drd_cond.c b/exp-drd/drd_cond.c index 11e962fab5..46b994e673 100644 --- a/exp-drd/drd_cond.c +++ b/exp-drd/drd_cond.c @@ -28,25 +28,30 @@ #include "drd_error.h" #include "drd_mutex.h" #include "drd_suppression.h" -#include "pub_tool_errormgr.h" // VG_(maybe_record_error)() -#include "pub_tool_libcassert.h" // tl_assert() -#include "pub_tool_libcprint.h" // VG_(printf)() -#include "pub_tool_machine.h" // VG_(get_IP)() -#include "pub_tool_options.h" // VG_(clo_backtrace_size) -#include "pub_tool_threadstate.h" // VG_(get_running_tid)() +#include "pub_tool_errormgr.h" /* VG_(maybe_record_error)() */ +#include "pub_tool_libcassert.h" /* tl_assert() */ +#include "pub_tool_libcprint.h" /* VG_(printf)() */ +#include "pub_tool_machine.h" /* VG_(get_IP)() */ +#include "pub_tool_options.h" /* VG_(clo_backtrace_size) */ +#include "pub_tool_threadstate.h" /* VG_(get_running_tid)() */ -// Local functions. +/* Local functions. */ static void cond_cleanup(struct cond_info* p); -// Local variables. +/* Global variables. */ + +Bool s_drd_report_signal_unlocked = True; + + +/* Local variables. */ static Bool s_trace_cond; -// Function definitions. +/* Function definitions. */ void cond_set_trace(const Bool trace_cond) { @@ -243,11 +248,11 @@ static void cond_signal(Addr const cond) if (cond_p && cond_p->waiter_count > 0) { - if (! mutex_is_locked_by(cond_p->mutex, drd_tid)) + if (s_drd_report_signal_unlocked + && ! mutex_is_locked_by(cond_p->mutex, drd_tid)) { /* A signal is sent while the associated mutex has not been locked. */ /* This can indicate but is not necessarily a race condition. */ -#if 0 CondRaceErrInfo cei; cei.cond = cond; cei.mutex = cond_p->mutex; @@ -256,7 +261,6 @@ static void cond_signal(Addr const cond) VG_(get_IP)(vg_tid), "CondErr", &cei); -#endif } } else diff --git a/exp-drd/drd_cond.h b/exp-drd/drd_cond.h index 23880f53b9..abe3f4896e 100644 --- a/exp-drd/drd_cond.h +++ b/exp-drd/drd_cond.h @@ -23,21 +23,26 @@ */ -// Condition variable state information: mutex specified in pthread_cond_wait() -// call. - - #ifndef __DRD_COND_H #define __DRD_COND_H -#include "drd_thread.h" // DrdThreadid -#include "pub_tool_basics.h" // Addr +#include "drd_thread.h" /* DrdThreadid */ +#include "pub_tool_basics.h" /* Addr */ +/* Forward declarations. */ + struct cond_info; +/* Global variables. */ + +extern Bool s_drd_report_signal_unlocked; + + +/* Function declarations. */ + void cond_set_trace(const Bool trace_cond); void cond_pre_init(const Addr cond); void cond_post_destroy(const Addr cond); diff --git a/exp-drd/drd_error.c b/exp-drd/drd_error.c index bb198a168b..e4f35815b6 100644 --- a/exp-drd/drd_error.c +++ b/exp-drd/drd_error.c @@ -170,7 +170,7 @@ static void drd_tool_error_pp(Error* const e) case CondRaceErr: { CondRaceErrInfo* cei = (CondRaceErrInfo*)(VG_(get_error_extra)(e)); VG_(message)(Vg_UserMsg, - "Race condition: condition variable 0x%lx has been" + "Probably a race condition: condition variable 0x%lx has been" " signalled but the associated mutex 0x%lx is not locked" " by the signalling thread", cei->cond, cei->mutex); diff --git a/exp-drd/drd_main.c b/exp-drd/drd_main.c index da89d8554f..70e15570b6 100644 --- a/exp-drd/drd_main.c +++ b/exp-drd/drd_main.c @@ -103,14 +103,15 @@ static Bool drd_process_cmd_line_option(Char* arg) VG_BOOL_CLO (arg, "--check-stack-var", s_drd_check_stack_accesses) else VG_BOOL_CLO(arg, "--drd-stats", s_drd_print_stats) + else VG_BOOL_CLO(arg,"--report-signal-unlocked",s_drd_report_signal_unlocked) else VG_BOOL_CLO(arg, "--segment-merging", segment_merging) else VG_BOOL_CLO(arg, "--show-confl-seg", show_confl_seg) else VG_BOOL_CLO(arg, "--show-stack-usage", s_show_stack_usage) else VG_BOOL_CLO(arg, "--trace-barrier", trace_barrier) else VG_BOOL_CLO(arg, "--trace-clientobj", trace_clientobj) else VG_BOOL_CLO(arg, "--trace-cond", trace_cond) - else VG_BOOL_CLO(arg, "--trace-csw", trace_csw) else VG_BOOL_CLO(arg, "--trace-conflict-set", trace_conflict_set) + else VG_BOOL_CLO(arg, "--trace-csw", trace_csw) else VG_BOOL_CLO(arg, "--trace-fork-join", s_drd_trace_fork_join) else VG_BOOL_CLO(arg, "--trace-mutex", trace_mutex) else VG_BOOL_CLO(arg, "--trace-rwlock", trace_rwlock) @@ -173,6 +174,10 @@ static void drd_print_usage(void) " stack variables [no].\n" " --exclusive-threshold= Print an error message if any mutex or\n" " writer lock is held longer than the specified time (in milliseconds).\n" +" --report-signal-unlocked=yes|no Whether to report calls to\n" +" pthread_cond_signal() where the mutex associated\n" +" with the signal via pthread_cond_wait() is not\n" +" locked at the time the signal is sent [yes].\n" " --segment-merging=yes|no Controls segment merging [yes].\n" " Segment merging is an algorithm to limit memory usage of the\n" " data race detection algorithm. Disabling segment merging may\n" diff --git a/exp-drd/tests/Makefile.am b/exp-drd/tests/Makefile.am index 5cbd70f243..2b78cbab8c 100644 --- a/exp-drd/tests/Makefile.am +++ b/exp-drd/tests/Makefile.am @@ -69,6 +69,8 @@ EXTRA_DIST = \ pth_cond_race.vgtest \ pth_cond_race2.stderr.exp \ pth_cond_race2.vgtest \ + pth_cond_race3.stderr.exp \ + pth_cond_race3.vgtest \ pth_create_chain.stderr.exp \ pth_create_chain.vgtest \ pth_detached.stderr.exp \ diff --git a/exp-drd/tests/pth_cond_race.stderr.exp b/exp-drd/tests/pth_cond_race.stderr.exp index d18786f806..6cea56a3b9 100644 --- a/exp-drd/tests/pth_cond_race.stderr.exp +++ b/exp-drd/tests/pth_cond_race.stderr.exp @@ -1,3 +1,10 @@ +Thread 2: +Probably a race condition: condition variable 0x........ has been signalled but the associated mutex 0x........ is not locked by the signalling thread + at 0x........: pthread_cond_signal* (drd_pthread_intercepts.c:?) + by 0x........: thread_func (pth_cond_race.c:?) + by 0x........: vg_thread_wrapper (drd_pthread_intercepts.c:?) + by 0x........: (within libpthread-?.?.so) + by 0x........: clone (in /...libc...) -ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) +ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) diff --git a/exp-drd/tests/pth_cond_race3.stderr.exp b/exp-drd/tests/pth_cond_race3.stderr.exp new file mode 100644 index 0000000000..d18786f806 --- /dev/null +++ b/exp-drd/tests/pth_cond_race3.stderr.exp @@ -0,0 +1,3 @@ + + +ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) diff --git a/exp-drd/tests/pth_cond_race3.vgtest b/exp-drd/tests/pth_cond_race3.vgtest new file mode 100644 index 0000000000..8e6b9cc96f --- /dev/null +++ b/exp-drd/tests/pth_cond_race3.vgtest @@ -0,0 +1,3 @@ +prereq: ./supported_libpthread +prog: pth_cond_race +vgopts: --report-signal-unlocked=no diff --git a/exp-drd/tests/tc23_bogus_condwait.stderr.exp b/exp-drd/tests/tc23_bogus_condwait.stderr.exp index ca0ec2ba30..b64e170388 100644 --- a/exp-drd/tests/tc23_bogus_condwait.stderr.exp +++ b/exp-drd/tests/tc23_bogus_condwait.stderr.exp @@ -3,17 +3,52 @@ The object at address 0x........ is not a mutex. at 0x........: pthread_cond_wait* (drd_pthread_intercepts.c:?) by 0x........: main (tc23_bogus_condwait.c:69) +Thread 3: +Probably a race condition: condition variable 0x........ has been signalled but the associated mutex 0x........ is not locked by the signalling thread + at 0x........: pthread_cond_signal* (drd_pthread_intercepts.c:?) + by 0x........: rescue_me (tc23_bogus_condwait.c:20) + by 0x........: vg_thread_wrapper (drd_pthread_intercepts.c:?) + by 0x........: (within libpthread-?.?.so) + by 0x........: clone (in /...libc...) + +Thread 1: Mutex not locked: mutex 0x........, recursion count 0, owner 0. at 0x........: pthread_cond_wait* (drd_pthread_intercepts.c:?) by 0x........: main (tc23_bogus_condwait.c:72) +Thread 3: +Probably a race condition: condition variable 0x........ has been signalled but the associated mutex 0x........ is not locked by the signalling thread + at 0x........: pthread_cond_signal* (drd_pthread_intercepts.c:?) + by 0x........: rescue_me (tc23_bogus_condwait.c:24) + by 0x........: vg_thread_wrapper (drd_pthread_intercepts.c:?) + by 0x........: (within libpthread-?.?.so) + by 0x........: clone (in /...libc...) + +Thread 1: The object at address 0x........ is not a mutex. at 0x........: pthread_cond_wait* (drd_pthread_intercepts.c:?) by 0x........: main (tc23_bogus_condwait.c:75) +Thread 3: +Probably a race condition: condition variable 0x........ has been signalled but the associated mutex 0x........ is not locked by the signalling thread + at 0x........: pthread_cond_signal* (drd_pthread_intercepts.c:?) + by 0x........: rescue_me (tc23_bogus_condwait.c:28) + by 0x........: vg_thread_wrapper (drd_pthread_intercepts.c:?) + by 0x........: (within libpthread-?.?.so) + by 0x........: clone (in /...libc...) + +Thread 1: Mutex not locked by calling thread: mutex 0x........, recursion count 1, owner 2. at 0x........: pthread_cond_wait* (drd_pthread_intercepts.c:?) by 0x........: main (tc23_bogus_condwait.c:78) + +Thread 3: +Probably a race condition: condition variable 0x........ has been signalled but the associated mutex 0x........ is not locked by the signalling thread + at 0x........: pthread_cond_signal* (drd_pthread_intercepts.c:?) + by 0x........: rescue_me (tc23_bogus_condwait.c:32) + by 0x........: vg_thread_wrapper (drd_pthread_intercepts.c:?) + by 0x........: (within libpthread-?.?.so) + by 0x........: clone (in /...libc...) The impossible happened: mutex 0x........ is locked simultaneously by two threads (recursion count 1, owners 2 and 1) ! Thread 2: @@ -24,4 +59,4 @@ Mutex not locked by calling thread: mutex 0x........, recursion count 2, owner 1 by 0x........: (within libpthread-?.?.so) by 0x........: clone (in /...libc...) -ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 0 from 0) +ERROR SUMMARY: 9 errors from 9 contexts (suppressed: 0 from 0) -- 2.47.2