Thread (1 message) 1 message, 1 author, 2023-01-20

Re: [PATCH] Makefile: suppress annotated leaks with certain ASan options

From: Junio C Hamano <hidden>
Date: 2023-01-20 20:47:02

Jeff King [off-list ref] 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help