On Thu, Jan 9, 2014 at 4:18 PM, Chandler Carruth <[email protected]>wrote:
> > On Thu, Jan 9, 2014 at 4:12 AM, Kostya Serebryany <[email protected]> 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
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
