[SanitizerCoverage] Use different module ctor names for trace-pc-guar…

…d and inline-8bit-counters

Fixes the main issue in PR41693

When both modes are used, two functions are created:
`sancov.module_ctor`, `sancov.module_ctor.$LastUnique`, where
$LastUnique is the current LastUnique counter that may be different in
another module.

`sancov.module_ctor.$LastUnique` belongs to the comdat group of the same
name (due to the non-null third field of the ctor in llvm.global_ctors).

    COMDAT group section [    9] `.group' [sancov.module_ctor] contains 6 sections:
       [Index]    Name
       [   10]   .text.sancov.module_ctor
       [   11]   .rela.text.sancov.module_ctor
       [   12]   .text.sancov.module_ctor.6
       [   13]   .rela.text.sancov.module_ctor.6
       [   23]   .init_array.2
       [   24]   .rela.init_array.2

    # 2 problems:
    # 1) If sancov.module_ctor in this module is discarded, this group
    # has a relocation to a discarded section. ld.bfd and gold will
    # error. (Another issue: it is silently accepted by lld)
    # 2) The comdat group has an unstable name that may be different in
    # another translation unit. Even if the linker allows the dangling relocation
    # (with --noinhibit-exec), there will be many undesired .init_array entries
    COMDAT group section [   25] `.group' [sancov.module_ctor.6] contains 2 sections:
       [Index]    Name
       [   26]   .init_array.2
       [   27]   .rela.init_array.2

By using different module ctor names, the associated comdat group names
will also be different and thus stable across modules.

Reviewed By: morehouse, phosek

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

llvm-svn: 360107

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

@@ -61,7 +61,10 @@ static const char *const SanCovTraceDiv4 = "__sanitizer_cov_trace_div4";
 static const char *const SanCovTraceDiv8 = "__sanitizer_cov_trace_div8";
 static const char *const SanCovTraceGep = "__sanitizer_cov_trace_gep";
 static const char *const SanCovTraceSwitchName = "__sanitizer_cov_trace_switch";
-static const char *const SanCovModuleCtorName = "sancov.module_ctor";
+static const char *const SanCovModuleCtorTracePcGuardName =
+    "sancov.module_ctor_trace_pc_guard";
+static const char *const SanCovModuleCtor8bitCountersName =
+    "sancov.module_ctor_8bit_counters";
 static const uint64_t SanCtorAndDtorPriority = 2;
 
 static const char *const SanCovTracePCGuardName =
@@ -209,8 +212,9 @@ class SanitizerCoverageModule : public ModulePass {
   void CreateFunctionLocalArrays(Function &F, ArrayRef<BasicBlock *> AllBlocks);
   void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
                              bool IsLeafFunc = true);
-  Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
-                                       Type *Ty, const char *Section);
+  Function *CreateInitCallsForSections(Module &M, const char *CtorName,
+                                       const char *InitFunctionName, Type *Ty,
+                                       const char *Section);
   std::pair<Value *, Value *> CreateSecStartEnd(Module &M, const char *Section,
                                                 Type *Ty);
 
@@ -275,18 +279,19 @@ SanitizerCoverageModule::CreateSecStartEnd(Module &M, const char *Section,
 }
 
 Function *SanitizerCoverageModule::CreateInitCallsForSections(
-    Module &M, const char *InitFunctionName, Type *Ty,
+    Module &M, const char *CtorName, const char *InitFunctionName, Type *Ty,
     const char *Section) {
   auto SecStartEnd = CreateSecStartEnd(M, Section, Ty);
   auto SecStart = SecStartEnd.first;
   auto SecEnd = SecStartEnd.second;
   Function *CtorFunc;
   std::tie(CtorFunc, std::ignore) = createSanitizerCtorAndInitFunctions(
-      M, SanCovModuleCtorName, InitFunctionName, {Ty, Ty}, {SecStart, SecEnd});
+      M, CtorName, InitFunctionName, {Ty, Ty}, {SecStart, SecEnd});
+  assert(CtorFunc->getName() == CtorName);
 
   if (TargetTriple.supportsCOMDAT()) {
     // Use comdat to dedup CtorFunc.
-    CtorFunc->setComdat(M.getOrInsertComdat(SanCovModuleCtorName));
+    CtorFunc->setComdat(M.getOrInsertComdat(CtorName));
     appendToGlobalCtors(M, CtorFunc, SanCtorAndDtorPriority, CtorFunc);
   } else {
     appendToGlobalCtors(M, CtorFunc, SanCtorAndDtorPriority);
@@ -403,10 +408,12 @@ bool SanitizerCoverageModule::runOnModule(Module &M) {
   Function *Ctor = nullptr;
 
   if (FunctionGuardArray)
-    Ctor = CreateInitCallsForSections(M, SanCovTracePCGuardInitName, Int32PtrTy,
+    Ctor = CreateInitCallsForSections(M, SanCovModuleCtorTracePcGuardName,
+                                      SanCovTracePCGuardInitName, Int32PtrTy,
                                       SanCovGuardsSectionName);
   if (Function8bitCounterArray)
-    Ctor = CreateInitCallsForSections(M, SanCov8bitCountersInitName, Int8PtrTy,
+    Ctor = CreateInitCallsForSections(M, SanCovModuleCtor8bitCountersName,
+                                      SanCov8bitCountersInitName, Int8PtrTy,
                                       SanCovCountersSectionName);
   if (Ctor && Options.PCTable) {
     auto SecStartEnd = CreateSecStartEnd(M, SanCovPCsSectionName, IntptrPtrTy);

llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-comdat.ll

@@ -38,5 +38,4 @@ entry:
 ; CHECK_TRACE_PC_GUARD: call void @__sanitizer_cov_trace_pc_indir
 ; CHECK_TRACE_PC_GUARD: ret void
 
-; CHECK_TRACE_PC_GUARD-LABEL: define internal void @sancov.module_ctor() comdat
-
+; CHECK_TRACE_PC_GUARD-LABEL: define internal void @sancov.module_ctor_trace_pc_guard() comdat

llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll

@@ -0,0 +1,13 @@
+; RUN: opt < %s -sancov -sanitizer-coverage-level=1 -sanitizer-coverage-trace-pc-guard -sanitizer-coverage-inline-8bit-counters -S | FileCheck %s
+
+; Module ctors should have stable names across modules, not something like
+; @sancov.module_ctor.3 that may cause duplicate ctors after linked together.
+
+; CHECK: define internal void @sancov.module_ctor_trace_pc_guard() comdat {
+; CHECK: define internal void @sancov.module_ctor_8bit_counters() comdat {
+
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
+target triple = "x86_64-unknown-linux-gnu"
+define void @foo() {
+  ret void
+}

llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-nocomdat.ll

@@ -38,5 +38,4 @@ entry:
 ; CHECK_TRACE_PC_GUARD: call void @__sanitizer_cov_trace_pc_indir
 ; CHECK_TRACE_PC_GUARD: ret void
 
-; CHECK_TRACE_PC_GUARD-LABEL: define internal void @sancov.module_ctor() {
-
+; CHECK_TRACE_PC_GUARD-LABEL: define internal void @sancov.module_ctor_trace_pc_guard() {