[PATCH] Don't localize globals that may serve as leak checker roots.

Alexey Samsonov samsonov at google.com
Thu Sep 26 01:36:05 PDT 2013


On Thu, Sep 26, 2013 at 12:37 AM, Rafael EspĂ­ndola <
rafael.espindola at gmail.com> wrote:

> >> I agree. Wouldn't it be better for clang to attach metadata to a
> >> global variable to say it is a GC root? That way the optimization is
> >> only disabled when using a sanitizer.
> >
> >
> > Huh? Isn't it just as broken with valgrind's leak checker? If not, why
> isn't
> > it?
>
> I assume it is broken with valgrind, but the llvm language spec
> doesn't say we cannot hide "leak check roots". That is why I think
> this should be controllable from the FE with an attribute (I now
> realize it cannot be metadata since we don't want it dropped).
>

I've verified that this optimization is indeed broken with valgrind.
Personally, I'm not sure this optimization is very useful - as Reid
mentions,
it fires for a very limited set of globals, and localizes them only to
main(). In the meantime,
it causes noticeable problems with leak checkers deployment - storing a
pointer to a
never-deleted object in a global variable seems to be a common pattern.

The long way - introduce a new -fsanitize=valgrind flag (are there another
use cases for this?),
teach FE to attach specific newly-introduced attributes to emitted globals
if any sanitizer is turned on,
and then consult these attributes in (already working) isLeakCheckerRoot()
function - seems like an overkill,
and will still be a problem for Valgrind users (after all, the benefit of
this tool is that you don't have to recompile
your code and adding -fsanitize=valgrind might be annoying)...


> Cheers,
> Rafael
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130926/fda1e1fe/attachment.html>


More information about the llvm-commits mailing list