[AMDGPU][clang] Fix clang driver check for multiple sanitizer arguments

[ro-i]

-fsanitize=address,fuzzer should be rejected like -fsanitize=fuzzer,address.
The address sanitizer enables the device sanitizer pipeline. The fuzzer implicitly turns on LLVMs SanitizerCoverage, which the driver then forwards to the device cc1. SanitizerCoverage is not supported on amdgcn.

[llvmbot]

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-backend-amdgpu

Author: Robert Imschweiler (ro-i)

Changes

-fsanitize=address,fuzzer should be rejected like -fsanitize=fuzzer,address.
The address sanitizer enables the device sanitizer pipeline. The fuzzer implicitly turns on LLVMs SanitizerCoverage, which the driver then forwards to the device cc1. SanitizerCoverage is not supported on amdgcn.

---

Full diff: https://github.com/llvm/llvm-project/pull/166851.diff

3 Files Affected:

• (modified) clang/lib/Driver/ToolChains/AMDGPU.cpp (+5-3)
• (modified) clang/lib/Driver/ToolChains/AMDGPU.h (+7-5)
• (modified) clang/test/Driver/amdgpu-openmp-sanitize-options.c (+13)

diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 654a382e87e40..1f82ddc23e321 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -1091,9 +1091,11 @@ bool AMDGPUToolChain::shouldSkipSanitizeOption(
   auto &Diags = TC.getDriver().getDiags();
 
   // For simplicity, we only allow -fsanitize=address
-  SanitizerMask K = parseSanitizerValue(A->getValue(), /*AllowGroups=*/false);
-  if (K != SanitizerKind::Address)
-    return true;
+  for (const char *Value : A->getValues()) {
+    SanitizerMask K = parseSanitizerValue(Value, /*AllowGroups=*/false);
+    if (K != SanitizerKind::Address)
+      return true;
+  }
 
   llvm::StringMap<bool> FeatureMap;
   auto OptionalGpuArch = parseTargetID(TC.getTriple(), TargetID, &FeatureMap);
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h b/clang/lib/Driver/ToolChains/AMDGPU.h
index e90a5736911e4..c5680a9d486bd 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.h
+++ b/clang/lib/Driver/ToolChains/AMDGPU.h
@@ -161,11 +161,13 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
       return;
     auto &Diags = getDriver().getDiags();
     for (auto *A : Args.filtered(options::OPT_fsanitize_EQ)) {
-      SanitizerMask K =
-          parseSanitizerValue(A->getValue(), /*Allow Groups*/ false);
-      if (K != SanitizerKind::Address)
-        Diags.Report(clang::diag::warn_drv_unsupported_option_for_target)
-            << A->getAsString(Args) << getTriple().str();
+      for (const char *Value : A->getValues()) {
+        SanitizerMask K =
+            parseSanitizerValue(Value, /*Allow Groups*/ false);
+        if (K != SanitizerKind::Address)
+          Diags.Report(clang::diag::warn_drv_unsupported_option_for_target)
+              << A->getAsString(Args) << getTriple().str();
+      }
     }
   }
 };
diff --git a/clang/test/Driver/amdgpu-openmp-sanitize-options.c b/clang/test/Driver/amdgpu-openmp-sanitize-options.c
index 914e01873089c..f7b869b6f3234 100644
--- a/clang/test/Driver/amdgpu-openmp-sanitize-options.c
+++ b/clang/test/Driver/amdgpu-openmp-sanitize-options.c
@@ -48,6 +48,18 @@
 // RUN:   %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ --offload-arch=gfx900:xnack+ -fsanitize=address -fno-gpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=HOSTSAN,NOGPUSAN,SAN %s
 
+// Catch invalid combination of sanitizers regardless of their order.
+// The address sanitizer enables the device sanitizer pipeline. The fuzzer
+// implicitly turns on LLVMs SanitizerCoverage, which the driver then forwards
+// to the device cc1. SanitizerCoverage is not supported on amdgcn
+// RUN:   %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ -fsanitize=address,fuzzer --rocm-path=%S/Inputs/rocm %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=HOSTSANCOMBINATION,INVALIDCOMBINATION1 %s
+// RUN:   %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ -fsanitize=fuzzer,address --rocm-path=%S/Inputs/rocm %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=HOSTSANCOMBINATION,INVALIDCOMBINATION2 %s
+
+// INVALIDCOMBINATION1: warning: ignoring '-fsanitize=address,fuzzer' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored]
+// INVALIDCOMBINATION2: warning: ignoring '-fsanitize=fuzzer,address' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored]
+
 // FAIL-DAG: error: cannot find ROCm device library for ABI version 5; provide its path via '--rocm-path' or '--rocm-device-lib-path', or pass '-nogpulib' to build without ROCm device library
 // NOTSUPPORTED-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa'
 
@@ -55,6 +67,7 @@
 // XNACKNEG: warning: ignoring '-fsanitize=address' option for offload arch 'gfx908:xnack-' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead
 
 // HOSTSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "x86_64-unknown-linux-gnu".* "-fopenmp".* "-fsanitize=address".* "--offload-targets=amdgcn-amd-amdhsa".* "-x" "c".*}}
+// HOSTSANCOMBINATION: {{"[^"]*clang[^"]*" "-cc1" "-triple" "x86_64-unknown-linux-gnu".* "-fopenmp".* "-fsanitize=address,fuzzer,fuzzer-no-link".* "--offload-targets=amdgcn-amd-amdhsa".* "-x" "c".*}}
 
 // GPUSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu".* "-emit-llvm-bc".* "-mlink-bitcode-file" "[^"]*asanrtl.bc".* "-mlink-bitcode-file" "[^"]*ockl.bc".* "-target-cpu" "(gfx908|gfx900)".* "-fopenmp".* "-fsanitize=address".* "-x" "c".*}}
 // NOGPUSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu".* "-emit-llvm-bc".* "-target-cpu" "(gfx908|gfx900)".* "-fopenmp".* "-x" "c".*}}

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[ro-i]

wait, forgot to push sth

[ro-i]

needed to rebase due to conflict with #166754

[jhuber6]

Is this supposed to be a warning or an error. Personally I think the user should be able to do stuff like -Xarch_host -fsanitize=a,b,c -Xarch_device -fsanitize=address since we only support one. Seems a little unintuitive to just ignore things requested by the user just because we can't handle them.

[ampandey-1995]

Can this functionality be included in this function diagnoseUnsupportedSanitizers as we try to warn and skip processing of the unsupported sanitizer options?

[ampandey-1995]

This function is a lower-level helper utility. We can check for whatever options supported by -fsanitize= and filter them at higher level. getSupportedSanitizers might help to cover this part of code.