From: Toon Claes <toon@iotcl•com>
To: Jeff King <peff@peff•net>, Junio C Hamano <gitster@pobox•com>
Cc: Todd Zullinger <tmz@pobox•com>,
Elia Pinto <gitter.spiros@gmail•com>,
Usman Akinyemi <usmanakinyemi202@gmail•com>,
Andreas Schwab <schwab@linux-m68k•org>,
Christian Couder <christian.couder@gmail•com>,
git@vger•kernel.org
Subject: Re: [PATCH] test-lib: check malloc debug LD_PRELOAD before using
Date: Wed, 13 Nov 2024 11:19:13 +0100 [thread overview]
Message-ID: <87zfm3iggu.fsf@iotcl.com> (raw)
In-Reply-To: <20241111070134.GA675125@coredump.intra.peff.net>
Jeff King <peff@peff•net> writes:
> Subject: [PATCH] test-lib: check malloc debug LD_PRELOAD before using
>
> This fixes test failures across the suite on glibc platforms that don't
> have libc_malloc_debug.so.0.
As I ran into this issue not so long as well, I'm really supportive of
adding a fix for this.
> We added support for glibc's malloc checking routines long ago in
> a731fa916e (Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test
> suite for detecting heap corruption, 2012-09-14). Back then we didn't
> need to do any checks to see if the platform supported it. We were just
> setting some environment variables which would either enable it or not.
>
> That changed in 131b94a10a (test-lib.sh: Use GLIBC_TUNABLES instead of
> MALLOC_CHECK_ on glibc >= 2.34, 2022-03-04). Now that glibc split this
> out into libc_malloc_debug.so, we have to add it to LD_PRELOAD. We only
> do that when we detect glibc, but it's possible to have glibc but not
> the malloc debug library. In that case LD_PRELOAD will complain to
> stderr, and tests which check for an empty stderr will fail.
>
> You can work around this by setting TEST_NO_MALLOC_CHECK, which disables
> the feature entirely. But it's not obvious to know you need to do that.
> Instead, since this malloc checking is best-effort anyway, let's just
> automatically disable it when the LD_PRELOAD appears not to work. We can
> check it by running something simple that should work (and produce
> nothing on stderr) like "git version".
>
> Signed-off-by: Jeff King <peff@peff•net>
> ---
> t/test-lib.sh | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index a278181a05..4fe757fe9a 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -593,9 +593,12 @@ then
> }
> else
> _USE_GLIBC_TUNABLES=
> + _USE_GLIBC_PRELOAD=
> if _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) &&
> _GLIBC_VERSION=${_GLIBC_VERSION#"glibc "} &&
> - expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null
> + expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null &&
> + stderr=$(LD_PRELOAD=$_USE_GLIBC_PRELOAD git version 2>&1 >/dev/null) &&
Can we assume some version of git is in the $PATH here? I see $PATH and
$GIT_EXEC_PATH are only determined at line 1440 and further.
> + test -z "$stderr"
> then
> _USE_GLIBC_TUNABLES=YesPlease
Shall we include a warning in a else clause to inform the user the tests
were started with malloc check, but libc_malloc_debug.so.0 was not found
and they should either install it or run with TEST_NO_MALLOC_CHECK?
> fi
> @@ -607,7 +610,7 @@ else
> if test -n "$_USE_GLIBC_TUNABLES"
> then
> g=
> - LD_PRELOAD="libc_malloc_debug.so.0"
> + LD_PRELOAD=$_USE_GLIBC_PRELOAD
> for t in \
> glibc.malloc.check=1 \
> glibc.malloc.perturb=165
> --
> 2.47.0.495.g1253739cc1
I've tested this patch with and without having glibc-utils installed, in
combination of having TEST_NO_MALLOC_CHECK set/unset and seems to work
like a charm.
--
Toon
next prev parent reply other threads:[~2024-11-13 10:19 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-09 6:01 [RFC]: Test Were failing on Fedora Linux Usman Akinyemi
2024-11-09 8:12 ` Christian Couder
2024-11-09 9:32 ` Usman Akinyemi
2024-11-09 14:01 ` Christian Couder
2024-11-09 14:35 ` Andreas Schwab
2024-11-09 15:02 ` Usman Akinyemi
2024-11-09 16:05 ` Todd Zullinger
2024-11-09 19:00 ` Jeff King
2024-11-09 19:12 ` Usman Akinyemi
2024-11-11 3:11 ` Junio C Hamano
2024-11-11 7:01 ` [PATCH] test-lib: check malloc debug LD_PRELOAD before using Jeff King
2024-11-13 10:19 ` Toon Claes [this message]
2024-11-14 1:27 ` Jeff King
2024-11-14 1:39 ` [PATCH 2/1] test-lib: move malloc-debug setup after $PATH setup Jeff King
2024-11-20 13:51 ` Toon Claes
2024-11-20 23:26 ` Junio C Hamano
2024-11-09 19:09 ` [RFC]: Test Were failing on Fedora Linux Usman Akinyemi
2024-11-09 14:59 ` Usman Akinyemi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zfm3iggu.fsf@iotcl.com \
--to=toon@iotcl$(echo .)com \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=gitter.spiros@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=schwab@linux-m68k$(echo .)org \
--cc=tmz@pobox$(echo .)com \
--cc=usmanakinyemi202@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox