r198858 - Disable LeakSanitizer in TableGen binaries, see PR18325

[Kostya Serebryany]

On Thu, Jan 9, 2014 at 4:18 PM, Chandler Carruth <chandlerc at google.com>wrote:

>
> On Thu, Jan 9, 2014 at 4:12 AM, Kostya Serebryany <kcc at google.com> wrote:
>
>> Hard to tell.
>> The standalone lsan exists and is tested in llvm (check-lsan).
>> On the one hand, we are not planing to use standalone lsan anywhere any
>> time soon.
>> 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.
>> 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.
>>
>> If we guard this code with something, I'd prefer it to be a regular macro
>> (e.g. LEAK_SANITIZER) rather than __has_feature:
>>   1. __has_feature is not supported by GCC (lsan is)
>>
>
> I don't understand. GCC can just as easily make '__has_feature(...)'
> expand to 1 as any other macro.
>

__has_feature is not a regular macro.

% cat has_feature.cc
#if __has_feature(address_sanitizer)
#error ASAN IS ON
#else
#error ASAN IS OFF
#endif
% clang  has_feature.cc
has_feature.cc:4:2: error: ASAN IS OFF
#error ASAN IS OFF
 ^
1 error generated.
% clang  has_feature.cc -fsanitize=address
has_feature.cc:2:2: error: ASAN IS ON
#error ASAN IS ON
 ^
1 error generated.
% g++  has_feature.cc
has_feature.cc:1:18: error: missing binary operator before token "("
has_feature.cc:4:2: error: #error ASAN IS OFF
%



Unless someone is planning to use LSan with a GCC host on an LLVM internal
> binary, this seems like a weak argument.
>

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)



>
> We also guard ASan stuff with __has_feature
>

Where we use __has_feature we always do the ugly

#ifdef __has_feature
  #if __has_feature(address_sanitizer)
  #endif
#endif



> and GCC has ASan...
>
GCC defines __SANITIZE_ADDRESS__ instead of supporting __has_feature. (old
and long story)



>
> 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.
>
>   2. in case someone wants to have a standalone lsan via LD_PRELOAD
>>
>
> 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?
>

Consider someone wants to run clang bootstrap with ld-preloaded lsan.
(S)he will do "LD_PRELOAD=/path/to/lsan-lib.so make check-all".
The current solution will just work.
With #ifdef LEAK_SANITIZER it will work if cmake was called with
...-DLEAK_SANITIZER regardless of the host compiler
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.

--kcc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140109/9fdfae0e/attachment.html>