Skip to content

Commit 5ec4303

Browse files
committed
[Driver][ASan] Refactor Clang-Driver "Sanitizer Bitcode" linking.
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 65df99c commit 5ec4303

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)