From: Andrzej Hunt Date: Sun, 14 Mar 2021 16:55:09 GMT Subject: Re: [PATCH 0/7] Fix all leaks in t0001 Message-ID: In-Reply-To: On 08/03/2021 19:57, Junio C Hamano wrote: > "Andrzej Hunt via GitGitGadget" writes: > >> hand: fixing or suppressing these leaks should make it easier to spot leaks >> that have more significant user impact (it's entirely plausible that no real >> impactful leaks exist). > > Even if there is no leaks that matter exist now, to help maintain > that state, suppressing false positives would be useful, provided if > such checkers are run sufficiently often. Expanding on my thoughts in response to Peff's comments regarding running leak checking under ASAN: I'm wondering whether it would be acceptable to piggyback leak-checking on top of the existing ASAN test runs: - It looks like people are already running tests with ASAN from time to time - enabling leak checking there would add leak-checking coverage without having to run LSAN separately (this would have to be restricted to those tests that are leak-free - more on that below). - I also saw some discussion around enabling ASAN in CI, although I don't think that went anywhere yet [1]. I'd be interested in trying to pick that up - ASAN seems quite valuable by itself, and running it in CI would be a simple way to also get leak-checking in CI. (Again, this would have to be limited to known leak-free tests.) (I'm planning to run some benchmarks to see how much enabling leak-checking with ASAN actually costs - I'm assuming it's fairly cheap if you're already running ASAN, but I'd like to verify that.) As to the mechanics of enabling leak-checking with ASAN: test-lib.sh currently completely disables ASAN's leak-checking. Instead we could add a simple allowlist, to enable leak-checking for those tests that don't - Fix LSAN crashleak - this needn't be much more complex than: $TEST_NUMBER <= highest_leak_free_test I.e. something like this: diff --git a/t/test-lib.sh b/t/test-lib.sh index d3f6af6a65..cf9f1ad827 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -41,7 +41,13 @@ GIT_BUILD_DIR="$TEST_DIRECTORY"/.. # the noise level. This needs to happen at the start of the script, # before we even do our "did we build git yet" check (since we don't # want that one to complain to stderr). -: ${ASAN_OPTIONS=detect_leaks=0:abort_on_error=1} +if test $TEST_NUMBER -le 10 +then + DETECT_LEAKS=1 +else + DETECT_LEAKS=0 +fi +: ${ASAN_OPTIONS=detect_leaks=$DETECT_LEAKS:abort_on_error=1} export ASAN_OPTIONS # If LSAN is in effect we _do_ want leak checking, but we still (This would also require moving TEST_NUMBER a bit earlier in test-lib.sh.) [1] https://public-inbox.org/git/20170710155831.3zxijp7bvbquvlau@sigill.intra.peff.net/