Commits on Sep 23, 2021

1. Makefile: add SANITIZE=leak flag to GIT-BUILD-OPTIONS …

   When SANITIZE=leak is specified we'll now add a SANITIZE_LEAK flag to
   GIT-BUILD-OPTIONS, this can then be picked up by the test-lib.sh,
   which sets a SANITIZE_LEAK prerequisite.
   
   We can then skip specific tests that are known to fail under
   SANITIZE=leak, add one such annotation to t0004-unwritable.sh, which
   now passes under SANITIZE=leak.
   
   Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
   Signed-off-by: Junio C Hamano <gitster@pobox.com>

   

   avar authored and gitster committed Sep 23, 2021
   Copy the full SHA

   2cdc292 View commit details

   Browse the repository at this point in the history

2. 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/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 31f9acf (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 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%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>

   

   avar authored and gitster committed Sep 23, 2021
   Copy the full SHA

   956d2e4 View commit details

   Browse the repository at this point in the history