[PATCH v2] test-lib.sh: Use GLIBC_TUNABLES instead of MALLOC_CHECK_ on glibc >= 2.34

[PATCH v2] test-lib.sh: Use GLIBC_TUNABLES instead of MALLOC_CHECK_ on glibc >= 2.34

[Elia Pinto]

In glibc >= 2.34 MALLOC_CHECK_ and MALLOC_PERTURB_ environment
variables have been replaced by GLIBC_TUNABLES.  Also the new
glibc requires that you preload a library called libc_malloc_debug.so
to get these features.

Using the ordinary glibc system variable detect if this is glibc >= 2.34 and
use GLIBC_TUNABLES and the new library.

This patch was inspired by a Richard W.M. Jones ndbkit patch

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
This is the second version of the patch.

Compared to the first version, the code has been simplified, based on Junio's
indications, introducing some redundancy in the setting of the glibc variables
covered by the patch
(https://lore.kernel.org/all/20220228160827.465488-1-gitter.spiros@gmail.com/)
 t/test-lib.sh | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9af5fb7674..b545013a14 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -548,11 +548,32 @@ then
 	}
 else
 	setup_malloc_check () {
+		if _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) &&
+		_GLIBC_VERSION=${_GLIBC_VERSION#"glibc "} &&
+		expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null
+		then
+			USE_GLIBC_TUNABLES=YesPlease
+		fi
 		MALLOC_CHECK_=3	MALLOC_PERTURB_=165
 		export MALLOC_CHECK_ MALLOC_PERTURB_
+		case "$USE_GLIBC_TUNABLES" in
+		YesPlease)
+			g=
+			LD_PRELOAD="libc_malloc_debug.so.0"
+			for t in \
+				glibc.malloc.check=1 \
+				glibc.malloc.perturb=165
+			do
+				g="${g##:}:$t"
+			done
+			GLIBC_TUNABLES=$g
+			export LD_PRELOAD GLIBC_TUNABLES
+			;;
+		esac
 	}
 	teardown_malloc_check () {
 		unset MALLOC_CHECK_ MALLOC_PERTURB_
+		unset LD_PRELOAD GLIBC_TUNABLES
 	}
 fi
 
-- 
2.35.1

Re: [PATCH v2] test-lib.sh: Use GLIBC_TUNABLES instead of MALLOC_CHECK_ on glibc >= 2.34

[Junio C Hamano]

Elia Pinto <gitter.spiros@gmail.com> writes:

> Compared to the first version, the code has been simplified, based on Junio's
> indications, introducing some redundancy in the setting of the glibc variables
> covered by the patch

I think we can even lose the separate case statement and write its
body inside if/then...fi.  The following shows the result of
shuffling the lines to do so.

>  	setup_malloc_check () {
> +		if _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) &&
> +		   _GLIBC_VERSION=${_GLIBC_VERSION#"glibc "} &&
> +		   expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null

I am wondering if we want to make sure "getconf" returned a string
that begins with "glibc " before stripping that prefix.  Hopefully
such a paranoia is probably unneeded.

> +		then
> +			g=
> +			LD_PRELOAD="libc_malloc_debug.so.0"
> +			for t in \
> +				glibc.malloc.check=1 \
> +				glibc.malloc.perturb=165
> +			do
> +				g="${g##:}:$t"

Cute.  As this strips a constant, we do not need to be ##greedy
and using just a single # would suffice, right?

> +			done
> +			GLIBC_TUNABLES=$g
> +			export LD_PRELOAD GLIBC_TUNABLES
> +			;;
> +		fi
>  		MALLOC_CHECK_=3	MALLOC_PERTURB_=165
>  		export MALLOC_CHECK_ MALLOC_PERTURB_
>  	}
>  	teardown_malloc_check () {
>  		unset MALLOC_CHECK_ MALLOC_PERTURB_
> +		unset LD_PRELOAD GLIBC_TUNABLES
>  	}
>  fi

especially the teardown side unsets these unconditionally.