llvm dev - Dec 2013 - [LLVMdev] [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

LGTM.

I think it is totally reasonable to just disable LSan directly in tablegen
at least for now.

I would leave some comments on the disabling to clarify:
1) What it does, the reader may not have heard about LSan.
2) Some of the high-level information from this thread as pointers for
anyone who wants to go and fix this some day.

-Chandler


On Wed, Dec 25, 2013 at 2:26 PM, Kostya Serebryany <kcc at google.com>
wrote:
>
>
>
> On Wed, Dec 25, 2013 at 11:21 PM, Alexey Samsonov <samsonov at
google.com>wrote:
>
>> On Wed, Dec 25, 2013 at 9:38 PM, dblaikie at gmail.com <dblaikie at
gmail.com>
>> wrote:
>> > Its generally been by design that tblgen leaks. Might be nice to
fix
>> one day
>> > but it's been that way for a while now so don't expect it
to be fixed
>> soon.
>> >
>> > If thats the suppression mechanism of choice (I would've
expected a
>> build
>> > flag option) for lsan, I'd say go for it.
>> >
>> > (Maybe there's some existing build flag handling for valgrind
so as not
>> to
>> > leak check tblgen, but that would be a runtime flag, not build
time)
>> >
>> > On Wednesday, December 25, 2013 7:08:27 AM, Kostya Serebryany
>> > <kcc at google.com> wrote:
>> >
>> > Hi,
>> >
>> > We are trying to enable LeakSanitizer on our asan/msan llvm
bootstrap
>> bot
>> >
>> >
(http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/).
>> >
>> > In clang itself there are two leaks
>> >
>> > (http://llvm.org/bugs/show_bug.cgi?id=18318,
>> > http://llvm-reviews.chandlerc.com/D2472)
>> >
>> > and one lsan-hostile feature (
>> http://llvm.org/bugs/show_bug.cgi?id=18320),
>> >
>> > all of which are easy to fix.
>> >
>> > And there are also lots of leaks in TableGen.
>>
>> I think the problem with TableGen is that we call leak checking too
>> late - after the global destructors,
>> not before them. IIRC pointers to objects "leaked" in
TableGen are
>> stored in a set with static storage duration.
>> If we're able to call __lsan_do_leak_check() right after main(), we
>> will treat that objects as reachable.
>>
>
> This is part of the problem. Indeed, there is code like this:
>
> void foo() {
>   static Pool ThePool;
>    ...
>   ThePool.Insert(new Stuff);
>   ...
> }
>
> This static variable is destructed w/o deleting the contents and lsan
> complains.
> I've replaced some of these objects with
>   static Pool *ThePool = new Pool
> and those leaks were gone.
>
> But then there were a few other leaks of different nature, all of which I
> did not want to investigate.
>
> --kcc
>
>
>
>> I considered adding a call to __lsan_do_leak_check into
>> llvm_shutdown() function, but that looks wrong -
>> for instance, llvm_shutdown() may be called when a dynamic library is
>> unloaded.
>>
>
>
>
>>
>> >
>> > Would anyone be interested in fixing TableGen leaks?
>> >
>> > I'd guess not since TableGen is not a user-facing utility,
leaks there
>> are
>> > not too harmful.
>> >
>> > If so, I'd like to disable lsan for TableGen so that
>> >
>> > we can enable lsan on the bootstrap bot soon. Objections?
>> >
>> > --kcc
>> >
>> > Index: utils/TableGen/TableGen.cpp
>> >
>> >
==================================================================>> >
>> > --- utils/TableGen/TableGen.cpp (revision 198007)
>> >
>> > +++ utils/TableGen/TableGen.cpp (working copy)
>> >
>> > @@ -180,3 +180,7 @@
>> >
>> >
>> >
>> >    return TableGenMain(argv[0], &LLVMTableGenMain);
>> >
>> >  }
>> >
>> > +
>> >
>> > +extern "C" {
>> >
>> > +int __lsan_is_turned_off() { return 1; }
>> >
>> > +}  // extern "C"
>> >
>> > Index: tools/clang/utils/TableGen/TableGen.cpp
>> >
>> >
==================================================================>> >
>> > --- tools/clang/utils/TableGen/TableGen.cpp     (revision 198007)
>> >
>> > +++ tools/clang/utils/TableGen/TableGen.cpp     (working copy)
>> >
>> > @@ -248,3 +248,7 @@
>> >
>> >
>> >
>> >    return TableGenMain(argv[0], &ClangTableGenMain);
>> >
>> >  }
>> >
>> > +
>> >
>> > +extern "C" {
>> >
>> > +int __lsan_is_turned_off() { return 1; }
>> >
>> > +}  // extern "C"
>> >
>> >
>> >
>> > _______________________________________________
>> > cfe-dev mailing list
>> > cfe-dev at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>> >
>>
>>
>>
>> --
>> Alexey Samsonov, MSK
>>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131225/2177a707/attachment.html>

Like this?

+extern "C" {
+// Disable LeakSanitizer, see http://llvm.org/bugs/show_bug.cgi?id=18325.
+int __lsan_is_turned_off() { return 1; }
+}  // extern "C"



