git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Jeff KingNext: Jeff King
Message 12 of 18 in “[RFC]: Test Were failing on Fedora Linux.”
  1. Usman AkinyemiNov 9, 2024
  2. Christian CouderNov 9, 2024
  3. Usman AkinyemiNov 9, 2024
  4. Christian CouderNov 9, 2024
  5. Andreas SchwabNov 9, 2024
  6. Usman AkinyemiNov 9, 2024
  7. Todd ZullingerNov 9, 2024
  8. Jeff KingNov 9, 2024
  9. Usman AkinyemiNov 9, 2024
  10. Junio C HamanoNov 11, 2024
  11. test-lib: check malloc debug LD_PRELOAD before usingJeff King, Nov 11, 2024
  12. Toon ClaesNov 13, 2024
  13. Jeff KingNov 14, 2024
  14. 2/1 test-lib: move malloc-debug setup after $PATH setupJeff King, Nov 14, 2024
  15. Toon ClaesNov 20, 2024
  16. Junio C HamanoNov 20, 2024
  17. Usman AkinyemiNov 9, 2024
  18. Usman AkinyemiNov 9, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.