Re: r288448 - [CUDA] "Support" ASAN arguments in CudaToolChain
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=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=288447=288448=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 , const llvm::Triple
> > ,
> > const ArgList )
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains.h
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=288448=288447=288448=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 ,
> > llvm::opt::ArgStringList ) const
> > override;
> >
> > + SanitizerMask getSupportedSanitizers() const override;
> > +
> >const ToolChain
> >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=288447=28844
Re: r288448 - [CUDA] "Support" ASAN arguments in CudaToolChain
- 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=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=288447=288448=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 , const llvm::Triple
> ,
> const ArgList )
>
> Modified: cfe/trunk/lib/Driver/ToolChains.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=288448=288447=288448=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 ,
> llvm::opt::ArgStringList ) const
> override;
>
> + SanitizerMask getSupportedSanitizers() const override;
> +
>const ToolChain
>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=288447=288448=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
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=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=288447=288448=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;
+}
+
/// XCore tool chain
XCoreToolChain::XCoreToolChain(const Driver , const llvm::Triple ,
const ArgList )
Modified: cfe/trunk/lib/Driver/ToolChains.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=288448=288447=288448=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 ,
llvm::opt::ArgStringList ) const override;
+ SanitizerMask getSupportedSanitizers() const override;
+
const ToolChain
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=288447=288448=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
