[PATCH] Don't localize globals that may serve as leak checker roots.
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
>> > 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
> 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)...
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
> Alexey Samsonov, MSK
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits