r288448 - [CUDA] "Support" ASAN arguments in CudaToolChain

[Jason Henline via cfe-commits]

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 <hfinkel at anl.gov> wrote:

> ----- Original Message -----
> > From: "Jason Henline via cfe-commits" <cfe-commits at lists.llvm.org>
> > To: cfe-commits at 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 at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161202/a07665f2/attachment.html>