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

[Eric Christopher via llvm-dev]

+Chris Lattner <clattner at nondot.org> to make sure Chris sees this
response too.

On Wed, Apr 14, 2021 at 7:04 PM Kostya Serebryany via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> I would expect this change to cause a large cleanup and render lsan
> unusable on any non-trivial code base.
> But indeed, it would be nice to see the data.
>
> #1 is undesirable as it will make asan/lsan behave differently depending
> on the opt level.
> We can disable the transformation under asan (which includes lsan),
> but it will be harder to disable it under pure lsan (which is a link-time
> option).
>
> #2 might be the right first step to demonstrate the impact on code size
> and on lsan.
>
> Suppressions are not a good way to fix this (and generally, suppressions
> are almost never a good long term answer).
> Creating suppressions will take more work than adding
> __attribute((used))__, will be harder to maintain,
> and will cause true leaks to be lost.
>
> __attribute((used))__ is not a complete solution either.
> imagine a code base which imports a third_party code base verbatim and
> can't add attributes there.
>
> Philip's suggestion is worth investigating.
>
> <untested proposal>
> For globals that are never read and are written to only once, we may have
> an lsan-specific solution,
> by calling __lsan_enable/__lsan_disable around the to-be-removed
> assignment.
> Globals that are written to multiple times will be harder to handle.
>
> --kcc
>
>
> On Wed, Apr 14, 2021 at 2:53 PM Vitaly Buka via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>>
>>
>> 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
>>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
> _______________________________________________
> 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/20210422/afbace2a/attachment.html>