lsan for LLVM bootstrap; leaks in TableGen

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

lsan for LLVM bootstrap; leaks in TableGen

Kostya Serebryany
Hi, 

We are trying to enable LeakSanitizer on our asan/msan llvm bootstrap bot
In clang itself there are two leaks 
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.
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"




_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

lsan for LLVM bootstrap; leaks in TableGen

David Blaikie
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 <[hidden email][hidden email][hidden email]> 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.

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"



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Alexey Samsonov
On Wed, Dec 25, 2013 at 9:38 PM, [hidden email] <[hidden email]> 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
> <[hidden email]> 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
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>



--
Alexey Samsonov, MSK
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: lsan for LLVM bootstrap; leaks in TableGen

Kostya Serebryany
In reply to this post by David Blaikie



On Wed, Dec 25, 2013 at 7:38 PM, [hidden email] <[hidden email]> 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
It is.  
(I would've expected a build flag option)

That would not be trivial. 
The link-time flag is -fsanitize=address, and we do want to pass it because we do want to check TableGen for addressability bugs
(and because if we don't pass it at link time we can not use it at compile time, which will cause a huge cmake/make challenge).
Inventing a link time flag that will keep asan but remove lsan is not worth it. 
There is the run-time flag (ASAN_OPTIONS=detect_leaks=0|1) which turns lsan on/off,
 but again passing it selectively to TableGen is harder than my simple patch. 
 
for lsan, I'd say go for it.

Ok, unless someone objects. 

--kcc 
 

(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 <[hidden email][hidden email][hidden email]> 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.

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"




_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Kostya Serebryany
In reply to this post by Alexey Samsonov



On Wed, Dec 25, 2013 at 11:21 PM, Alexey Samsonov <[hidden email]> wrote:
On Wed, Dec 25, 2013 at 9:38 PM, [hidden email] <[hidden email]> 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
> <[hidden email]> 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
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>



--
Alexey Samsonov, MSK


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Chandler Carruth-2
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 <[hidden email]> wrote:



On Wed, Dec 25, 2013 at 11:21 PM, Alexey Samsonov <[hidden email]> wrote:
On Wed, Dec 25, 2013 at 9:38 PM, [hidden email] <[hidden email]> 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
> <[hidden email]> 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
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>



--
Alexey Samsonov, MSK


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Kostya Serebryany
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 <[hidden email]> 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 <[hidden email]> wrote:



On Wed, Dec 25, 2013 at 11:21 PM, Alexey Samsonov <[hidden email]> wrote:
On Wed, Dec 25, 2013 at 9:38 PM, [hidden email] <[hidden email]> 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
> <[hidden email]> 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
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>



--
Alexey Samsonov, MSK


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev




_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Chandler Carruth-2

On Thu, Dec 26, 2013 at 2:40 AM, Kostya Serebryany <[hidden email]> 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.

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Kostya Serebryany
This? 

+extern "C" {
+// Disable LeakSanitizer for this binary as it has too many leaks that are not
+// very interesting to fix. __lsan_is_turned_off is explained in
+// compiler-rt/include/sanitizer/lsan_interface.h
+int __lsan_is_turned_off() { return 1; }
+}  // extern "C"



On Thu, Dec 26, 2013 at 11:49 AM, Chandler Carruth <[hidden email]> wrote:

On Thu, Dec 26, 2013 at 2:40 AM, Kostya Serebryany <[hidden email]> 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.


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Kostya Serebryany
Thoughts? 


On Thu, Dec 26, 2013 at 11:55 AM, Kostya Serebryany <[hidden email]> wrote:
This? 

+extern "C" {
+// Disable LeakSanitizer for this binary as it has too many leaks that are not
+// very interesting to fix. __lsan_is_turned_off is explained in
+// compiler-rt/include/sanitizer/lsan_interface.h
+int __lsan_is_turned_off() { return 1; }
+}  // extern "C"



On Thu, Dec 26, 2013 at 11:49 AM, Chandler Carruth <[hidden email]> wrote:

On Thu, Dec 26, 2013 at 2:40 AM, Kostya Serebryany <[hidden email]> 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.



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Chandler Carruth-2
LGTM


On Thu, Jan 9, 2014 at 3:43 AM, Kostya Serebryany <[hidden email]> wrote:
Thoughts? 


On Thu, Dec 26, 2013 at 11:55 AM, Kostya Serebryany <[hidden email]> wrote:
This? 

+extern "C" {
+// Disable LeakSanitizer for this binary as it has too many leaks that are not
+// very interesting to fix. __lsan_is_turned_off is explained in
+// compiler-rt/include/sanitizer/lsan_interface.h
+int __lsan_is_turned_off() { return 1; }
+}  // extern "C"



On Thu, Dec 26, 2013 at 11:49 AM, Chandler Carruth <[hidden email]> wrote:

On Thu, Dec 26, 2013 at 2:40 AM, Kostya Serebryany <[hidden email]> 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.




_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Marshall Clow-2
In reply to this post by Kostya Serebryany
On Dec 25, 2013, at 11:55 PM, Kostya Serebryany <[hidden email]> wrote:
On Thu, Dec 26, 2013 at 11:49 AM, Chandler Carruth <[hidden email]> wrote:

On Thu, Dec 26, 2013 at 2:40 AM, Kostya Serebryany <[hidden email]> 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.

This? 

+extern "C" {
+// Disable LeakSanitizer for this binary as it has too many leaks that are not
+// very interesting to fix. __lsan_is_turned_off is explained in
+// compiler-rt/include/sanitizer/lsan_interface.h
+int __lsan_is_turned_off() { return 1; }
+}  // extern “C"

[ Sorry to be joining the conversation late ]

What is the reasoning behind having them define a function to disable lsan, rather than calling __lsan_disable?
Is it so that lsan can be turned off before main() is entered?

I’m not really happy with the idea of the user having to define a function with a reserved name in their code.

— Marshall


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Kostya Serebryany



On Fri, Jan 10, 2014 at 12:53 AM, Marshall Clow <[hidden email]> wrote:
On Dec 25, 2013, at 11:55 PM, Kostya Serebryany <[hidden email]> wrote:
On Thu, Dec 26, 2013 at 11:49 AM, Chandler Carruth <[hidden email]> wrote:

On Thu, Dec 26, 2013 at 2:40 AM, Kostya Serebryany <[hidden email]> 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.

This? 

+extern "C" {
+// Disable LeakSanitizer for this binary as it has too many leaks that are not
+// very interesting to fix. __lsan_is_turned_off is explained in
+// compiler-rt/include/sanitizer/lsan_interface.h
+int __lsan_is_turned_off() { return 1; }
+}  // extern “C"

[ Sorry to be joining the conversation late ]

What is the reasoning behind having them define a function to disable lsan, rather than calling __lsan_disable?
Is it so that lsan can be turned off before main() is entered?

Yes. Also, this allows us to disable lsan w/o modifying the source file where main() is defined

--kcc

 
I’m not really happy with the idea of the user having to define a function with a reserved name in their code.

— Marshall



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] lsan for LLVM bootstrap; leaks in TableGen

Kostya Serebryany
FTR: LLVM is now LeakSanitizer-clean, lsan is enabled on the sanitizer bootstrap bot, which is green: 

--kcc 


On Fri, Jan 10, 2014 at 8:02 AM, Kostya Serebryany <[hidden email]> wrote:



On Fri, Jan 10, 2014 at 12:53 AM, Marshall Clow <[hidden email]> wrote:
On Dec 25, 2013, at 11:55 PM, Kostya Serebryany <[hidden email]> wrote:
On Thu, Dec 26, 2013 at 11:49 AM, Chandler Carruth <[hidden email]> wrote:

On Thu, Dec 26, 2013 at 2:40 AM, Kostya Serebryany <[hidden email]> 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.

This? 

+extern "C" {
+// Disable LeakSanitizer for this binary as it has too many leaks that are not
+// very interesting to fix. __lsan_is_turned_off is explained in
+// compiler-rt/include/sanitizer/lsan_interface.h
+int __lsan_is_turned_off() { return 1; }
+}  // extern “C"

[ Sorry to be joining the conversation late ]

What is the reasoning behind having them define a function to disable lsan, rather than calling __lsan_disable?
Is it so that lsan can be turned off before main() is entered?

Yes. Also, this allows us to disable lsan w/o modifying the source file where main() is defined

--kcc

 
I’m not really happy with the idea of the user having to define a function with a reserved name in their code.

— Marshall




_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev