yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added subscribers: kerbowa, nhaehnle, jvesely.
yaxunl requested review of this revision.
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.
An enum for bitcode link option and a struct BitCodeLibraryInfo is
introduced in ToolChain as a generic approach to pass the bitcode
linking option between ToolChain and Tool.
https://reviews.llvm.org/D110304
Files:
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/Inputs/amdgpu-asanrtl.ll
clang/test/CodeGenCUDA/amdgpu-asan.cu
clang/test/Driver/hip-sanitize-options.hip
Index: clang/test/Driver/hip-sanitize-options.hip
===================================================================
--- clang/test/Driver/hip-sanitize-options.hip
+++ 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"}}
Index: clang/test/CodeGenCUDA/amdgpu-asan.cu
===================================================================
--- clang/test/CodeGenCUDA/amdgpu-asan.cu
+++ 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
Index: clang/test/CodeGenCUDA/Inputs/amdgpu-asanrtl.ll
===================================================================
--- /dev/null
+++ 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
+}
Index: clang/lib/Driver/ToolChains/HIP.h
===================================================================
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -83,7 +83,7 @@
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;
Index: clang/lib/Driver/ToolChains/HIP.cpp
===================================================================
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -88,8 +88,8 @@
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,13 @@
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) {
+ assert(BCFile.LinkOpt == ToolChain::BCLO_LinkBitCodeFile ||
+ BCFile.LinkOpt == ToolChain::BCLO_LinkBuiltinBitCode);
+ CC1Args.push_back(BCFile.LinkOpt == ToolChain::BCLO_LinkBitCodeFile
+ ? "-mlink-bitcode-file"
+ : "-mlink-builtin-bitcode");
+ CC1Args.push_back(DriverArgs.MakeArgString(BCFile.Path));
});
}
@@ -359,9 +363,9 @@
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 +386,7 @@
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 +413,15 @@
getDriver().Diag(DiagID);
return {};
} else
- BCLibs.push_back(AsanRTL.str());
+ BCLibs.push_back({AsanRTL.str(), BCLO_LinkBitCodeFile});
}
// 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 +429,7 @@
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;
}
Index: clang/lib/Driver/ToolChain.cpp
===================================================================
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -1024,7 +1024,7 @@
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 {};
}
Index: clang/include/clang/Driver/ToolChain.h
===================================================================
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -113,6 +113,21 @@
RM_Disabled,
};
+ // Enums corresponding to clang options for linking bitcode, i.e.,
+ // -mlink-builtin-bitcode or -mlink-bitcode-file
+ enum BitCodeLinkOpt {
+ BCLO_LinkBuiltinBitCode,
+ BCLO_LinkBitCodeFile,
+ };
+
+ struct BitCodeLibraryInfo {
+ std::string Path;
+ BitCodeLinkOpt LinkOpt;
+ BitCodeLibraryInfo(StringRef Path,
+ BitCodeLinkOpt LinkOpt = BCLO_LinkBuiltinBitCode)
+ : Path(Path), LinkOpt(LinkOpt) {}
+ };
+
enum FileType { FT_Object, FT_Static, FT_Shared };
private:
@@ -681,7 +696,7 @@
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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
