https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/196737
>From 882122f1254331686f6e49b8d71e4c4a505d2b00 Mon Sep 17 00:00:00 2001 From: Matt Arsenault <[email protected]> Date: Thu, 7 May 2026 20:57:02 +0100 Subject: [PATCH] clang: Refactor handling of offload sanitizer arguments Previously the AMDGPU toolchains hackily handled -fsanitize arguments. They would lie and report that all host side sanitizers are available, then TranslateArgs would filter out the device side cases that do not work, providing diagnostics for the skipped cases. Move that logic into the base sanitizer argument parsing. This makes the produced diagnostics more consistent. Previously we would get repeated warnings when a sanitizer is fully unsupported by amdgpu, which should now be once for the toolchain. These could be further improved; we're printing the specific field of -fsanitize in more cases where it could be skipped. In other cases we have the opposite problem, where we aren't reporting the exact sanitizer from the -f flag in the case that depends on a subtarget feature. This will help fix other broken target specific flag forwarding bugs in the future. Co-authored-by: Claude Sonnet 4 <[email protected]> --- clang/include/clang/Driver/SanitizerArgs.h | 5 +- clang/include/clang/Driver/ToolChain.h | 19 +++- clang/lib/Driver/SanitizerArgs.cpp | 94 ++++++++++++++-- clang/lib/Driver/ToolChain.cpp | 11 +- clang/lib/Driver/ToolChains/AMDGPU.cpp | 101 ++++++++++-------- clang/lib/Driver/ToolChains/AMDGPU.h | 82 ++------------ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp | 38 +++---- clang/lib/Driver/ToolChains/AMDGPUOpenMP.h | 5 +- clang/lib/Driver/ToolChains/Clang.cpp | 4 +- clang/lib/Driver/ToolChains/HIPAMD.cpp | 45 ++++---- clang/lib/Driver/ToolChains/HIPAMD.h | 5 +- .../Driver/amdgpu-openmp-sanitize-options.c | 4 +- clang/test/Driver/amdgpu-validate-sanitize.cl | 23 ++++ clang/test/Driver/hip-sanitize-options.hip | 28 ++--- 14 files changed, 259 insertions(+), 205 deletions(-) create mode 100644 clang/test/Driver/amdgpu-validate-sanitize.cl diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h index ed2eb6852b124..d4ee17802fd8e 100644 --- a/clang/include/clang/Driver/SanitizerArgs.h +++ b/clang/include/clang/Driver/SanitizerArgs.h @@ -9,6 +9,7 @@ #define LLVM_CLANG_DRIVER_SANITIZERARGS_H #include "clang/Basic/Sanitizers.h" +#include "clang/Driver/Action.h" #include "clang/Driver/Types.h" #include "llvm/Option/Arg.h" #include "llvm/Option/ArgList.h" @@ -85,7 +86,9 @@ class SanitizerArgs { public: /// Parses the sanitizer arguments from an argument list. SanitizerArgs(const ToolChain &TC, const llvm::opt::ArgList &Args, - bool DiagnoseErrors = true); + bool DiagnoseErrors = true, bool DiagnoseBoundArchErrors = true, + StringRef BoundArch = "", + Action::OffloadKind DeviceOffloadKind = Action::OFK_None); bool needsSharedRt() const { return SharedRuntime; } bool needsStableAbi() const { return StableABI; } diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h index 8676318d6f9d4..7df9565da8c2a 100644 --- a/clang/include/clang/Driver/ToolChain.h +++ b/clang/include/clang/Driver/ToolChain.h @@ -18,6 +18,7 @@ #include "llvm/ADT/APFloat.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/FloatingPointMode.h" +#include "llvm/ADT/SmallSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Frontend/Debug/Options.h" @@ -185,8 +186,13 @@ class ToolChain { Tool *getOffloadPackager() const; Tool *getLinkerWrapper() const; + /// Track if diagnostics have been emitted for sanitizer arguments already to + /// avoid duplicate diagnostics. mutable bool SanitizerArgsChecked = false; + /// Set of BoundArch values which have already had diagnostics emitted. + mutable llvm::SmallSet<StringRef, 4> BoundArchSanitizerArgsChecked; + /// The effective clang triple for the current Job. mutable llvm::Triple EffectiveTriple; @@ -338,7 +344,18 @@ class ToolChain { /// -print-multi-flags-experimental argument. Multilib::flags_list getMultilibFlags(const llvm::opt::ArgList &) const; - SanitizerArgs getSanitizerArgs(const llvm::opt::ArgList &JobArgs) const; + SanitizerArgs getSanitizerArgs( + const llvm::opt::ArgList &JobArgs, StringRef BoundArch = "", + Action::OffloadKind DeviceOffloadKind = Action::OFK_None) const; + + /// Returns the feature requirement for a sanitizer on a specific arch for + /// diagnostic purposes. Returns the required feature name (e.g., "xnack+") if + /// the sanitizer is generally supported but requires a specific feature for + /// the given BoundArch, or an empty StringRef otherwise. + virtual StringRef getSanitizerRequirement(SanitizerMask Kinds, + StringRef BoundArch) const { + return {}; + } const XRayArgs getXRayArgs(const llvm::opt::ArgList &) const; diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index 4bf011ab3dedf..31660dd29407c 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -398,7 +398,9 @@ bool SanitizerArgs::needsLTO() const { SanitizerArgs::SanitizerArgs(const ToolChain &TC, const llvm::opt::ArgList &Args, - bool DiagnoseErrors) { + bool DiagnoseErrors, bool DiagnoseBoundArchErrors, + StringRef BoundArch, + Action::OffloadKind DeviceOffloadKind) { SanitizerMask AllRemove; // During the loop below, the accumulated set of // sanitizers disabled by the current sanitizer // argument or any argument after it. @@ -412,8 +414,16 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, SanitizerMask IgnoreForUbsanFeature; // Accumulated set of values passed to // `-fsanitize-ignore-for-ubsan-feature`. SanitizerMask Kinds; - const SanitizerMask Supported = - setGroupBits(TC.getSupportedSanitizers("", Action::OFK_None)); + + // Figure out the base toolchain's sanitizer support so we can diagnose the + // diff for a specific BoundArch. + const SanitizerMask ToolChainSupported = + setGroupBits(TC.getSupportedSanitizers("", DeviceOffloadKind)); + + const SanitizerMask BoundArchSupported = + BoundArch.empty() ? ToolChainSupported + : setGroupBits(TC.getSupportedSanitizers( + BoundArch, DeviceOffloadKind)); CfiCrossDso = Args.hasFlag(options::OPT_fsanitize_cfi_cross_dso, options::OPT_fno_sanitize_cfi_cross_dso, false); @@ -540,15 +550,79 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, DiagnosedKinds |= SanitizerKind::CFIMFCall; } - if (SanitizerMask KindsToDiagnose = Add & ~Supported & ~DiagnosedKinds) { - if (DiagnoseErrors) { - std::string Desc = describeSanitizeArg(Arg, KindsToDiagnose); + // Check for sanitizers that are supported by the toolchain but not for + // this specific arch (e.g., AMDGPU requires specific subtarget features + // for address sanitizer.) + if (SanitizerMask ArchSpecificUnsupported = + Add & ToolChainSupported & ~BoundArchSupported & ~DiagnosedKinds; + ArchSpecificUnsupported && DiagnoseBoundArchErrors) { + // Upgrade the warning to an error if the unsupported sanitizer was + // explicitly specified for the bound arch. + + // FIXME: There are additional options which explicitly bind to this + // device. + bool IsExplicitDevice = + Arg->getBaseArg().getOption().matches(options::OPT_Xarch_device); + + // Check if the toolchain provides a feature requirement hint for + // any of the unsupported sanitizers + StringRef Requirement = + TC.getSanitizerRequirement(ArchSpecificUnsupported, BoundArch); + if (!Requirement.empty()) { + // Emit diagnostic with feature requirement + // + // TODO: Use variant of unsupported_option_part_for_target that + // includes offload_arch_req_feature + D.Diag( + IsExplicitDevice + ? diag:: + err_drv_unsupported_option_for_offload_arch_req_feature + : diag:: + warn_drv_unsupported_option_for_offload_arch_req_feature) + << Arg->getAsString(Args) << BoundArch << Requirement; + } else { + // Fall back to generic diagnostic if no requirement was provided + SanitizerSet UnsupportedSet; + UnsupportedSet.Mask = ArchSpecificUnsupported; + D.Diag(diag::warn_drv_unsupported_option_part_for_target) + << toString(UnsupportedSet) << Arg->getAsString(Args) + << Triple.str(); + } + + DiagnosedKinds |= ArchSpecificUnsupported; + } + + // Check for sanitizers that are not supported at all by the toolchain + if (SanitizerMask KindsToDiagnose = + Add & ~ToolChainSupported & ~DiagnosedKinds; + DiagnoseErrors && KindsToDiagnose) { + bool IsExplicitDevice = + Arg->getBaseArg().getOption().matches(options::OPT_Xarch_device); + // For device offload compilation, emit a warning since the sanitizer + // may still work on the host. For non-offload compilation or explicit + // device specification, emit an error. + if (DeviceOffloadKind != Action::OFK_None && + DeviceOffloadKind != Action::OFK_Host) { + // For warnings, extract just the sanitizer names (e.g., "fuzzer") + // instead of the full argument (e.g., "-fsanitize=fuzzer") + SanitizerSet KindSet; + KindSet.Mask = KindsToDiagnose; + D.Diag(IsExplicitDevice + ? diag::err_drv_unsupported_option_part_for_target + : diag::warn_drv_unsupported_option_part_for_target) + << toString(KindSet) << Arg->getAsString(Args) + << TC.getTriple().str(); + } else { + // For non-offload targets, use the shorter diagnostic format D.Diag(diag::err_drv_unsupported_opt_for_target) - << Desc << TC.getTriple().str(); + << describeSanitizeArg(Arg, KindsToDiagnose) + << TC.getTriple().str(); } + DiagnosedKinds |= KindsToDiagnose; } - Add &= Supported; + + Add &= BoundArchSupported; // Test for -fno-rtti + explicit -fsanitizer=vptr before expanding groups // so we don't error out if -fno-rtti and -fsanitize=undefined were @@ -599,7 +673,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, options::OPT_fno_wrapv_pointer, S)) Add &= ~SanitizerKind::PointerOverflow; } - Add &= Supported; + Add &= BoundArchSupported; if (Add & SanitizerKind::Fuzzer) Add |= SanitizerKind::FuzzerNoLink; @@ -714,7 +788,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, // c++abi-specific parts of UBSan runtime, and they are not provided by the // toolchain. We don't have a good way to check the latter, so we just // check if the toolchan supports vptr. - if (~Supported & SanitizerKind::Vptr) { + if (~BoundArchSupported & SanitizerKind::Vptr) { SanitizerMask KindsToDiagnose = Kinds & ~TrappingKinds & NeedsUbsanCxxRt; // The runtime library supports the Microsoft C++ ABI, but only well enough // for CFI. FIXME: Remove this once we support vptr on Windows. diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp index 6e5148101d4ec..e0ff70c32ddfd 100644 --- a/clang/lib/Driver/ToolChain.cpp +++ b/clang/lib/Driver/ToolChain.cpp @@ -516,8 +516,15 @@ ToolChain::getMultilibFlags(const llvm::opt::ArgList &Args) const { } SanitizerArgs -ToolChain::getSanitizerArgs(const llvm::opt::ArgList &JobArgs) const { - SanitizerArgs SanArgs(*this, JobArgs, !SanitizerArgsChecked); +ToolChain::getSanitizerArgs(const llvm::opt::ArgList &JobArgs, + StringRef BoundArch, + Action::OffloadKind DeviceOffloadKind) const { + SanitizerArgs SanArgs(*this, JobArgs, + /*DiagnoseErrors=*/!SanitizerArgsChecked, + /*DiagnoseBoundArchErrors=*/ + BoundArchSanitizerArgsChecked.insert(BoundArch).second, + BoundArch, DeviceOffloadKind); + SanitizerArgsChecked = true; return SanArgs; } diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp index b457ee2cde1c3..7f59d59585eb0 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -908,6 +908,9 @@ AMDGPUToolChain::getGPUArch(const llvm::opt::ArgList &DriverArgs) const { AMDGPUToolChain::ParsedTargetIDType AMDGPUToolChain::getParsedTargetID(const llvm::opt::ArgList &DriverArgs) const { StringRef TargetID = DriverArgs.getLastArgValue(options::OPT_mcpu_EQ); + if (TargetID.empty()) + TargetID = DriverArgs.getLastArgValue(options::OPT_march_EQ); + if (TargetID.empty()) return {std::nullopt, std::nullopt, std::nullopt}; @@ -980,15 +983,14 @@ void ROCMToolChain::addClangTargetOptions( if (TT.getEnvironment() == llvm::Triple::LLVM) return; - // Get the device name and canonicalize it - const StringRef GpuArch = getGPUArch(DriverArgs); - auto Kind = llvm::AMDGPU::parseArchAMDGCN(GpuArch); - const StringRef CanonArch = llvm::AMDGPU::getArchNameAMDGCN(Kind); - StringRef LibDeviceFile = RocmInstallation->getLibDeviceFile(CanonArch); + AMDGPUToolChain::ParsedTargetIDType TargetID = getParsedTargetID(DriverArgs); + StringRef GpuArch = + TargetID.OptionalGPUArch ? *TargetID.OptionalGPUArch : StringRef(); + + StringRef LibDeviceFile = RocmInstallation->getLibDeviceFile(GpuArch); auto ABIVer = DeviceLibABIVersion::fromCodeObjectVersion( getAMDGPUCodeObjectVersion(getDriver(), DriverArgs)); - if (!RocmInstallation->checkCommonBitcodeLibs(CanonArch, LibDeviceFile, - ABIVer)) + if (!RocmInstallation->checkCommonBitcodeLibs(GpuArch, LibDeviceFile, ABIVer)) return; // Add the OpenCL specific bitcode library. @@ -998,7 +1000,9 @@ void ROCMToolChain::addClangTargetOptions( // Add the generic set of libraries. BCLibs.append(RocmInstallation->getCommonBitcodeLibs( DriverArgs, LibDeviceFile, GpuArch, DeviceOffloadingKind, - getSanitizerArgs(DriverArgs).needsAsanRt())); + getSanitizerArgs(DriverArgs, TargetID.OptionalTargetID.value_or(""), + DeviceOffloadingKind) + .needsAsanRt())); for (auto [BCFile, Internalize] : BCLibs) { if (Internalize) @@ -1074,8 +1078,8 @@ RocmInstallationDetector::getCommonBitcodeLibs( llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> ROCMToolChain::getCommonDeviceLibNames( - const llvm::opt::ArgList &DriverArgs, llvm::StringRef GPUArch, - Action::OffloadKind DeviceOffloadingKind) const { + const llvm::opt::ArgList &DriverArgs, llvm::StringRef TargetID, + llvm::StringRef GPUArch, Action::OffloadKind DeviceOffloadingKind) const { auto Kind = llvm::AMDGPU::parseArchAMDGCN(GPUArch); const StringRef CanonArch = llvm::AMDGPU::getArchNameAMDGCN(Kind); @@ -1088,46 +1092,51 @@ ROCMToolChain::getCommonDeviceLibNames( return RocmInstallation->getCommonBitcodeLibs( DriverArgs, LibDeviceFile, GPUArch, DeviceOffloadingKind, - getSanitizerArgs(DriverArgs).needsAsanRt()); + getSanitizerArgs(DriverArgs, TargetID, DeviceOffloadingKind) + .needsAsanRt()); } -bool AMDGPUToolChain::shouldSkipSanitizeOption( - const ToolChain &TC, const llvm::opt::ArgList &DriverArgs, - StringRef TargetID, const llvm::opt::Arg *A) const { - auto &Diags = TC.getDriver().getDiags(); - bool IsExplicitDevice = - A->getBaseArg().getOption().matches(options::OPT_Xarch_device); - - // Check 'xnack+' availability by default - llvm::StringRef Processor = - getProcessorFromTargetID(TC.getTriple(), TargetID); - auto ProcKind = TC.getTriple().isAMDGCN() - ? llvm::AMDGPU::parseArchAMDGCN(Processor) - : llvm::AMDGPU::parseArchR600(Processor); - auto Features = TC.getTriple().isAMDGCN() - ? llvm::AMDGPU::getArchAttrAMDGCN(ProcKind) - : llvm::AMDGPU::getArchAttrR600(ProcKind); - if (Features & llvm::AMDGPU::FEATURE_XNACK_ALWAYS) - return false; +static bool isXnackAvailable(const llvm::Triple &TT, llvm::StringRef TargetID) { + // Arch-specific check - only report as supported if arch has xnack+ + llvm::StringRef Processor = getProcessorFromTargetID(TT, TargetID); + auto ProcKind = TT.isAMDGCN() ? llvm::AMDGPU::parseArchAMDGCN(Processor) + : llvm::AMDGPU::parseArchR600(Processor); + auto Features = TT.isAMDGPU() ? llvm::AMDGPU::getArchAttrAMDGCN(ProcKind) + : llvm::AMDGPU::getArchAttrR600(ProcKind); + + // If processor has xnack always on, Address sanitizer is supported + bool XnackAvailable = (Features & llvm::AMDGPU::FEATURE_XNACK_ALWAYS); + if (XnackAvailable) + return true; - // Look for the xnack feature in TargetID + // Otherwise, check if xnack+ is explicitly enabled in the target ID llvm::StringMap<bool> FeatureMap; - auto OptionalGpuArch = parseTargetID(TC.getTriple(), TargetID, &FeatureMap); - assert(OptionalGpuArch && "Invalid Target ID"); - (void)OptionalGpuArch; + auto OptionalGpuArch = parseTargetID(TT, TargetID, &FeatureMap); + if (!OptionalGpuArch) + return false; auto Loc = FeatureMap.find("xnack"); - if (Loc == FeatureMap.end() || !Loc->second) { - 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 (Loc != FeatureMap.end() && Loc->second); +} - return false; +SanitizerMask AMDGPUToolChain::getSupportedSanitizers( + StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const { + SanitizerMask SupportedMask = + SanitizerKind::Undefined | SanitizerKind::UndefinedGroup; + + // Address sanitizer is potentially supported, but depends on the exact target + // arch xnack support. + if (BoundArch.empty() || isXnackAvailable(getTriple(), BoundArch)) + SupportedMask |= SanitizerKind::Address; + + return SupportedMask; +} + +StringRef AMDGPUToolChain::getSanitizerRequirement(SanitizerMask Kinds, + StringRef BoundArch) const { + // Address sanitizer requires xnack+ feature + if ((Kinds & SanitizerKind::Address) && !BoundArch.empty() && + !isXnackAvailable(getTriple(), BoundArch)) { + return "xnack+"; + } + return ""; } diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h b/clang/lib/Driver/ToolChains/AMDGPU.h index 1698786f0e27a..2a5e28224808c 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.h +++ b/clang/lib/Driver/ToolChains/AMDGPU.h @@ -101,11 +101,8 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public Generic_ELF { /// Needed for translating LTO options. const char *getDefaultLinker() const override { return "ld.lld"; } - /// Should skip sanitize option. - bool shouldSkipSanitizeOption(const ToolChain &TC, - const llvm::opt::ArgList &DriverArgs, - StringRef TargetID, - const llvm::opt::Arg *A) const; + StringRef getSanitizerRequirement(SanitizerMask Kinds, + StringRef BoundArch) const override; /// Uses amdgpu-arch tool to get arch of the system GPU. Will return error /// if unable to find one. @@ -134,6 +131,10 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public Generic_ELF { /// Common warning options shared by AMDGPU HIP, OpenCL and OpenMP toolchains. /// Language specific warning options should go to derived classes. void addClangWarningOptions(llvm::opt::ArgStringList &CC1Args) const override; + + SanitizerMask + getSupportedSanitizers(StringRef BoundArch, + Action::OffloadKind DeviceOffloadKind) const override; }; class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain { @@ -148,23 +149,9 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain { // Returns a list of device library names shared by different languages llvm::SmallVector<BitCodeLibraryInfo, 12> getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs, - llvm::StringRef GPUArch, + llvm::StringRef TargetID, llvm::StringRef GPUArch, Action::OffloadKind DeviceOffloadingKind) const; - // FIXME: Remove this and make use of OffloadKind argument to - // getSupportedSanitizers - static constexpr SanitizerMask getOffloadSupportedSanitizers() { - return SanitizerKind::Address | SanitizerKind::Undefined | - SanitizerKind::UndefinedGroup; - } - - SanitizerMask - getSupportedSanitizers(StringRef BoundArch, - Action::OffloadKind DeviceOffloadKind) const override { - assert(DeviceOffloadKind == Action::OFK_None); - return getOffloadSupportedSanitizers(); - } - bool diagnoseUnsupportedOption(const llvm::opt::Arg *A, const llvm::opt::DerivedArgList &DAL, const llvm::opt::ArgList &DriverArgs, @@ -189,61 +176,6 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain { } 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; - - SanitizerMask Supported = ROCMToolChain::getOffloadSupportedSanitizers(); - SanitizerMask SupportedMask; - for (const char *Value : A->getValues()) { - SanitizerMask K = parseSanitizerValue(Value, /*Allow Groups*/ true); - if (K & Supported) { - SupportedSanitizers.push_back(Value); - SupportedMask |= K; - } 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); - } - } - // The xnack+ feature is only required for ASan on AMDGPU. - if ((SupportedMask & SanitizerKind::Address) && - 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; - } }; } // end namespace toolchains diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp index 226779d9eb80b..8c7efbf491187 100644 --- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp @@ -65,9 +65,18 @@ llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs( const OptTable &Opts = getDriver().getOpts(); for (Arg *A : Args) { - // Filter unsupported sanitizers passed from the HostTC. - if (!handleSanitizeOption(*this, *DAL, Args, BoundArch, A)) - DAL->append(A); + // Sanitizer coverage is currently not supported for AMDGPU. + if (A->getOption().matches(options::OPT_fsan_cov_Group)) { + diagnoseUnsupportedOption(A, *DAL, Args); + continue; + } + + if (A->getOption().matches(options::OPT_fsanitize_EQ) && + !Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize, + true)) + continue; + + DAL->append(A); } if (!BoundArch.empty()) { @@ -105,21 +114,6 @@ void AMDGPUOpenMPToolChain::AddIAMCUIncludeArgs(const ArgList &Args, HostTC.AddIAMCUIncludeArgs(Args, CC1Args); } -SanitizerMask AMDGPUOpenMPToolChain::getSupportedSanitizers( - StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) 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 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 - // tolerate flags meant only for the host toolchain. - - // FIXME: Be accurate and use DeviceOffloadKind. - return HostTC.getSupportedSanitizers(BoundArch, DeviceOffloadKind); -} - VersionTuple AMDGPUOpenMPToolChain::computeMSVCVersion(const Driver *D, const ArgList &Args) const { @@ -133,12 +127,14 @@ AMDGPUOpenMPToolChain::getDeviceLibs( if (!Args.hasFlag(options::OPT_offloadlib, options::OPT_no_offloadlib, true)) return {}; - StringRef GpuArch = getProcessorFromTargetID( - getTriple(), Args.getLastArgValue(options::OPT_march_EQ)); + AMDGPUToolChain::ParsedTargetIDType TargetID = getParsedTargetID(Args); + if (!TargetID.OptionalTargetID) + return {}; SmallVector<BitCodeLibraryInfo, 12> BCLibs; for (auto BCLib : - getCommonDeviceLibNames(Args, GpuArch.str(), DeviceOffloadingKind)) + getCommonDeviceLibNames(Args, *TargetID.OptionalTargetID, + *TargetID.OptionalGPUArch, DeviceOffloadingKind)) BCLibs.emplace_back(BCLib); return BCLibs; diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h index ff4ba9cc9cda4..d030246d02cbb 100644 --- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h +++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h @@ -36,6 +36,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUOpenMPToolChain final llvm::opt::DerivedArgList * TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const override; + void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args, @@ -51,10 +52,6 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUOpenMPToolChain final void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args) const override; - SanitizerMask - getSupportedSanitizers(StringRef BoundArch, - Action::OffloadKind DeviceOffloadKind) const override; - VersionTuple computeMSVCVersion(const Driver *D, const llvm::opt::ArgList &Args) const override; diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index bdffa4fdd7e6b..7071d59f0930c 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6113,7 +6113,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, Args.AddLastArg(CmdArgs, options::OPT_fno_knr_functions); - auto SanitizeArgs = TC.getSanitizerArgs(Args); + const char *OffloadArch = JA.getOffloadingArch(); + auto SanitizeArgs = TC.getSanitizerArgs(Args, OffloadArch ? OffloadArch : "", + JA.getOffloadingDeviceKind()); Args.AddLastArg(CmdArgs, options::OPT_fallow_runtime_check_skip_hot_cutoff_EQ); diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp index 81fe156721281..8b80bc49bd6d7 100644 --- a/clang/lib/Driver/ToolChains/HIPAMD.cpp +++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp @@ -303,9 +303,18 @@ HIPAMDToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args, const OptTable &Opts = getDriver().getOpts(); for (Arg *A : Args) { - // Filter unsupported sanitizers passed from the HostTC. - if (!handleSanitizeOption(*this, *DAL, Args, BoundArch, A)) - DAL->append(A); + // Sanitizer coverage is currently not supported for AMDGPU. + if (A->getOption().matches(options::OPT_fsan_cov_Group)) { + diagnoseUnsupportedOption(A, *DAL, Args); + continue; + } + + if (A->getOption().matches(options::OPT_fsanitize_EQ) && + !Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize, + true)) + continue; + + DAL->append(A); } if (!BoundArch.empty()) { @@ -357,21 +366,6 @@ void HIPAMDToolChain::AddHIPIncludeArgs(const ArgList &DriverArgs, RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args); } -SanitizerMask HIPAMDToolChain::getSupportedSanitizers( - StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) 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 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 - // tolerate flags meant only for the host toolchain. - // - // FIXME: Be accurate and use DeviceOffloadKind. - return HostTC.getSupportedSanitizers(BoundArch, DeviceOffloadKind); -} - VersionTuple HIPAMDToolChain::computeMSVCVersion(const Driver *D, const ArgList &Args) const { return HostTC.computeMSVCVersion(D, Args); @@ -385,9 +379,13 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs, if (!DriverArgs.hasFlag(options::OPT_offloadlib, options::OPT_no_offloadlib, true) || - TT.getEnvironment() == llvm::Triple::LLVM || - getGPUArch(DriverArgs) == "amdgcnspirv") + TT.getEnvironment() == llvm::Triple::LLVM) + return {}; + + AMDGPUToolChain::ParsedTargetIDType TargetID = getParsedTargetID(DriverArgs); + if (!TargetID.OptionalTargetID || TargetID.OptionalTargetID == "amdgcnspirv") return {}; + ArgStringList LibraryPaths; // Find in --hip-device-lib-path and HIP_LIBRARY_PATH. @@ -420,12 +418,11 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs, getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 0; return {}; } - StringRef GpuArch = getGPUArch(DriverArgs); - assert(!GpuArch.empty() && "Must have an explicit GPU arch."); // Add common device libraries like ocml etc. - for (auto N : - getCommonDeviceLibNames(DriverArgs, GpuArch, DeviceOffloadingKind)) + for (auto N : getCommonDeviceLibNames( + DriverArgs, *TargetID.OptionalTargetID, *TargetID.OptionalGPUArch, + DeviceOffloadingKind)) BCLibs.emplace_back(N); // Add instrument lib. diff --git a/clang/lib/Driver/ToolChains/HIPAMD.h b/clang/lib/Driver/ToolChains/HIPAMD.h index d7e10493195ce..ef1ebb83c1023 100644 --- a/clang/lib/Driver/ToolChains/HIPAMD.h +++ b/clang/lib/Driver/ToolChains/HIPAMD.h @@ -64,6 +64,7 @@ class LLVM_LIBRARY_VISIBILITY HIPAMDToolChain final : public ROCMToolChain { llvm::opt::DerivedArgList * TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const override; + void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args, @@ -84,10 +85,6 @@ class LLVM_LIBRARY_VISIBILITY HIPAMDToolChain final : public ROCMToolChain { getDeviceLibs(const llvm::opt::ArgList &Args, Action::OffloadKind DeviceOffloadKind) const override; - SanitizerMask - getSupportedSanitizers(StringRef BoundArch, - Action::OffloadKind DeviceOffloadKind) const override; - VersionTuple computeMSVCVersion(const Driver *D, const llvm::opt::ArgList &Args) const override; diff --git a/clang/test/Driver/amdgpu-openmp-sanitize-options.c b/clang/test/Driver/amdgpu-openmp-sanitize-options.c index 061dd1d9110f4..43cf323f45a86 100644 --- a/clang/test/Driver/amdgpu-openmp-sanitize-options.c +++ b/clang/test/Driver/amdgpu-openmp-sanitize-options.c @@ -91,11 +91,11 @@ // RUN: | FileCheck -check-prefixes=ERRSANCOV %s +// NOTSUPPORTED-DAG: warning: ignoring 'leak' in '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' // INVALIDCOMBINATION1: warning: ignoring 'fuzzer' in '-fsanitize=address,fuzzer' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] // INVALIDCOMBINATION2: warning: ignoring 'fuzzer' in '-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' // NOXNACK: warning: ignoring '-fsanitize=address' option for offload arch 'gfx908' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead // 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 @@ -111,7 +111,7 @@ // SAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "x86_64-unknown-linux-gnu".* "-fopenmp".* "-fsanitize=address".* "--offload-targets=amdgcn-amd-amdhsa".* "-x" "ir".*}} // SAN: {{"[^"]*clang-linker-wrapper[^"]*".* "--host-triple=x86_64-unknown-linux-gnu".* "--linker-path=[^"]*".* "--whole-archive" "[^"]*(libclang_rt.asan_static.a|libclang_rt.asan_static-x86_64.a)".* "--whole-archive" "[^"]*(libclang_rt.asan.a|libclang_rt.asan-x86_64.a)".*}} -// UNSUPPORTEDERROR: error: '-fsanitize=leak' option is not currently supported for target 'amdgcn-amd-amdhsa' +// UNSUPPORTEDERROR: error: 'leak' in '-fsanitize=leak' option is not currently supported for target 'amdgcn-amd-amdhsa' // XNACKERROR: error: '-fsanitize=address' option for offload arch 'gfx908:xnack-' is not currently supported there. Use it with an offload arch containing 'xnack+' instead // INVALIDCOMBINATIONERROR: error: 'fuzzer' in '-fsanitize=fuzzer,address' option is not currently supported for target 'amdgcn-amd-amdhsa' diff --git a/clang/test/Driver/amdgpu-validate-sanitize.cl b/clang/test/Driver/amdgpu-validate-sanitize.cl new file mode 100644 index 0000000000000..6d60e03dc451a --- /dev/null +++ b/clang/test/Driver/amdgpu-validate-sanitize.cl @@ -0,0 +1,23 @@ +// RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx900:xnack+ \ +// RUN: -fsanitize=address \ +// RUN: -nogpuinc --rocm-path=%S/Inputs/rocm \ +// RUN: %s 2>&1 | FileCheck %s + +// RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1250 \ +// RUN: -fsanitize=address \ +// RUN: -nogpuinc --rocm-path=%S/Inputs/rocm \ +// RUN: %s 2>&1 | FileCheck %s + +// FIXME: This should error, but is silently ignored +// RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx900:xnack- \ +// RUN: -fsanitize=address \ +// RUN: -nogpuinc --rocm-path=%S/Inputs/rocm \ +// RUN: %s 2>&1 | FileCheck -check-prefix=ERR %s + +// CHECK: "-triple" "amdgcn-amd-amdhsa" +// CHECK-SAME: "-mlink-bitcode-file" "{{.*}}/amdgcn/bitcode/asanrtl.bc" +// CHECK-SAME: "-fsanitize=address" + +// FIXME: Should not be forwarding argument +// ERR-NOT: asanrtl.bc +// ERR: "-fsanitize=address" diff --git a/clang/test/Driver/hip-sanitize-options.hip b/clang/test/Driver/hip-sanitize-options.hip index 4deffc32430fb..a5b06d5cbb26f 100644 --- a/clang/test/Driver/hip-sanitize-options.hip +++ b/clang/test/Driver/hip-sanitize-options.hip @@ -110,9 +110,9 @@ // FAIL: 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 -// XNACK-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' -// XNACK-DAG: warning: ignoring '-fsanitize=address' option for offload arch 'gfx900:xnack-' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead -// XNACK-DAG: warning: ignoring '-fsanitize=address' option for offload arch 'gfx906' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead +// XNACK: warning: ignoring 'leak' in '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' +// XNACK: warning: ignoring '-fsanitize=address' option for offload arch 'gfx900:xnack-' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead +// XNACK: warning: ignoring '-fsanitize=address' option for offload arch 'gfx906' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead // XNACK-DAG: {{"[^"]*clang[^"]*".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-target-cpu" "gfx900".* "-target-feature" "\+xnack".* "-fsanitize=address"}} // XNACK-DAG: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx900".* "-target-feature" "-xnack"}} // XNACK-DAG: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx906"}} @@ -150,23 +150,23 @@ // INVALIDCOMBINATION-DAG: {{"[^"]*clang[^"]*".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-target-cpu" "gfx900".* "-target-feature" "\+xnack".* "-fsanitize=address"}} // INVALIDCOMBINATION-DAG: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address,fuzzer,fuzzer-no-link"}} -// MULT1-DAG: warning: ignoring 'fuzzer' in '-fsanitize=address,fuzzer' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] -// MULT1-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] -// MULT1-DAG: warning: ignoring 'fuzzer' in '-fsanitize=address,fuzzer' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] -// MULT1-DAG: warning: ignoring '-fsanitize=address,fuzzer' option for offload arch 'gfx908:xnack-' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead [-Woption-ignored] -// MULT1-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] +// MULT1: warning: ignoring 'leak' in '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] +// MULT1: warning: ignoring 'fuzzer' in '-fsanitize=address,fuzzer' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] -// MULT2-DAG: warning: ignoring 'fuzzer' in '-fsanitize=fuzzer,address' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] -// MULT2-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] -// MULT2-DAG: warning: ignoring 'fuzzer' in '-fsanitize=fuzzer,address' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] -// MULT2-DAG: warning: ignoring '-fsanitize=fuzzer,address' option for offload arch 'gfx908:xnack-' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead [-Woption-ignored] -// MULT2-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] +// FIXME: This should produce a separate warning for address and fuzzer. The xnack+ hint only applies to the address part +// MULT1: warning: ignoring '-fsanitize=address,fuzzer' option for offload arch 'gfx908:xnack-' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead [-Woption-ignored] + +// MULT2: warning: ignoring 'leak' in '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] +// MULT2: warning: ignoring 'fuzzer' in '-fsanitize=fuzzer,address' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored] + +// FIXME: This should produce a separate warning for address and fuzzer. The xnack+ hint only applies to the address part +// MULT2: warning: ignoring '-fsanitize=fuzzer,address' option for offload arch 'gfx908:xnack-' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead [-Woption-ignored] // XNACK2-DAG: {{"[^"]*clang[^"]*".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-target-cpu" "gfx900".* "-target-feature" "\+xnack".* "-fsanitize=address"}} // XNACK2-DAG: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx908"}} // XNACK2-DAG: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address,fuzzer,fuzzer-no-link,leak"}} -// UNSUPPORTEDERROR: error: '-fsanitize=leak' option is not currently supported for target 'amdgcn-amd-amdhsa' +// UNSUPPORTEDERROR: error: 'leak' in '-fsanitize=leak' option is not currently supported for target 'amdgcn-amd-amdhsa' // XNACKERROR: error: '-fsanitize=address' option for offload arch 'gfx900:xnack-' is not currently supported there. Use it with an offload arch containing 'xnack+' instead // INVALIDCOMBINATIONERROR: error: 'fuzzer' in '-fsanitize=fuzzer,address' option is not currently supported for target 'amdgcn-amd-amdhsa' _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
