Alexey Samsonov
2013-Dec-25 19:21 UTC
[LLVMdev] [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen
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. 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
Kostya Serebryany
2013-Dec-25 19:26 UTC
[LLVMdev] [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen
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 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131225/f18c14d2/attachment.html>
Chandler Carruth
2013-Dec-26 04:53 UTC
[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>