Skip to content

Commit

Permalink
tests/valgrind: improve handling for fork() and exec()
Browse files Browse the repository at this point in the history
- use trace-children=yes.  (This actually means "trace exec"; the
  valgrind man page even admits that the option is badly named!)

- allow our tests to only check the parent pid

In more detail, in our tests/??-*.sh, we would set up:

    scenario_cmd() {
        ...
        c_valgrind_cmd=$(valgrind_setup_cmd "valgrind-parent")
        ${c_valgrind_cmd} ./test_something_with_forks

If we have two pids from a single binary, then our valgrind test will
assume that we're using daemonize() and ignore any leaks from the parent
pid.  (This is exactly what we want for daemonize().)

However, if we use fork() but keep the parent, then we want the
opposite: we should check for leaks in the parent, and ignore leaks from
the children.

We need to explicitly add something to the log filename beacuse if we
didn't, then a valgrind check with two pids wouldn't know whether to
complain about leaks in the parent or child.
  • Loading branch information
gperciva committed Sep 20, 2023
1 parent e5871e6 commit 38e07b4
Showing 1 changed file with 25 additions and 4 deletions.
29 changes: 25 additions & 4 deletions tests/shared_valgrind_functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ set -o noclobber -o nounset
# - valgrind_init():
# Clear previous valgrind output, and prepare for running valgrind tests
# (if applicable).
# - valgrind_setup_cmd():
# - valgrind_setup_cmd(str):
# Set up the valgrind command if ${USE_VALGRIND} is greater than or equal to
# ${valgrind_min}.
# ${valgrind_min}. If ${str} is not blank, include it in the log filename.
# - valgrind_check_basenames(exitfile):
# Check for any memory leaks recorded in valgrind logfiles associated with a
# test exitfile. Return the filename if there's a leak; otherwise return an
Expand Down Expand Up @@ -203,6 +203,7 @@ _valgrind_ensure_suppression() {
printf "\n" | (valgrind \
--leak-check=full --show-leak-kinds=all \
--gen-suppressions=all \
--trace-children=yes \
--suppressions="${valgrind_suppressions}" \
--log-file="${this_valgrind_supp}" \
"${potential_memleaks_binary}" \
Expand All @@ -224,17 +225,27 @@ _valgrind_ensure_suppression() {

## valgrind_setup_cmd ():
# Set up the valgrind command if ${USE_VALGRIND} is greater than or equal to
# ${valgrind_min}.
# ${valgrind_min}. If ${str} is not blank, include it in the log filename.
valgrind_setup_cmd() {
str=${1:-}

# Bail if we don't want to use valgrind for this check.
if [ "${USE_VALGRIND}" -lt "${c_valgrind_min}" ]; then
return
fi

val_logfilename="${s_val_basename}-${c_count_str}-%p.log"
# Set up the log filename.
if [ -n "${str}" ]; then
val_logfilename="${s_val_basename}-${c_count_str}-${str}-%p.log"
else
val_logfilename="${s_val_basename}-${c_count_str}-%p.log"
fi

# Set up valgrind command.
c_valgrind_cmd="valgrind \
--log-file=${val_logfilename} \
--track-fds=yes \
--trace-children=yes \
--leak-check=full --show-leak-kinds=all \
--errors-for-leak-kinds=all \
--suppressions=${valgrind_suppressions}"
Expand Down Expand Up @@ -345,6 +356,16 @@ valgrind_check_basenames() {
return
fi

# If the valgrind logfiles contain "-valgrind-parent-", then we only
# want to check the parent (the lowest pid).
for logfile in ${logfiles} ; do
if [ "${logfile#*-valgrind-parent-}" != "${logfile}" ]; then
# Only check the parent
valgrind_check_logfile "${logfile}"
return "$?"
fi
done

# If there's two files, there's a fork() -- likely within
# daemonize() -- so only pay attention to the child.
if [ "${num_logfiles}" -eq "2" ]; then
Expand Down

0 comments on commit 38e07b4

Please sign in to comment.