[clang] b68b4f6 - [Driver][ASan] Refactor Clang-Driver "Sanitizer Bitcode" linking. (#123922)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 30 02:58:07 PST 2025
Author: Amit Kumar Pandey
Date: 2025-01-30T16:28:03+05:30
New Revision: b68b4f64a2bd2e0a22375cf89a4d655fc3667e11
URL: https://github.com/llvm/llvm-project/commit/b68b4f64a2bd2e0a22375cf89a4d655fc3667e11
DIFF: https://github.com/llvm/llvm-project/commit/b68b4f64a2bd2e0a22375cf89a4d655fc3667e11.diff
LOG: [Driver][ASan] Refactor Clang-Driver "Sanitizer Bitcode" linking. (#123922)
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.
Added:
Modified:
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/lib/Driver/ToolChains/AMDGPU.h
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
clang/lib/Driver/ToolChains/HIPAMD.cpp
clang/lib/Driver/ToolChains/ROCm.h
clang/test/Driver/hip-sanitize-options.hip
clang/test/Driver/rocm-device-libs.cl
Removed:
################################################################################
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index a8061ffd9321f5..83f486611bc946 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -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),
+ getSanitizerArgs(DriverArgs));
+
bool Wave64 = isWave64(DriverArgs, Kind);
// TODO: There are way too many flags that change this. Do we need to check
@@ -965,21 +970,19 @@ 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) {
- CC1Args.push_back("-mlink-builtin-bitcode");
+ 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));
}
}
@@ -1002,18 +1005,35 @@ 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 = [&]() {
+ if (GPUSanEnabled()) {
+ auto SanArgs = std::get<const SanitizerArgs>(GPUSan);
+ 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 +1047,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 {
@@ -1044,6 +1064,10 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
// If --hip-device-lib is not set, add the default bitcode libraries.
// TODO: There are way too many flags that change this. Do we need to check
// them all?
+ std::tuple<bool, const SanitizerArgs> GPUSan(
+ DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
+ options::OPT_fno_gpu_sanitize, true),
+ getSanitizerArgs(DriverArgs));
bool DAZ = DriverArgs.hasFlag(options::OPT_fgpu_flush_denormals_to_zero,
options::OPT_fno_gpu_flush_denormals_to_zero,
getDefaultDenormsAreZeroForTarget(Kind));
@@ -1061,7 +1085,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
diff erent 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..158a2520759846 100644
--- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -382,7 +382,7 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
llvm::sys::path::append(Path, BCName);
FullName = Path;
if (llvm::sys::fs::exists(FullName)) {
- BCLibs.push_back(FullName);
+ BCLibs.emplace_back(FullName);
return;
}
}
@@ -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());
+ BCLibs.emplace_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.
@@ -426,7 +409,7 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
if (InstLib.empty())
return BCLibs;
if (llvm::sys::fs::exists(InstLib))
- BCLibs.push_back(InstLib);
+ BCLibs.emplace_back(InstLib);
else
getDriver().Diag(diag::err_drv_no_such_file) << InstLib;
}
diff --git a/clang/lib/Driver/ToolChains/ROCm.h b/clang/lib/Driver/ToolChains/ROCm.h
index dceb0ab0366933..681c242b0678e2 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,12 +174,11 @@ class RocmInstallationDetector {
/// Get file paths of default bitcode libraries common to AMDGPU based
/// toolchains.
- llvm::SmallVector<std::string, 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;
+ 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,
+ 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,
diff --git a/clang/test/Driver/hip-sanitize-options.hip b/clang/test/Driver/hip-sanitize-options.hip
index d94cbdacdaeb3a..8de0ee9e18426b 100644
--- a/clang/test/Driver/hip-sanitize-options.hip
+++ b/clang/test/Driver/hip-sanitize-options.hip
@@ -18,7 +18,7 @@
// RUN: -nogpuinc --rocm-path=%S/Inputs/rocm \
// RUN: %s 2>&1 | FileCheck -check-prefixes=RDC %s
-// RUN: not %clang -### --target=x86_64-unknown-linux-gnu --offload-arch=gfx900:xnack+ \
+// RUN: not %clang -### --target=x86_64-unknown-linux-gnu -mcode-object-version=5 --offload-arch=gfx900:xnack+ \
// RUN: -fsanitize=address -fgpu-sanitize \
// RUN: -nogpuinc --rocm-path=%S/Inputs/rocm-invalid \
// RUN: %s 2>&1 | FileCheck -check-prefixes=FAIL %s
@@ -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: 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
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"
More information about the cfe-commits
mailing list