[PATCH] D69428: [GlobalOpt] Remove valgrind specific hacks (revert r160529)

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 20 10:11:29 PDT 2021


MaskRay added a comment.

In D69428#2700667 <https://reviews.llvm.org/D69428#2700667>, @JonChesterfield wrote:

> In D69428#2689270 <https://reviews.llvm.org/D69428#2689270>, @jyknight wrote:
>
>> MaskRay's example does not show an improvement in LSAN behavior, but rather a new false positive. Memory which is still accessible at process termination is not a leak, so the source -- as written -- had no leak
>
> This seems an unusual definition of memory leak.
>
> Is it predicated on the assumption of a hosted environment which will perform the memory free on behalf of the application at termination, where said host+application in some sense can never leak memory?

I've provided some context why we need to retain global pointer variable retained objects: https://lists.llvm.org/pipermail/llvm-dev/2021-April/149857.html
Actually I just applied this pattern yesterday for lldb: D100806 <https://reviews.llvm.org/D100806>.

The global variable may have readers but that readers can be optimized out. Then with this patch, such global variables will be optimized away and cause a leak.

If we ignore such patterns, we would cause a huge list of false positives in many projects, with all of valgrind, -fsanitize=leak, -fsanitize=address.

As of how to retain the potential missing optimization (to the best of my knowledge it is unmeasured), I've suggested that we can ignore function pointers as root-set if deemed useful.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69428/new/

https://reviews.llvm.org/D69428



More information about the llvm-commits mailing list