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

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

[Elia Pinto]

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.

But the test suite can use also valgrind(memcheck) via 'make valgrind'
or 'make GIT_TEST_OPTS="--valgrind"'.

Memcheck wraps client calls to malloc(), and puts a "red zone" on
each end of each block in order to detect access overruns.
Memcheck already detects double free() (up to the limit of the buffer
which remembers pending free()). Thus memcheck subsumes all the
documented coverage of MALLOC_CHECK_.

If MALLOC_CHECK_ is set non-zero when running memcheck, then the
overruns that might be detected by MALLOC_CHECK_ would be overruns
on the wrapped blocks which include the red zones.  Thus MALLOC_CHECK_
would be checking memcheck, and not the client.  This is not useful,
and actually is wasteful.  The only possible [documented] advantage
of using MALLOC_CHECK_ and memcheck together, would be if MALLOC_CHECK_
detected duplicate free() in more cases than memcheck because memcheck's
buffer is too small.

Therefore we don't use MALLOC_CHECK_ and valgrind(memcheck) at the
same time.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
This the third reroll of the original patch.

I redid the patch correcting the export command in a more portable
way thanks to the Junio observation and not setting MALLOC_CHECK_
at the same time we are using valgrind. Added in the commit the reason,
not so simple to find. Hope the better :=)


 t/test-lib.sh |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 78c4286..f34b861 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -93,6 +93,15 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
 export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
 export EDITOR
 
+# Add libc MALLOC and MALLOC_PERTURB test 
+# only if we are not executing the test with valgrind
+expr "$GIT_TEST_OPTS" : ".*\(--valgrind\)" >/dev/null || {
+	MALLOC_CHECK_=3
+	export MALLOC_CHECK_
+	MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)"
+	export MALLOC_PERTURB_
+}
+
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
 unset CDPATH
-- 
1.7.10.4

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

[Junio C Hamano]

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

> 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.
> ...
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
> This the third reroll of the original patch.

Well written.  I have one suggestion and a question, though.

>  t/test-lib.sh |    9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 78c4286..f34b861 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -93,6 +93,15 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
>  export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
>  export EDITOR
>  
> +# Add libc MALLOC and MALLOC_PERTURB test 
> +# only if we are not executing the test with valgrind
> +expr "$GIT_TEST_OPTS" : ".*\(--valgrind\)" >/dev/null || {

I would write this like

	if ! expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null
	then
		...
	fi

> +	MALLOC_CHECK_=3
> +	export MALLOC_CHECK_
> +	MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)"

How was this expression chosen?  The only effect I can think of to
use a randomly picked value here is to make it impossible to make
the test repeatable and reproducible, so you must have had some
benefit that outweighs the downside, but I cannot think of any.
Wouldn't MALLOC_PERTURB_=165 (i.e. 0xA5, half of the bits in the
byte is set, including the MSB, and is an odd number) be equally a
good choice without repeatability downside, for example?

Also, doesn't the above give 256 sometimes, which would not fit in a
byte?

> +	export MALLOC_PERTURB_
> +}
> +
>  # Protect ourselves from common misconfiguration to export
>  # CDPATH into the environment
>  unset CDPATH

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Elia Pinto <gitter.spiros@gmail.com> writes:
>
>> 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.
>> ...
>> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
>> ---
>> This the third reroll of the original patch.
>
> Well written.  I have one suggestion and a question, though.

After looking at it a bit more, there are a few more things I would
suggest, in the form of an squashable patch on top.

Points to note:

 - When test-lib.sh is used from perf-lib, we would not want to be
   affected with MALLOC_CHECK_; we would want to run as vanilla as
   possible in that case.

 - We are interested in checking "git" and whatever we test using
   the test harness, i.e. what comes inside test_expect_success.
   Setting MALLOC_CHECK_ at the beginning will cover a lot more than
   what we want to test.

 - That "165" thing I mentioned earlier.

 - Update the "expr" expression to make sure the --valgrind we catch
   is indeed an option, not a substring of something else.  Also
   there is no need to capture the substring.

 t/perf/perf-lib.sh |  1 +
 t/test-lib.sh      | 26 ++++++++++++++++++++------
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git i/t/perf/perf-lib.sh w/t/perf/perf-lib.sh
index a1361e5..1d0bb9d 100644
--- i/t/perf/perf-lib.sh
+++ w/t/perf/perf-lib.sh
@@ -42,6 +42,7 @@ else
 fi
 
 TEST_NO_CREATE_REPO=t
