[InstrProf] Use separate comdat group for data and counters

Summary:
I hadn't realized that instrumentation runs before inlining, so we can't
use the function as the comdat group. Doing so can create relocations
against discarded sections when references to discarded __profc_
variables are inlined into functions outside the function's comdat
group.

In the future, perhaps we should consider standardizing the comdat group
names that ELF and COFF use. It will save object file size, since
__profv_$sym won't appear in the symbol table again.

Reviewers: xur, vsk

Subscribers: eraman, hiraditya, cfe-commits, #sanitizers, llvm-commits

Tags: #clang, #sanitizers, #llvm

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@355044 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/Instrumentation/InstrProfiling.cpp b/lib/Transforms/Instrumentation/InstrProfiling.cpp
index bf91e6d7fa8e3cb5101ae44f626259ccaad4d5d8..4ed431b216e2c10529f90e257c667b6e3d727cac 100644 (file)
--- a/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -743,30 +743,23 @@ InstrProfiling::getOrCreateRegionCounters(InstrProfIncrementInst *Inc) {
 
   // Move the name variable to the right section. Place them in a COMDAT group
   // if the associated function is a COMDAT. This will make sure that only one
-  // copy of counters of the COMDAT function will be emitted after linking.
+  // copy of counters of the COMDAT function will be emitted after linking. Keep
+  // in mind that this pass may run before the inliner, so we need to create a
+  // new comdat group for the counters and profiling data. If we use the comdat
+  // of the parent function, that will result in relocations against discarded
+  // sections.
   Comdat *Cmdt = nullptr;
   GlobalValue::LinkageTypes CounterLinkage = Linkage;
   if (needsComdatForCounter(*Fn, *M)) {
+    StringRef CmdtPrefix = getInstrProfComdatPrefix();
     if (TT.isOSBinFormatCOFF()) {
-      // There are two cases that need a comdat on COFF:
-      // 1. Functions that already have comdats (standard case)
-      // 2. available_externally functions (dllimport and C99 inline)
-      // In the first case, put all the data in the original function comdat. In
-      // the second case, create a new comdat group using the counter as the
-      // leader. It's linkage must be external, so use linkonce_odr linkage in
-      // that case.
-      if (Comdat *C = Fn->getComdat()) {
-        Cmdt = C;
-      } else {
-        Cmdt = M->getOrInsertComdat(
-            getVarName(Inc, getInstrProfCountersVarPrefix()));
-        CounterLinkage = GlobalValue::LinkOnceODRLinkage;
-      }
-    } else {
-      // For other platforms that use comdats (ELF), make a new comdat group for
-      // all the profile data. It will be deduplicated within the current DSO.
-      Cmdt = M->getOrInsertComdat(getVarName(Inc, getInstrProfComdatPrefix()));
+      // For COFF, the comdat group name must be the name of a symbol in the
+      // group. Use the counter variable name, and upgrade its linkage to
+      // something externally visible, like linkonce_odr.
+      CmdtPrefix = getInstrProfCountersVarPrefix();
+      CounterLinkage = GlobalValue::LinkOnceODRLinkage;
     }
+    Cmdt = M->getOrInsertComdat(getVarName(Inc, CmdtPrefix));
   }
 
   uint64_t NumCounters = Inc->getNumCounters()->getZExtValue();

diff --git a/test/Instrumentation/InstrProfiling/PR23499.ll b/test/Instrumentation/InstrProfiling/PR23499.ll
index 7f6bed689954119c4e96d63ecb3873fd0b53ec00..af95f9a455806da9eeedcce3ef87ded690717ef1 100644 (file)
--- a/test/Instrumentation/InstrProfiling/PR23499.ll
+++ b/test/Instrumentation/InstrProfiling/PR23499.ll
@@ -20,8 +20,8 @@ $_Z3barIvEvv = comdat any
 
 
 ; COFF-NOT: __profn__Z3barIvEvv
-; COFF: @__profc__Z3barIvEvv = internal global [1 x i64] zeroinitializer, section "{{.*}}prfc$M", comdat($_Z3barIvEvv), align 8
-; COFF: @__profd__Z3barIvEvv = internal global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", comdat($_Z3barIvEvv), align 8
+; COFF: @__profc__Z3barIvEvv = linkonce_odr dso_local global [1 x i64] zeroinitializer, section "{{.*}}prfc$M", comdat, align 8
+; COFF: @__profd__Z3barIvEvv = internal global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", comdat($__profc__Z3barIvEvv), align 8
 
 
 declare void @llvm.instrprof.increment(i8*, i64, i32, i32) #1

diff --git a/test/Instrumentation/InstrProfiling/comdat.ll b/test/Instrumentation/InstrProfiling/comdat.ll
index c3bb7be47f6d2dc8578d803a6b2e7aea1967835b..03ee905fa65c1a142b58428b9211d198182fd268 100644 (file)
--- a/test/Instrumentation/InstrProfiling/comdat.ll
+++ b/test/Instrumentation/InstrProfiling/comdat.ll
@@ -17,8 +17,8 @@ $foo_inline = comdat any
 
 ; ELF: @__profc_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_cnts", comdat($__profv_foo_inline), align 8
 ; ELF: @__profd_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_data", comdat($__profv_foo_inline), align 8
-; COFF: @__profc_foo_inline = internal global{{.*}}, section ".lprfc$M", comdat($foo_inline), align 8
-; COFF: @__profd_foo_inline = internal global{{.*}}, section ".lprfd$M", comdat($foo_inline), align 8
+; COFF: @__profc_foo_inline = linkonce_odr dso_local global{{.*}}, section ".lprfc$M", comdat, align 8
+; COFF: @__profd_foo_inline = internal global{{.*}}, section ".lprfd$M", comdat($__profc_foo_inline), align 8
 define weak_odr void @foo_inline() comdat {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__profn_foo_inline, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void