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

diff --git a/exp-drd/drd_cond.c b/exp-drd/drd_cond.c
index 11e962fab5e877a1042ba180d7be51906f850080..46b994e6730fbff1c9d8d6d2ae35011832373542 100644 (file)
--- 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 23880f53b9746f547441a7e016fd3f5591afcf53..abe3f4896ec332ed955ac5aa53bc8ed5de3d3453 100644 (file)
--- 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 bb198a168b9dbb1c0957f34bc101c0f684e0e839..e4f35815b691e9f25aae5f3544cd3bb5caedd8f7 100644 (file)
--- 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 da89d8554f99462c3af1b5492e7dbf41e363884d..70e15570b694e855d96ce324c5b086fcd4e0e907 100644 (file)
--- 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=<n> 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 5cbd70f2430576149339bde90f25918c2209cfe1..2b78cbab8c992cf24add09fe19280038994c9040 100644 (file)
--- 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 d18786f80668a209115b4a13cf5e8afa8d9cd471..6cea56a3b93964dda24970bd746b5c6263eadae5 100644 (file)
--- 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 (file)
index 0000000..d18786f
--- /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 (file)
index 0000000..8e6b9cc
--- /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 ca0ec2ba30f5c973c0f626a6712985f81c073771..b64e170388272acf61c48ed7b6b8603818e9eade 100644 (file)
--- 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)