commit faececa53f904845b615631f61f158a622c2ce08
author Ævar Arnfjörð Bjarmason <avarab@gmail.com> Thu Jul 28 01:13:39 2022 +0200
committer Junio C Hamano <gitster@pobox.com> Wed Jul 27 16:35:40 2022 -0700
tree 7abea90431f950e2cd3c06eac4f4e5ef0cfe1012
parent e92684e1a26a2e61f53c691d65e01b446914784d

test-lib: have the "check" mode for SANITIZE=leak consider leak logs

As noted in previous on-list discussions[1] we have various tests that
will falsely report being leak-free because we're missing the relevant
exit code from LSAN as summarized below.

We should fix those issues, but in the meantime and as an additional
sanity check we can and should consider our own ASAN logs before
reporting that a test is leak-free.

Before this compiling with SANITIZE=leak and running:

    ./t6407-merge-binary.sh

Will exit successfully, now we'll get an error and an informative
message on:

    GIT_TEST_SANITIZE_LEAK_LOG=true ./t6407-merge-binary.sh

Even better, as noted in the updated t/README we'll now error out when
combined with the "check" mode:

    GIT_TEST_PASSING_SANITIZE_LEAK=check \
    GIT_TEST_SANITIZE_LEAK_LOG=true \
	./t4058-diff-duplicates.sh

Why do we miss these leaks? Because:

 * We have leaks inside "test_expect_failure" blocks, which by design
   will not distinguish a "normal" failure from an abort() or
   segfault. See [1] for a discussion of it shortcomings.

 * We have "git" invocations outside of "test_expect_success",
   e.g. setup code in the main body of the test, or in test helper
   functions that don't use &&-chaining.

 * Our tests will otherwise catch segfaults and abort(), but if we
   invoke a command that invokes another command it needs to ferry the
   exit code up to us.

   Notably a command that e.g. might invoke "git pack-objects" might
   itself exit with status 128 if that "pack-objects" segfaults or
   abort()'s. If the test invoking the parent command(s) is using
   "test_must_fail" we'll consider it an expected "ok" failure.

 * run-command.c doesn't (but probably should) ferry up such exit
   codes, so for e.g. "git push" tests where we expect a failure and an
   underlying "git" command fails we won't ferry up the segfault or
   abort exit code.

 * We have gitweb.perl and some other perl code ignoring return values
   from close(), i.e. ignoring exit codes from "git rev-parse" et al.

 * We have in-tree shellscripts like "git-merge-one-file.sh" invoking
   git commands, they'll usually return their own exit codes on "git"
   failure, rather then ferrying up segfault or abort() exit code.

   E.g. these invocations in git-merge-one-file.sh leak, but aren't
   reflected in the "git merge" exit code:

	src1=$(git unpack-file $2)
	src2=$(git unpack-file $3)

   That case would be easily "fixed" by adding a line like this after
   each assignment:

	test $? -ne 0 && exit $?

   But we'd then in e.g. "t6407-merge-binary.sh" run into
   write_tree_trivial() in "builtin/merge.c" calling die() instead of
   ferrying up the relevant exit code.

Let's remove "TEST_PASSES_SANITIZE_LEAK=true" from tests we
were falsely marking as leak-free.

In the case of t6407-merge-binary.sh it was marked as leak-free in
9081a421a6d (checkout: fix "branch info" memory leaks,
2021-11-16). I'd previously removed other bad
"TEST_PASSES_SANITIZE_LEAK=true" opt-ins in the series merged in
ea05fd5fbf7 (Merge branch 'ab/keep-git-exit-codes-in-tests',
2022-03-16). The case of t1060-object-corruption.sh is more subtle,
and will be discussed in a subsequent commit.

1. https://lore.kernel.org/git/cover-0.7-00000000000-20220318T002951Z-avarab@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>