Re: [PATCH] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 12, 2012, 17:51 UTC
Message-ID: <7vy5kfceod.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1347452248-12222-1-git-send-email-gitter.spiros@gmail.com>

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

Show 32 quoted lines

> Recent versions of Linux libc (later than 5.4.23) and glibc (2.x)
> include a malloc() implementation which is tunable via environment
> variables. When MALLOC_CHECK_ is set, a special (less efficient)
> implementation is used which is designed to be tolerant against
> simple errors, such as double calls of free() with the same argument,
> or overruns of a single byte (off-by-one bugs). When MALLOC_CHECK_
> is set to 3, a diagnostic message is printed on stderr
> and the program is aborted.
>
> Setting the MALLOC_PERTURB_ environment variable causes the malloc
> functions in libc to return memory which has been wiped and clear
> memory when it is returned.
> Of course this does not affect calloc which always does clear the memory.
>
> The reason for this exercise is, of course, to find code which uses
> memory returned by malloc without initializing it and code which uses
> code after it is freed. valgrind can do this but it's costly to run.
> The MALLOC_PERTURB_ exchanges the ability to detect problems in 100%
> of the cases with speed.
>
> The byte value used to initialize values returned by malloc is the byte
> value of the environment value. The value used to clear memory is the
> bitwise inverse. Setting MALLOC_PERTURB_ to zero disables the feature.
>
> This technique can find hard to detect bugs.
> It is therefore suggested to always use this flag (at least temporarily)
> when testing out code or a new distribution.
>
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
>  t/test-lib.sh | 6 ++++++
>  1 file changed, 6 insertions(+)

Interesting, but it bothers me to make it enabled unconditionally. At least, this shouldn't be enabled under GIT_TEST_OPTS=--valgrind, no?

By the way, "export VAR=VAL" all on the same line, even though it is in POSIX.1, is reported to be unsupported by some shells people care about, and needs to be corrected to "VAR=VAL" and "export VAR" as separate commands. I think we saw a patch to fix an instance or two that snuck in recently.

Show 18 quoted lines

>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 78c4286..98c90b0 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -93,6 +93,12 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
>  export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
>  export EDITOR
>  
> +# Add libc malloc_check and MALLOC_PERTURB test 
> +export MALLOC_CHECK_=3
> +export MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)"
> +#
> +
> +
>  # Protect ourselves from common misconfiguration to export
>  # CDPATH into the environment
>  unset CDPATH