[llvm-dev] Eliminating global memory roots (or not) to help leak checkers

[Vitaly Buka via llvm-dev]

On Wed, 14 Apr 2021 at 09:39, Sterling Augustine via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> [Continuing discussion from https://reviews.llvm.org/D69428]
>
> Llvm is fairly conservative when eliminating global variables (or fields
> of such) that may point to dynamically allocated memory. This behavior is
> entirely to help leak checking tools such as Valgrind, Google's
> HeapLeakChecker, and LSAN, all of which treat memory that is reachable at
> exit as "not leaked", even though it will never be freed. Without these
> global variables to hold the pointer, the leak checkers can't determine
> that it is actually reachable, and will report a leak. Global variables
> that dynamically allocate memory but don't clean themselves up are fairly
> common in the wild, and various leak checkers have long not reported errors.
>
> This behavior was added all the way back in 2012 in
> https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20120625/145646.html
> .
>
> https://reviews.llvm.org/D69428 removed this behavior, and I subsequently
> reverted it when many internal Google tests started failing, but I believe
> many other users who use leak checking will encounter errors when this hits
> more mainstream releases.
>
> So: What to do?
>
> Preventing a valid transformation (the global variables are never read and
> can be eliminated) to help the leak checkers leaves some performance and
> code size on the table. Just how much is unclear.
>
> On the other hand, having leak checkers suddenly start reporting failures
> where they didn't before also seems suboptimal. Cleaning this somewhat
> common scenario up is surprisingly difficult at the user level.
>
> Some possibilities:
>
> 1. Only do this at high optimization levels, say -O3. This would give
> aggressive users all the performance we can, but also make leak checkers
> report leaks sometimes, but not others.
>

This could be disabled for Asan by default as it very likely runs with lsan.


>
> 2. Hide it behind a flag or configurable option. Users who care can set it
> as they prefer. Creates more confusing options, different testing matrices
> and such, but everyone can get the behaviour that they want.
>
> 3. Do it all the time, and users who encounter issues can clean up their
> code. Users get the most performance they possibly can, but have to clean
> up code or drop leak checking. Seems a little user hostile.
>

I expect this requires significant cleanup effort, and not just in Google.
It's quite a common pattern, but it would be nice to see some real data.


>
> Other possibilities?:
>

5.  Maybe replace writes into such removed global with no-op callback which
can be intercepted by LeakChecker? This will prevent other
useful optimizations.

_______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210414/5f4f1e72/attachment.html>