[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

[Alexey Samsonov via cfe-commits]

samsonov added inline comments.

================
Comment at: test/Driver/fsanitize.c:221
@@ +220,3 @@
+// RUN: %clang -target x86_64-apple-darwin10 -resource-dir=%S/Inputs/resource_dir -fsanitize=memory -fsanitize=thread,memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-TSAN-MSAN-DARWIN1
+// CHECK-MSAN-TSAN-MSAN-DARWIN1: unsupported option '-fsanitize=thread,memory' for target 'x86_64-apple-darwin10'
+// CHECK-MSAN-TSAN-MSAN-DARWIN1-NOT: unsupported option
----------------
beanz wrote:
> kubabrecka wrote:
> > samsonov wrote:
> > > Again, I feel like we're lying to users here: `-fsanitize=thread` *is* supported for this target, it just requires building a runtime.
> > I'd like to see this from the point-of-view of a binary distribution.  If the binary distribution (e.g. the one from llvm.org or Apple's Clang in Xcode) doesn't contain a runtime library, then the sanitizer is *not* supported in that distribution.  Also, see http://reviews.llvm.org/D14846, we'd like to have CMake options to select which runtimes will be built.  If you deliberately choose not to build ThreadSanitizer, then that sanitizer is *not* supported in your version of Clang.  If you're experimenting and porting a runtime to a new platform, then this sanitizer *is* supported in your version of Clang.
> Maybe the point is we should have a different error message for cases where the runtime is just missing. Something like "runtime components for '-fsanitize=thread' not available"
I see, so essentially you want to use a different approach for determining sanitizer availability (on OS X for now): if the library is present, then we support sanitizer, otherwise we don't: i.e. the binary distribution is the source of truth, not the list of sanitizers hardcoded into Clang driver source code. I'm fine with that, and see why it would make sense.

It's just that error message looks misleading: the problem is not TSan is unsupported for target, it's just unavailable in this distribution for one reason or another.


http://reviews.llvm.org/D15225