Skip to content

Commit b68b4f6

Browse files
ampandey-1995ampandey-1995
authored
[Driver][ASan] Refactor Clang-Driver "Sanitizer Bitcode" linking. (llvm#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.
1 parent c75b251 commit b68b4f6

File tree

7 files changed

+58
-56
lines changed

7 files changed

+58
-56
lines changed

‎clang/lib/Driver/ToolChains/AMDGPU.cpp‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,11 @@ void ROCMToolChain::addClangTargetOptions(
950950
ABIVer))
951951
return;
952952

953+
std::tuple<bool, const SanitizerArgs> GPUSan(
954+
DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
955+
options::OPT_fno_gpu_sanitize, true),
956+
getSanitizerArgs(DriverArgs));
957+
953958
bool Wave64 = isWave64(DriverArgs, Kind);
954959

955960
// TODO: There are way too many flags that change this. Do we need to check
@@ -965,21 +970,19 @@ void ROCMToolChain::addClangTargetOptions(
965970
DriverArgs.hasArg(options::OPT_cl_fp32_correctly_rounded_divide_sqrt);
966971

967972
// Add the OpenCL specific bitcode library.
968-
llvm::SmallVector<std::string, 12> BCLibs;
969-
BCLibs.push_back(RocmInstallation->getOpenCLPath().str());
973+
llvm::SmallVector<BitCodeLibraryInfo, 12> BCLibs;
974+
BCLibs.emplace_back(RocmInstallation->getOpenCLPath().str());
970975

971976
// Add the generic set of libraries.
972977
BCLibs.append(RocmInstallation->getCommonBitcodeLibs(
973978
DriverArgs, LibDeviceFile, Wave64, DAZ, FiniteOnly, UnsafeMathOpt,
974-
FastRelaxedMath, CorrectSqrt, ABIVer, false));
979+
FastRelaxedMath, CorrectSqrt, ABIVer, GPUSan, false));
975980

976-
if (getSanitizerArgs(DriverArgs).needsAsanRt()) {
977-
CC1Args.push_back("-mlink-bitcode-file");
978-
CC1Args.push_back(
979-
DriverArgs.MakeArgString(RocmInstallation->getAsanRTLPath()));
980-
}
981-
for (StringRef BCFile : BCLibs) {
982-
CC1Args.push_back("-mlink-builtin-bitcode");
981+
for (auto [BCFile, Internalize] : BCLibs) {
982+
if (Internalize)
983+
CC1Args.push_back("-mlink-builtin-bitcode");
984+
else
985+
CC1Args.push_back("-mlink-bitcode-file");
983986
CC1Args.push_back(DriverArgs.MakeArgString(BCFile));
984987
}
985988
}
@@ -1002,18 +1005,35 @@ bool RocmInstallationDetector::checkCommonBitcodeLibs(
10021005
return true;
10031006
}
10041007

1005-
llvm::SmallVector<std::string, 12>
1008+
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
10061009
RocmInstallationDetector::getCommonBitcodeLibs(
10071010
const llvm::opt::ArgList &DriverArgs, StringRef LibDeviceFile, bool Wave64,
10081011
bool DAZ, bool FiniteOnly, bool UnsafeMathOpt, bool FastRelaxedMath,
1009-
bool CorrectSqrt, DeviceLibABIVersion ABIVer, bool isOpenMP = false) const {
1010-
llvm::SmallVector<std::string, 12> BCLibs;
1011-
1012-
auto AddBCLib = [&](StringRef BCFile) { BCLibs.push_back(BCFile.str()); };
1012+
bool CorrectSqrt, DeviceLibABIVersion ABIVer,
1013+
const std::tuple<bool, const SanitizerArgs> &GPUSan,
1014+
bool isOpenMP = false) const {
1015+
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> BCLibs;
1016+
1017+
auto GPUSanEnabled = [GPUSan]() { return std::get<bool>(GPUSan); };
1018+
auto AddBCLib = [&](ToolChain::BitCodeLibraryInfo BCLib,
1019+
bool Internalize = true) {
1020+
BCLib.ShouldInternalize = Internalize;
1021+
BCLibs.emplace_back(BCLib);
1022+
};
1023+
auto AddSanBCLibs = [&]() {
1024+
if (GPUSanEnabled()) {
1025+
auto SanArgs = std::get<const SanitizerArgs>(GPUSan);
1026+
if (SanArgs.needsAsanRt())
1027+
AddBCLib(getAsanRTLPath(), false);
1028+
}
1029+
};
10131030

1031+
AddSanBCLibs();
10141032
AddBCLib(getOCMLPath());
10151033
if (!isOpenMP)
10161034
AddBCLib(getOCKLPath());
1035+
else if (GPUSanEnabled() && isOpenMP)
1036+
AddBCLib(getOCKLPath(), false);
10171037
AddBCLib(getDenormalsAreZeroPath(DAZ));
10181038
AddBCLib(getUnsafeMathPath(UnsafeMathOpt || FastRelaxedMath));
10191039
AddBCLib(getFiniteOnlyPath(FiniteOnly || FastRelaxedMath));
@@ -1027,7 +1047,7 @@ RocmInstallationDetector::getCommonBitcodeLibs(
10271047
return BCLibs;
10281048
}
10291049

1030-
llvm::SmallVector<std::string, 12>
1050+
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
10311051
ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
10321052
const std::string &GPUArch,
10331053
bool isOpenMP) const {
@@ -1044,6 +1064,10 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
10441064
// If --hip-device-lib is not set, add the default bitcode libraries.
10451065
// TODO: There are way too many flags that change this. Do we need to check
10461066
// them all?
1067+
std::tuple<bool, const SanitizerArgs> GPUSan(
1068+
DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
1069+
options::OPT_fno_gpu_sanitize, true),
1070+
getSanitizerArgs(DriverArgs));
10471071
bool DAZ = DriverArgs.hasFlag(options::OPT_fgpu_flush_denormals_to_zero,
10481072
options::OPT_fno_gpu_flush_denormals_to_zero,
10491073
getDefaultDenormsAreZeroForTarget(Kind));
@@ -1061,7 +1085,7 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
10611085

