[sancov] Put .SCOV* sections into the right comdat groups on COFF

Avoids linker errors about relocations against discarded sections.

This was uncovered during the Chromium clang roll here:
https://chromium-review.googlesource.com/c/chromium/src/+/1321863#message-717516acfcf829176f6a2f50980f7a4bdd66469a

After this change, Chromium's libGLESv2 links successfully for me.

Reviewers: metzman, hans, morehouse

Differential Revision: https://reviews.llvm.org/D54232

llvm-svn: 346381

llvm/include/llvm/Transforms/Instrumentation.h

@@ -24,6 +24,7 @@
 
 namespace llvm {
 
+class Triple;
 class FunctionPass;
 class ModulePass;
 class OptimizationRemarkEmitter;
@@ -45,7 +46,8 @@ GlobalVariable *createPrivateGlobalForString(Module &M, StringRef Str,
 // Returns F.getComdat() if it exists.
 // Otherwise creates a new comdat, sets F's comdat, and returns it.
 // Returns nullptr on failure.
-Comdat *GetOrCreateFunctionComdat(Function &F, const std::string &ModuleId);
+Comdat *GetOrCreateFunctionComdat(Function &F, Triple &T,
+                                  const std::string &ModuleId);
 
 // Insert GCOV profiling instrumentation
 struct GCOVOptions {

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

@@ -743,7 +743,8 @@ void HWAddressSanitizer::createFrameGlobal(Function &F,
   appendToCompilerUsed(M, GV);
   // Put GV into the F's Comadat so that if F is deleted GV can be deleted too.
   if (&F != HwasanCtorFunction)
-    if (auto Comdat = GetOrCreateFunctionComdat(F, CurModuleUniqueId))
+    if (auto Comdat =
+            GetOrCreateFunctionComdat(F, TargetTriple, CurModuleUniqueId))
       GV->setComdat(Comdat);
 }
 

llvm/lib/Transforms/Instrumentation/Instrumentation.cpp

@@ -14,6 +14,7 @@
 
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm-c/Initialization.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Module.h"
 #include "llvm/InitializePasses.h"
@@ -70,19 +71,31 @@ GlobalVariable *llvm::createPrivateGlobalForString(Module &M, StringRef Str,
   return GV;
 }
 
-Comdat *llvm::GetOrCreateFunctionComdat(Function &F,
+Comdat *llvm::GetOrCreateFunctionComdat(Function &F, Triple &T,
                                         const std::string &ModuleId) {
   if (auto Comdat = F.getComdat()) return Comdat;
   assert(F.hasName());
   Module *M = F.getParent();
   std::string Name = F.getName();
-  if (F.hasLocalLinkage()) {
+
+  // Make a unique comdat name for internal linkage things on ELF. On COFF, the
+  // name of the comdat group identifies the leader symbol of the comdat group.
+  // The linkage of the leader symbol is considered during comdat resolution,
+  // and internal symbols with the same name from different objects will not be
+  // merged.
+  if (T.isOSBinFormatELF() && F.hasLocalLinkage()) {
     if (ModuleId.empty())
       return nullptr;
     Name += ModuleId;
   }
-  F.setComdat(M->getOrInsertComdat(Name));
-  return F.getComdat();
+
+  // Make a new comdat for the function. Use the "no duplicates" selection kind
+  // for non-weak symbols if the object file format supports it.
+  Comdat *C = M->getOrInsertComdat(Name);
+  if (T.isOSBinFormatCOFF() && !F.isWeakForLinker())
+    C->setSelectionKind(Comdat::NoDuplicates);
+  F.setComdat(C);
+  return C;
 }
 
 /// initializeInstrumentation - Initialize all passes in the TransformUtils

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

@@ -577,8 +577,9 @@ GlobalVariable *SanitizerCoverageModule::CreateFunctionLocalArrayInSection(
       *CurModule, ArrayTy, false, GlobalVariable::PrivateLinkage,
       Constant::getNullValue(ArrayTy), "__sancov_gen_");
 
-  if (TargetTriple.isOSBinFormatELF())
-    if (auto Comdat = GetOrCreateFunctionComdat(F, CurModuleUniqueId))
+  if (TargetTriple.supportsCOMDAT())
+    if (auto Comdat =
+            GetOrCreateFunctionComdat(F, TargetTriple, CurModuleUniqueId))
       Array->setComdat(Comdat);
   Array->setSection(getSectionName(Section));
   Array->setAlignment(Ty->isPointerTy() ? DL->getPointerSize()

llvm/test/Instrumentation/SanitizerCoverage/coff-comdat.ll

@@ -0,0 +1,85 @@
+; RUN: opt < %s -sancov -sanitizer-coverage-level=1 -sanitizer-coverage-inline-8bit-counters=1 -sanitizer-coverage-pc-table=1 -S | FileCheck %s
+
+; Make sure we use the right comdat groups for COFF to avoid relocations
+; against discarded sections. Internal linkage functions are also different from
+; ELF. We don't add a module unique identifier.
+
+; Test based on this source:
+; int baz(int);
+; static int __attribute__((noinline)) bar(int x) {
+;   if (x)
+;     return baz(x);
+;   return 0;
+; }
+; int foo(int x) {
+;   if (baz(0))
+;     x = bar(x);
+;   return x;
+; }
+
+; Both new comdats should no duplicates on COFF.
+
+; CHECK: $foo = comdat noduplicates
+; CHECK: $bar = comdat noduplicates
+
+; Tables for 'foo' should be in the 'foo' comdat.
+
+; CHECK: @__sancov_gen_{{.*}} = private global [1 x i8] zeroinitializer, section ".SCOV$CM", comdat($foo), align 1
+
+; CHECK: @__sancov_gen_{{.*}} = private constant [2 x i64*]
+; CHECK-SAME: [i64* bitcast (i32 (i32)* @foo to i64*), i64* inttoptr (i64 1 to i64*)],
+; CHECK-SAME: section ".SCOVP$M", comdat($foo), align 8
+
+; Tables for 'bar' should be in the 'bar' comdat.
+
+; CHECK: @__sancov_gen_{{.*}} = private global [1 x i8] zeroinitializer, section ".SCOV$CM", comdat($bar), align 1
+
+; CHECK: @__sancov_gen_{{.*}} = private constant [2 x i64*]
+; CHECK-SAME: [i64* bitcast (i32 (i32)* @bar to i64*), i64* inttoptr (i64 1 to i64*)],
+; CHECK-SAME: section ".SCOVP$M", comdat($bar), align 8
+
+; 'foo' and 'bar' should be in their new comdat groups.
+
+; CHECK: define dso_local i32 @foo(i32 %x){{.*}} comdat {
+; CHECK: define internal fastcc i32 @bar(i32 %x){{.*}} comdat {
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.14.26433"
+
+; Function Attrs: nounwind uwtable
+define dso_local i32 @foo(i32 %x) local_unnamed_addr #0 {
+entry:
+  %call = tail call i32 @baz(i32 0) #3
+  %tobool = icmp eq i32 %call, 0
+  br i1 %tobool, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %call1 = tail call fastcc i32 @bar(i32 %x)
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.then
+  %x.addr.0 = phi i32 [ %call1, %if.then ], [ %x, %entry ]
+  ret i32 %x.addr.0
+}
+
+declare dso_local i32 @baz(i32) local_unnamed_addr #1
+
+; Function Attrs: noinline nounwind uwtable
+define internal fastcc i32 @bar(i32 %x) unnamed_addr #2 {
+entry:
+  %tobool = icmp eq i32 %x, 0
+  br i1 %tobool, label %return, label %if.then
+
+if.then:                                          ; preds = %entry
+  %call = tail call i32 @baz(i32 %x) #3
+  br label %return
+
+return:                                           ; preds = %entry, %if.then
+  %retval.0 = phi i32 [ %call, %if.then ], [ 0, %entry ]
+  ret i32 %retval.0
+}
+
+attributes #0 = { nounwind uwtable }
+attributes #1 = { "asdf" }
+attributes #2 = { noinline nounwind uwtable }
+attributes #3 = { nounwind }