[CUDA] "Support" ASAN arguments in CudaToolChain

[jhen]

Summary

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.

[jhen]

Before this patch, the following command would fail:

clang++ -fsanitize=address --cuda-gpu-arch=sm_20 -c example.cu

with error:

clang-4.0: error: unsupported option '-fsanitize=address' for target 'nvptx64-nvidia-cuda'

After this patch, there is no such compilation error.

I don't understand this part of the driver code very well, so I would appreciate any advice on whether this is the right way to fix this bug, and where would be the right place for me to put a test to check that it is working (i.e. ASAN still works for CUDA host code).

[jlebar]

where would be the right place for me to put a test to check that it is working (i.e. ASAN still works for CUDA host code).

I just ran the test from rL281680, and it passes because it doesn't notice clang raising the "unsupported option '-fsanitize=address" error. :)

I don't understand this part of the driver code very well

What I would probably do (other than ask Kostya, who may just Know) is first identify which commit caused the regression. That will probably give us some idea of whether or not this is the right fix (and whether or not the check in rL281680 is still necessary).

lib/Driver/SanitizerArgs.cpp

214 ↗

(On Diff #79963)

Nit, remove braces.

[kcc]

at the very least this requires a test... Let me thing about the logic a bit more...

[jlebar]

In D27316#611160, @kcc wrote:

at the very least this requires a test... Let me thing about the logic a bit more...

I think we just need to fix the test from the previous CL, which is buggy and didn't catch this regression.

[kcc]

I am not sure this is going to work.
You essentially break on the first iteration in the beginning of the loop.
Why not exit the function before the loop?
Also, the loop "claims" the args and by breaking early you leave the args unclaimed.

I don't remember this code well enough to argue about it w/o tests. :(

[jhen]

In D27316#611162, @kcc wrote:

I am not sure this is going to work.
You essentially break on the first iteration in the beginning of the loop.
Why not exit the function before the loop?
Also, the loop "claims" the args and by breaking early you leave the args unclaimed.

I don't remember this code well enough to argue about it w/o tests. :(

Thanks for taking a look. I just wanted to make sure nobody knew of a simple solution for this problem before I dug in further. Since none of us seems too familiar with this code, I'll do more investigation to try to find the right way to fix this. Once I do that I'll post it to this review (with a test or two to make sure it's working).

[jhen]

• "Support" ASAN in CudaToolChain

[jlebar]

This takes it back to how it used to be before I regressed it, which I think is good enough for now. If we want to revisit how we make this work for offloading toolchains in general, we can do that separately.

lib/Driver/ToolChains.h

public:
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                             llvm::opt::ArgStringList &CC1Args) const override;
   void AddClangCXXStdlibIncludeArgs(
       const llvm::opt::ArgList &Args,
       llvm::opt::ArgStringList &CC1Args) const override;
   void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                            llvm::opt::ArgStringList &CC1Args) const override;
 
+  SanitizerMask getSupportedSanitizers() const override;
+
   const ToolChain &HostTC;
   CudaInstallationDetector CudaInstallation;
 
 protected:
   Tool *buildAssembler() const override;  // ptxas
   Tool *buildLinker() const override;     // fatbinary (ok, not really a linker)
 };
 

lib/Driver/ToolChains.cpp

void CudaToolChain::AddClangCXXStdlibIncludeArgs(const ArgList &Args,
   HostTC.AddClangCXXStdlibIncludeArgs(Args, CC1Args);
 }
 
 void CudaToolChain::AddIAMCUIncludeArgs(const ArgList &Args,
                                         ArgStringList &CC1Args) const {
   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 &D, const llvm::Triple &Triple,
                                const ArgList &Args)
     : ToolChain(D, Triple, Args) {
   // ProgramPaths are found via 'PATH' environment variable.
 }
 
 Tool *XCoreToolChain::buildAssembler() const {

test/Driver/cuda-no-sanitizers.cu

 // Check that -fsanitize=foo doesn't get passed down to device-side
 // compilation.
 //
 // REQUIRES: clang-driver
 //
 // 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"
 // CHECK: "-fsanitize=address"