10621086
return RocmInstallation->getCommonBitcodeLibs(
10631087
DriverArgs, LibDeviceFile, Wave64, DAZ, FiniteOnly, UnsafeMathOpt,
1064-
FastRelaxedMath, CorrectSqrt, ABIVer, isOpenMP);
1088+
FastRelaxedMath, CorrectSqrt, ABIVer, GPUSan, isOpenMP);
10651089
}
10661090

10671091
bool AMDGPUToolChain::shouldSkipSanitizeOption(

‎clang/lib/Driver/ToolChains/AMDGPU.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
142142
Action::OffloadKind DeviceOffloadKind) const override;
143143

144144
// Returns a list of device library names shared by different languages
145-
llvm::SmallVector<std::string, 12>
145+
llvm::SmallVector<BitCodeLibraryInfo, 12>
146146
getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
147147
const std::string &GPUArch,
148148
bool isOpenMP = false) const;

‎clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#include "AMDGPUOpenMP.h"
1010
#include "AMDGPU.h"
1111
#include "CommonArgs.h"
12-
#include "ToolChains/ROCm.h"
12+
#include "ROCm.h"
1313
#include "clang/Basic/DiagnosticDriver.h"
1414
#include "clang/Driver/Compilation.h"
1515
#include "clang/Driver/Driver.h"
@@ -159,11 +159,6 @@ AMDGPUOpenMPToolChain::getDeviceLibs(const llvm::opt::ArgList &Args) const {
159159
if (Args.hasArg(options::OPT_nogpulib))
160160
return {};
161161

162-
if (!RocmInstallation->hasDeviceLibrary()) {
163-
getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 0;
164-
return {};
165-
}
166-
167162
StringRef GpuArch = getProcessorFromTargetID(
168163
getTriple(), Args.getLastArgValue(options::OPT_march_EQ));
169164

‎clang/lib/Driver/ToolChains/HIPAMD.cpp‎

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
382382
llvm::sys::path::append(Path, BCName);
383383
FullName = Path;
384384
if (llvm::sys::fs::exists(FullName)) {
385-
BCLibs.push_back(FullName);
385+
BCLibs.emplace_back(FullName);
386386
return;
387387
}
388388
}
@@ -396,28 +396,11 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
396396
StringRef GpuArch = getGPUArch(DriverArgs);
397397
assert(!GpuArch.empty() && "Must have an explicit GPU arch.");
398398

399-
// If --hip-device-lib is not set, add the default bitcode libraries.
400-
if (DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
401-
options::OPT_fno_gpu_sanitize, true) &&
402-
getSanitizerArgs(DriverArgs).needsAsanRt()) {
403-
auto AsanRTL = RocmInstallation->getAsanRTLPath();
404-
if (AsanRTL.empty()) {
405-
unsigned DiagID = getDriver().getDiags().getCustomDiagID(
406-
DiagnosticsEngine::Error,
407-
"AMDGPU address sanitizer runtime library (asanrtl) is not found. "
408-
"Please install ROCm device library which supports address "
409-
"sanitizer");
410-
getDriver().Diag(DiagID);
411-
return {};
412-
} else
413-
BCLibs.emplace_back(AsanRTL, /*ShouldInternalize=*/false);
414-
}
415-
416399
// Add the HIP specific bitcode library.
417-
BCLibs.push_back(RocmInstallation->getHIPPath());
400+
BCLibs.emplace_back(RocmInstallation->getHIPPath());
418401

