From 4e1be63c3b05fd379b51b611b7e869ff6977d8ab Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:25:59 +0100 Subject: Add valgrind support in test scripts This patch adds the ability to use valgrind's memcheck tool to diagnose memory problems in Git while running the test scripts. It requires valgrind 3.4.0 or newer. It works by creating symlinks to a valgrind script, which have the same name as our Git binaries, and then putting that directory in front of the test script's PATH as well as set GIT_EXEC_PATH to that directory. Git scripts are symlinked from that directory directly. That way, Git binaries called by Git scripts are valgrinded, too. Valgrind can be used by specifying "GIT_TEST_OPTS=--valgrind" in the make invocation. Any invocation of git that finds any errors under valgrind will exit with failure code 126. Any valgrind output will go to the usual stderr channel for tests (i.e., /dev/null, unless -v has been specified). If you need to pass options to valgrind -- you might want to run another tool than memcheck, for example -- you can set the environment variable GIT_VALGRIND_OPTIONS. A few default suppressions are included, since libz seems to trigger quite a few false positives. We'll assume that libz works and that we can ignore any errors which are reported there. Note: it is safe to run the valgrind tests in parallel, as the links in t/valgrind/bin/ are created using proper locking. Initial patch and all the hard work by Jeff King. Signed-off-by: Johannes Schindelin Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/README | 8 ++++++- t/test-lib.sh | 58 +++++++++++++++++++++++++++++++++++++++++++++++-- t/valgrind/.gitignore | 2 ++ t/valgrind/default.supp | 21 ++++++++++++++++++ t/valgrind/valgrind.sh | 13 +++++++++++ 5 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 t/valgrind/.gitignore create mode 100644 t/valgrind/default.supp create mode 100755 t/valgrind/valgrind.sh (limited to 't') diff --git a/t/README b/t/README index f208cf1db9..7560db5c02 100644 --- a/t/README +++ b/t/README @@ -39,7 +39,8 @@ this: * passed all 3 test(s) You can pass --verbose (or -v), --debug (or -d), and --immediate -(or -i) command line argument to the test. +(or -i) command line argument to the test, or by setting GIT_TEST_OPTS +appropriately before running "make". --verbose:: This makes the test more verbose. Specifically, the @@ -58,6 +59,11 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate This causes additional long-running tests to be run (where available), for more exhaustive testing. +--valgrind:: + Execute all Git binaries with valgrind and exit with status + 126 on errors (just like regular tests, this will only stop + the test script when running under -i). Valgrind errors + go to stderr, so you might want to pass the -v option, too. Skipping Tests -------------- diff --git a/t/test-lib.sh b/t/test-lib.sh index 6f6244ab7e..da12b0aa43 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -94,6 +94,8 @@ do --no-python) # noop now... shift ;; + --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind) + valgrind=t; shift ;; *) break ;; esac @@ -492,8 +494,60 @@ test_done () { # Test the binaries we have just built. The tests are kept in # t/ subdirectory and are run in 'trash directory' subdirectory. TEST_DIRECTORY=$(pwd) -PATH=$TEST_DIRECTORY/..:$PATH -GIT_EXEC_PATH=$(pwd)/.. +if test -z "$valgrind" +then + PATH=$TEST_DIRECTORY/..:$PATH + GIT_EXEC_PATH=$TEST_DIRECTORY/.. +else + make_symlink () { + test -h "$2" && + test "$1" = "$(readlink "$2")" || { + # be super paranoid + if mkdir "$2".lock + then + rm -f "$2" && + ln -s "$1" "$2" && + rm -r "$2".lock + else + while test -d "$2".lock + do + say "Waiting for lock on $2." + sleep 1 + done + fi + } + } + + make_valgrind_symlink () { + # handle only executables + test -x "$1" || return + + base=$(basename "$1") + symlink_target=$TEST_DIRECTORY/../$base + # do not override scripts + if test -x "$symlink_target" && + test ! -d "$symlink_target" && + test "#!" != "$(head -c 2 < "$symlink_target")" + then + symlink_target=../valgrind.sh + fi + # create the link, or replace it if it is out of date + make_symlink "$symlink_target" "$GIT_VALGRIND/bin/$base" || exit + } + + # override all git executables in TEST_DIRECTORY/.. + GIT_VALGRIND=$TEST_DIRECTORY/valgrind + mkdir -p "$GIT_VALGRIND"/bin + for file in $TEST_DIRECTORY/../git* $TEST_DIRECTORY/../test-* + do + make_valgrind_symlink $file + done + PATH=$GIT_VALGRIND/bin:$PATH + GIT_EXEC_PATH=$GIT_VALGRIND/bin + export GIT_VALGRIND + + make_symlink ../../../templates "$GIT_VALGRIND"/bin/templates || exit +fi GIT_TEMPLATE_DIR=$(pwd)/../templates/blt unset GIT_CONFIG GIT_CONFIG_NOSYSTEM=1 diff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore new file mode 100644 index 0000000000..d4ae6676d1 --- /dev/null +++ b/t/valgrind/.gitignore @@ -0,0 +1,2 @@ +/bin/ +/templates diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp new file mode 100644 index 0000000000..2482b3b06a --- /dev/null +++ b/t/valgrind/default.supp @@ -0,0 +1,21 @@ +{ + ignore-zlib-errors-cond + Memcheck:Cond + obj:*libz.so* +} + +{ + ignore-zlib-errors-value4 + Memcheck:Value4 + obj:*libz.so* +} + +{ + writing-data-from-zlib-triggers-errors + Memcheck:Param + write(buf) + obj:/lib/ld-*.so + fun:write_in_full + fun:write_buffer + fun:write_loose_object +} diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh new file mode 100755 index 0000000000..dc9261265b --- /dev/null +++ b/t/valgrind/valgrind.sh @@ -0,0 +1,13 @@ +#!/bin/sh + +base=$(basename "$0") + +exec valgrind -q --error-exitcode=126 \ + --leak-check=no \ + --suppressions="$GIT_VALGRIND/default.supp" \ + --gen-suppressions=all \ + --track-origins=yes \ + --log-fd=4 \ + --input-fd=4 \ + $GIT_VALGRIND_OPTIONS \ + "$GIT_VALGRIND"/../../"$base" "$@" -- cgit v1.2.3 From 6a7e37c99f733821bd3a17432fa6e4591b63866c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 4 Feb 2009 00:26:03 +0100 Subject: valgrind: ignore ldso and more libz errors On some Linux systems, we get a host of Cond and Addr errors from calls to dlopen that are caused by nss modules. We should be able to safely ignore anything happening in ld-*.so as "not our problem." [Johannes: I added some more... unfortunately using valgrind 3.4.0 syntax] Signed-off-by: Jeff King Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/valgrind/default.supp | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) (limited to 't') diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp index 2482b3b06a..5f341b8598 100644 --- a/t/valgrind/default.supp +++ b/t/valgrind/default.supp @@ -4,6 +4,12 @@ obj:*libz.so* } +{ + ignore-zlib-errors-value8 + Memcheck:Value8 + obj:*libz.so* +} + { ignore-zlib-errors-value4 Memcheck:Value4 @@ -11,11 +17,27 @@ } { - writing-data-from-zlib-triggers-errors + ignore-ldso-cond + Memcheck:Cond + obj:*ld-*.so +} + +{ + ignore-ldso-addr8 + Memcheck:Addr8 + obj:*ld-*.so +} + +{ + ignore-ldso-addr4 + Memcheck:Addr4 + obj:*ld-*.so +} + +{ + writing-data-from-zlib-triggers-even-more-errors Memcheck:Param write(buf) - obj:/lib/ld-*.so - fun:write_in_full - fun:write_buffer + ... fun:write_loose_object } -- cgit v1.2.3 From efd92ffd316d03360e1c8a348091e4f50f749d6f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:08 +0100 Subject: Valgrind support: check for more than just programming errors This patch makes --valgrind try to override _all_ Git binaries in the PATH, and it makes it an error to call *.sh and *.perl scripts directly. While it is not strictly necessary to look through the whole PATH to find git binaries to override, it is in line with running an expensive test (which valgrind is) to make extra sure that only binaries are tested that actually come from the git.git checkout. In the same spirit, we can test that neither our test suite nor our scripts try to run the *.sh or *.perl scripts directly. It's more like a "because we can" than a "this is tightly connected to valgrind", but in the author's opinion "because we can" is "so we should" in this case. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/test-lib.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 't') diff --git a/t/test-lib.sh b/t/test-lib.sh index da12b0aa43..5a58356c72 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -531,6 +531,10 @@ else then symlink_target=../valgrind.sh fi + case "$base" in + *.sh|*.perl) + symlink_target=../unprocessed-script + esac # create the link, or replace it if it is out of date make_symlink "$symlink_target" "$GIT_VALGRIND/bin/$base" || exit } @@ -542,6 +546,17 @@ else do make_valgrind_symlink $file done + OLDIFS=$IFS + IFS=: + for path in $PATH + do + ls "$path"/git-* 2> /dev/null | + while read file + do + make_valgrind_symlink "$file" + done + done + IFS=$OLDIFS PATH=$GIT_VALGRIND/bin:$PATH GIT_EXEC_PATH=$GIT_VALGRIND/bin export GIT_VALGRIND -- cgit v1.2.3 From 44138559e8b7c89768a2450220b831847059311c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:12 +0100 Subject: test-lib.sh: optionally output to test-results/$TEST.out, too When tests are run in parallel and a few tests fail, it does not help that the output of the terminal is totally confusing, as you rarely know which test which line came from. So introduce the option '--tee' which triggers that the output of the tests will be written to t/test-results/$TEST.out in addition to the terminal, where $TEST is the basename of the script. Unfortunately, there seems to be no way to redirect a given file descriptor to a specified subprocess in POSIX shell, only redirection to a file is supported via 'exec > $FILE'. At least with bash, one might think that 'exec >($COMMAND)' would work as intended, but it does not. The common way to work around the lack of proper tools support is to work with named pipes, alas, one of our most beloved platforms does not really support named pipes. Besides, we would need a pipe for every script, as the whole point of this patch is to allow parallel execution. Therefore, we handle the redirection in the following way: when '--tee' was passed to the test script, the variable GIT_TEST_TEE_STARTED is set (to avoid triggering that code path again) and the script is started _again_, in a subshell, redirected to the command "tee". Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/README | 6 ++++++ t/test-lib.sh | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) (limited to 't') diff --git a/t/README b/t/README index 7560db5c02..ed1ebb6a5c 100644 --- a/t/README +++ b/t/README @@ -65,6 +65,12 @@ appropriately before running "make". the test script when running under -i). Valgrind errors go to stderr, so you might want to pass the -v option, too. +--tee:: + In addition to printing the test output to the terminal, + write it to files named 't/test-results/$TEST_NAME.out'. + As the names depend on the tests' file names, it is safe to + run the tests with this option in parallel. + Skipping Tests -------------- diff --git a/t/test-lib.sh b/t/test-lib.sh index 5a58356c72..34f372c92f 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -3,6 +3,22 @@ # Copyright (c) 2005 Junio C Hamano # +# if --tee was passed, write the output not only to the terminal, but +# additionally to the file test-results/$BASENAME.out, too. +case "$GIT_TEST_TEE_STARTED, $* " in +done,*) + # do not redirect again + ;; +*' --tee '*) + mkdir -p test-results + BASE=test-results/$(basename "$0" .sh) + (GIT_TEST_TEE_STARTED=done ${SHELL-sh} "$0" "$@" 2>&1; + echo $? > $BASE.exit) | tee $BASE.out + test "$(cat $BASE.exit)" = 0 + exit + ;; +esac + # Keep the original TERM for say_color ORIGINAL_TERM=$TERM @@ -96,6 +112,8 @@ do shift ;; --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind) valgrind=t; shift ;; + --tee) + shift ;; # was handled already *) break ;; esac -- cgit v1.2.3 From 7f6fdea1198cd7599c25cf9435df8540e9378a15 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:18 +0100 Subject: t/Makefile: provide a 'valgrind' target It is easy to forget running valgrinded tests without -v, and it is also easy to forget to redirect the output to "tee" (lest the output scroll out of the terminal's buffer). Running "make valgrind" will take care of all that. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 't') diff --git a/t/Makefile b/t/Makefile index ed49c20b16..e544493d10 100644 --- a/t/Makefile +++ b/t/Makefile @@ -38,4 +38,7 @@ full-svn-test: $(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=1 LC_ALL=C $(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=0 LC_ALL=en_US.UTF-8 -.PHONY: pre-clean $(T) aggregate-results clean +valgrind: + GIT_TEST_OPTS='--valgrind -v --tee' $(MAKE) -k + +.PHONY: pre-clean $(T) aggregate-results clean valgrind -- cgit v1.2.3 From 268fac6919ef673094e50e2d944d09f017f5ad33 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:22 +0100 Subject: Add a script to coalesce the valgrind outputs After running the valgrind tests with GIT_TEST_TREE=t, the test output is in the test-results/$TEST.out files. Call ./valgrind/analyze.sh in $GIT_ROOT/t/ to group the valgrind errors by backtrace. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/valgrind/analyze.sh | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100755 t/valgrind/analyze.sh (limited to 't') diff --git a/t/valgrind/analyze.sh b/t/valgrind/analyze.sh new file mode 100755 index 0000000000..d8105d9fab --- /dev/null +++ b/t/valgrind/analyze.sh @@ -0,0 +1,123 @@ +#!/bin/sh + +out_prefix=$(dirname "$0")/../test-results/valgrind.out +output= +count=0 +total_count=0 +missing_message= +new_line=' +' + +# start outputting the current valgrind error in $out_prefix.++$count, +# and the test case which failed in the corresponding .message file +start_output () { + test -z "$output" || return + + # progress + total_count=$(($total_count+1)) + test -t 2 && printf "\rFound %d errors" $total_count >&2 + + count=$(($count+1)) + output=$out_prefix.$count + : > $output + + echo "*** $1 ***" > $output.message +} + +finish_output () { + test ! -z "$output" || return + output= + + # if a test case has more than one valgrind error, we need to + # copy the last .message file to the previous errors + test -z "$missing_message" || { + while test $missing_message -lt $count + do + cp $out_prefix.$count.message \ + $out_prefix.$missing_message.message + missing_message=$(($missing_message+1)) + done + missing_message= + } +} + +# group the valgrind errors by backtrace +output_all () { + last_line= + j=0 + i=1 + while test $i -le $count + do + # output + echo "$i $(tr '\n' ' ' < $out_prefix.$i)" + i=$(($i+1)) + done | + sort -t ' ' -k 2 | # order by + while read number line + do + # find duplicates, do not output backtrace twice + if test "$line" != "$last_line" + then + last_line=$line + j=$(($j+1)) + printf "\nValgrind error $j:\n\n" + cat $out_prefix.$number + printf "\nfound in:\n" + fi + # print the test case where this came from + printf "\n" + cat $out_prefix.$number.message + done +} + +handle_one () { + OLDIFS=$IFS + IFS="$new_line" + while read line + do + case "$line" in + # backtrace, possibly a new one + ==[0-9]*) + + # Does the current valgrind error have a message yet? + case "$output" in + *.message) + test -z "$missing_message" && + missing_message=$count + output= + esac + + start_output $(basename $1) + echo "$line" | + sed 's/==[0-9]*==/==valgrind==/' >> $output + ;; + # end of backtrace + '}') + test -z "$output" || { + echo "$line" >> $output + test $output = ${output%.message} && + output=$output.message + } + ;; + # end of test case + '') + finish_output + ;; + # normal line; if $output is set, print the line + *) + test -z "$output" || echo "$line" >> $output + ;; + esac + done < $1 + IFS=$OLDIFS + + # just to be safe + finish_output +} + +for test_script in "$(dirname "$0")"/../test-results/*.out +do + handle_one $test_script +done + +output_all -- cgit v1.2.3 From 3da936523445eb7a74dfe3ab1fe0ce82fac56174 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:26 +0100 Subject: Tests: let --valgrind imply --verbose and --tee It does not make much sense to run the (expensive) valgrind tests and not look at the output. To prevent output from scrolling out of reach, the parameter --tee is implied, too. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/Makefile | 2 +- t/README | 4 ++++ t/test-lib.sh | 4 ++-- 3 files changed, 7 insertions(+), 3 deletions(-) (limited to 't') diff --git a/t/Makefile b/t/Makefile index e544493d10..09623414a7 100644 --- a/t/Makefile +++ b/t/Makefile @@ -39,6 +39,6 @@ full-svn-test: $(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=0 LC_ALL=en_US.UTF-8 valgrind: - GIT_TEST_OPTS='--valgrind -v --tee' $(MAKE) -k + GIT_TEST_OPTS=--valgrind $(MAKE) .PHONY: pre-clean $(T) aggregate-results clean valgrind diff --git a/t/README b/t/README index ed1ebb6a5c..d8f6c7de6d 100644 --- a/t/README +++ b/t/README @@ -65,6 +65,10 @@ appropriately before running "make". the test script when running under -i). Valgrind errors go to stderr, so you might want to pass the -v option, too. + Since it makes no sense to run the tests with --valgrind and + not see any output, this option implies --verbose. For + convenience, it also implies --tee. + --tee:: In addition to printing the test output to the terminal, write it to files named 't/test-results/$TEST_NAME.out'. diff --git a/t/test-lib.sh b/t/test-lib.sh index 34f372c92f..bc87936bab 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -9,7 +9,7 @@ case "$GIT_TEST_TEE_STARTED, $* " in done,*) # do not redirect again ;; -*' --tee '*) +*' --tee '*|*' --va'*) mkdir -p test-results BASE=test-results/$(basename "$0" .sh) (GIT_TEST_TEE_STARTED=done ${SHELL-sh} "$0" "$@" 2>&1; @@ -111,7 +111,7 @@ do # noop now... shift ;; --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind) - valgrind=t; shift ;; + valgrind=t; verbose=t; shift ;; --tee) shift ;; # was handled already *) -- cgit v1.2.3 From a6d63b749369f3ba9c6d2f382efd27838604b7db Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:31 +0100 Subject: test-lib: avoid assuming that templates/ are in the GIT_EXEC_PATH Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/test-lib.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 't') diff --git a/t/test-lib.sh b/t/test-lib.sh index bc87936bab..20214468ae 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -454,7 +454,7 @@ test_create_repo () { repo="$1" mkdir -p "$repo" cd "$repo" || error "Cannot setup test environment" - "$GIT_EXEC_PATH/git" init "--template=$GIT_EXEC_PATH/templates/blt/" >&3 2>&4 || + "$GIT_EXEC_PATH/git" init "--template=$owd/../templates/blt/" >&3 2>&4 || error "cannot run git init -- have you built things yet?" mv .git/hooks .git/hooks-disabled cd "$owd" @@ -578,8 +578,6 @@ else PATH=$GIT_VALGRIND/bin:$PATH GIT_EXEC_PATH=$GIT_VALGRIND/bin export GIT_VALGRIND - - make_symlink ../../../templates "$GIT_VALGRIND"/bin/templates || exit fi GIT_TEMPLATE_DIR=$(pwd)/../templates/blt unset GIT_CONFIG -- cgit v1.2.3 From 5caa81d1b4f1b0b9ed73605ab1e4d91ba31a56b4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 5 Feb 2009 22:03:00 +0100 Subject: valgrind: do not require valgrind 3.4.0 or newer Valgrind 3.4.0 is pretty new, and even if --track-origins is a nice feature, it is not the end of the world if that is not available. So play nice and use that option only when only an older version of valgrind is available. In the same spirit, refrain from the use of '...' in suppression files, which is also a feature only valgrind 3.4 and newer understand. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/valgrind/default.supp | 4 +++- t/valgrind/valgrind.sh | 11 ++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp index 5f341b8598..9e013fa3b2 100644 --- a/t/valgrind/default.supp +++ b/t/valgrind/default.supp @@ -38,6 +38,8 @@ writing-data-from-zlib-triggers-even-more-errors Memcheck:Param write(buf) - ... + obj:/lib/ld-*.so + fun:write_in_full + fun:write_buffer fun:write_loose_object } diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh index dc9261265b..582b4dca94 100755 --- a/t/valgrind/valgrind.sh +++ b/t/valgrind/valgrind.sh @@ -2,11 +2,20 @@ base=$(basename "$0") +TRACK_ORIGINS= + +VALGRIND_VERSION=$(valgrind --version) +VALGRIND_MAJOR=$(expr "$VALGRIND_VERSION" : '[^0-9]*\([0-9]*\)') +VALGRIND_MINOR=$(expr "$VALGRIND_VERSION" : '[^0-9]*[0-9]*\.\([0-9]*\)') +test 3 -gt "$VALGRIND_MAJOR" || +test 3 -eq "$VALGRIND_MAJOR" -a 4 -gt "$VALGRIND_MINOR" || +TRACK_ORIGINS=--track-origins=yes + exec valgrind -q --error-exitcode=126 \ --leak-check=no \ --suppressions="$GIT_VALGRIND/default.supp" \ --gen-suppressions=all \ - --track-origins=yes \ + $TRACK_ORIGINS \ --log-fd=4 \ --input-fd=4 \ $GIT_VALGRIND_OPTIONS \ -- cgit v1.2.3