kubabrecka 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
----------------
ygribov wrote:
> samsonov wrote:
> > 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.
> > the binary distribution is the source of truth, not the list of sanitizers 
> > hardcoded into Clang driver source code.
> 
> This will not work for cross-compilers. It _may_ be ok for OSX but not for 
> other platforms.
Why not?  On Linux, there are statically-linked sanitizers, if you want to 
cross-compile, you need to have the runtime libraries for the target.  And 
dynamic libraries are a similar story – they're version-tied to the compiler 
and your compiler should really have the libraries of the sanitizers that it 
supports.


http://reviews.llvm.org/D15225



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to