419402
// Add common device libraries like ocml etc.
420-
for (StringRef N : getCommonDeviceLibNames(DriverArgs, GpuArch.str()))
403+
for (auto N : getCommonDeviceLibNames(DriverArgs, GpuArch.str()))
421404
BCLibs.emplace_back(N);
422405

423406
// Add instrument lib.
@@ -426,7 +409,7 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
426409
if (InstLib.empty())
427410
return BCLibs;
428411
if (llvm::sys::fs::exists(InstLib))
429-
BCLibs.push_back(InstLib);
412+
BCLibs.emplace_back(InstLib);
430413
else
431414
getDriver().Diag(diag::err_drv_no_such_file) << InstLib;
432415
}

‎clang/lib/Driver/ToolChains/ROCm.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "clang/Basic/LLVM.h"
1414
#include "clang/Driver/Driver.h"
1515
#include "clang/Driver/Options.h"
16+
#include "clang/Driver/SanitizerArgs.h"
1617
#include "llvm/ADT/SmallString.h"
1718
#include "llvm/ADT/StringMap.h"
1819
#include "llvm/Option/ArgList.h"
@@ -173,12 +174,11 @@ class RocmInstallationDetector {
173174

174175
/// Get file paths of default bitcode libraries common to AMDGPU based
175176
/// toolchains.
176-
llvm::SmallVector<std::string, 12>
177-
getCommonBitcodeLibs(const llvm::opt::ArgList &DriverArgs,
178-
StringRef LibDeviceFile, bool Wave64, bool DAZ,
179-
bool FiniteOnly, bool UnsafeMathOpt,
180-
bool FastRelaxedMath, bool CorrectSqrt,
181-
DeviceLibABIVersion ABIVer, bool isOpenMP) const;
177+
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> getCommonBitcodeLibs(
178+
const llvm::opt::ArgList &DriverArgs, StringRef LibDeviceFile,
179+
bool Wave64, bool DAZ, bool FiniteOnly, bool UnsafeMathOpt,
180+
bool FastRelaxedMath, bool CorrectSqrt, DeviceLibABIVersion ABIVer,
181+
const std::tuple<bool, const SanitizerArgs> &GPUSan, bool isOpenMP) const;
182182
/// Check file paths of default bitcode libraries common to AMDGPU based
183183
/// toolchains. \returns false if there are invalid or missing files.
184184
bool checkCommonBitcodeLibs(StringRef GPUArch, StringRef LibDeviceFile,

‎clang/test/Driver/hip-sanitize-options.hip‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
// RUN: -nogpuinc --rocm-path=%S/Inputs/rocm \
1919
// RUN: %s 2>&1 | FileCheck -check-prefixes=RDC %s
2020

21-
// RUN: not %clang -### --target=x86_64-unknown-linux-gnu --offload-arch=gfx900:xnack+ \
21+
// RUN: not %clang -### --target=x86_64-unknown-linux-gnu -mcode-object-version=5 --offload-arch=gfx900:xnack+ \
2222
// RUN: -fsanitize=address -fgpu-sanitize \
2323
// RUN: -nogpuinc --rocm-path=%S/Inputs/rocm-invalid \
2424
// RUN: %s 2>&1 | FileCheck -check-prefixes=FAIL %s
@@ -52,15 +52,15 @@
5252
// CHECK-NOT: {{"[^"]*lld(\.exe){0,1}".* ".*hip.bc"}}
5353
// CHECK: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
5454

55-
// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]"
55+
// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-mlink-builtin-bitcode" ".*hip.bc".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]"
5656
// NORDC-NOT: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
5757
// NORDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
5858

5959
// RDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
60-
// RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]"
60+
// RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-mlink-builtin-bitcode" ".*hip.bc".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]"
6161
// RDC-NOT: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
6262

63-
// FAIL: AMDGPU address sanitizer runtime library (asanrtl) is not found. Please install ROCm device library which supports address sanitizer
63+
// 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
6464

6565
// XNACK-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa'
6666
// 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

‎clang/test/Driver/rocm-device-libs.cl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@
145145
// RUN: 2>&1 | FileCheck --check-prefixes=NOASAN %s
146146

147147
// COMMON: "-triple" "amdgcn-amd-amdhsa"
148-
// ASAN-SAME: "-mlink-bitcode-file" "{{.*}}/amdgcn/bitcode/asanrtl.bc"
149148
// COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/opencl.bc"
149+
// ASAN-SAME: "-mlink-bitcode-file" "{{.*}}/amdgcn/bitcode/asanrtl.bc"
150150
// COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ocml.bc"
151151
// COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ockl.bc"
152152

0 commit comments

Comments
 (0)