configure: Optionally pass --track-destroy to helgrind

Message ID 20260702233226.50725-1-amerey@redhat.com
State New
Headers
Series configure: Optionally pass --track-destroy to helgrind |

Commit Message

Aaron Merey July 2, 2026, 11:32 p.m. UTC
  Helgrind in Valgrind >= 3.27 has a --track-destroy option that reports
missing pthread_mutex_destroy/pthread_rwlock_destroy calls and warns
when pthread_*_init is called on an address that still has a live lock
record.  This is useful for auditing lock lifecycles.

Keep it off by default. Helgrind emits these reports as advisory
messages and not errors.  Calling pthread_*_destroy is not strictly
necessary and does not always indicate an error. For example
heap-allocated std::mutex instances are never pthread_mutex_destroy'd
by libstdc++, so allocator memory reuse produces benign re-init
reports (this may occur during libabigail testruns).

	* configure.ac: Add --enable-helgrind-track-destroy=yes|no|all,
	off by default.
	* autoconf-archive/ax_valgrind_check.m4
	(VALGRIND_helgrind_FLAGS): Append --track-destroy to the default
	helgrind flags if it was enabled.

Signed-off-by: Aaron Merey <amerey@redhat.com>
---
 autoconf-archive/ax_valgrind_check.m4 |  2 +-
 configure.ac                          | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)
  

Comments

Dodji Seketeli July 7, 2026, 4:08 p.m. UTC | #1
Hello Aaron,

Aaron Merey <amerey@redhat.com> a écrit:

> Helgrind in Valgrind >= 3.27 has a --track-destroy option that reports
> missing pthread_mutex_destroy/pthread_rwlock_destroy calls and warns
> when pthread_*_init is called on an address that still has a live lock
> record.  This is useful for auditing lock lifecycles.
>
> Keep it off by default. Helgrind emits these reports as advisory
> messages and not errors.  Calling pthread_*_destroy is not strictly
> necessary and does not always indicate an error. For example
> heap-allocated std::mutex instances are never pthread_mutex_destroy'd
> by libstdc++, so allocator memory reuse produces benign re-init
> reports (this may occur during libabigail testruns).

Thanks for the patch which obviously makes complete sense to me.

>
> 	* configure.ac: Add --enable-helgrind-track-destroy=yes|no|all,
> 	off by default.
> 	* autoconf-archive/ax_valgrind_check.m4
> 	(VALGRIND_helgrind_FLAGS): Append --track-destroy to the default
> 	helgrind flags if it was enabled.

I am presuming this patch is intended for the multithreading branch,
right?

I am asking because after all, the patch could be useful also for
tools/abipkgdiff.cc, even in the master branch.  I'd then re-base the
multithreading branch on top of master.

Which one you'd prefer.

[...]

Cheers,
  
Dodji Seketeli July 10, 2026, 1:12 p.m. UTC | #2
Hello,

Dodji Seketeli <dodji@seketeli.org> a écrit:

>>
>> 	* configure.ac: Add --enable-helgrind-track-destroy=yes|no|all,
>> 	off by default.
>> 	* autoconf-archive/ax_valgrind_check.m4
>> 	(VALGRIND_helgrind_FLAGS): Append --track-destroy to the default
>> 	helgrind flags if it was enabled.
>
> I am presuming this patch is intended for the multithreading branch,
> right?
>
> I am asking because after all, the patch could be useful also for
> tools/abipkgdiff.cc, even in the master branch.  I'd then re-base the
> multithreading branch on top of master.
>
> Which one you'd prefer.

Finally, I have applied it to the multithreading branch at
https://sourceware.org/cgit/libabigail/commit/?h=users/dodji/multithreading&id=023c6e7f89c8899c886a8ce1412e5c66b1a7eb1f.
The patch wasn't applying cleanly to the mainline so that made the
difference ;-)

Thanks again!

[...]
  

Patch

diff --git a/autoconf-archive/ax_valgrind_check.m4 b/autoconf-archive/ax_valgrind_check.m4
index be834904..9ef740aa 100644
--- a/autoconf-archive/ax_valgrind_check.m4
+++ b/autoconf-archive/ax_valgrind_check.m4
@@ -159,7 +159,7 @@  m4_if(m4_defn([en_dflt_valgrind_]vgtool), [off], [= "yes"], [!= "no"]),[
 VALGRIND_SUPPRESSIONS ?= $(addprefix --suppressions=,$(VALGRIND_SUPPRESSIONS_FILES))
 VALGRIND_FLAGS ?= --num-callers=30
 VALGRIND_memcheck_FLAGS ?= --leak-check=full --show-reachable=no
-VALGRIND_helgrind_FLAGS ?= --history-level=approx
+VALGRIND_helgrind_FLAGS ?= --history-level=approx $(HELGRIND_TRACK_DESTROY)
 VALGRIND_drd_FLAGS ?=
 VALGRIND_sgcheck_FLAGS ?=
 
diff --git a/configure.ac b/configure.ac
index 7aed81f4..52f9a041 100644
--- a/configure.ac
+++ b/configure.ac
@@ -991,6 +991,31 @@  AX_VALGRIND_DFLT(sgcheck, off)
 
 AX_VALGRIND_CHECK
 
+AC_ARG_ENABLE(helgrind-track-destroy,
+	      AS_HELP_STRING([--enable-helgrind-track-destroy=yes|no|all],
+			     [have "make check-valgrind-helgrind" run helgrind with --track-destroy]),
+	      ENABLE_HELGRIND_TRACK_DESTROY=$enableval,
+	      ENABLE_HELGRIND_TRACK_DESTROY=no)
+
+if test x$ENABLE_HELGRIND_TRACK_DESTROY != xyes \
+	-a x$ENABLE_HELGRIND_TRACK_DESTROY != xno \
+	-a x$ENABLE_HELGRIND_TRACK_DESTROY != xall; then
+   AC_MSG_ERROR([bad value --enable-helgrind-track-destroy; use yes, no or all])
+fi
+
+HELGRIND_TRACK_DESTROY=
+if test x$ENABLE_HELGRIND_TRACK_DESTROY != xno; then
+   if test x$enable_valgrind_helgrind != xyes; then
+      AC_MSG_ERROR([--enable-helgrind-track-destroy needs helgrind; reconfigure with --enable-valgrind])
+   fi
+   if $VALGRIND --tool=helgrind --help 2>/dev/null | grep -qe "--track-destroy"; then
+      HELGRIND_TRACK_DESTROY="--track-destroy=$ENABLE_HELGRIND_TRACK_DESTROY"
+   else
+      AC_MSG_ERROR([helgrind does not support --track-destroy])
+   fi
+fi
+AC_SUBST(HELGRIND_TRACK_DESTROY)
+
 
 if test x$ENABLE_MULTITHREADING != xno; then
 saved_CPPFLAGS="$CPPFLAGS"
@@ -1742,6 +1767,7 @@  AC_MSG_NOTICE([
     Enable BTF front-end                           : ${ENABLE_BTF}
     Enable Big Libabigail tests                    : ${ENABLE_BIG_TESTS}
     Enable running tests under Valgrind            : ${enable_valgrind}
+    Enable helgrind --track-destroy                : ${ENABLE_HELGRIND_TRACK_DESTROY}
     Enable build with -fsanitize=address    	   : ${ENABLE_ASAN}
     Enable build with -fsanitize=memory    	   : ${ENABLE_MSAN}
     Enable build with -fsanitize=thread    	   : ${ENABLE_TSAN}