]> git.ipfire.org Git - thirdparty/git.git/commitdiff
tests: add a test mode for SANITIZE=leak, run it in CI
authorÆvar Arnfjörð Bjarmason <avarab@gmail.com>
Thu, 23 Sep 2021 09:20:46 +0000 (11:20 +0200)
committerJunio C Hamano <gitster@pobox.com>
Thu, 23 Sep 2021 18:29:45 +0000 (11:29 -0700)
While git can be compiled with SANITIZE=leak, we have not run
regression tests under that mode. Memory leaks have only been fixed as
one-offs without structured regression testing.

This change adds CI testing for it. We'll now build and small set of
whitelisted t00*.sh tests under Linux with a new job called
"linux-leaks".

The CI target uses a new GIT_TEST_PASSING_SANITIZE_LEAK=true test
mode. When running in that mode, we'll assert that we were compiled
with SANITIZE=leak. We'll then skip all tests, except those that we've
opted-in by setting "TEST_PASSES_SANITIZE_LEAK=true".

A test setting "TEST_PASSES_SANITIZE_LEAK=true" setting can in turn
make use of the "SANITIZE_LEAK" prerequisite, should they wish to
selectively skip tests even under
"GIT_TEST_PASSING_SANITIZE_LEAK=true". In the preceding commit we
started doing this in "t0004-unwritable.sh" under SANITIZE=leak, now
it'll combine nicely with "GIT_TEST_PASSING_SANITIZE_LEAK=true".

This is how tests that don't set "TEST_PASSES_SANITIZE_LEAK=true" will
be skipped under GIT_TEST_PASSING_SANITIZE_LEAK=true:

    $ GIT_TEST_PASSING_SANITIZE_LEAK=true ./t0001-init.sh
    1..0 # SKIP skip all tests in t0001 under SANITIZE=leak, TEST_PASSES_SANITIZE_LEAK not set

The intent is to add more TEST_PASSES_SANITIZE_LEAK=true annotations
as follow-up change, but let's start small to begin with.

In ci/run-build-and-tests.sh we make use of the default "*" case to
run "make test" without any GIT_TEST_* modes. SANITIZE=leak is known
to fail in combination with GIT_TEST_SPLIT_INDEX=true in
t0016-oidmap.sh, and we're likely to have other such failures in
various GIT_TEST_* modes. Let's focus on getting the base tests
passing, we can expand coverage to GIT_TEST_* modes later.

It would also be possible to implement a more lightweight version of
this by only relying on setting "LSAN_OPTIONS". See
<YS9OT/pn5rRK9cGB@coredump.intra.peff.net>[1] and
<YS9ZIDpANfsh7N+S@coredump.intra.peff.net>[2] for a discussion of
that. I've opted for this approach of adding a GIT_TEST_* mode instead
because it's consistent with how we handle other special test modes.

Being able to add a "!SANITIZE_LEAK" prerequisite and calling
"test_done" early if it isn't satisfied also means that we can more
incrementally add regression tests without being forced to fix
widespread and hard-to-fix leaks at the same time.

We have tests that do simple checking of some tool we're interested
in, but later on in the script might be stressing trace2, or common
sources of leaks like "git log" in combination with the tool (e.g. the
commit-graph tests). To be clear having a prerequisite could also be
accomplished by using "LSAN_OPTIONS" directly.

On the topic of "LSAN_OPTIONS": It would be nice to have a mode to
aggregate all failures in our various scripts, see [2] for a start at
doing that which sets "log_path" in "LSAN_OPTIONS". I've punted on
that for now, it can be added later.

As of writing this we've got major regressions between master..seen,
i.e. the t000*.sh tests and more fixed since 31f9acf9ce2 (Merge branch
'ah/plugleaks', 2021-08-04) have regressed recently.

See the discussion at <87czsv2idy.fsf@evledraar.gmail.com>[3] about
the lack of this sort of test mode, and 0e5bba53af (add UNLEAK
annotation for reducing leak false positives, 2017-09-08) for the
initial addition of SANITIZE=leak.

See also 09595ab381 (Merge branch 'jk/leak-checkers', 2017-09-19),
7782066f67 (Merge branch 'jk/apache-lsan', 2019-05-19) and the recent
936e58851a (Merge branch 'ah/plugleaks', 2021-05-07) for some of the
past history of "one-off" SANITIZE=leak (and more) fixes.

As noted in [5] we can't support this on OSX yet until Clang 14 is
released, at that point we'll probably want to resurrect that
"osx-leaks" job.

1. https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer
2. https://lore.kernel.org/git/YS9OT%2Fpn5rRK9cGB@coredump.intra.peff.net/
3. https://lore.kernel.org/git/87czsv2idy.fsf@evledraar.gmail.com/
4. https://lore.kernel.org/git/YS9ZIDpANfsh7N+S@coredump.intra.peff.net/
5. https://lore.kernel.org/git/20210916035603.76369-1-carenas@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
13 files changed:
.github/workflows/main.yml
ci/install-dependencies.sh
ci/lib.sh
t/README
t/t0004-unwritable.sh
t/t0011-hashmap.sh
t/t0016-oidmap.sh
t/t0017-env-helper.sh
t/t0018-advice.sh
t/t0030-stripspace.sh
t/t0063-string-list.sh
t/t0091-bugreport.sh
t/test-lib.sh

index 68596f25927570c7725250f41cad6c459b4c5acc..be5cbb37748e10a313837048e35feea08ceb203f 100644 (file)
@@ -232,6 +232,9 @@ jobs:
           - jobname: linux-gcc-default
             cc: gcc
             pool: ubuntu-latest
+          - jobname: linux-leaks
+            cc: gcc
+            pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
       jobname: ${{matrix.vector.jobname}}
index 5772081b6e55dac5cd6973df8609852036d9cfe9..1d0e48f451558e685602ac362270d169b42caa92 100755 (executable)
@@ -12,7 +12,7 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
 case "$jobname" in
-linux-clang|linux-gcc)
+linux-clang|linux-gcc|linux-leaks)
        sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
        sudo apt-get -q update
        sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
index 476c3f369f549d69f52723c21d65bd4bf8e93da6..82cb17f8eea732967860ffe5b6c82f5be92d96c7 100755 (executable)
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -183,7 +183,7 @@ export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
 case "$jobname" in
-linux-clang|linux-gcc)
+linux-clang|linux-gcc|linux-leaks)
        if [ "$jobname" = linux-gcc ]
        then
                export CC=gcc-8
@@ -233,4 +233,11 @@ linux-musl)
        ;;
 esac
 
+case "$jobname" in
+linux-leaks)
+       export SANITIZE=leak
+       export GIT_TEST_PASSING_SANITIZE_LEAK=true
+       ;;
+esac
+
 MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
index 9e7012230203805da49deeba5f9a66af2199c792..8b5f86a46f32d1ee1a3b2f47a5931e7fa46fa906 100644 (file)
--- a/t/README
+++ b/t/README
@@ -366,6 +366,13 @@ excluded as so much relies on it, but this might change in the future.
 GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
 test suite. Accept any boolean values that are accepted by git-config.
 
+GIT_TEST_PASSING_SANITIZE_LEAK=<boolean> when compiled with
+SANITIZE=leak will run only those tests that have whitelisted
+themselves as passing with no memory leaks. Tests can be whitelisted
+by setting "TEST_PASSES_SANITIZE_LEAK=true" before sourcing
+"test-lib.sh" itself at the top of the test script. This test mode is
+used by the "linux-leaks" CI target.
+
 GIT_TEST_PROTOCOL_VERSION=<n>, when set, makes 'protocol.version'
 default to n.
 
index fbdcb926b3ab127365706c6414495b0a487cd3c8..37d68ef03be4a468a4926d37f3cfab40f78ac9bb 100755 (executable)
@@ -2,6 +2,7 @@
 
 test_description='detect unwritable repository and fail correctly'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success setup '
index 5343ffd3f92c1637bb60719d812a01e0ab6d4064..e094975b13bf270992d9cac8999bb0972a633198 100755 (executable)
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test hashmap and string hash functions'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_hashmap() {
index 31f8276ba82bad3129c4a3cfa92f2d0ba798073c..0faef1f4f110dc0ce5006bcc9195d2a6216cd76a 100755 (executable)
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test oidmap'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # This purposefully is very similar to t0011-hashmap.sh
index 4a159f99e44342d04f2810e2f136637c51f5e400..2e42fba9567d5b35bf72eda214d336a7c3564566 100755 (executable)
@@ -2,6 +2,7 @@
 
 test_description='test env--helper'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 
index 39e5e4b34f8729e361f7cc7801dadb4db6d6c76b..c13057a4ca3be22e431771fd60d5ab0325349bdc 100755 (executable)
@@ -2,6 +2,7 @@
 
 test_description='Test advise_if_enabled functionality'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'advice should be printed when config variable is unset' '
index 0c24a0f9a377103f49fa7cb749a5657058da3490..ae1ca380c1aa54709a557374d08a700337759b75 100755 (executable)
@@ -5,6 +5,7 @@
 
 test_description='git stripspace'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 t40='A quick brown fox jumps over the lazy do'
index c6ee9f66b11d55f312ee673106ddf295c39bb50d..46d4839194bb2322b2a8cd9b80618a855e0fb070 100755 (executable)
@@ -5,6 +5,7 @@
 
 test_description='Test string list functionality'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_split () {
index 526304ff95b3e91e7db69f823bfc0e173b4e0764..eeedbfa919327b206a1d730069edb8cd1ed59930 100755 (executable)
@@ -2,6 +2,7 @@
 
 test_description='git bugreport'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # Headers "[System Info]" will be followed by a non-empty line if we put some
index 4ab18914a3d5c68e5db539d44bd971049c1b3388..3b7acfec23b3d6f924677429dbcc0671746d8b04 100644 (file)
@@ -1379,6 +1379,26 @@ then
        test_done
 fi
 
+# skip non-whitelisted tests when compiled with SANITIZE=leak
+if test -n "$SANITIZE_LEAK"
+then
+       if test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
+       then
+               # We need to see it in "git env--helper" (via
+               # test_bool_env)
+               export TEST_PASSES_SANITIZE_LEAK
+
+               if ! test_bool_env TEST_PASSES_SANITIZE_LEAK false
+               then
+                       skip_all="skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true"
+                       test_done
+               fi
+       fi
+elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
+then
+       error "GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except when compiled with SANITIZE=leak"
+fi
+
 # Last-minute variable setup
 HOME="$TRASH_DIRECTORY"
 GNUPGHOME="$HOME/gnupg-home-not-used"