Merge pull request #1990 from DLR-AMR/1921-run-valgrind-check-in-ci-in-parallel

Improvement: Add possibility to use mpi with valgrind to check_valgrind scripts

Author: holke

.github/workflows/tests_cmake_valgrind.yml

@@ -104,6 +104,7 @@ jobs:
     - name: ninja install
       run: cd build && ninja install $MAKEFLAGS
     # Execute script that runs a Valgrind check for all test binaries. 
+    # We use 3 parallel processes to run the binaries in parallel with MPI.
     - name: Valgrind check
-      run: cd scripts && bash ./check_all_test_binaries_valgrind.sh
+      run: cd scripts && bash ./check_all_test_binaries_valgrind.sh --ntasks=3
 

scripts/README.md

@@ -32,11 +32,11 @@ This script indents all t8code source files at once. This script should only be
 
 #### find_all_source_files.sh
 
-List all source files of t8code in the `src/` `example/` and `test/` subfolders.
+List all source files of t8code in the `src/`, `example/` and `test/` subfolders.
 
 #### check_valgrind.sh
 
-This script runs Valgrind on a binary path provided as a parameter with specified memory leak detection flags. The Valgrind output is parsed. As a second argument, you can provide a path to a suppression file that is used by Valgrind to suppress certain errors (e.g. [valgrind_suppressions_file](valgrind_suppressions_file.supp)).
+This script runs Valgrind on a binary path provided as a parameter with specified memory leak detection flags. The Valgrind output is parsed. Using `--supp=[FILE]`, you can provide a path to a suppression file that is used by Valgrind to suppress certain errors (e.g. [valgrind_suppressions_file](valgrind_suppressions_file.supp)). With `--ntasks=[NUMBER]`, you can provide the number of processes to use with mpi (default is 1).
 
 #### find_all_test_binary_paths.sh
 
@@ -46,7 +46,7 @@ The paths are relative paths assuming an execution from the test/ folder in the
 #### check_all_test_binaries_valgrind.sh
 
 This script performs a valgrind check on each test binary found by [find_all_test_binary_paths.sh](find_all_test_binary_paths.sh).
-The valgrind check is done by [check_valgrind.sh](check_valgrind.sh). It is assumed that the build folder ../build/test/ with the correct test binaries exists.
+The valgrind check is done by [check_valgrind.sh](check_valgrind.sh). It is assumed that the build folder ../build/test/ with the correct test binaries exists. With `--ntasks=[NUMBER]`, you can provide the number of processes to use with mpi (default is 1).
 
 ## Others
 

scripts/check_all_test_binaries_valgrind.sh

@@ -26,15 +26,36 @@
 # The script returns 1 if an error is found and 0 otherwise. 
 # This script must be executed from the scripts/ folder.
 # It is assumed that the build folder ../build/test/ with the correct test binaries exists.
+# With "--ntasks=[NUMBER]", you can provide the number of processes to use with MPI (default is 1).
 #
 
+USAGE="\nUSAGE: This script executes valgrind in parallel on each test binary available. Use the syntax \n
+$0 --ntasks=[NUM_TASKS]\n 
+Providing the number of parallel processes to use with MPI is optional.\n"
+
+# Check if a number of processes is provided. If not, set to 1.
+num_procs=1
+for arg in "$@"; do
+  if [[ "$arg" == --ntasks=* ]]; then
+    ntasks_val="${arg#--ntasks=}"
+    if [[ "$ntasks_val" =~ ^[0-9]+$ ]]; then
+      num_procs="$ntasks_val"
+    else
+      echo "ERROR: --ntasks value '$ntasks_val' is not a valid number."
+      echo -e "$USAGE"
+      exit 1
+    fi
+  fi
+done
+
 # Script must be executed from the scripts/ folder.
 if [ `basename $PWD` != scripts ]; then
   if [ -d scripts ]; then
     # The directory stack is automatically reset on script exit.
     pushd scripts/ > /dev/null
   else
     echo "ERROR: scripts/ directory not found."
+    echo -e "$USAGE"
     exit 1
   fi
 fi
@@ -50,6 +71,7 @@ if [ -d ../build/test ]; then
   pushd ../build/test/ > /dev/null
 else
   echo "ERROR: Couldn't find a the directory ../build/test/."
+  echo -e "$USAGE"
   exit 1
 fi
 
@@ -59,9 +81,9 @@ valgrind_suppressions_file=../../scripts/valgrind_suppressions_file.supp
 
 for bin_path in $test_bin_paths; do
   counter=$(( $counter + 1 ))
-  echo "[$counter/$num_paths] Valgrind check of $bin_path..."
+  echo -n "[$counter/$num_paths] "
   # Run check_valgrind script for each test binary.
-  bash ../../scripts/check_valgrind.sh $bin_path $valgrind_suppressions_file 2>&1
+  bash ../../scripts/check_valgrind.sh $bin_path --supp=$valgrind_suppressions_file --ntasks=$num_procs 2>&1
   status=$?
   # If status is not 0, an error occurred.
   if test $status -ne 0; then

scripts/check_valgrind.sh

@@ -23,12 +23,18 @@
 #
 # This script runs Valgrind on an input binary paths with specified memory leak detection flags. 
 # The Valgrind output is parsed. If any errors are found, they are printed and the script exits with a status of 1.
-# As a second argument, you can provide a path to a suppression file that is used by Valgrind to suppress certain errors.
+# If errors are found, the Valgrind output is kept in the file valgrind-output.log for further inspection.
+# Using "--supp=[FILE]", you can provide a path to a suppression file that is used by Valgrind to suppress certain errors.
+# With "--ntasks=[NUMBER]", you can provide the number of processes to use with MPI (default is 1).
 #
+USAGE="\nUSAGE: This script executes valgrind in parallel on a given input file. Use \n
+$0 [FILE] --supp=[SUPPRESSION_FILE] --ntasks=[NUM_TASKS]\n
+to run valgrind on FILE. Optionally you can provide a suppression file and the number of parallel processes to use with MPI.\n"
 
 # Check that an argument is given and that the argument is a file.
 if [ ${1-x} = x ]; then
-  echo ERROR: Need to provide a file as first argument.
+  echo "ERROR: Need to provide a file as first argument."
+  echo -e "$USAGE"
   exit 1
 fi
 if [ -f "$1" ]; then
@@ -39,14 +45,47 @@ else
     FILE="../$1"
   else
     echo "ERROR: Non existing file: $1"
+    echo -e "$USAGE"
     exit 1
   fi
 fi
 
+# Check if a suppression file is provided. If yes, add the flag to incorporate the Valgrind suppression file.
+VALGRIND_FLAGS=""
+for arg in "$@"; do
+  if [[ "$arg" == --supp=* ]]; then
+    supp_file="${arg#--supp=}"
+    if [ -f "$supp_file" ]; then
+      VALGRIND_FLAGS="${VALGRIND_FLAGS} --suppressions=${supp_file}"
+    else
+      echo "ERROR: Suppression file '$supp_file' does not exist."
+      echo -e "$USAGE"
+      exit 1
+    fi
+  fi
+done
+
+# Check if a number of processes is provided. If not, set to 1.
+num_procs=1
+for arg in "$@"; do
+  if [[ "$arg" == --ntasks=* ]]; then
+    ntasks_val="${arg#--ntasks=}"
+    if [[ "$ntasks_val" =~ ^[0-9]+$ ]]; then
+      num_procs="$ntasks_val"
+    else
+      echo "ERROR: --ntasks value '$ntasks_val' is not a valid number."
+      echo -e "$USAGE"
+      exit 1
+    fi
+  fi
+done
+
+echo "Valgrind check of ${FILE} using ${num_procs} processes..."
+
 # Write valgrind output to variable OUTPUT_FILE.
 OUTPUT_FILE="valgrind-output.log"
 # Set valgrind flags.
-VALGRIND_FLAGS="--leak-check=full --track-origins=yes \
+VALGRIND_FLAGS="${VALGRIND_FLAGS} --leak-check=full --track-origins=yes \
     --trace-children=yes --show-leak-kinds=definite,indirect,possible \
     --errors-for-leak-kinds=definite,indirect,possible"
 # There are some more flags that can be reasonable to use, e.g., for debugging reasons if you found an error.
@@ -55,18 +94,8 @@ VALGRIND_FLAGS="--leak-check=full --track-origins=yes \
 # For more detailed outputs: -read-var-info=yes --read-inline-info=yes --gen-suppressions=all
 # Warning: --show-leak-kinds=all will find a lot of still reachable leaks. This is not necessarily a problem.
 
-# Check if a second argument is provided. If yes, add the flag to incorporate the Valgrind suppression file.
-if ! [ ${2-x} = x ]; then
-  if [ -f "$2" ]; then
-    VALGRIND_FLAGS="${VALGRIND_FLAGS} --suppressions=${2}"
-  else
-    echo "ERROR: If a second argument is provided, this must be a valid valgrind suppression file."
-    exit 1
-  fi
-fi
-
 # Run valgrind on given file with flags and write output to OUTPUT_FILE.
-valgrind $VALGRIND_FLAGS "${FILE}" > /dev/null 2>"${OUTPUT_FILE}"
+mpirun -n $num_procs valgrind $VALGRIND_FLAGS "${FILE}" > /dev/null 2>"${OUTPUT_FILE}"
 
 # Parse valgrind output.
 declare -a VALGRIND_RULES=(
@@ -82,6 +111,7 @@ declare -a VALGRIND_RULES=(
         "^==.*== Argument .* of function .* has a fishy (possibly negative) value: .*$"
         "^==.*== .*alloc() with size 0$"
         "^==.*== Invalid alignment value: .* (should be power of 2)$"
+        "^==.*== Conditional jump or move depends on uninitialised value(s)"
     )
 report_id=1
 status=0
@@ -122,5 +152,8 @@ while IFS= read -r line; do
   fi
 done < "${OUTPUT_FILE}"
 
-rm -f "${OUTPUT_FILE}"
+# Remove the output file only if the checks were error free.
+if [ "$status" -eq 0 ]; then
+  rm -f "${OUTPUT_FILE}"
+fi
 exit "${status}"