Bug 502871 - Make Helgrind "pthread_cond_{signal,broadcast}: dubious: associated...

diff --git a/NEWS b/NEWS
index afdbacd331e208f5854772fe6b2378392dc34b2a..b6b822202d07103e23d200b2f42c5e2aa801e11e 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,15 @@ AMD64/macOS 10.13 and nanoMIPS/Linux.
   And when the stdin/out/err descriptors are reassigned they are
   now treated as normal (non-inherited) file descriptors.
 
+* Helgrind:
+  There is a change to warnings about calls to pthread_cond_signal and
+  pthread_cond_broadcast when the associated mutex is unlocked. Previously
+  Helgrind would always warn about this. Now this error is controlled by
+  a command line option, --check-cond-signal-mutex=yes|no. The default is
+  no. This change has been made because some C and C++ standard libraries
+  use pthread_cond_signal/pthread_cond_broadcast in this way. Users are
+  obliged to use suppressions if they wish to avoid this noise.
+
 * ==================== FIXED BUGS ====================
 
 The following bugs have been fixed or resolved.  Note that "n-i-bz"
@@ -80,6 +89,8 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 502288  s390x: Memcheck false positives with NNPA last tensor dimension
 502324  s390x: Memcheck false positives with TMxx and TM/TMY
 502679  Use LTP for testing valgrind
+502871  Make Helgrind "pthread_cond_{signal,broadcast}: dubious: associated
+        lock is not held by any thread" optional
 
 To see details of a given bug, visit
   https://bugs.kde.org/show_bug.cgi?id=XXXXXX

diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c
index 6a300b83bb7bdc0294f458c20d501feb28e627ea..135c4d3b9896675ce8b04f0a93ca41d4aec89c32 100644 (file)
--- a/helgrind/hg_main.c
+++ b/helgrind/hg_main.c
@@ -165,6 +165,7 @@ Bool HG_(clo_ignore_thread_creation) = True;
 #else
 Bool HG_(clo_ignore_thread_creation) = False;
 #endif /* VGO_solaris */
+Bool HG_(clo_check_cond_signal_mutex) = False;
 
 static
 ThreadId map_threads_maybe_reverse_lookup_SLOW ( Thread* thr ); /*fwds*/
@@ -2454,7 +2455,7 @@ static void evh__HG_PTHREAD_COND_SIGNAL_PRE ( ThreadId tid, void* cond )
             HG_(record_error_Misc)(thr,
                "pthread_cond_{signal,broadcast}: associated lock is a rwlock");
          }
-         if (lk->heldBy == NULL) {
+         if (HG_(clo_check_cond_signal_mutex) && lk->heldBy == NULL) {
             HG_(record_error_Dubious)(thr,
                "pthread_cond_{signal,broadcast}: dubious: "
                "associated lock is not held by any thread");
@@ -5840,7 +5841,8 @@ static Bool hg_process_cmd_line_option ( const HChar* arg )
                             HG_(clo_check_stack_refs)) {}
    else if VG_BOOL_CLO(arg, "--ignore-thread-creation",
                             HG_(clo_ignore_thread_creation)) {}
-
+   else if VG_BOOL_CLO(arg, "--check-cond-signal-mutex",
+                            HG_(clo_check_cond_signal_mutex)) {}
    else
       return VG_(replacement_malloc_process_cmd_line_option)(arg);
 
@@ -5859,14 +5861,19 @@ static void hg_print_usage ( void )
 "    --history-backtrace-size=<number>  record <number> callers for full\n"
 "        history level [8]\n"
 "    --delta-stacktrace=no|yes [yes on linux amd64/x86]\n"
-"        no : always compute a full history stacktrace from unwind info\n"
-"        yes : derive a stacktrace from the previous stacktrace\n"
+"        no: always compute a full history stacktrace from unwind info\n"
+"        yes: derive a stacktrace from the previous stacktrace\n"
 "          if there was no call/return or similar instruction\n"
 "    --conflict-cache-size=N   size of 'full' history cache [2000000]\n"
 "    --check-stack-refs=no|yes race-check reads and writes on the\n"
 "                              main stack and thread stacks? [yes]\n"
 "    --ignore-thread-creation=yes|no Ignore activities during thread\n"
-"                              creation [%s]\n",
+"                              creation [%s]\n"""
+"    --check-cond-signal-mutex=yes|no [no]\n"
+"        no: do not check that the associated mutex is locked for calls\n"
+"          to pthread_cond_{signal,broadcast}\n"
+"        yes: generate 'dubious' error messages if the associated mutex\n"
+"          is unlocked\n",
 HG_(clo_ignore_thread_creation) ? "yes" : "no"
    );
 }

