diff --git a/t/README b/t/README index e13063195e..2f439f9658 100644 --- a/t/README +++ b/t/README @@ -385,6 +385,12 @@ GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to "dedup_token" (see +"ASAN_OPTIONS=help=1 ./git") and other options to make logs +machine-readable. +With GIT_TEST_SANITIZE_LEAK_LOG=true we'll look at the leak logs +before exiting and exit on failure if the logs showed that we had a +memory leak, even if the test itself would have otherwise passed. This +allows us to catch e.g. missing &&-chaining. This is especially useful +when combined with "GIT_TEST_PASSING_SANITIZE_LEAK", see below. + GIT_TEST_PASSING_SANITIZE_LEAK=check when combined with "--immediate" will run to completion faster, and result in the same failing tests. The only practical reason to run @@ -393,6 +399,15 @@ combine it with "GIT_TEST_SANITIZE_LEAK_LOG=true". If we stop at the first failing test case our leak logs won't show subsequent leaks we might have run into. +GIT_TEST_PASSING_SANITIZE_LEAK=(true|check) will not catch all memory +leaks unless combined with GIT_TEST_SANITIZE_LEAK_LOG=true. Some tests +run "git" (or "test-tool" etc.) without properly checking the exit +code, or git will invoke itself and fail to ferry the abort() exit +code to the original caller. When the two modes are combined we'll +look at the "test-results/$TEST_NAME.leak/trace.*" files at the end of +the test run to see if had memory leaks which the test itself didn't +catch. + GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version' default to n. diff --git a/t/t1060-object-corruption.sh b/t/t1060-object-corruption.sh index 5b8e47e346..e8a58b1589 100755 --- a/t/t1060-object-corruption.sh +++ b/t/t1060-object-corruption.sh @@ -2,7 +2,6 @@ test_description='see how we handle various forms of corruption' -TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh # convert "1234abcd" to ".git/objects/12/34abcd" diff --git a/t/t6407-merge-binary.sh b/t/t6407-merge-binary.sh index 0753fc95f4..e8a28717ce 100755 --- a/t/t6407-merge-binary.sh +++ b/t/t6407-merge-binary.sh @@ -5,7 +5,6 @@ test_description='ask merge-recursive to merge binary files' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME -TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success setup ' diff --git a/t/test-lib.sh b/t/test-lib.sh index 3d0c8896e0..10258def7b 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -309,6 +309,7 @@ TEST_RESULTS_SAN_FILE_PFX=trace TEST_RESULTS_SAN_DIR_SFX=leak TEST_RESULTS_SAN_FILE= TEST_RESULTS_SAN_DIR="$TEST_RESULTS_DIR/$TEST_NAME.$TEST_RESULTS_SAN_DIR_SFX" +TEST_RESULTS_SAN_DIR_NR_LEAKS_STARTUP= TRASH_DIRECTORY="trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX" test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY" case "$TRASH_DIRECTORY" in @@ -316,6 +317,16 @@ case "$TRASH_DIRECTORY" in *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;; esac +# Utility functions using $TEST_RESULTS_* variables +nr_san_dir_leaks_ () { + # stderr piped to /dev/null because the directory may have + # been "rmdir"'d already. + find "$TEST_RESULTS_SAN_DIR" \ + -type f \ + -name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null | + wc -l +} + # If --stress was passed, run this test repeatedly in several parallel loops. if test "$GIT_TEST_STRESS_STARTED" = "done" then @@ -1191,6 +1202,66 @@ test_atexit_handler () { teardown_malloc_check } +sanitize_leak_log_message_ () { + local new="$1" && + local old="$2" && + local file="$3" && + + printf "With SANITIZE=leak at exit we have %d leak logs, but started with %d + +This means that we have a blindspot where git is leaking but we're +losing the exit code somewhere, or not propagating it appropriately +upwards! + +See the logs at \"%s.*\"; +those logs are reproduced below." \ + "$new" "$old" "$file" +} + +check_test_results_san_file_ () { + if test -z "$TEST_RESULTS_SAN_FILE" + then + return + fi && + local old="$TEST_RESULTS_SAN_DIR_NR_LEAKS_STARTUP" && + local new="$(nr_san_dir_leaks_)" && + + if test $new -le $old + then + return + fi && + local out="$(sanitize_leak_log_message_ "$new" "$old" "$TEST_RESULTS_SAN_FILE")" && + say_color error "$out" && + if test "$old" != 0 + then + echo && + say_color error "The logs include output from past runs to avoid" && + say_color error "that remove 'test-results' between runs." + fi && + say_color error "$(cat "$TEST_RESULTS_SAN_FILE".*)" && + + if test -n "$passes_sanitize_leak" && test "$test_failure" = 0 + then + say "As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, exit non-zero!" && + invert_exit_code=t + elif test -n "$passes_sanitize_leak" + then + say "As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, and we're failing for other reasons too..." && + invert_exit_code= + elif test -n "$sanitize_leak_check" && test "$test_failure" = 0 + then + say "As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check" && + invert_exit_code= + elif test -n "$sanitize_leak_check" + then + say "As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check" && + invert_exit_code=t + else + say "With GIT_TEST_SANITIZE_LEAK_LOG=true our logs revealed a memory leak, exit non-zero!" && + invert_exit_code=t + fi +} + test_done () { # Run the atexit commands _before_ the trash directory is # removed, so the commands can access pidfiles and socket files. @@ -1270,6 +1341,8 @@ test_done () { error "Tests passed but test cleanup failed; aborting" fi + check_test_results_san_file_ "$test_failure" + if test -z "$skip_all" && test -n "$invert_exit_code" then say_color warn "# faking up non-zero exit with --invert-exit-code" @@ -1286,6 +1359,8 @@ test_done () { say_color error "# failed $test_failure among $msg" say "1..$test_count" + check_test_results_san_file_ "$test_failure" + if test -n "$invert_exit_code" then _invert_exit_code_failure_end_blurb @@ -1464,6 +1539,7 @@ then if test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check" then + sanitize_leak_check=t if test -n "$invert_exit_code" then BAIL_OUT "cannot use --invert-exit-code under GIT_TEST_PASSING_SANITIZE_LEAK=check" @@ -1489,6 +1565,10 @@ then fi && TEST_RESULTS_SAN_FILE="$TEST_RESULTS_SAN_DIR/$TEST_RESULTS_SAN_FILE_PFX" + # In case "test-results" is left over from a previous + # run: Only report if new leaks show up. + TEST_RESULTS_SAN_DIR_NR_LEAKS_STARTUP=$(nr_san_dir_leaks_) + # Don't litter *.leak dirs if there was nothing to report test_atexit "rmdir \"$TEST_RESULTS_SAN_DIR\" 2>/dev/null || :"