From: Junio C Hamano <gitster@pobox•com>
To: Elia Pinto <gitter.spiros@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption
Date: Fri, 14 Sep 2012 16:18:50 -0700 [thread overview]
Message-ID: <7vligc19d1.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7v392k5w7u.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Fri, 14 Sep 2012 10:51:33 -0700")
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
next prev parent reply other threads:[~2012-09-14 23:19 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-14 16:54 [PATCH] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption Elia Pinto
2012-09-14 17:51 ` Junio C Hamano
2012-09-14 23:18 ` Junio C Hamano [this message]
2012-09-17 12:17 ` Elia Pinto
2012-09-17 20:28 ` Junio C Hamano
2012-09-18 4:22 ` Elia Pinto
2012-09-26 20:16 ` René Scharfe
2012-09-27 6:39 ` Junio C Hamano
-- strict thread matches above, loose matches on Subject: below --
2012-09-12 12:17 Elia Pinto
2012-09-12 17:51 ` Junio C Hamano
2012-09-13 16:36 ` Elia Pinto
2012-09-13 17:46 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vligc19d1.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitter.spiros@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox