git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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.
Previous: Jeff KingNext: Jeff King
Message 4 of 5 in “Makefile: suppress annotated leaks with certain ASan options”
  1. Makefile: suppress annotated leaks with certain ASan optionsTaylor Blau, Jan 20, 2023
  2. Junio C HamanoJan 20, 2023
  3. Jeff KingJan 20, 2023
  4. Junio C HamanoJan 20, 2023
  5. Jeff KingJan 20, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.