Add support for Valgrind's helgrind tool (#38706)

Also fix one locking issue that helgrind found: Maintain stats->lock
while stats->reference is used.

diff --git a/bin/tests/system/README b/bin/tests/system/README
index a2d73d176ddfcd6e453978f69bde4cc06be2f0e3..69a3937ac88d9d57df5f8cfa05256a9205243721 100644 (file)
--- a/bin/tests/system/README
+++ b/bin/tests/system/README
@@ -62,4 +62,6 @@ To run all the tests, just type "make test".
 When running system tests, named and lwresd can be run under
 Valgrind. The output from Valgrind are sent to per-process files that
 can be reviewed after the test has completed. To enable this, set the
-USE_VALGRIND environment variable.
+USE_VALGRIND environment variable to "helgrind" to run the Helgrind
+tool, or any other value to run the Memcheck tool. To use "helgrind"
+effectively, build BIND with --disable-atomic.

diff --git a/bin/tests/system/cleanall.sh b/bin/tests/system/cleanall.sh
index c3796982e9fb493998b7c61cb2bcfa5f74e1e07e..82c8f30863a95872a12b86c4793ae66fb67431a8 100644 (file)
--- a/bin/tests/system/cleanall.sh
+++ b/bin/tests/system/cleanall.sh
@@ -24,9 +24,10 @@ SYSTEMTESTTOP=.
 
 
 find . -type f \( \
-    -name 'K*' -o -name '*~' -o -name '*.core' -o -name '*.log' \
-    -o -name '*.pid' -o -name '*.keyset' -o -name named.run \
-    -o -name lwresd.run -o -name ans.run \) -print | xargs rm -f
+    -name 'K*' -o -name '*~' -o -name 'core' -o -name '*.core' \
+    -o -name '*.log' -o -name '*.pid' -o -name '*.keyset' \
+    -o -name named.run -o -name lwresd.run -o -name ans.run \
+    -o -name '*-valgrind-*.log' \) -print | xargs rm -f
 
 status=0
 

diff --git a/bin/tests/system/start.pl b/bin/tests/system/start.pl
index 89b37950e26e58e5fad48396bd3d3aa67f9446a4..1f9ed5c5fcb0e69c6ef09a530d593cad9614e5e5 100644 (file)
--- a/bin/tests/system/start.pl
+++ b/bin/tests/system/start.pl
@@ -150,7 +150,13 @@ sub start_server {
        if ($server =~ /^ns/) {
                $cleanup_files = "{*.jnl,*.bk,*.st,named.run}";
                if ($ENV{'USE_VALGRIND'}) {
-                       $command = "valgrind -q --gen-suppressions=all --track-origins=yes --num-callers=48 --leak-check=full --fullpath-after= --log-file=named-$server-valgrind-%p.log $NAMED -m none -M external ";
+                       $command = "valgrind -q --gen-suppressions=all --num-callers=48 --fullpath-after= --log-file=named-$server-valgrind-%p.log ";
+                       if ($ENV{'USE_VALGRIND'} eq 'helgrind') {
+                               $command .= "--tool=helgrind ";
+                       } else {
+                               $command .= "--tool=memcheck --track-origins=yes --leak-check=full ";
+                       }
+                       $command .= "$NAMED -m none -M external ";
                } else {
                        $command = "$NAMED ";
                }
@@ -200,7 +206,13 @@ sub start_server {
        } elsif ($server =~ /^lwresd/) {
                $cleanup_files = "{lwresd.run}";
                if ($ENV{'USE_VALGRIND'}) {
-                       $command = "valgrind -q --gen-suppressions=all --track-origins=yes --num-callers=48 --leak-check=full --fullpath-after= --log-file=lwresd-valgrind-%p.log $LWRESD -m none -M external ";
+                       $command = "valgrind -q --gen-suppressions=all --num-callers=48 --fullpath-after= --log-file=lwresd-valgrind-%p.log ";
+                       if ($ENV{'USE_VALGRIND'} eq 'helgrind') {
+                               $command .= "--tool=helgrind ";
+                       } else {
+                               $command .= "--tool=memcheck --track-origins=yes --leak-check=full ";
+                       }
+                       $command .= "$LWRESD -m none -M external ";
                } else {
                        $command = "$LWRESD ";
                }

diff --git a/lib/isc/stats.c b/lib/isc/stats.c
index 9d2ba0cb98273715f2c20da539d72a44a35a2a05..040b8003d833bcb8ea5b2976892fecb37c1dad70 100644 (file)
--- a/lib/isc/stats.c
+++ b/lib/isc/stats.c
@@ -169,19 +169,22 @@ isc_stats_detach(isc_stats_t **statsp) {
 
        LOCK(&stats->lock);
        stats->references--;
-       UNLOCK(&stats->lock);
 
        if (stats->references == 0) {
                isc_mem_put(stats->mctx, stats->copiedcounters,
                            sizeof(isc_stat_t) * stats->ncounters);
                isc_mem_put(stats->mctx, stats->counters,
                            sizeof(isc_stat_t) * stats->ncounters);
+               UNLOCK(&stats->lock);
                DESTROYLOCK(&stats->lock);
 #ifdef ISC_RWLOCK_USEATOMIC
                isc_rwlock_destroy(&stats->counterlock);
 #endif
                isc_mem_putanddetach(&stats->mctx, stats, sizeof(*stats));
+               return;
        }
+
+       UNLOCK(&stats->lock);
 }
 
 int