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

Reid Kleckner rnk at google.com
Thu Sep 26 09:00:51 PDT 2013

On Thu, Sep 26, 2013 at 1:36 AM, Alexey Samsonov <samsonov at google.com>wrote:

> 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.

IMO moving globals to locals in main is basically a SPEC benchmark hack.
 It's only profitable when most of the program can be inlined into main and
the globals become register allocated.

The C++ standard sez [basic.start.main]p6: "The function main shall not be
used (3.2) within a program."  The C standard has no such language, so this
optimization isn't really valid for C.  IIRC this caused PNaCl some

So if we want to remove this optimization, it should be on *that* basis,
and not because we want leak checking to work out of the box.

> 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
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130926/5dcf8da3/attachment.html>

More information about the llvm-commits mailing list