[clang] c4afb5f - [HIP] Fix linking of asanrt.bc
Yaxun Liu via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 27 10:26:06 PDT 2021
Author: Yaxun (Sam) Liu
Date: 2021-09-27T13:25:46-04:00
New Revision: c4afb5f81b62b903e4af8a92235e1b901e184040
URL: https://github.com/llvm/llvm-project/commit/c4afb5f81b62b903e4af8a92235e1b901e184040
DIFF: https://github.com/llvm/llvm-project/commit/c4afb5f81b62b903e4af8a92235e1b901e184040.diff
LOG: [HIP] Fix linking of asanrt.bc
HIP currently uses -mlink-builtin-bitcode to link all bitcode libraries, which
changes the linkage of functions to be internal once they are linked in. This
works for common bitcode libraries since these functions are not intended
to be exposed for external callers.
However, the functions in the sanitizer bitcode library is intended to be
called by instructions generated by the sanitizer pass. If their linkage is
changed to internal, their parameters may be altered by optimizations before
the sanitizer pass, which renders them unusable by the sanitizer pass.
To fix this issue, HIP toolchain links the sanitizer bitcode library with
-mlink-bitcode-file, which does not change the linkage.
A struct BitCodeLibraryInfo is introduced in ToolChain as a generic
approach to pass the bitcode library information between ToolChain and Tool.
Reviewed by: Artem Belevich
Differential Revision: https://reviews.llvm.org/D110304
Added:
clang/test/CodeGenCUDA/Inputs/amdgpu-asanrtl.ll
Modified:
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChain.cpp
clang/lib/Driver/ToolChains/HIP.cpp
clang/lib/Driver/ToolChains/HIP.h
clang/test/CodeGenCUDA/amdgpu-asan.cu
clang/test/Driver/hip-sanitize-options.hip
Removed:
################################################################################
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index 882ae40086cea..fa5095e98018b 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -113,6 +113,13 @@ class ToolChain {
RM_Disabled,
};
+ struct BitCodeLibraryInfo {
+ std::string Path;
+ bool ShouldInternalize;
+ BitCodeLibraryInfo(StringRef Path, bool ShouldInternalize = true)
+ : Path(Path), ShouldInternalize(ShouldInternalize) {}
+ };
+
enum FileType { FT_Object, FT_Static, FT_Shared };
private:
@@ -681,7 +688,7 @@ class ToolChain {
const llvm::opt::ArgList &Args) const;
/// Get paths of HIP device libraries.
- virtual llvm::SmallVector<std::string, 12>
+ virtual llvm::SmallVector<BitCodeLibraryInfo, 12>
getHIPDeviceLibs(const llvm::opt::ArgList &Args) const;
/// Return sanitizers which are available in this toolchain.
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 7272cc29cc7c0..302bfb348b29f 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1026,7 +1026,7 @@ void ToolChain::AddCudaIncludeArgs(const ArgList &DriverArgs,
void ToolChain::AddHIPIncludeArgs(const ArgList &DriverArgs,
ArgStringList &CC1Args) const {}
-llvm::SmallVector<std::string, 12>
+llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
ToolChain::getHIPDeviceLibs(const ArgList &DriverArgs) const {
return {};
}
diff --git a/clang/lib/Driver/ToolChains/HIP.cpp b/clang/lib/Driver/ToolChains/HIP.cpp
index c4e840de86e15..665d5bab7218d 100644
--- a/clang/lib/Driver/ToolChains/HIP.cpp
+++ b/clang/lib/Driver/ToolChains/HIP.cpp
@@ -88,8 +88,8 @@ void AMDGCN::Linker::constructLldCommand(Compilation &C, const JobAction &JA,
if (Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize,
false))
- llvm::for_each(TC.getHIPDeviceLibs(Args), [&](StringRef BCFile) {
- LldArgs.push_back(Args.MakeArgString(BCFile));
+ llvm::for_each(TC.getHIPDeviceLibs(Args), [&](auto BCFile) {
+ LldArgs.push_back(Args.MakeArgString(BCFile.Path));
});
const char *Lld = Args.MakeArgString(getToolChain().GetProgramPath("lld"));
@@ -276,9 +276,10 @@ void HIPToolChain::addClangTargetOptions(
CC1Args.push_back("-fapply-global-visibility-to-externs");
}
- llvm::for_each(getHIPDeviceLibs(DriverArgs), [&](StringRef BCFile) {
- CC1Args.push_back("-mlink-builtin-bitcode");
- CC1Args.push_back(DriverArgs.MakeArgString(BCFile));
+ llvm::for_each(getHIPDeviceLibs(DriverArgs), [&](auto BCFile) {
+ CC1Args.push_back(BCFile.ShouldInternalize ? "-mlink-builtin-bitcode"
+ : "-mlink-bitcode-file");
+ CC1Args.push_back(DriverArgs.MakeArgString(BCFile.Path));
});
}
@@ -359,9 +360,9 @@ VersionTuple HIPToolChain::computeMSVCVersion(const Driver *D,
return HostTC.computeMSVCVersion(D, Args);
}
-llvm::SmallVector<std::string, 12>
+llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
HIPToolChain::getHIPDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
- llvm::SmallVector<std::string, 12> BCLibs;
+ llvm::SmallVector<BitCodeLibraryInfo, 12> BCLibs;
if (DriverArgs.hasArg(options::OPT_nogpulib))
return {};
ArgStringList LibraryPaths;
@@ -382,7 +383,7 @@ HIPToolChain::getHIPDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
llvm::sys::path::append(Path, BCName);
FullName = Path;
if (llvm::sys::fs::exists(FullName)) {
- BCLibs.push_back(FullName.str());
+ BCLibs.push_back(FullName);
return;
}
}
@@ -409,14 +410,15 @@ HIPToolChain::getHIPDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
getDriver().Diag(DiagID);
return {};
} else
- BCLibs.push_back(AsanRTL.str());
+ BCLibs.push_back({AsanRTL.str(), /*ShouldInternalize=*/false});
}
// Add the HIP specific bitcode library.
- BCLibs.push_back(RocmInstallation.getHIPPath().str());
+ BCLibs.push_back(RocmInstallation.getHIPPath());
// Add common device libraries like ocml etc.
- BCLibs.append(getCommonDeviceLibNames(DriverArgs, GpuArch.str()));
+ for (auto N : getCommonDeviceLibNames(DriverArgs, GpuArch.str()))
+ BCLibs.push_back(StringRef(N));
// Add instrument lib.
auto InstLib =
@@ -424,7 +426,7 @@ HIPToolChain::getHIPDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
if (InstLib.empty())
return BCLibs;
if (llvm::sys::fs::exists(InstLib))
- BCLibs.push_back(InstLib.str());
+ BCLibs.push_back(InstLib);
else
getDriver().Diag(diag::err_drv_no_such_file) << InstLib;
}
diff --git a/clang/lib/Driver/ToolChains/HIP.h b/clang/lib/Driver/ToolChains/HIP.h
index 00df4a5addb01..60b3d69b3f525 100644
--- a/clang/lib/Driver/ToolChains/HIP.h
+++ b/clang/lib/Driver/ToolChains/HIP.h
@@ -83,7 +83,7 @@ class LLVM_LIBRARY_VISIBILITY HIPToolChain final : public ROCMToolChain {
llvm::opt::ArgStringList &CC1Args) const override;
void AddHIPIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const override;
- llvm::SmallVector<std::string, 12>
+ llvm::SmallVector<BitCodeLibraryInfo, 12>
getHIPDeviceLibs(const llvm::opt::ArgList &Args) const override;
SanitizerMask getSupportedSanitizers() const override;
diff --git a/clang/test/CodeGenCUDA/Inputs/amdgpu-asanrtl.ll b/clang/test/CodeGenCUDA/Inputs/amdgpu-asanrtl.ll
new file mode 100644
index 0000000000000..9c4f30d37d626
--- /dev/null
+++ b/clang/test/CodeGenCUDA/Inputs/amdgpu-asanrtl.ll
@@ -0,0 +1,13 @@
+; Sample code for amdgpu address sanitizer runtime.
+
+; Note the runtime functions need to have weak linkage and default
+; visibility, otherwise they may be internalized and removed by GlobalOptPass.
+
+define weak void @__amdgpu_device_library_preserve_asan_functions() {
+ tail call void @__asan_report_load1(i64 0)
+ ret void
+}
+
+define weak void @__asan_report_load1(i64 %0) {
+ ret void
+}
diff --git a/clang/test/CodeGenCUDA/amdgpu-asan.cu b/clang/test/CodeGenCUDA/amdgpu-asan.cu
index fab0d48ef3c95..e392b9617cdd9 100644
--- a/clang/test/CodeGenCUDA/amdgpu-asan.cu
+++ b/clang/test/CodeGenCUDA/amdgpu-asan.cu
@@ -1,6 +1,20 @@
+// Create a sample address sanitizer bitcode library.
+
+// RUN: %clang_cc1 -x ir -fcuda-is-device -triple amdgcn-amd-amdhsa -emit-llvm-bc \
+// RUN: -disable-llvm-passes -o %t.asanrtl.bc %S/Inputs/amdgpu-asanrtl.ll
+
+// Check sanitizer runtime library functions survive
+// optimizations without being removed or parameters altered.
+
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
+// RUN: -fcuda-is-device -target-cpu gfx906 -fsanitize=address \
+// RUN: -mlink-bitcode-file %t.asanrtl.bc -x hip \
+// RUN: | FileCheck -check-prefix=ASAN %s
+
// RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
// RUN: -fcuda-is-device -target-cpu gfx906 -fsanitize=address \
-// RUN: -x hip | FileCheck -check-prefix=ASAN %s
+// RUN: -O3 -mlink-bitcode-file %t.asanrtl.bc -x hip \
+// RUN: | FileCheck -check-prefix=ASAN %s
// RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
// RUN: -fcuda-is-device -target-cpu gfx906 -x hip \
@@ -8,8 +22,10 @@
// REQUIRES: amdgpu-registered-target
-// ASAN-DAG: declare void @__amdgpu_device_library_preserve_asan_functions()
+// ASAN-DAG: define weak void @__amdgpu_device_library_preserve_asan_functions()
// ASAN-DAG: @__amdgpu_device_library_preserve_asan_functions_ptr = weak addrspace(1) constant void ()* @__amdgpu_device_library_preserve_asan_functions
// ASAN-DAG: @llvm.compiler.used = {{.*}}@__amdgpu_device_library_preserve_asan_functions_ptr
+// ASAN-DAG: define weak void @__asan_report_load1(i64 %{{.*}})
-// CHECK-NOT: @__amdgpu_device_library_preserve_asan_functions_ptr
+// CHECK-NOT: @__amdgpu_device_library_preserve_asan_functions
+// CHECK-NOT: @__asan_report_load1
diff --git a/clang/test/Driver/hip-sanitize-options.hip b/clang/test/Driver/hip-sanitize-options.hip
index 7df800f0c04f0..d617b00fa9e6f 100644
--- a/clang/test/Driver/hip-sanitize-options.hip
+++ b/clang/test/Driver/hip-sanitize-options.hip
@@ -34,7 +34,7 @@
// 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".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]"
+// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]"
// NORDC: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
// NORDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
More information about the cfe-commits
mailing list