Hi Hal,
I don't understand why only ASAN is enabled. What about TSAN, etc.?
Shouldn't you query whatever the host toolchain is?
I think you're right, I should just implement it as
SanitizerMask CudaToolChain::getSupportedSanitizers() const { return
HostTC.getSupportedSanitizers(); }
I will get a patch up later today to do that.
Thanks for the suggestion!
-Jason
On Fri, Dec 2, 2016 at 4:31 AM Hal Finkel <hfin...@anl.gov> wrote:
> ----- Original Message -----
> > From: "Jason Henline via cfe-commits" <cfe-commits@lists.llvm.org>
> > To: cfe-commits@lists.llvm.org
> > Sent: Thursday, December 1, 2016 7:42:54 PM
> > Subject: r288448 - [CUDA] "Support" ASAN arguments in CudaToolChain
> >
> > Author: jhen
> > Date: Thu Dec 1 19:42:54 2016
> > New Revision: 288448
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=288448&view=rev
> > Log:
> > [CUDA] "Support" ASAN arguments in CudaToolChain
> >
> > This fixes a bug that was introduced in rL287285. The bug made it
> > illegal to pass -fsanitize=address during CUDA compilation because
> > the
> > CudaToolChain class was switched from deriving from the Linux
> > toolchain
> > class to deriving directly from the ToolChain toolchain class. When
> > CudaToolChain derived from Linux, it used Linux's
> > getSupportedSanitizers
> > method, and that method allowed ASAN, but when it switched to
> > deriving
> > directly from ToolChain, it inherited a getSupportedSanitizers method
> > that didn't allow for ASAN.
> >
> > This patch fixes that bug by creating a getSupportedSanitizers method
> > for CudaToolChain that supports ASAN.
> >
> > This patch also fixes the test that checks that -fsanitize=address is
> > passed correctly for CUDA builds. That test didn't used to notice if
> > an
> > error message was emitted, and that's why it didn't catch this bug
> > when
> > it was first introduced. With the fix from this patch, that test will
> > now catch any similar bug in the future.
> >
> > Modified:
> > cfe/trunk/lib/Driver/ToolChains.cpp
> > cfe/trunk/lib/Driver/ToolChains.h
> > cfe/trunk/test/Driver/cuda-no-sanitizers.cu
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=288448&r1=288447&r2=288448&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Driver/ToolChains.cpp (original)
> > +++ cfe/trunk/lib/Driver/ToolChains.cpp Thu Dec 1 19:42:54 2016
> > @@ -4973,6 +4973,15 @@ void CudaToolChain::AddIAMCUIncludeArgs(
> > HostTC.AddIAMCUIncludeArgs(Args, CC1Args);
> > }
> >
> > +SanitizerMask CudaToolChain::getSupportedSanitizers() const {
> > + // The CudaToolChain only supports address sanitization in the
> > sense that it
> > + // allows ASAN arguments on the command line. It must not error
> > out on these
> > + // command line arguments because the host code compilation
> > supports them.
> > + // However, it doesn't actually do any address sanitization for
> > device code;
> > + // instead, it just ignores any ASAN command line arguments it
> > sees.
> > + return SanitizerKind::Address;
> > +}
>
> I don't understand why only ASAN is enabled. What about TSAN, etc.?
> Shouldn't you query whatever the host toolchain is?
>
> Thanks again,
> Hal
>
> > +
> > /// XCore tool chain
> > XCoreToolChain::XCoreToolChain(const Driver &D, const llvm::Triple
> > &Triple,
> > const ArgList &Args)
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains.h
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=288448&r1=288447&r2=288448&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Driver/ToolChains.h (original)
> > +++ cfe/trunk/lib/Driver/ToolChains.h Thu Dec 1 19:42:54 2016
> > @@ -912,6 +912,8 @@ public:
> > void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs,
> > llvm::opt::ArgStringList &CC1Args) const
> > override;
> >
> > + SanitizerMask getSupportedSanitizers() const override;
> > +
> > const ToolChain &HostTC;
> > CudaInstallationDetector CudaInstallation;
> >
> >
> > Modified: cfe/trunk/test/Driver/cuda-no-sanitizers.cu
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cuda-no-sanitizers.cu?rev=288448&r1=288447&r2=288448&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/test/Driver/cuda-no-sanitizers.cu (original)
> > +++ cfe/trunk/test/Driver/cuda-no-sanitizers.cu Thu Dec 1 19:42:54
> > 2016
> > @@ -6,6 +6,7 @@
> > // RUN: %clang -### -target x86_64-linux-gnu -c
> > --cuda-gpu-arch=sm_20 -fsanitize=address %s 2>&1 | \
> > // RUN: FileCheck %s
> >
> > +// CHECK-NOT: error:
> > // CHECK-DAG: "-fcuda-is-device"
> > // CHECK-NOT: "-fsanitize=address"
> > // CHECK-DAG: "-triple" "x86_64--linux-gnu"
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
