[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

clang/include/clang/Driver/ToolChain.h

@@ -113,6 +113,13 @@ public:
     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 @@ public:
                                           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.

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 {};
 }

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;
   }

clang/lib/Driver/ToolChains/HIP.h

@@ -83,7 +83,7 @@ public:
                            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;

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
+}

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:   -x hip | FileCheck -check-prefix=ASAN %s
+// 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:   -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

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"}}