diff --git a/helgrind/tests/bug392331.vgtest b/helgrind/tests/bug392331.vgtest
index 3889b15f77c45f7f92f11b102a786d5f62aa95ae..f300c5cc13b58a04c0338a81ded5371f4d86d6c7 100644 (file)
--- a/helgrind/tests/bug392331.vgtest
+++ b/helgrind/tests/bug392331.vgtest
@@ -1,4 +1,4 @@
 prereq: test -e bug392331
-vgopts: -q
+vgopts: -q --check-cond-signal-mutex=yes
 prog: bug392331
 stderr_filter: filter_bug392331

diff --git a/helgrind/tests/tc23_bogus_condwait.stderr.exp b/helgrind/tests/tc23_bogus_condwait.stderr.exp
index e1611a5d8f85ffc4960c064bceb925f9ed2e9aef..84289f1684759899346e9cca8ed29649b406a269 100644 (file)
--- a/helgrind/tests/tc23_bogus_condwait.stderr.exp
+++ b/helgrind/tests/tc23_bogus_condwait.stderr.exp
@@ -1,4 +1,3 @@
-
 ---Thread-Announcement------------------------------------------
 
 Thread #x is the program's root thread
@@ -52,5 +51,3 @@ Thread #x: pthread_cond_{timed}wait: cond is associated with a different mutex
    by 0x........: pthread_cond_wait@* (hg_intercepts.c:...)
    by 0x........: main (tc23_bogus_condwait.c:85)
 
-
-ERROR SUMMARY: 7 errors from 7 contexts (suppressed: 0 from 0)

diff --git a/helgrind/tests/tc23_bogus_condwait.stderr.exp-freebsd b/helgrind/tests/tc23_bogus_condwait.stderr.exp-freebsd
index 1eee38a90ea62c01a11febfa7b1be1b4301f8460..562bf71ebe3a61829b82bcf73685405b66510015 100644 (file)
--- a/helgrind/tests/tc23_bogus_condwait.stderr.exp-freebsd
+++ b/helgrind/tests/tc23_bogus_condwait.stderr.exp-freebsd
@@ -1,4 +1,3 @@
-
 ---Thread-Announcement------------------------------------------
 
 Thread #x is the program's root thread
@@ -96,5 +95,3 @@ Thread #x: pthread_cond_{signal,broadcast}: dubious: associated lock is not held
    by 0x........: mythread_wrapper (hg_intercepts.c:...)
    ...
 
-
-ERROR SUMMARY: 11 errors from 11 contexts (suppressed: 0 from 0)

diff --git a/helgrind/tests/tc23_bogus_condwait.stderr.exp-mips32 b/helgrind/tests/tc23_bogus_condwait.stderr.exp-mips32
index 6ed9ef8239c5aecfa583c95f9b0bc06d53316b43..8ce9de66c2225434d66487aab88210cd197a9e7e 100644 (file)
--- a/helgrind/tests/tc23_bogus_condwait.stderr.exp-mips32
+++ b/helgrind/tests/tc23_bogus_condwait.stderr.exp-mips32
@@ -1,4 +1,3 @@
-
 ---Thread-Announcement------------------------------------------
 
 Thread #x is the program's root thread
@@ -60,5 +59,3 @@ Thread #x: pthread_cond_{timed}wait: cond is associated with a different mutex
    by 0x........: pthread_cond_wait@* (hg_intercepts.c:...)
    by 0x........: main (tc23_bogus_condwait.c:78)
 
-
-ERROR SUMMARY: 8 errors from 8 contexts (suppressed: 0 from 0)

diff --git a/helgrind/tests/tc23_bogus_condwait.vgtest b/helgrind/tests/tc23_bogus_condwait.vgtest
index 31c3ed8b9a8d5d1bebc2e3e578ab0e65bae4bdf5..918cd86e4cfe841f4210a8944fa49d1e5c5a211e 100644 (file)
--- a/helgrind/tests/tc23_bogus_condwait.vgtest
+++ b/helgrind/tests/tc23_bogus_condwait.vgtest
@@ -1 +1,3 @@
 prog: tc23_bogus_condwait
+vgopts: -q  --check-cond-signal-mutex=yes
+