]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
Bug 494337 - All threaded applications cause still holding lock errors
authorPaul Floyd <pjfloyd@wanadoo.fr>
Sun, 3 Nov 2024 18:47:12 +0000 (19:47 +0100)
committerPaul Floyd <pjfloyd@wanadoo.fr>
Sun, 3 Nov 2024 18:47:12 +0000 (19:47 +0100)
Don't count the first call to pthread_mutex_lock() from exit().

Needed to update a couple of expecteds as exit() is now wrapped
so it turns up in the callstacks.

This is only for FreeBSD.

NEWS
helgrind/hg_intercepts.c
helgrind/hg_main.c
helgrind/tests/Makefile.am
helgrind/tests/tc04_free_lock.stderr.exp
helgrind/tests/tc04_free_lock.stderr.exp-freebsd [moved from helgrind/tests/tc04_free_lock.stderr.exp-freebsd15 with 100% similarity]
helgrind/tests/tc09_bad_unlock.stderr.exp-freebsd
helgrind/tests/tc09_bad_unlock.stderr.exp-freebsd15 [deleted file]

diff --git a/NEWS b/NEWS
index 97247050907525ce42afb4783de67f18488d3d7c..b0c0b80cfa6312afd4f69064d87b619770261490 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,7 @@ bugzilla (https://bugs.kde.org/enter_bug.cgi?product=valgrind) rather
 than mailing the developers (or mailing lists) directly -- bugs that
 are not entered into bugzilla tend to get forgotten about or ignored.
 
+494337  All threaded applications cause still holding lock errors
 495488  Add FreeBSD getrlimitusage syscall wrapper
 
 To see details of a given bug, visit
index 0c27b0a662c4a18627a8d5e88ec5cfe1a263544c..2e63dad87a3e9028206909c25b974bac17ea9256 100644 (file)
@@ -915,6 +915,48 @@ static int mutex_destroy_WRK(pthread_mutex_t *mutex)
 #  error "Unsupported OS"
 #endif
 
+#if defined(VGO_freebsd)
+
+/*
+ * Bugzilla 494337
+ *
+ * Hacks'R'Us
+ * FreeBSD 15 (backported to 14.2) add a mutex lock to
+ * exit to ensure that it is thread-safe. But this lock
+ * never gets unlocked. That meant this lock was causing
+ * all threaded apps to generate a Helgrind still holding
+ * lock errors. So in time honoured tradition, this can
+ * be fixed with a hack. This adds a wrapper to exit()
+ * that sets a global variable hg_in_exit. In the next
+ * call to pthread_mutex_lock, if that variable is 1
+ * then the pthread_mutex_lock wrapper only calls the
+ * wrapped function. It does not call the PRE and POST
+ * userreq functions. It then resets hg_in_exit to 0
+ * (because there will be more locks in the atexit
+ * processing).
+ */
+
+int hg_in_exit = 0;
+
+static int exit_WRK(int status)
+{
+   int ret;
+   OrigFn fn;
+   VALGRIND_GET_ORIG_FN(fn);
+
+   hg_in_exit = 1;
+
+   CALL_FN_W_W(ret, fn, status);
+
+   return ret;
+}
+
+LIBC_FUNC(int, exit, int res) {
+   return exit_WRK(res);
+}
+
+#endif
+
 
 //-----------------------------------------------------------
 // glibc:   pthread_mutex_lock
@@ -931,6 +973,14 @@ static int mutex_lock_WRK(pthread_mutex_t *mutex)
       fprintf(stderr, "<< pthread_mxlock %p", mutex); fflush(stderr);
    }
 
+#if defined(VGO_freebsd)
+   if (hg_in_exit) {
+      CALL_FN_W_W(ret, fn, mutex);
+      hg_in_exit = 0;
+      goto HG_MUTEX_LOCK_OUT;
+   }
+#endif
+
    DO_CREQ_v_WW(_VG_USERREQ__HG_PTHREAD_MUTEX_LOCK_PRE,
                 pthread_mutex_t*,mutex, long,0/*!isTryLock*/);
 
@@ -944,6 +994,8 @@ static int mutex_lock_WRK(pthread_mutex_t *mutex)
    DO_CREQ_v_WW(_VG_USERREQ__HG_PTHREAD_MUTEX_LOCK_POST,
                 pthread_mutex_t *, mutex, long, (ret == 0) ? True : False);
 
+HG_MUTEX_LOCK_OUT:
+
    if (ret != 0) {
       DO_PthAPIerror( "pthread_mutex_lock", ret );
    }