On Thu, Dec 26, 2013 at 8:53 AM, Chandler Carruth <chandlerc at
google.com>wrote:
> LGTM.
>
> I think it is totally reasonable to just disable LSan directly in tablegen
> at least for now.
>
> I would leave some comments on the disabling to clarify:
> 1) What it does, the reader may not have heard about LSan.
> 2) Some of the high-level information from this thread as pointers for
> anyone who wants to go and fix this some day.
>
> -Chandler
>
>
> On Wed, Dec 25, 2013 at 2:26 PM, Kostya Serebryany <kcc at
google.com> wrote:
>
>>
>>
>>
>> On Wed, Dec 25, 2013 at 11:21 PM, Alexey Samsonov <samsonov at
google.com>wrote:
>>
>>> On Wed, Dec 25, 2013 at 9:38 PM, dblaikie at gmail.com <dblaikie
at gmail.com>
>>> wrote:
>>> > Its generally been by design that tblgen leaks. Might be nice
to fix
>>> one day
>>> > but it's been that way for a while now so don't expect
it to be fixed
>>> soon.
>>> >
>>> > If thats the suppression mechanism of choice (I would've
expected a
>>> build
>>> > flag option) for lsan, I'd say go for it.
>>> >
>>> > (Maybe there's some existing build flag handling for
valgrind so as
>>> not to
>>> > leak check tblgen, but that would be a runtime flag, not build
time)
>>> >
>>> > On Wednesday, December 25, 2013 7:08:27 AM, Kostya Serebryany
>>> > <kcc at google.com> wrote:
>>> >
>>> > Hi,
>>> >
>>> > We are trying to enable LeakSanitizer on our asan/msan llvm
bootstrap
>>> bot
>>> >
>>> >
(http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/).
>>> >
>>> > In clang itself there are two leaks
>>> >
>>> > (http://llvm.org/bugs/show_bug.cgi?id=18318,
>>> > http://llvm-reviews.chandlerc.com/D2472)
>>> >
>>> > and one lsan-hostile feature (
>>> http://llvm.org/bugs/show_bug.cgi?id=18320),
>>> >
>>> > all of which are easy to fix.
>>> >
>>> > And there are also lots of leaks in TableGen.
>>>
>>> I think the problem with TableGen is that we call leak checking too
>>> late - after the global destructors,
>>> not before them. IIRC pointers to objects "leaked" in
TableGen are
>>> stored in a set with static storage duration.
>>> If we're able to call __lsan_do_leak_check() right after
main(), we
>>> will treat that objects as reachable.
>>>
>>
>> This is part of the problem. Indeed, there is code like this:
>>
>> void foo() {
>>   static Pool ThePool;
>>    ...
>>   ThePool.Insert(new Stuff);
>>   ...
>> }
>>
>> This static variable is destructed w/o deleting the contents and lsan
>> complains.
>> I've replaced some of these objects with
>>   static Pool *ThePool = new Pool
>> and those leaks were gone.
>>
>> But then there were a few other leaks of different nature, all of which
I
>> did not want to investigate.
>>
>>  --kcc
>>
>>
>>
>>> I considered adding a call to __lsan_do_leak_check into
>>> llvm_shutdown() function, but that looks wrong -
>>> for instance, llvm_shutdown() may be called when a dynamic library
is
>>> unloaded.
>>>
>>
>>
>>
>>>
>>> >
>>> > Would anyone be interested in fixing TableGen leaks?
>>> >
>>> > I'd guess not since TableGen is not a user-facing utility,
leaks there
>>> are
>>> > not too harmful.
>>> >
>>> > If so, I'd like to disable lsan for TableGen so that
>>> >
>>> > we can enable lsan on the bootstrap bot soon. Objections?
>>> >
>>> > --kcc
>>> >
>>> > Index: utils/TableGen/TableGen.cpp
>>> >
>>> >
==================================================================>>>
>
>>> > --- utils/TableGen/TableGen.cpp (revision 198007)
>>> >
>>> > +++ utils/TableGen/TableGen.cpp (working copy)
>>> >
>>> > @@ -180,3 +180,7 @@
>>> >
>>> >
>>> >
>>> >    return TableGenMain(argv[0], &LLVMTableGenMain);
>>> >
>>> >  }
>>> >
>>> > +
>>> >
>>> > +extern "C" {
>>> >
>>> > +int __lsan_is_turned_off() { return 1; }
>>> >
>>> > +}  // extern "C"
>>> >
>>> > Index: tools/clang/utils/TableGen/TableGen.cpp
>>> >
>>> >
==================================================================>>>
>
>>> > --- tools/clang/utils/TableGen/TableGen.cpp     (revision
198007)
>>> >
>>> > +++ tools/clang/utils/TableGen/TableGen.cpp     (working copy)
>>> >
>>> > @@ -248,3 +248,7 @@
>>> >
>>> >
>>> >
>>> >    return TableGenMain(argv[0], &ClangTableGenMain);
>>> >
>>> >  }
>>> >
>>> > +
>>> >
>>> > +extern "C" {
>>> >
>>> > +int __lsan_is_turned_off() { return 1; }
>>> >
>>> > +}  // extern "C"
>>> >
>>> >
>>> >
>>> > _______________________________________________
>>> > cfe-dev mailing list
>>> > cfe-dev at cs.uiuc.edu
>>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>> >
>>>
>>>
>>>
>>> --
>>> Alexey Samsonov, MSK
>>>
>>
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131226/3bc2673a/attachment.html>

On Thu, Dec 26, 2013 at 2:40 AM, Kostya Serebryany <kcc at google.com>
wrote:
> Like this?
>
> +extern "C" {
> +// Disable LeakSanitizer, see http://llvm.org/bugs/show_bug.cgi?id=18325.
>
We don't often reference bugs in comments. I would give a brief summary in
the text of the comment, and mention the bug in the commit log.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131226/80334505/attachment.html>