<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jan 9, 2014 at 4:18 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">
<div class="im">On Thu, Jan 9, 2014 at 4:12 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div>Hard to tell. </div><div>The standalone lsan exists and is tested in llvm (check-lsan).<br>
</div><div>On the one hand, we are not planing to use standalone lsan anywhere any time soon.</div>
<div>We are quite satisfied with the performance of asan+lsan and we don't want to have a separate build with standalone lsan for things like LLVM or Chromium.</div>
<div>On the other hand, there are already users of standalone lsan (outside of llvm community) and they find standalone lsan worth using because they can't pay the price of asan slowdown. </div><div><br></div><div>If we guard this code with something, I'd prefer it to be a regular macro (e.g. LEAK_SANITIZER) rather than __has_feature:</div>


<div>  1. __has_feature is not supported by GCC (lsan is)</div></blockquote><div><br></div></div><div>I don't understand. GCC can just as easily make '__has_feature(...)' expand to 1 as any other macro.</div>
</div></div></div></blockquote><div><br></div><div>__has_feature is not a regular macro.</div><div><br></div><div><div>% cat has_feature.cc</div><div>#if __has_feature(address_sanitizer)</div><div>#error ASAN IS ON</div><div>
#else</div><div>#error ASAN IS OFF</div><div>#endif</div><div>% clang  has_feature.cc</div><div>has_feature.cc:4:2: error: ASAN IS OFF</div><div>#error ASAN IS OFF</div><div> ^</div><div>1 error generated.</div><div>% clang  has_feature.cc -fsanitize=address </div>
<div>has_feature.cc:2:2: error: ASAN IS ON</div><div>#error ASAN IS ON</div><div> ^</div><div>1 error generated.</div><div>% g++  has_feature.cc </div><div>has_feature.cc:1:18: error: missing binary operator before token "("</div>
<div>has_feature.cc:4:2: error: #error ASAN IS OFF</div><div>% </div><div><br></div></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> Unless someone is planning to use LSan with a GCC host on an LLVM internal binary, this seems like a weak argument.</div></div></div></div></blockquote>
<div><br></div><div>I am not planing to do so, but it's not entirely unlikely that someone will (e.g. just to test the GCC support for lsan)</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><br></div><div>We also guard ASan stuff with __has_feature </div></div></div></div></blockquote><div><br></div><div>Where we use __has_feature we always do the ugly </div><div><br></div><div>#ifdef __has_feature</div>
<div>  #if __has_feature(address_sanitizer)</div><div>  #endif</div><div>#endif</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>and GCC has ASan...</div></div></div></div></blockquote>GCC defines __SANITIZE_ADDRESS__ instead of supporting __has_feature. (old and long story)<div>
<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">
<div class="gmail_quote"><div><br></div><div>If __has_feature is the right way to do this, we should use that. If it isn't, I suspect the reasons should not have anything to do with GCC.</div><div class="im">
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div>  2. in case someone wants to have a standalone lsan via LD_PRELOAD </div>
</blockquote></div></div><br>So, we're not talking about disabling all of LSan. Just the ability to turn LSan off for a single program using a global. I don't really see a problem with LD_PRELOAD not necessarily working with this one feature -- you could always just not preload LSan when running that binary?</div>
</div></blockquote><div><br></div><div>Consider someone wants to run clang bootstrap with ld-preloaded lsan. </div><div>(S)he will do "LD_PRELOAD=/path/to/lsan-lib.so make check-all".</div><div>The current solution will just work.</div>
With #ifdef LEAK_SANITIZER it will work if cmake was called with ...-DLEAK_SANITIZER regardless of the host compiler</div><div class="gmail_quote">With __has_feature it will only work if clang is the host compiler and if we used the appropriate __has_feature. (which needs to be implemented). Too much trouble for no gain, imho. <br>
<div><br></div><div>--kcc </div></div></div></div>