index 5cdabb1127dd269dce0c09729edb0891794fc0fc..6a300b83bb7bdc0294f458c20d501feb28e627ea 100644 (file)
@@ -1716,18 +1716,7 @@ void evh__pre_thread_ll_exit ( ThreadId quit_tid )
    /* Complain if this thread holds any locks. */
    nHeld = HG_(cardinalityWS)( univ_lsets, thr_q->locksetA );
    tl_assert(nHeld >= 0);
-   Bool lock_at_exit = False;
-#if defined(VGO_freebsd)
-   /* Bugzilla 494337
-    * temporary (?): turn off this check on FreeBSD 14.2+
-    * there is a lock during exit() to make it thread safe
-    * but that lock gets leaked.
-    */
-   if (VG_(getosreldate)() > 1401500) {
-      lock_at_exit = True;
-   }
-#endif
-   if (nHeld > 0 && (lock_at_exit == False)) {
+   if (nHeld > 0) {
       HChar buf[80];
       VG_(sprintf)(buf, "Exiting thread still holds %d lock%s",
                         nHeld, nHeld > 1 ? "s" : "");
index d7101ed1aa2e286c7aaf15ba0f1d6e6189f85a32..bffd5bf76ae9e178156bef7b37a353f6d8e808b0 100755 (executable)
@@ -87,7 +87,7 @@ EXTRA_DIST = \
                tc03_re_excl.stderr.exp \
        tc04_free_lock.vgtest tc04_free_lock.stdout.exp \
                tc04_free_lock.stderr.exp \
-               tc04_free_lock.stderr.exp-freebsd15 \
+               tc04_free_lock.stderr.exp-freebsd \
        tc05_simple_race.vgtest tc05_simple_race.stdout.exp \
                tc05_simple_race.stderr.exp \
        tc06_two_races.vgtest tc06_two_races.stdout.exp \
@@ -100,7 +100,6 @@ EXTRA_DIST = \
        tc09_bad_unlock.vgtest tc09_bad_unlock.stdout.exp \
                tc09_bad_unlock.stderr.exp tc09_bad_unlock.stderr.exp-solaris \
                tc09_bad_unlock.stderr.exp-freebsd \
-               tc09_bad_unlock.stderr.exp-freebsd15 \
        tc10_rec_lock.vgtest tc10_rec_lock.stdout.exp tc10_rec_lock.stderr.exp \
        tc11_XCHG.vgtest tc11_XCHG.stdout.exp tc11_XCHG.stderr.exp \
        tc12_rwl_trivial.vgtest tc12_rwl_trivial.stdout.exp \
index 772531160b915cec5f55ba978055053ce71c4794..57cbc8eeca50ad60b2623370e2210ea797e26fe3 100644 (file)
@@ -7,6 +7,9 @@ Thread #x is the program's root thread
 
 Thread #x: Exiting thread still holds 2 locks
    ...
+   by 0x........: exit_WRK (hg_intercepts.c:...)
+   by 0x........: exit (hg_intercepts.c:...)
+   ...
 
 
 ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
index 1053cbebf299762534b14b0519ad22d90cd39dbc..41df4cdcc1ee3c0097d31390f425dbeaea23957e 100644 (file)
@@ -145,6 +145,9 @@ Thread #x's call to pthread_mutex_unlock failed
 
 Thread #x: Exiting thread still holds 1 lock
    ...
+   by 0x........: exit_WRK (hg_intercepts.c:...)
+   by 0x........: exit (hg_intercepts.c:...)
+   ...
 
 
 ERROR SUMMARY: 11 errors from 11 contexts (suppressed: 0 from 0)
diff --git a/helgrind/tests/tc09_bad_unlock.stderr.exp-freebsd15 b/helgrind/tests/tc09_bad_unlock.stderr.exp-freebsd15
deleted file mode 100644 (file)
index c9d6d3d..0000000
+++ /dev/null
@@ -1,145 +0,0 @@
-
----Thread-Announcement------------------------------------------
-
-Thread #x is the program's root thread
-
-----------------------------------------------------------------
-
-Thread #x unlocked a not-locked lock at 0x........
-   at 0x........: mutex_unlock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_unlock (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:28)
-   by 0x........: main (tc09_bad_unlock.c:52)
- Lock at 0x........ was first observed
-   at 0x........: pthread_mutex_init (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:24)
-   by 0x........: main (tc09_bad_unlock.c:52)
- Address 0x........ is on thread #x's stack
- in frame #x, created by nearly_main (tc09_bad_unlock.c:17)
-
-
-----------------------------------------------------------------
-
-Thread #x's call to pthread_mutex_unlock failed
-   with error code 1 (EPERM: Operation not permitted)
-   at 0x........: mutex_unlock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_unlock (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:28)
-   by 0x........: main (tc09_bad_unlock.c:52)
-
----Thread-Announcement------------------------------------------
-
-Thread #x was created
-   ...
-   by 0x........: pthread_create@* (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:36)
-   by 0x........: main (tc09_bad_unlock.c:52)
-
-----------------------------------------------------------------
-
-Thread #x unlocked lock at 0x........ currently held by thread #x
-   at 0x........: mutex_unlock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_unlock (hg_intercepts.c:...)
-   by 0x........: child_fn (tc09_bad_unlock.c:12)
-   by 0x........: mythread_wrapper (hg_intercepts.c:...)
-   ...
- Lock at 0x........ was first observed
-   at 0x........: pthread_mutex_init (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:32)
-   by 0x........: main (tc09_bad_unlock.c:52)
- Address 0x........ is on thread #x's stack
- in frame #x, created by nearly_main (tc09_bad_unlock.c:17)
-
-
-----------------------------------------------------------------
-
-Thread #x's call to pthread_mutex_unlock failed
-   with error code 1 (EPERM: Operation not permitted)
-   at 0x........: mutex_unlock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_unlock (hg_intercepts.c:...)
-   by 0x........: child_fn (tc09_bad_unlock.c:12)
-   by 0x........: mythread_wrapper (hg_intercepts.c:...)
-   ...
-
----------------------
-----------------------------------------------------------------
-
-Thread #x unlocked a not-locked lock at 0x........
-   at 0x........: mutex_unlock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_unlock (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:28)
-   by 0x........: main (tc09_bad_unlock.c:53)
- Lock at 0x........ was first observed
-   at 0x........: pthread_mutex_init (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:24)
-   by 0x........: main (tc09_bad_unlock.c:52)
- Address 0x........ is on thread #x's stack
- in frame #x, created by nearly_main (tc09_bad_unlock.c:17)
-
-
-----------------------------------------------------------------
-
-Thread #x's call to pthread_mutex_unlock failed
-   with error code 1 (EPERM: Operation not permitted)
-   at 0x........: mutex_unlock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_unlock (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:28)
-   by 0x........: main (tc09_bad_unlock.c:53)
-
-----------------------------------------------------------------
-
-Thread #x: Attempt to re-lock a non-recursive lock I already hold
-   at 0x........: mutex_lock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_lock (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:33)
-   by 0x........: main (tc09_bad_unlock.c:53)
- Lock was previously acquired
-   at 0x........: mutex_lock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_lock (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:33)
-   by 0x........: main (tc09_bad_unlock.c:52)
-
-----------------------------------------------------------------
-
-Thread #x: Bug in libpthread: recursive write lock granted on mutex/wrlock which does not support recursion
-   at 0x........: mutex_lock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_lock (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:33)
-   by 0x........: main (tc09_bad_unlock.c:53)
-
----Thread-Announcement------------------------------------------
-
-Thread #x was created
-   ...
-   by 0x........: pthread_create@* (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:36)
-   by 0x........: main (tc09_bad_unlock.c:53)
-
-----------------------------------------------------------------
-
-Thread #x unlocked lock at 0x........ currently held by thread #x
-   at 0x........: mutex_unlock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_unlock (hg_intercepts.c:...)
-   by 0x........: child_fn (tc09_bad_unlock.c:12)
-   by 0x........: mythread_wrapper (hg_intercepts.c:...)
-   ...
- Lock at 0x........ was first observed
-   at 0x........: pthread_mutex_init (hg_intercepts.c:...)
-   by 0x........: nearly_main (tc09_bad_unlock.c:32)
-   by 0x........: main (tc09_bad_unlock.c:52)
- Address 0x........ is on thread #x's stack
- in frame #x, created by nearly_main (tc09_bad_unlock.c:17)
-
-
-----------------------------------------------------------------
-
-Thread #x's call to pthread_mutex_unlock failed
-   with error code 1 (EPERM: Operation not permitted)
-   at 0x........: mutex_unlock_WRK (hg_intercepts.c:...)
-   by 0x........: pthread_mutex_unlock (hg_intercepts.c:...)
-   by 0x........: child_fn (tc09_bad_unlock.c:12)
-   by 0x........: mythread_wrapper (hg_intercepts.c:...)
-   ...
-
-
-ERROR SUMMARY: 10 errors from 10 contexts (suppressed: 0 from 0)