Re: [PATCH] test-lib: check malloc debug LD_PRELOAD before using
- From
Toon Claes <toon@iotcl.com>
- Date
- Nov 13, 2024, 10:19 UTC
- Message-ID
- <87zfm3iggu.fsf@iotcl.com>
- 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.
Show 39 quoted lines
> 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?
Show 12 quoted lines
> 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