Skip to content

Commit 956d2e4

Browse files
avargitster
authored andcommitted
tests: add a test mode for SANITIZE=leak, run it in CI
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/[email protected]>[1] and <[email protected]>[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 31f9acf (Merge branch 'ah/plugleaks', 2021-08-04) have regressed recently. See the discussion at <[email protected]>[3] about the lack of this sort of test mode, and 0e5bba5 (add UNLEAK annotation for reducing leak false positives, 2017-09-08) for the initial addition of SANITIZE=leak. See also 09595ab (Merge branch 'jk/leak-checkers', 2017-09-19), 7782066 (Merge branch 'jk/apache-lsan', 2019-05-19) and the recent 936e588 (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%[email protected]/ 3. https://lore.kernel.org/git/[email protected]/ 4. https://lore.kernel.org/git/[email protected]/ 5. https://lore.kernel.org/git/[email protected]/ Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Signed-off-by: Carlo Marcelo Arenas Belón <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 2cdc292 commit 956d2e4

13 files changed

+49
-2
lines changed

.github/workflows/main.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,9 @@ jobs:
232232
- jobname: linux-gcc-default
233233
cc: gcc
234234
pool: ubuntu-latest
235+
- jobname: linux-leaks
236+
cc: gcc
237+
pool: ubuntu-latest
235238
env:
236239
CC: ${{matrix.vector.cc}}
237240
jobname: ${{matrix.vector.jobname}}

ci/install-dependencies.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
1212
libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
1313

1414
case "$jobname" in
15-
linux-clang|linux-gcc)
15+
linux-clang|linux-gcc|linux-leaks)
1616
sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
1717
sudo apt-get -q update
1818
sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \

ci/lib.sh

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ export GIT_TEST_CLONE_2GB=true
183183
export SKIP_DASHED_BUILT_INS=YesPlease
184184

185185
case "$jobname" in
186-
linux-clang|linux-gcc)
186+
linux-clang|linux-gcc|linux-leaks)
187187
if [ "$jobname" = linux-gcc ]
188188
then
189189
export CC=gcc-8
@@ -233,4 +233,11 @@ linux-musl)
233233
;;
234234
esac
235235

236+
case "$jobname" in
237+
linux-leaks)
238+
export SANITIZE=leak
239+
export GIT_TEST_PASSING_SANITIZE_LEAK=true
240+
;;
241+
esac
242+
236243
MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"

t/README

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,13 @@ excluded as so much relies on it, but this might change in the future.
366366
GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
367367
test suite. Accept any boolean values that are accepted by git-config.
368368

369+
GIT_TEST_PASSING_SANITIZE_LEAK=<boolean> when compiled with
370+
SANITIZE=leak will run only those tests that have whitelisted
371+
themselves as passing with no memory leaks. Tests can be whitelisted
372+
by setting "TEST_PASSES_SANITIZE_LEAK=true" before sourcing
373+
"test-lib.sh" itself at the top of the test script. This test mode is
374+
used by the "linux-leaks" CI target.
375+
369376
GIT_TEST_PROTOCOL_VERSION=<n>, when set, makes 'protocol.version'
370377
default to n.
371378

t/t0004-unwritable.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
test_description='detect unwritable repository and fail correctly'
44

5+
TEST_PASSES_SANITIZE_LEAK=true
56
. ./test-lib.sh
67

78
test_expect_success setup '

t/t0011-hashmap.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/bin/sh
22

33
test_description='test hashmap and string hash functions'
4+
5+
TEST_PASSES_SANITIZE_LEAK=true
46
. ./test-lib.sh
57

68
test_hashmap() {

t/t0016-oidmap.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/bin/sh
22

33
test_description='test oidmap'
4+
5+
TEST_PASSES_SANITIZE_LEAK=true
46
. ./test-lib.sh
57

68
# This purposefully is very similar to t0011-hashmap.sh

t/t0017-env-helper.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
test_description='test env--helper'
44

5+
TEST_PASSES_SANITIZE_LEAK=true
56
. ./test-lib.sh
67

78

t/t0018-advice.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
test_description='Test advise_if_enabled functionality'
44

5+
TEST_PASSES_SANITIZE_LEAK=true
56
. ./test-lib.sh
67

78
test_expect_success 'advice should be printed when config variable is unset' '

t/t0030-stripspace.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
test_description='git stripspace'
77

8+
TEST_PASSES_SANITIZE_LEAK=true
89
. ./test-lib.sh
910

1011
t40='A quick brown fox jumps over the lazy do'

t/t0063-string-list.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
test_description='Test string list functionality'
77

8+
TEST_PASSES_SANITIZE_LEAK=true
89
. ./test-lib.sh
910

1011
test_split () {

t/t0091-bugreport.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
test_description='git bugreport'
44

5+
TEST_PASSES_SANITIZE_LEAK=true
56
. ./test-lib.sh
67

78
# Headers "[System Info]" will be followed by a non-empty line if we put some

t/test-lib.sh

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1379,6 +1379,26 @@ then
13791379
test_done
13801380
fi
13811381

1382+
# skip non-whitelisted tests when compiled with SANITIZE=leak
1383+
if test -n "$SANITIZE_LEAK"
1384+
then
1385+
if test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
1386+
then
1387+
# We need to see it in "git env--helper" (via
1388+
# test_bool_env)
1389+
export TEST_PASSES_SANITIZE_LEAK
1390+
1391+
if ! test_bool_env TEST_PASSES_SANITIZE_LEAK false
1392+
then
1393+
skip_all="skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true"
1394+
test_done
1395+
fi
1396+
fi
1397+
elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
1398+
then
1399+
error "GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except when compiled with SANITIZE=leak"
1400+
fi
1401+
13821402
# Last-minute variable setup
13831403
HOME="$TRASH_DIRECTORY"
13841404
GNUPGHOME="$HOME/gnupg-home-not-used"

0 commit comments

Comments
 (0)