On 08/03/2021 19:57, Junio C Hamano wrote:
"Andrzej Hunt via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx/
