Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+86 −29
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bc0aad0
[AMDGPU][clang] Fix clang driver check for multiple sanitizer arguments
ro-i Nov 6, 2025
e2a11d3
fix formatting
ro-i Nov 6, 2025
cfa15f4
ignore sanitizers selectively
ro-i Nov 7, 2025
1d0052b
add forgotten diagnostic
ro-i Nov 7, 2025
da9e715
fix after rebase
ro-i Nov 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticDriverKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ def warn_drv_unsupported_option_for_offload_arch_req_feature : Warning<
def warn_drv_unsupported_option_for_target : Warning<
"ignoring '%0' option as it is not currently supported for target '%1'">,
InGroup<OptionIgnored>;
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 warn_drv_invalid_argument_for_flang : Warning<
"'%0' is not valid for Fortran">,
InGroup<OptionIgnored>;
Expand Down
28 changes: 17 additions & 11 deletions clang/lib/Driver/ToolChains/AMDGPU.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,27 +1074,32 @@ ROCMToolChain::getCommonDeviceLibNames(
getSanitizerArgs(DriverArgs).needsAsanRt());
}

bool AMDGPUToolChain::shouldSkipSanitizeOption(
std::optional<std::string> AMDGPUToolChain::filterSanitizeOption(
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;
return std::nullopt;
Option O = A->getOption();

if (!O.matches(options::OPT_fsanitize_EQ))
return false;
return std::nullopt;

if (!DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
options::OPT_fno_gpu_sanitize, true))
return true;
return "";

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;
// We only allow the address sanitizer and ignore all other sanitizers.
Copy link
Contributor

@ampandey-1995 ampandey-1995 Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@ThorBl ThorBl Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Centralizing the handling of unsupported sanitizers in diagnoseUnsupportedSanitizers is a good idea. However, in its current form, it only issues a warning and doesn’t prevent unsupported sanitizers from being forwarded to the device compilation. For this issue, the logic needs to not just warn but also actively filter (drop) unsupported sanitizers from the device job to avoid the crash. Extending this function to perform the filtering would help reduce code duplication across the HIP and OpenMP toolchains.

Copy link
Contributor Author

@ro-i ro-i Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that it's currently not possible to modify the arglist in the constructor of a ToolChain. On the other hand, TranslateArgs is called multiple times and while you'd want to drop args there, we need to be careful with doing diagnostics there because they would then be yielded unnecessarily multiple times. That's why the current code did split the diagnostics and the actual arg dropping. I'm currently figuring out a solution where we can have everything nice :)

SmallVector<std::string, 4> SupportedSanitizers;
for (const char *Value : A->getValues()) {
SanitizerMask K = parseSanitizerValue(Value, /*AllowGroups=*/false);
if (K == SanitizerKind::Address)
SupportedSanitizers.push_back(std::string(Value));
}
if (SupportedSanitizers.empty())
return "";

// Check 'xnack+' availability by default
llvm::StringRef Processor =
Expand All @@ -1106,7 +1111,7 @@ bool AMDGPUToolChain::shouldSkipSanitizeOption(
? llvm::AMDGPU::getArchAttrAMDGCN(ProcKind)
: llvm::AMDGPU::getArchAttrR600(ProcKind);
if (Features & llvm::AMDGPU::FEATURE_XNACK_ALWAYS)
return false;
return std::nullopt;

// Look for the xnack feature in TargetID
llvm::StringMap<bool> FeatureMap;
Expand All @@ -1118,7 +1123,8 @@ bool AMDGPUToolChain::shouldSkipSanitizeOption(
Diags.Report(
clang::diag::warn_drv_unsupported_option_for_offload_arch_req_feature)
<< A->getAsString(DriverArgs) << TargetID << "xnack+";
return true;
return "";
}
return false;

return llvm::join(SupportedSanitizers, ",");
}
23 changes: 13 additions & 10 deletions clang/lib/Driver/ToolChains/AMDGPU.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,13 @@ 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.
bool shouldSkipSanitizeOption(const ToolChain &TC,
const llvm::opt::ArgList &DriverArgs,
StringRef TargetID,
const llvm::opt::Arg *A) const;
/// Filter supported sanitizers from the sanitize option and return them. If
/// there should be no filtering and Arg should be kept as-is, return
/// std::nullopt. If no sanitizers are supported, return an empty string.
std::optional<std::string>
filterSanitizeOption(const ToolChain &TC,
const llvm::opt::ArgList &DriverArgs, StringRef TargetID,
const llvm::opt::Arg *A) const;

