Re: [PATCH] Makefile: suppress annotated leaks with certain ASan options
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 20, 2023, 20:46 UTC
- Message-ID
- <xmqqmt6dvsqk.fsf@gitster.g>
- In-Reply-To
- <Y8r2Tn75g52YTIij@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 22 quoted lines
> I do think there's some complexity here, though. > > One problem UNLEAK() is that compile-time switch, but whether ASan does > leak detection is a run-time choice. So you are stuck with either: > > - you always turn on UNLEAK() for ASan builds, in which case test runs > using the default ASAN_OPTIONS we set do the extra work even though > they are not doing any leak detection. I doubt it's very measurable, > though (it's just shoving a few bytes onto a linked list), > especially compared to the overall slowness of ASan. > > - you predicate the build-time choice on ASAN_OPTIONS. But this means > that: > > make SANITIZE=address > cd t > ASAN_OPTIONS=detect_leaks=1 ./t0000-*.sh > > will confusingly fail to use UNLEAK(). > > Your patch does the second one, but I think the first may be the > least-bad option.
Thanks, I totally missed the fact that ASAN_OPTIONS was a runtime thing. If we were to pursue this topic of enabling UNLEAK() outside LSan, I agree the first would be necessary.
Show 8 quoted lines
> But for leak-checking, we've already seen real cases where using LSan > with higher optimization levels can lead to false positives (because the > optimizer drops a value that is still in scope but not used in a code > path that leads to exit()). > ... > So it may be that we really do want to keep leak-checking to "-O0 > -fsanitize=leak", and reserve "-O2 -fsanitize=address" for finding > address bugs.
Yup, we have been burned a few times with this, IIRC.