[AMDGPU][clang] Fix clang driver check for multiple sanitizer arguments (llvm#166851)

`-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.

Author: ro-i

clang/include/clang/Basic/DiagnosticDriverKinds.td

@@ -126,13 +126,23 @@ def err_drv_bad_offload_arch_combo : Error<
   "invalid offload arch combinations: '%0' and '%1' (for a specific processor, "
   "a feature should either exist in all offload archs, or not exist in any "
   "offload archs)">;
+def err_drv_unsupported_option_for_offload_arch_req_feature : Error<
+  "'%0' option for offload arch '%1' is not currently supported "
+  "there. Use it with an offload arch containing '%2' instead">;
 def warn_drv_unsupported_option_for_offload_arch_req_feature : Warning<
   "ignoring '%0' option for offload arch '%1' as it is not currently supported "
   "there. Use it with an offload arch containing '%2' instead">,
   InGroup<OptionIgnored>;
 def warn_drv_unsupported_option_for_target : Warning<
   "ignoring '%0' option as it is not currently supported for target '%1'">,
   InGroup<OptionIgnored>;
+def err_drv_unsupported_option_for_target : Error<
+  "'%0' option is not currently supported for target '%1'">;
+def warn_drv_unsupported_option_part_for_target : Warning<
+  "ignoring '%0' in '%1' option as it is not currently supported for target '%2'">,
+  InGroup<OptionIgnored>;
+def err_drv_unsupported_option_part_for_target : Error<
+  "'%0' in '%1' option is not currently supported for target '%2'">;
 def warn_drv_invalid_argument_for_flang : Warning<
   "'%0' is not valid for Fortran">,
   InGroup<OptionIgnored>;

clang/include/clang/Options/Options.td

@@ -201,6 +201,10 @@ def hlsl_Group : OptionGroup<"<HLSL group>">, Group<f_Group>,
                    DocName<"HLSL options">,
                    Visibility<[ClangOption]>;
 
+def fsan_cov_Group : OptionGroup<"<-fsanitize-coverage group>">,
+                   Group<f_clang_Group>,
+                   DocName<"Sanitizer Coverage options">;
+
 // Feature groups - these take command line options that correspond directly to
 // target specific features and can be translated directly from command line
 // options.
@@ -2407,26 +2411,26 @@ def : Flag<["-"], "fno-sanitize-blacklist">,
   Group<f_clang_Group>, Flags<[HelpHidden]>, Alias<fno_sanitize_ignorelist>;
 
 def fsanitize_coverage : CommaJoined<["-"], "fsanitize-coverage=">,
-  Group<f_clang_Group>,
+  Group<fsan_cov_Group>,
   HelpText<"Specify the type of coverage instrumentation for Sanitizers">;
 def fno_sanitize_coverage : CommaJoined<["-"], "fno-sanitize-coverage=">,
-  Group<f_clang_Group>, Visibility<[ClangOption, CLOption]>,
+  Group<fsan_cov_Group>, Visibility<[ClangOption, CLOption]>,
   HelpText<"Disable features of coverage instrumentation for Sanitizers">,
   Values<"func,bb,edge,indirect-calls,trace-bb,trace-cmp,trace-div,trace-gep,"
          "8bit-counters,trace-pc,trace-pc-guard,no-prune,inline-8bit-counters,"
          "inline-bool-flag">;
 def fsanitize_coverage_allowlist : Joined<["-"], "fsanitize-coverage-allowlist=">,
-    Group<f_clang_Group>, Visibility<[ClangOption, CLOption]>,
+    Group<fsan_cov_Group>, Visibility<[ClangOption, CLOption]>,
     HelpText<"Restrict sanitizer coverage instrumentation exclusively to modules and functions that match the provided special case list, except the blocked ones">,
     MarshallingInfoStringVector<CodeGenOpts<"SanitizeCoverageAllowlistFiles">>;
 def fsanitize_coverage_ignorelist : Joined<["-"], "fsanitize-coverage-ignorelist=">,
-    Group<f_clang_Group>, Visibility<[ClangOption, CLOption]>,
+    Group<fsan_cov_Group>, Visibility<[ClangOption, CLOption]>,
     HelpText<"Disable sanitizer coverage instrumentation for modules and functions "
              "that match the provided special case list, even the allowed ones">,
     MarshallingInfoStringVector<CodeGenOpts<"SanitizeCoverageIgnorelistFiles">>;
 def fsanitize_coverage_stack_depth_callback_min_EQ
     : Joined<["-"], "fsanitize-coverage-stack-depth-callback-min=">,
-      Group<f_clang_Group>,
+      Group<fsan_cov_Group>,
       MetaVarName<"<M>">,
       HelpText<"Use callback for max stack depth tracing with minimum stack "
                "depth M">,
@@ -7901,70 +7905,87 @@ def linker_option : Joined<["--"], "linker-option=">,
   HelpText<"Add linker option">,
   MarshallingInfoStringVector<CodeGenOpts<"LinkerOptions">>;
 def fsanitize_coverage_type : Joined<["-"], "fsanitize-coverage-type=">,
+                              Group<fsan_cov_Group>,
                               HelpText<"Sanitizer coverage type">,
                               MarshallingInfoInt<CodeGenOpts<"SanitizeCoverageType">>;
 def fsanitize_coverage_indirect_calls
     : Flag<["-"], "fsanitize-coverage-indirect-calls">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable sanitizer coverage for indirect calls">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageIndirectCalls">>;
 def fsanitize_coverage_trace_bb
     : Flag<["-"], "fsanitize-coverage-trace-bb">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable basic block tracing in sanitizer coverage">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageTraceBB">>;
 def fsanitize_coverage_trace_cmp
     : Flag<["-"], "fsanitize-coverage-trace-cmp">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable cmp instruction tracing in sanitizer coverage">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageTraceCmp">>;
 def fsanitize_coverage_trace_div
     : Flag<["-"], "fsanitize-coverage-trace-div">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable div instruction tracing in sanitizer coverage">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageTraceDiv">>;
 def fsanitize_coverage_trace_gep
     : Flag<["-"], "fsanitize-coverage-trace-gep">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable gep instruction tracing in sanitizer coverage">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageTraceGep">>;
 def fsanitize_coverage_8bit_counters
     : Flag<["-"], "fsanitize-coverage-8bit-counters">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable frequency counters in sanitizer coverage">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverage8bitCounters">>;
 def fsanitize_coverage_inline_8bit_counters
     : Flag<["-"], "fsanitize-coverage-inline-8bit-counters">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable inline 8-bit counters in sanitizer coverage">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageInline8bitCounters">>;
 def fsanitize_coverage_inline_bool_flag
     : Flag<["-"], "fsanitize-coverage-inline-bool-flag">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable inline bool flag in sanitizer coverage">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageInlineBoolFlag">>;
 def fsanitize_coverage_pc_table
     : Flag<["-"], "fsanitize-coverage-pc-table">,
+      Group<fsan_cov_Group>,
       HelpText<"Create a table of coverage-instrumented PCs">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoveragePCTable">>;
 def fsanitize_coverage_control_flow
     : Flag<["-"], "fsanitize-coverage-control-flow">,
+      Group<fsan_cov_Group>,
       HelpText<"Collect control flow of function">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageControlFlow">>;
 def fsanitize_coverage_trace_pc
     : Flag<["-"], "fsanitize-coverage-trace-pc">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable PC tracing in sanitizer coverage">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageTracePC">>;
 def fsanitize_coverage_trace_pc_guard
     : Flag<["-"], "fsanitize-coverage-trace-pc-guard">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable PC tracing with guard in sanitizer coverage">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageTracePCGuard">>;
 def fsanitize_coverage_no_prune
     : Flag<["-"], "fsanitize-coverage-no-prune">,
+      Group<fsan_cov_Group>,
       HelpText<"Disable coverage pruning (i.e. instrument all blocks/edges)">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageNoPrune">>;
 def fsanitize_coverage_stack_depth
     : Flag<["-"], "fsanitize-coverage-stack-depth">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable max stack depth tracing">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageStackDepth">>;
 def fsanitize_coverage_trace_loads
     : Flag<["-"], "fsanitize-coverage-trace-loads">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable tracing of loads">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageTraceLoads">>;
 def fsanitize_coverage_trace_stores
     : Flag<["-"], "fsanitize-coverage-trace-stores">,
+      Group<fsan_cov_Group>,
       HelpText<"Enable tracing of stores">,
       MarshallingInfoFlag<CodeGenOpts<"SanitizeCoverageTraceStores">>;
 def fexperimental_sanitize_metadata_EQ_covered

clang/lib/Driver/ToolChains/AMDGPU.cpp

@@ -1074,24 +1074,9 @@ ROCMToolChain::getCommonDeviceLibNames(
 bool AMDGPUToolChain::shouldSkipSanitizeOption(
     const ToolChain &TC, const llvm::opt::ArgList &DriverArgs,
     StringRef TargetID, const llvm::opt::Arg *A) const {
-  // For actions without targetID, do nothing.
-  if (TargetID.empty())
-    return false;
-  Option O = A->getOption();
-
-  if (!O.matches(options::OPT_fsanitize_EQ))
-    return false;
-
-  if (!DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
-                          options::OPT_fno_gpu_sanitize, true))
-    return true;
-
   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;
+  bool IsExplicitDevice =
+      A->getBaseArg().getOption().matches(options::OPT_Xarch_device);
 
   // Check 'xnack+' availability by default
   llvm::StringRef Processor =
@@ -1112,10 +1097,17 @@ bool AMDGPUToolChain::shouldSkipSanitizeOption(
   (void)OptionalGpuArch;
   auto Loc = FeatureMap.find("xnack");
   if (Loc == FeatureMap.end() || !Loc->second) {
-    Diags.Report(
-        clang::diag::warn_drv_unsupported_option_for_offload_arch_req_feature)
-        << A->getAsString(DriverArgs) << TargetID << "xnack+";
+    if (IsExplicitDevice) {
+      Diags.Report(
+          clang::diag::err_drv_unsupported_option_for_offload_arch_req_feature)
+          << A->getAsString(DriverArgs) << TargetID << "xnack+";
+    } else {
+      Diags.Report(
+          clang::diag::warn_drv_unsupported_option_for_offload_arch_req_feature)
+          << A->getAsString(DriverArgs) << TargetID << "xnack+";
+    }
     return true;
   }
+
   return false;
 }

clang/lib/Driver/ToolChains/AMDGPU.h

@@ -101,7 +101,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public Generic_ELF {
   /// Needed for translating LTO options.
   const char *getDefaultLinker() const override { return "ld.lld"; }
 
-  /// Should skip sanitize options.
+  /// Should skip sanitize option.
   bool shouldSkipSanitizeOption(const ToolChain &TC,
                                 const llvm::opt::ArgList &DriverArgs,
                                 StringRef TargetID,
@@ -155,18 +155,79 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
     return SanitizerKind::Address;
   }
 
-  void diagnoseUnsupportedSanitizers(const llvm::opt::ArgList &Args) const {
-    if (!Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize,
-                      true))
-      return;
+  bool diagnoseUnsupportedOption(const llvm::opt::Arg *A,
+                                 const llvm::opt::DerivedArgList &DAL,
+                                 const llvm::opt::ArgList &DriverArgs,
+                                 const char *Value = nullptr) const {
     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();
+    bool IsExplicitDevice =
+        A->getBaseArg().getOption().matches(options::OPT_Xarch_device);
+
+    if (Value) {
+      unsigned DiagID =
+          IsExplicitDevice
+              ? clang::diag::err_drv_unsupported_option_part_for_target
+              : clang::diag::warn_drv_unsupported_option_part_for_target;
+      Diags.Report(DiagID) << Value << A->getAsString(DriverArgs)
+                           << getTriple().str();
+    } else {
+      unsigned DiagID =
+          IsExplicitDevice
+              ? clang::diag::err_drv_unsupported_option_for_target
+              : clang::diag::warn_drv_unsupported_option_for_target;
+      Diags.Report(DiagID) << A->getAsString(DAL) << getTriple().str();
     }
+    return true;
+  }
+
+  bool handleSanitizeOption(const ToolChain &TC, llvm::opt::DerivedArgList &DAL,
+                            const llvm::opt::ArgList &DriverArgs,
+                            StringRef TargetID, const llvm::opt::Arg *A) const {
+    if (TargetID.empty())
+      return false;
+    // If we shouldn't do sanitizing, skip it.
+    if (!DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
+                            options::OPT_fno_gpu_sanitize, true))
+      return true;
+    const llvm::opt::Option &Opt = A->getOption();
+    // Sanitizer coverage is currently not supported for AMDGPU, so warn/error
+    // on every related option.
+    if (Opt.matches(options::OPT_fsan_cov_Group)) {
+      diagnoseUnsupportedOption(A, DAL, DriverArgs);
+    }
+    // If this isn't a sanitizer option, don't handle it.
+    if (!Opt.matches(options::OPT_fsanitize_EQ))
+      return false;
+
+    SmallVector<const char *, 4> SupportedSanitizers;
+    SmallVector<const char *, 4> UnSupportedSanitizers;
+
+    for (const char *Value : A->getValues()) {
+      SanitizerMask K = parseSanitizerValue(Value, /*Allow Groups*/ false);
+      if (K & ROCMToolChain::getSupportedSanitizers())
+        SupportedSanitizers.push_back(Value);
+      else
+        UnSupportedSanitizers.push_back(Value);
+    }
+
+    // If there are no supported sanitizers, drop the whole argument.
+    if (SupportedSanitizers.empty()) {
+      diagnoseUnsupportedOption(A, DAL, DriverArgs);
+      return true;
+    }
+    // If only some sanitizers are unsupported, report each one individually.
+    if (!UnSupportedSanitizers.empty()) {
+      for (const char *Value : UnSupportedSanitizers) {
+        diagnoseUnsupportedOption(A, DAL, DriverArgs, Value);
+      }
+    }
+    // If we know the target arch, check if the sanitizer is supported for it.
+    if (shouldSkipSanitizeOption(TC, DriverArgs, TargetID, A))
+      return true;
+
+    // Add a new argument with only the supported sanitizers.
+    DAL.AddJoinedArg(A, A->getOption(), llvm::join(SupportedSanitizers, ","));
+    return true;
   }
 };
 

clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp

@@ -28,8 +28,6 @@ AMDGPUOpenMPToolChain::AMDGPUOpenMPToolChain(const Driver &D,
   // Lookup binaries into the driver directory, this is used to
   // discover the 'amdgpu-arch' executable.
   getProgramPaths().push_back(getDriver().Dir);
-  // Diagnose unsupported sanitizer options only once.
-  diagnoseUnsupportedSanitizers(Args);
 }
 
 void AMDGPUOpenMPToolChain::addClangTargetOptions(
@@ -66,16 +64,11 @@ llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs(
 
   const OptTable &Opts = getDriver().getOpts();
 
-  // Skip sanitize options passed from the HostTC. Claim them early.
-  // The decision to sanitize device code is computed only by
-  // 'shouldSkipSanitizeOption'.
-  if (DAL->hasArg(options::OPT_fsanitize_EQ))
-    DAL->claimAllArgs(options::OPT_fsanitize_EQ);
-
-  for (Arg *A : Args)
-    if (!shouldSkipSanitizeOption(*this, Args, BoundArch, A) &&
-        !llvm::is_contained(*DAL, A))
+  for (Arg *A : Args) {
+    // Filter unsupported sanitizers passed from the HostTC.
+    if (!handleSanitizeOption(*this, *DAL, Args, BoundArch, A))
       DAL->append(A);
+  }
 
   if (!BoundArch.empty()) {
     DAL->eraseArg(options::OPT_march_EQ);
@@ -115,9 +108,8 @@ void AMDGPUOpenMPToolChain::AddIAMCUIncludeArgs(const ArgList &Args,
 SanitizerMask AMDGPUOpenMPToolChain::getSupportedSanitizers() const {
   // The AMDGPUOpenMPToolChain only supports sanitizers in the sense that it
   // allows sanitizer arguments on the command line if they are supported by the
-  // host toolchain. The AMDGPUOpenMPToolChain will actually ignore any command
-  // line arguments for any of these "supported" sanitizers. That means that no
-  // sanitization of device code is actually supported at this time.
+  // host toolchain. The AMDGPUOpenMPToolChain will later filter unsupported
+  // sanitizers from the command line arguments.
   //
   // This behavior is necessary because the host and device toolchains
   // invocations often share the command line, so the device toolchain must

clang/lib/Driver/ToolChains/HIPAMD.cpp

@@ -219,8 +219,6 @@ HIPAMDToolChain::HIPAMDToolChain(const Driver &D, const llvm::Triple &Triple,
   // Lookup binaries into the driver directory, this is used to
   // discover the clang-offload-bundler executable.
   getProgramPaths().push_back(getDriver().Dir);
-  // Diagnose unsupported sanitizer options only once.
-  diagnoseUnsupportedSanitizers(Args);
 }
 
 void HIPAMDToolChain::addClangTargetOptions(
@@ -292,7 +290,8 @@ HIPAMDToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args,
   const OptTable &Opts = getDriver().getOpts();
 
   for (Arg *A : Args) {
-    if (!shouldSkipSanitizeOption(*this, Args, BoundArch, A))
+    // Filter unsupported sanitizers passed from the HostTC.
+    if (!handleSanitizeOption(*this, *DAL, Args, BoundArch, A))
       DAL->append(A);
   }
 
@@ -348,9 +347,8 @@ void HIPAMDToolChain::AddHIPIncludeArgs(const ArgList &DriverArgs,
 SanitizerMask HIPAMDToolChain::getSupportedSanitizers() const {
   // The HIPAMDToolChain only supports sanitizers in the sense that it allows
   // sanitizer arguments on the command line if they are supported by the host
-  // toolchain. The HIPAMDToolChain will actually ignore any command line
-  // arguments for any of these "supported" sanitizers. That means that no
-  // sanitization of device code is actually supported at this time.
+  // toolchain. The HIPAMDToolChain will later filter unsupported sanitizers
+  // from the command line arguments.
   //
   // This behavior is necessary because the host and device toolchains
   // invocations often share the command line, so the device toolchain must