Thread (3 messages) 3 messages, 3 authors, 2022-09-29

Re: [PATCH] test-lib: have SANITIZE=leak imply TEST_NO_MALLOC_CHECK

From: Rubén Justo <hidden>
Date: 2022-09-28 23:21:08

On 28/9/22 12:01, Ævar Arnfjörð Bjarmason wrote:
quoted hunk ↗ jump to hunk
Since 131b94a10a7 (test-lib.sh: Use GLIBC_TUNABLES instead of
MALLOC_CHECK_ on glibc >= 2.34, 2022-03-04) compiling with
SANITIZE=leak has missed reporting some leaks. The old MALLOC_CHECK
method used before glibc 2.34 seems to have been (mostly?) compatible
with it, but after 131b94a10a7 e.g. running:

	TEST_NO_MALLOC_CHECK=1 make SANITIZE=leak test T=t6437-submodule-merge.sh

Would report a leak in builtin/commit.c, but this would not:

	TEST_NO_MALLOC_CHECK= make SANITIZE=leak test T=t6437-submodule-merge.sh

Since the interaction is clearly breaking the SANITIZE=leak mode,
let's mark them as explicitly incompatible.

A related regression for SANITIZE=address was fixed in
067109a5e7d (tests: make SANITIZE=address imply TEST_NO_MALLOC_CHECK,
2022-04-09).

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---

Junio: I think this is worth considering for v2.38.0. We've had this
check since v2.36.0

But 2.34 just recently got migrated to Debian testing (just a few days
ago), I suspect other distros are either upgrading to it now, or will
soon: https://tracker.debian.org/pkg/glibc;

When I upgraded to it I discovered that all of our tests pass with
SANITIZE=leak, i.e. unless TEST_NO_MALLOC_CHECK=1 is provided we
completely disable the LeakSanitizer in favor of glibc.

 t/test-lib.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index a65df2fd220..02f438d47ec 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -563,8 +563,10 @@ case $GIT_TEST_FSYNC in
 esac
 
 # Add libc MALLOC and MALLOC_PERTURB test only if we are not executing
-# the test with valgrind and have not compiled with SANITIZE=address.
+# the test with valgrind and have not compiled with conflict SANITIZE
+# options.
 if test -n "$valgrind" ||
+   test -n "$SANITIZE_LEAK" ||
    test -n "$SANITIZE_ADDRESS" ||
    test -n "$TEST_NO_MALLOC_CHECK"
 then
Thank you! A quick test with this on master shows clearly the leak in ref-filter.c
we discussed recently.  No need to dig with valgrind.  I also found that other case
you pointed out, from checkout. I'll reroll with that if you don't mind.

It is nice to have this working.

Thanks.

Un saludo.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help