Jeff King <peff@xxxxxxxx> writes: > 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. > 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.