+TEST_NO_MALLOC_=t
 
 . ../test-lib.sh
 
diff --git i/t/test-lib.sh w/t/test-lib.sh
index b0c0c84..aad4606 100644
--- i/t/test-lib.sh
+++ w/t/test-lib.sh
@@ -95,12 +95,24 @@ export EDITOR
 
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
-expr "$GIT_TEST_OPTS" : ".*\(--valgrind\)" >/dev/null || {
-	MALLOC_CHECK_=3
-	export MALLOC_CHECK_
-	MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)"
-	export MALLOC_PERTURB_
-}
+if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
+   test -n "TEST_NO_MALLOC_"
+then
+	setup_malloc_check () {
+		: nothing
+	}
+	teardown_malloc_check () {
+		: nothing
+	}
+else
+	setup_malloc_check () {
+		MALLOC_CHECK_=3	MALLOC_PERTURB_=165
+		export MALLOC_CHECK_ MALLOC_PERTURB_
+	}
+	teardown_malloc_check () {
+		unset MALLOC_CHECK_ MALLOC_PERTURB_
+	}
+fi
 
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
@@ -311,7 +323,9 @@ test_run_ () {
 
 	if test -z "$immediate" || test $eval_ret = 0 || test -n "$expecting_failure"
 	then
+		setup_malloc_check
 		test_eval_ "$test_cleanup"
+		teardown_malloc_check
 	fi
 	if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"
 	then

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

[Elia Pinto]

2012/9/15 Junio C Hamano <gitster@pobox.com>:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Elia Pinto <gitter.spiros@gmail.com> writes:
>>
>>> 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.
>>> ...
>>> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
>>> ---
>>> This the third reroll of the original patch.
>>
>> Well written.  I have one suggestion and a question, though.
>
> After looking at it a bit more, there are a few more things I would
> suggest, in the form of an squashable patch on top.
>
> Points to note:
>
>  - When test-lib.sh is used from perf-lib, we would not want to be
>    affected with MALLOC_CHECK_; we would want to run as vanilla as
>    possible in that case.
>
>  - We are interested in checking "git" and whatever we test using
>    the test harness, i.e. what comes inside test_expect_success.
>    Setting MALLOC_CHECK_ at the beginning will cover a lot more than
>    what we want to test.
>
>  - That "165" thing I mentioned earlier.

Thank you so much for the comments, that's fine. A single
consideration for  MALLOC_PERTURB.

You can use any value between 1..255 for MALLOC_PERTURB_
That chooses the byte that glibc will use to memset all freed buffers.
In general it is defined as

    export MALLOC_PERTURB_=$(($RANDOM % 255 + 1))

(as drepper pointed out http://udrepper.livejournal.com/11429.html)

I don't use RANDOM because it is not portable but $$ is sufficient for
the bias. I use this in popt
(http://rpm5.org/community/rpm-devel/5311.html but the site is down
now)

Using a random value is slightly better than using a fixed one
in case your fixed value is someday just the right/wrong value to mask
a problem.  At least with a random value, if you rerun
the test in a different shell, the odds are good you won't use
the unfortunate setting again. % is the module operation so the above
expression returns a value between 1 and 255 always (for the euclidean
division
property :=).

So OK per the original expression ? I prefer a perfect commit for git,
but no problem to follow your advice and reroll the patch.

Opinions ?

Best Regards

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

[Junio C Hamano]

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

>>  - That "165" thing I mentioned earlier.
>
> Thank you so much for the comments, that's fine. A single
> consideration for  MALLOC_PERTURB.
>
> You can use any value between 1..255 for MALLOC_PERTURB_
> That chooses the byte that glibc will use to memset all freed buffers.
> In general it is defined as
>
>     export MALLOC_PERTURB_=$(($RANDOM % 255 + 1))
>
> (as drepper pointed out http://udrepper.livejournal.com/11429.html)

Drepper never recommends RANDOM there.

> Using a random value is slightly better than using a fixed one
> in case your fixed value is someday just the right/wrong value to mask
> a problem.

Quite the contrary.  When you use a fixed pattern, it is easy which
other pieces of memory has uninitailized contents.  When you use a
random value, you sometimes get an error and sometimes the test
mysteriously pass, which does not help debugging.

openSUSE folks seem to use a fixed value for this exact reason of
repeatability of tests.

http://jaegerandi.blogspot.com/2012/01/finding-subtile-malloc-bugs.html

> So OK per the original expression?

No.

I am not convinced 165 is the perfect value, but I am fairly certain
any fixed value is better than using a random to deliberately worsen
repeatability of the tests.

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

[Elia Pinto]

Ok.

Please use the patch that you have already queued in the ep integration branch.

Thank you

2012/9/17, Junio C Hamano <gitster@pobox.com>:
> Elia Pinto <gitter.spiros@gmail.com> writes:
>
>>>  - That "165" thing I mentioned earlier.
>>
>> Thank you so much for the comments, that's fine. A single
>> consideration for  MALLOC_PERTURB.
>>
>> You can use any value between 1..255 for MALLOC_PERTURB_
>> That chooses the byte that glibc will use to memset all freed buffers.
>> In general it is defined as
>>
>>     export MALLOC_PERTURB_=$(($RANDOM % 255 + 1))
>>
>> (as drepper pointed out http://udrepper.livejournal.com/11429.html)
>
> Drepper never recommends RANDOM there.
>
>> Using a random value is slightly better than using a fixed one
>> in case your fixed value is someday just the right/wrong value to mask
>> a problem.
>
> Quite the contrary.  When you use a fixed pattern, it is easy which
> other pieces of memory has uninitailized contents.  When you use a
> random value, you sometimes get an error and sometimes the test
> mysteriously pass, which does not help debugging.
>
> openSUSE folks seem to use a fixed value for this exact reason of
> repeatability of tests.
>
> http://jaegerandi.blogspot.com/2012/01/finding-subtile-malloc-bugs.html
>
>> So OK per the original expression?
>
> No.
>
> I am not convinced 165 is the perfect value, but I am fairly certain
> any fixed value is better than using a random to deliberately worsen
> repeatability of the tests.
>

-- 
Inviato dal mio dispositivo mobile

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

[René Scharfe]

Sorry for being late.  Just wanted to try out this new feature and
ended up reading this old thread.

Am 15.09.2012 01:18, schrieb Junio C Hamano:
>   t/perf/perf-lib.sh |  1 +
>   t/test-lib.sh      | 26 ++++++++++++++++++++------
>   2 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git i/t/perf/perf-lib.sh w/t/perf/perf-lib.sh
> index a1361e5..1d0bb9d 100644
> --- i/t/perf/perf-lib.sh
> +++ w/t/perf/perf-lib.sh
> @@ -42,6 +42,7 @@ else
>  fi
>   
>  TEST_NO_CREATE_REPO=t
> +TEST_NO_MALLOC_=t

Why the trailing underscore?  Perhaps add "CHECK" before the equal sign?

>   . ../test-lib.sh
>   
> diff --git i/t/test-lib.sh w/t/test-lib.sh
> index b0c0c84..aad4606 100644
> --- i/t/test-lib.sh
> +++ w/t/test-lib.sh
> @@ -95,12 +95,24 @@ export EDITOR
>  
>  # Add libc MALLOC and MALLOC_PERTURB test
>  # only if we are not executing the test with valgrind
> -expr "$GIT_TEST_OPTS" : ".*\(--valgrind\)" >/dev/null || {
> -	MALLOC_CHECK_=3
> -	export MALLOC_CHECK_
> -	MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)"
> -	export MALLOC_PERTURB_
> -}
> +if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
> +   test -n "TEST_NO_MALLOC_"

Without a $, you'll get nothing. ;-)

('test -n "string"' is always true, unlike 'test -n "$variable"'.)

> +then
> +	setup_malloc_check () {
> +		: nothing
> +	}
> +	teardown_malloc_check () {
> +		: nothing
> +	}
> +else
> +	setup_malloc_check () {
> +		MALLOC_CHECK_=3	MALLOC_PERTURB_=165
> +		export MALLOC_CHECK_ MALLOC_PERTURB_
> +	}
> +	teardown_malloc_check () {
> +		unset MALLOC_CHECK_ MALLOC_PERTURB_
> +	}

Would it make sense to restore the previous values?  Hrm, can't think of a use case.

> +fi
>   
>  # Protect ourselves from common misconfiguration to export
>  # CDPATH into the environment
> @@ -311,7 +323,9 @@ test_run_ () {
>  
>  	if test -z "$immediate" || test $eval_ret = 0 || test -n "$expecting_failure"
>  	then
> +		setup_malloc_check
>  		test_eval_ "$test_cleanup"
> +		teardown_malloc_check
>  	fi
>  	if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"
>  	then

-- >8 --
Subject: [PATCH] MALLOC_CHECK: enable it, unless disabled explicitly

The malloc checks in tests are currently disabled.  Actually evaluate
the variable for turning them off and enable them if it's unset.

Also use this opportunity to give it the more descriptive and
consistent name TEST_NO_MALLOC_CHECK.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 t/perf/perf-lib.sh | 2 +-
 t/test-lib.sh      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 1d0bb9d..a816fbc 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -42,7 +42,7 @@ else
 fi
 
 TEST_NO_CREATE_REPO=t
-TEST_NO_MALLOC_=t
+TEST_NO_MALLOC_CHECK=t
 
 . ../test-lib.sh
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bff3d75..617d831 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -105,7 +105,7 @@ export EDITOR
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
-   test -n "TEST_NO_MALLOC_"
+   test -n "$TEST_NO_MALLOC_CHECK"
 then
 	setup_malloc_check () {
 		: nothing
-- 
1.7.12

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

[Junio C Hamano]

Thanks.

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

[Junio C Hamano]

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

> 2012/9/12 Junio C Hamano <gitster@pobox.com>:
>>
>> Interesting, but it bothers me to make it enabled unconditionally.
>> At least, this shouldn't be enabled under GIT_TEST_OPTS=--valgrind, no?
> Sorry for the late response and thanks.
>
> No, setting MALLOC_CHECK don't require
> valgrind ...

You never said anything like that, and I didn't question it.

> and it considered a best QA to have the test suite with it
> defined always. If the test suite fail with MALLOC_CHECK, well, there
> is some problem, no ?

I never said using MALLOC_CHECK_ is a bad idea.

Let me ask the same question in a different way, as I seem to have
failed in the previous message.

If you are using valgrind to run tests, is it sane to also enable
MALLOC_CHECK?  If you were testing "cat", would it make sense to do:

	$ MALLOC_CHECK_=3 valgrind cat README

Because we are not interested in testing how valgrind (not cat)
uses malloc, we may be better off running

	$ valgrind cat README

without MALLOC_CHECK_; it will reduce the risk of MALLOC_CHECK_
potentially disturbing what we really want to check (i.e. cat) by
triggering for something whose problems we are not trying to see
(i.e. valgrind), no?

That was my question.

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

[Elia Pinto]

2012/9/12 Junio C Hamano <gitster@pobox.com>:
>
> Interesting, but it bothers me to make it enabled unconditionally.
> At least, this shouldn't be enabled under GIT_TEST_OPTS=--valgrind, no?
Sorry for the late response and thanks.

No, setting MALLOC_CHECK don't require
valgrind and it considered a best QA to have the test suite with it
defined always. If the test suite fail with MALLOC_CHECK, well, there
is some problem, no ?
Some distro do it already in building packages (fedora for example)
http://emacs.1067599.n5.nabble.com/please-set-both-MALLOC-PERTURB-and-MALLOC-CHECK-envvars-td150144.html

At @rpm5.org we do the same for popt, for example, from years

http://rpm5.org/community/rpm-devel/4156.html
>
> 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.
Yes, right, my bad. I will reroll.

Thank you

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

[Junio C Hamano]

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

> 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.

>
> 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

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

[Elia Pinto]

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(+)

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
-- 
1.7.11.rc1