Do not append terminating NUL to the string with embedded GPU binary.

[tra]

Summary

Extra NUL does not impact functionality of the generated code, but it confuses
various NVIDIA tools used to examine embedded GPU binaries.

[efriedma]

clang/lib/CodeGen/CGCUDANV.cpp

95

Please don't abuse GetAddrOfConstantCString like this. Like the name says, it's meant for strings, in the normal sections strings would go in. If you just want an array global, please just use "new llvm::GlobalVariable" directly.

(AddNull itself is not a big deal, but messing with the section/alignment/unnamed_addr of globals in CodeGenModule's ConstantStringMap is a bad idea.)

[tra]

Reworked string/array generation.

[tra]

clang/lib/CodeGen/CGCUDANV.cpp

95

Agreed. That was not the best choice. I've separated generation of regular strings from generation of constant arrays we need to place just so.

[yaxunl]

LGTM. Thanks.

clang/lib/CodeGen/CGCUDANV.cpp

private:
   llvm::Function *makeRegisterGlobalsFn();
 
   /// Helper function that generates a constant string and returns a pointer to
   /// the start of the string.  The result of this function can be used anywhere
   /// where the C code specifies const char*.
   llvm::Constant *makeConstantString(const std::string &Str,
                                      const std::string &Name = "",
                                      const std::string &SectionName = "",
-                                     unsigned Alignment = 0) {
+                                     unsigned Alignment = 0,
+                                     bool AddNull = true) {
     llvm::Constant *Zeros[] = {llvm::ConstantInt::get(SizeTy, 0),
                                llvm::ConstantInt::get(SizeTy, 0)};
-    auto ConstStr = CGM.GetAddrOfConstantCString(Str, Name.c_str());
+    auto ConstStr = CGM.GetAddrOfConstantCString(Str, Name.c_str(), AddNull);
     llvm::GlobalVariable *GV =
         cast<llvm::GlobalVariable>(ConstStr.getPointer());
     if (!SectionName.empty()) {
       GV->setSection(SectionName);
       // Mark the address as used which make sure that this section isn't
       // merged and we will really have it in the object file.
       GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::None);
     }
if (IsHIP) {
 
     ModuleIDSectionName = "__hip_module_id";
     ModuleIDPrefix = "__hip_";
 
     if (CudaGpuBinary) {
       // If fatbin is available from early finalization, create a string
       // literal containing the fat binary loaded from the given file.
       const unsigned HIPCodeObjectAlign = 4096;
-      FatBinStr =
-          makeConstantString(std::string(CudaGpuBinary->getBuffer()), "",
-                             FatbinConstantName, HIPCodeObjectAlign);
+      FatBinStr = makeConstantString(std::string(CudaGpuBinary->getBuffer()),
+                                     "", FatbinConstantName, HIPCodeObjectAlign,
+                                     /*AddNull=*/false);
     } else {
       // If fatbin is not available, create an external symbol
       // __hip_fatbin in section .hip_fatbin. The external symbol is supposed
       // to contain the fat binary but will be populated somewhere else,
       // e.g. by lld through link script.
       FatBinStr = new llvm::GlobalVariable(
         CGM.getModule(), CGM.Int8Ty,
         /*isConstant=*/true, llvm::GlobalValue::ExternalLinkage, nullptr,
if (IsHIP) {
     ModuleIDSectionName = CGM.getTriple().isMacOSX()
                               ? "__NV_CUDA,__nv_module_id"
                               : "__nv_module_id";
     ModuleIDPrefix = "__nv_";
 
     // For CUDA, create a string literal containing the fat binary loaded from
     // the given file.
     FatBinStr = makeConstantString(std::string(CudaGpuBinary->getBuffer()), "",
-                                   FatbinConstantName, 8);
+                                   FatbinConstantName, 8, /*AddNull=*/false);
     FatMagic = CudaFatMagic;
   }
 
   // Create initialized wrapper structure that points to the loaded GPU binary
   ConstantInitBuilder Builder(CGM);
   auto Values = Builder.beginStruct(FatbinWrapperTy);
   // Fatbin wrapper magic.
   Values.addInt(IntTy, FatMagic);

clang/lib/CodeGen/CodeGenModule.h

public:
   ConstantAddress
   GetAddrOfConstantStringFromLiteral(const StringLiteral *S,
                                      StringRef Name = ".str");
 
   /// Return a pointer to a constant array for the given ObjCEncodeExpr node.
   ConstantAddress
   GetAddrOfConstantStringFromObjCEncode(const ObjCEncodeExpr *);
 
-  /// Returns a pointer to a character array containing the literal and a
-  /// terminating '\0' character. The result has pointer to array type.
+  /// Returns a pointer to a character array containing the literal and an
+  /// optional terminating '\0' character. The result has pointer to array type.
   ///
   /// \param GlobalName If provided, the name to use for the global (if one is
   /// created).
-  ConstantAddress
-  GetAddrOfConstantCString(const std::string &Str,
-                           const char *GlobalName = nullptr);
+  ConstantAddress GetAddrOfConstantCString(const std::string &Str,
+                                           const char *GlobalName = nullptr,
+                                           bool AddNull = true);
 
   /// Returns a pointer to a constant global variable for the given file-scope
   /// compound literal expression.
   ConstantAddress GetAddrOfConstantCompoundLiteral(const CompoundLiteralExpr*E);
 
   /// If it's been emitted already, returns the GlobalVariable corresponding to
   /// a compound literal. Otherwise, returns null.
   llvm::GlobalVariable *

clang/lib/CodeGen/CodeGenModule.cpp

 CodeGenModule::GetAddrOfConstantStringFromObjCEncode(const ObjCEncodeExpr *E) {
   std::string Str;
   getContext().getObjCEncodingForType(E->getEncodedType(), Str);
 
   return GetAddrOfConstantCString(Str);
 }
 
 /// GetAddrOfConstantCString - Returns a pointer to a character array containing
-/// the literal and a terminating '\0' character.
+/// the literal and an optional terminating '\0' character.
 /// The result has pointer to array type.
-ConstantAddress CodeGenModule::GetAddrOfConstantCString(
-    const std::string &Str, const char *GlobalName) {
-  StringRef StrWithNull(Str.c_str(), Str.size() + 1);
+ConstantAddress CodeGenModule::GetAddrOfConstantCString(const std::string &Str,
+                                                        const char *GlobalName,
+                                                        bool AddNull) {
   CharUnits Alignment =
     getContext().getAlignOfGlobalVarInChars(getContext().CharTy);
 
   llvm::Constant *C =
-      llvm::ConstantDataArray::getString(getLLVMContext(), StrWithNull, false);
+      llvm::ConstantDataArray::getString(getLLVMContext(), Str, AddNull);
 
   // Don't share any string literals if strings aren't constant.
   llvm::GlobalVariable **Entry = nullptr;
   if (!LangOpts.WritableStrings) {
     Entry = &ConstantStringMap[C];
     if (auto GV = *Entry) {
       if (uint64_t(Alignment.getQuantity()) > GV->getAlignment())
         GV->setAlignment(Alignment.getAsAlign());

clang/test/CodeGenCUDA/device-stub.cu

-// RUN: echo "GPU binary would be here" > %t
+// RUN: echo -n "GPU binary would be here." > %t
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN:     -target-sdk-version=8.0 -fcuda-include-gpubinary %t -o - \
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s \
 // RUN:       --check-prefixes=ALL,LNX,NORDC,CUDA,CUDANORDC,CUDA-OLD
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN:     -target-sdk-version=8.0  -fcuda-include-gpubinary %t \
 // RUN:     -o - -DNOGLOBALS \
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s \

 // ALL: @0 = private unnamed_addr constant [18 x i8] c"_Z10kernelfunciii\00"
 // * constant unnamed string with the device-side kernel name to be passed to
 //   __hipRegisterVar/__cudaRegisterVar.
 // ALL: @1 = private unnamed_addr constant [11 x i8] c"device_var\00"
 // ALL: @2 = private unnamed_addr constant [13 x i8] c"constant_var\00"
 // ALL: @3 = private unnamed_addr constant [19 x i8] c"ext_device_var_def\00"
 // ALL: @4 = private unnamed_addr constant [21 x i8] c"ext_constant_var_def\00"
 // * constant unnamed string with GPU binary
-// CUDA: @[[FATBIN:.*]] = private constant{{.*GPU binary would be here.*}}\00",
-// HIPEF: @[[FATBIN:.*]] = private constant{{.*GPU binary would be here.*}}\00",{{.*}}align 4096
+// CUDA: @[[FATBIN:.*]] = private constant{{.*}} c"GPU binary would be here.",
+// HIPEF: @[[FATBIN:.*]] = private constant{{.*}} c"GPU binary would be here.",{{.*}}align 4096
 // HIPNEF: @[[FATBIN:__hip_fatbin]] = external constant i8, section ".hip_fatbin"
 // CUDANORDC-SAME: section ".nv_fatbin", align 8
 // CUDARDC-SAME: section "__nv_relfatbin", align 8
 // * constant struct that wraps GPU binary
 // ALL: @__[[PREFIX:cuda|hip]]_fatbin_wrapper = internal constant
 // LNX-SAME: { i32, i32, i8*, i8* }
 // CUDA-SAME: { i32 1180844977, i32 1,
 // HIP-SAME: { i32 1212764230, i32 1,