/// Uses amdgpu-arch tool to get arch of the system GPU. Will return error
/// if unable to find one.
Expand Down Expand Up @@ -161,11 +163,12 @@ 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_part_for_target)
<< Value << A->getAsString(Args) << getTriple().str();
}
}
}
};
Expand Down
14 changes: 9 additions & 5 deletions clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,20 @@ llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs(

const OptTable &Opts = getDriver().getOpts();

// Skip sanitize options passed from the HostTC. Claim them early.
// Skip sanitize options passed from the HostTC. Remove 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);
DAL->eraseArg(options::OPT_fsanitize_EQ);

for (Arg *A : Args)
if (!shouldSkipSanitizeOption(*this, Args, BoundArch, A) &&
!llvm::is_contained(*DAL, A))
for (Arg *A : Args) {
std::optional<std::string> SupportedSanitizers =
filterSanitizeOption(*this, Args, BoundArch, A);
if (!SupportedSanitizers)
DAL->append(A);
else if (!SupportedSanitizers->empty())
DAL->AddJoinedArg(A, A->getOption(), *SupportedSanitizers);
}

if (!BoundArch.empty()) {
DAL->eraseArg(options::OPT_march_EQ);
Expand Down
12 changes: 11 additions & 1 deletion clang/lib/Driver/ToolChains/HIPAMD.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,19 @@ HIPAMDToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args,

const OptTable &Opts = getDriver().getOpts();

// Skip sanitize options passed from the HostTC. Remove them early.
// The decision to sanitize device code is computed only by
// 'shouldSkipSanitizeOption'.
if (DAL->hasArg(options::OPT_fsanitize_EQ))
DAL->eraseArg(options::OPT_fsanitize_EQ);

for (Arg *A : Args) {
if (!shouldSkipSanitizeOption(*this, Args, BoundArch, A))
std::optional<std::string> SupportedSanitizers =
filterSanitizeOption(*this, Args, BoundArch, A);
if (!SupportedSanitizers)
DAL->append(A);
else if (!SupportedSanitizers->empty())
DAL->AddJoinedArg(A, A->getOption(), *SupportedSanitizers);
}

if (!BoundArch.empty()) {
Expand Down
16 changes: 15 additions & 1 deletion clang/test/Driver/amdgpu-openmp-sanitize-options.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,27 @@
// 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 and ignore
// them selectively.
// (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 '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]

Copy link
Contributor

@ThorBl ThorBl Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test that includes multiple -fsanitize arguments and multi-arch scenarios (e.g., --offload-arch=gfx908:xnack+,gfx908:xnack-)?

// 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'
// NOTSUPPORTED-DAG: warning: ignoring 'leak' in '-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

// 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|gfx1250|gfx1251)".* "-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".*}}
Expand Down
19 changes: 18 additions & 1 deletion clang/test/Driver/hip-sanitize-options.hip
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,18 @@
// RUN: -fsanitize=leak -nogpuinc --rocm-path=%S/Inputs/rocm \
// RUN: %s 2>&1 | FileCheck -check-prefixes=NOGPUNEG %s

// Catch invalid combination of sanitizers regardless of their order and ignore
// them selectively.
// (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 -### --target=x86_64-unknown-linux-gnu --offload-arch=gfx900:xnack+ \
// RUN: -fsanitize=address,fuzzer --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=INVALIDCOMBINATION,INVALIDCOMBINATION1 %s
// RUN: %clang -### --target=x86_64-unknown-linux-gnu --offload-arch=gfx900:xnack+ \
// RUN: -fsanitize=fuzzer,address --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=INVALIDCOMBINATION,INVALIDCOMBINATION2 %s

// CHECK-NOT: {{"[^"]*clang[^"]*".* "-fcuda-is-device".* "-fsanitize=address"}}
// CHECK-NOT: {{"[^"]*clang[^"]*".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc"}}
// CHECK-NOT: {{"[^"]*lld(\.exe){0,1}".* ".*hip.bc"}}
Expand All @@ -67,7 +79,7 @@

// 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 'leak' in '-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-DAG: {{"[^"]*clang[^"]*".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-target-cpu" "gfx900".* "-target-feature" "\+xnack".* "-fsanitize=address"}}
Expand Down Expand Up @@ -101,3 +113,8 @@
// NOGPUNEG-NOT: {{"[^"]*clang[^"]*".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-target-cpu" "gfx900".* "-target-feature" "-xnack"}}
// NOGPUNEG-NOT: {{"[^"]*clang[^"]*".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-target-cpu" "gfx906"}}
// NOGPUNEG-NOT: {{"[^"]*lld(\.exe){0,1}".* ".*hip.bc"}}

// INVALIDCOMBINATION1-DAG: warning: ignoring 'fuzzer' in '-fsanitize=address,fuzzer' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored]
// INVALIDCOMBINATION2-DAG: warning: ignoring 'fuzzer' in '-fsanitize=fuzzer,address' option as it is not currently supported for target 'amdgcn-amd-amdhsa' [-Woption-ignored]
// 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"}}