[Driver][ASan] Refactor Clang-Driver "Sanitizer Bitcode" linking. #123922
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-amdgpu Author: Amit Kumar Pandey (ampandey-1995) ChangesASan bitcode linking is currently available for HIPAMD,OpenMP and OpenCL. Moving sanitizer specific common parts of logic to appropriate API's so as to reduce code redundancy and maintainability. Full diff: https://github.com/llvm/llvm-project/pull/123922.diff 8 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 42c39ac6606c7f..14125ecff5a114 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -70,6 +70,9 @@ def err_drv_no_rocm_device_lib : Error<
"cannot find ROCm device library%select{| for %1| for ABI version %1}0; provide its path via "
"'--rocm-path' or '--rocm-device-lib-path', or pass '-nogpulib' to build "
"without ROCm device library">;
+def err_drv_no_asan_rt_lib : Error<
+ "AMDGPU address sanitizer runtime library (asanrtl) is not found. "
+ "Please install ROCm device library which supports address sanitizer">;
def err_drv_no_hip_runtime : Error<
"cannot find HIP runtime; provide its path via '--rocm-path', or pass "
"'-nogpuinc' to build without HIP runtime">;
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index a8061ffd9321f5..bdbd9c6351078a 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -946,8 +946,9 @@ void ROCMToolChain::addClangTargetOptions(
StringRef LibDeviceFile = RocmInstallation->getLibDeviceFile(CanonArch);
auto ABIVer = DeviceLibABIVersion::fromCodeObjectVersion(
getAMDGPUCodeObjectVersion(getDriver(), DriverArgs));
+ std::tuple<bool,const SanitizerArgs> GPUSan(DriverArgs.hasFlag(options::OPT_fgpu_sanitize,options::OPT_fno_gpu_sanitize,true),getSanitizerArgs(DriverArgs));
if (!RocmInstallation->checkCommonBitcodeLibs(CanonArch, LibDeviceFile,
- ABIVer))
+ ABIVer,GPUSan))
return;
bool Wave64 = isWave64(DriverArgs, Kind);
@@ -965,28 +966,31 @@ void ROCMToolChain::addClangTargetOptions(
DriverArgs.hasArg(options::OPT_cl_fp32_correctly_rounded_divide_sqrt);
// Add the OpenCL specific bitcode library.
- llvm::SmallVector<std::string, 12> BCLibs;
- BCLibs.push_back(RocmInstallation->getOpenCLPath().str());
+ llvm::SmallVector<BitCodeLibraryInfo, 12> BCLibs;
+ BCLibs.emplace_back(RocmInstallation->getOpenCLPath().str());
// Add the generic set of libraries.
BCLibs.append(RocmInstallation->getCommonBitcodeLibs(
DriverArgs, LibDeviceFile, Wave64, DAZ, FiniteOnly, UnsafeMathOpt,
- FastRelaxedMath, CorrectSqrt, ABIVer, false));
+ FastRelaxedMath, CorrectSqrt, ABIVer, GPUSan, false));
- if (getSanitizerArgs(DriverArgs).needsAsanRt()) {
- CC1Args.push_back("-mlink-bitcode-file");
- CC1Args.push_back(
- DriverArgs.MakeArgString(RocmInstallation->getAsanRTLPath()));
- }
- for (StringRef BCFile : BCLibs) {
+ for (auto [BCFile,Internalize] : BCLibs) {
+ if(Internalize)
CC1Args.push_back("-mlink-builtin-bitcode");
+ else
+ CC1Args.push_back("-mlink-bitcode-file");
CC1Args.push_back(DriverArgs.MakeArgString(BCFile));
}
}
bool RocmInstallationDetector::checkCommonBitcodeLibs(
StringRef GPUArch, StringRef LibDeviceFile,
- DeviceLibABIVersion ABIVer) const {
+ DeviceLibABIVersion ABIVer,std::tuple<bool,const SanitizerArgs> &GPUSan) const {
+ if(std::get<bool>(GPUSan))
+ if(std::get<const SanitizerArgs>(GPUSan).needsAsanRt() && getAsanRTLPath().empty()){
+ D.Diag(diag::err_drv_no_asan_rt_lib);
+ return false;
+ }
if (!hasDeviceLibrary()) {
D.Diag(diag::err_drv_no_rocm_device_lib) << 0;
return false;
@@ -1002,18 +1006,28 @@ bool RocmInstallationDetector::checkCommonBitcodeLibs(
return true;
}
-llvm::SmallVector<std::string, 12>
+llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
RocmInstallationDetector::getCommonBitcodeLibs(
const llvm::opt::ArgList &DriverArgs, StringRef LibDeviceFile, bool Wave64,
bool DAZ, bool FiniteOnly, bool UnsafeMathOpt, bool FastRelaxedMath,
- bool CorrectSqrt, DeviceLibABIVersion ABIVer, bool isOpenMP = false) const {
- llvm::SmallVector<std::string, 12> BCLibs;
-
- auto AddBCLib = [&](StringRef BCFile) { BCLibs.push_back(BCFile.str()); };
-
+ bool CorrectSqrt, DeviceLibABIVersion ABIVer,const std::tuple<bool,const SanitizerArgs> &GPUSan, bool isOpenMP = false) const {
+ llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> BCLibs;
+
+ auto GPUSanEnabled = [GPUSan](){return std::get<bool>(GPUSan);};
+ auto AddBCLib = [&](ToolChain::BitCodeLibraryInfo BCLib,bool Internalize = true) { BCLib.ShouldInternalize=Internalize;BCLibs.emplace_back(BCLib); };
+ auto AddSanBCLibs = [&](){
+ auto SanArgs = std::get<const SanitizerArgs>(GPUSan);
+ if(GPUSanEnabled()){
+ if(SanArgs.needsAsanRt())
+ AddBCLib(getAsanRTLPath(),false);
+ }};
+
+ AddSanBCLibs();
AddBCLib(getOCMLPath());
if (!isOpenMP)
AddBCLib(getOCKLPath());
+ else if(GPUSanEnabled() && isOpenMP)
+ AddBCLib(getOCKLPath(),false);
AddBCLib(getDenormalsAreZeroPath(DAZ));
AddBCLib(getUnsafeMathPath(UnsafeMathOpt || FastRelaxedMath));
AddBCLib(getFiniteOnlyPath(FiniteOnly || FastRelaxedMath));
@@ -1027,7 +1041,7 @@ RocmInstallationDetector::getCommonBitcodeLibs(
return BCLibs;
}
-llvm::SmallVector<std::string, 12>
+llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
const std::string &GPUArch,
bool isOpenMP) const {
@@ -1037,8 +1051,9 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
StringRef LibDeviceFile = RocmInstallation->getLibDeviceFile(CanonArch);
auto ABIVer = DeviceLibABIVersion::fromCodeObjectVersion(
getAMDGPUCodeObjectVersion(getDriver(), DriverArgs));
+ std::tuple<bool,const SanitizerArgs> GPUSan(DriverArgs.hasFlag(options::OPT_fgpu_sanitize,options::OPT_fno_gpu_sanitize,true),getSanitizerArgs(DriverArgs));
if (!RocmInstallation->checkCommonBitcodeLibs(CanonArch, LibDeviceFile,
- ABIVer))
+ ABIVer,GPUSan))
return {};
// If --hip-device-lib is not set, add the default bitcode libraries.
@@ -1061,7 +1076,7 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
return RocmInstallation->getCommonBitcodeLibs(
DriverArgs, LibDeviceFile, Wave64, DAZ, FiniteOnly, UnsafeMathOpt,
- FastRelaxedMath, CorrectSqrt, ABIVer, isOpenMP);
+ FastRelaxedMath, CorrectSqrt, ABIVer, GPUSan, isOpenMP);
}
bool AMDGPUToolChain::shouldSkipSanitizeOption(
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h b/clang/lib/Driver/ToolChains/AMDGPU.h
index a9b4552a1f91a4..aad6bc75dffafc 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.h
+++ b/clang/lib/Driver/ToolChains/AMDGPU.h
@@ -142,7 +142,7 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Action::OffloadKind DeviceOffloadKind) const override;
// Returns a list of device library names shared by different languages
- llvm::SmallVector<std::string, 12>
+ llvm::SmallVector<BitCodeLibraryInfo, 12>
getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
const std::string &GPUArch,
bool isOpenMP = false) const;
diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index 3f0b3f2d86b3ed..2b8917106afc14 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -9,7 +9,7 @@
#include "AMDGPUOpenMP.h"
#include "AMDGPU.h"
#include "CommonArgs.h"
-#include "ToolChains/ROCm.h"
+#include "ROCm.h"
#include "clang/Basic/DiagnosticDriver.h"
#include "clang/Driver/Compilation.h"
#include "clang/Driver/Driver.h"
@@ -159,11 +159,6 @@ AMDGPUOpenMPToolChain::getDeviceLibs(const llvm::opt::ArgList &Args) const {
if (Args.hasArg(options::OPT_nogpulib))
return {};
- if (!RocmInstallation->hasDeviceLibrary()) {
- getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 0;
- return {};
- }
-
StringRef GpuArch = getProcessorFromTargetID(
getTriple(), Args.getLastArgValue(options::OPT_march_EQ));
diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp
index ccee065b590647..38bbe280806a64 100644
--- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -396,28 +396,11 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
StringRef GpuArch = getGPUArch(DriverArgs);
assert(!GpuArch.empty() && "Must have an explicit GPU arch.");
- // If --hip-device-lib is not set, add the default bitcode libraries.
- if (DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
- options::OPT_fno_gpu_sanitize, true) &&
- getSanitizerArgs(DriverArgs).needsAsanRt()) {
- auto AsanRTL = RocmInstallation->getAsanRTLPath();
- if (AsanRTL.empty()) {
- unsigned DiagID = getDriver().getDiags().getCustomDiagID(
- DiagnosticsEngine::Error,
- "AMDGPU address sanitizer runtime library (asanrtl) is not found. "
- "Please install ROCm device library which supports address "
- "sanitizer");
- getDriver().Diag(DiagID);
- return {};
- } else
- BCLibs.emplace_back(AsanRTL, /*ShouldInternalize=*/false);
- }
-
// Add the HIP specific bitcode library.
BCLibs.push_back(RocmInstallation->getHIPPath());
// Add common device libraries like ocml etc.
- for (StringRef N : getCommonDeviceLibNames(DriverArgs, GpuArch.str()))
+ for (auto N : getCommonDeviceLibNames(DriverArgs, GpuArch.str()))
BCLibs.emplace_back(N);
// Add instrument lib.
diff --git a/clang/lib/Driver/ToolChains/ROCm.h b/clang/lib/Driver/ToolChains/ROCm.h
index dceb0ab0366933..424fdec6d85ca6 100644
--- a/clang/lib/Driver/ToolChains/ROCm.h
+++ b/clang/lib/Driver/ToolChains/ROCm.h
@@ -13,6 +13,7 @@
#include "clang/Basic/LLVM.h"
#include "clang/Driver/Driver.h"
#include "clang/Driver/Options.h"
+#include "clang/Driver/SanitizerArgs.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Option/ArgList.h"
@@ -173,16 +174,16 @@ class RocmInstallationDetector {
/// Get file paths of default bitcode libraries common to AMDGPU based
/// toolchains.
- llvm::SmallVector<std::string, 12>
+ llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
getCommonBitcodeLibs(const llvm::opt::ArgList &DriverArgs,
StringRef LibDeviceFile, bool Wave64, bool DAZ,
bool FiniteOnly, bool UnsafeMathOpt,
bool FastRelaxedMath, bool CorrectSqrt,
- DeviceLibABIVersion ABIVer, bool isOpenMP) const;
+ DeviceLibABIVersion ABIVer,const std::tuple<bool,const SanitizerArgs> &GPUSan, bool isOpenMP) const;
/// Check file paths of default bitcode libraries common to AMDGPU based
/// toolchains. \returns false if there are invalid or missing files.
bool checkCommonBitcodeLibs(StringRef GPUArch, StringRef LibDeviceFile,
- DeviceLibABIVersion ABIVer) const;
+ DeviceLibABIVersion ABIVer,std::tuple<bool,const SanitizerArgs> &GPUSan) const;
/// Check whether we detected a valid HIP runtime.
bool hasHIPRuntime() const { return HasHIPRuntime; }
diff --git a/clang/test/Driver/hip-sanitize-options.hip b/clang/test/Driver/hip-sanitize-options.hip
index d94cbdacdaeb3a..10b00153501fef 100644
--- a/clang/test/Driver/hip-sanitize-options.hip
+++ b/clang/test/Driver/hip-sanitize-options.hip
@@ -52,15 +52,15 @@
// CHECK-NOT: {{"[^"]*lld(\.exe){0,1}".* ".*hip.bc"}}
// CHECK: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
-// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]"
+// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-mlink-builtin-bitcode" ".*hip.bc".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]"
// NORDC-NOT: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
// NORDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
// RDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
-// RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]"
+// RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-mlink-builtin-bitcode" ".*hip.bc".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]"
// RDC-NOT: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
-// FAIL: AMDGPU address sanitizer runtime library (asanrtl) is not found. Please install ROCm device library which supports address sanitizer
+// FAIL: error: AMDGPU address sanitizer runtime library (asanrtl) is not found. Please install ROCm device library which supports address sanitizer
// 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
diff --git a/clang/test/Driver/rocm-device-libs.cl b/clang/test/Driver/rocm-device-libs.cl
index 6837e219dc35de..f9766e6fa4d996 100644
--- a/clang/test/Driver/rocm-device-libs.cl
+++ b/clang/test/Driver/rocm-device-libs.cl
@@ -145,8 +145,8 @@
// RUN: 2>&1 | FileCheck --check-prefixes=NOASAN %s
// COMMON: "-triple" "amdgcn-amd-amdhsa"
-// ASAN-SAME: "-mlink-bitcode-file" "{{.*}}/amdgcn/bitcode/asanrtl.bc"
// COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/opencl.bc"
+// ASAN-SAME: "-mlink-bitcode-file" "{{.*}}/amdgcn/bitcode/asanrtl.bc"
// COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ocml.bc"
// COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ockl.bc"
|
ampandey-1995
force-pushed
the
main
branch
2 times, most recently
from
0c62a28 to
bb51865
Compare
arsenm
reviewed
Jan 22, 2025
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ampandey-1995
force-pushed
the
main
branch
2 times, most recently
from
5a80048 to
55582ee
Compare
yxsamliu
approved these changes
Jan 27, 2025
ASan bitcode linking is currently available for HIPAMD,OpenMP and OpenCL. Moving sanitizer specific common parts of logic to appropriate API's so as to reduce code redundancy and maintainability.
Enable device code ASan instrumentation for openmp offload applications using option '-fsanitize=address'.
…#2) Enable device code ASan instrumentation for openmp offload applications using option '-fsanitize=address'.
Revert "[OpenMP][ASan] Enable ASan Instrumentation for AMDGPUOpenMPToolChain."
|
Hi, an LLVM OpenMP Offloading CI/CD detected a new failure during testing on an AMD MI210 and MI300A. The Failed test is test_requires_atomic_default_mem_order_seq_cst.F90. The error: For full details of the failure see: https://gitlab.e4s.io/uo-public/llvm-openmp-offloading-v2/-/jobs/360327 |
Probably needs to be listed as a FCC1 option. |
jhuber6
reviewed
Feb 6, 2025
| @@ -950,6 +950,11 @@ void ROCMToolChain::addClangTargetOptions( | |||
| ABIVer)) | |||
| return; | |||
|
|
|||
| std::tuple<bool, const SanitizerArgs> GPUSan( | |||
| DriverArgs.hasFlag(options::OPT_fgpu_sanitize, | |||
| options::OPT_fno_gpu_sanitize, true), | |||
There was a problem hiding this comment.
This cannot be default true, it included ockl.bc which breaks OpenMP's printf and includes some other conflicting definitions. Furthermore you disable internalization so they stick around in every single OpenMP program now.
There was a problem hiding this comment.
Yes , sorry one misstep is here is that GPUSan object created by default while not checking that -fsanitize=address available in Args list. This GPUSan should be a pointer and must be initialized when -fsanitize=address is detected on Args.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Feb 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ASan bitcode linking is currently available for HIPAMD,OpenMP and OpenCL. Moving sanitizer specific common parts of logic to appropriate API's so as to reduce code redundancy and maintainability.