[InstrProfiling] Use external weak reference for bias variable

We need the compiler generated variable to override the weak symbol of
the same name inside the profile runtime, but using LinkOnceODRLinkage
results in weak symbol being emitted which leads to an issue where the
linker might choose either of the weak symbols potentially disabling the
runtime counter relocation.

This change replaces the use of weak definition inside the runtime with
an external weak reference to address the issue. We also place the
compiler generated symbol inside a COMDAT group so dead definition can
be garbage collected by the linker.

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

compiler-rt/lib/profile/CMakeLists.txt

@@ -53,7 +53,6 @@ set(PROFILE_SOURCES
   InstrProfiling.c
   InstrProfilingInternal.c
   InstrProfilingValue.c
-  InstrProfilingBiasVar.c
   InstrProfilingBuffer.c
   InstrProfilingFile.c
   InstrProfilingMerge.c

compiler-rt/lib/profile/InstrProfiling.h

@@ -320,10 +320,10 @@ extern uint64_t INSTR_PROF_RAW_VERSION_VAR; /* __llvm_profile_raw_version */
 extern char INSTR_PROF_PROFILE_NAME_VAR[1]; /* __llvm_profile_filename. */
 
 /*!
- * This variable is a weak symbol defined in InstrProfilingBiasVar.c. It
- * allows compiler instrumentation to provide overriding definition with
- * value from compiler command line. This variable has hidden visibility.
+ * This variable is a weak external reference which could be used to detect
+ * whether or not the compiler defined this symbol.
  */
-COMPILER_RT_VISIBILITY extern intptr_t __llvm_profile_counter_bias;
+COMPILER_RT_VISIBILITY COMPILER_RT_WEAK extern intptr_t
+    __llvm_profile_counter_bias;
 
 #endif /* PROFILE_INSTRPROFILING_H_ */

compiler-rt/lib/profile/InstrProfilingBiasVar.c

@@ -1,15 +0,0 @@
-/*===- InstrProfilingBiasVar.c - profile counter bias variable setup ------===*\
-|*
-|* Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-|* See https://llvm.org/LICENSE.txt for license information.
-|* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-|*
-\*===----------------------------------------------------------------------===*/
-
-#include "InstrProfiling.h"
-
-/* The runtime should only provide its own definition of this symbol when the
- * user has not specified one. Set this up by moving the runtime's copy of this
- * symbol to an object file within the archive.
- */
-COMPILER_RT_WEAK intptr_t __llvm_profile_counter_bias = -1;

compiler-rt/lib/profile/InstrProfilingFile.c

@@ -999,7 +999,9 @@ void __llvm_profile_initialize_file(void) {
   ProfileNameSpecifier PNS = PNS_unknown;
   int hasCommandLineOverrider = (INSTR_PROF_PROFILE_NAME_VAR[0] != 0);
 
-  if (__llvm_profile_counter_bias != -1)
+  /* This symbol is defined by the compiler when runtime counter relocation is
+   * used and runtime provides a weak external reference so we can check it. */
+  if (&__llvm_profile_counter_bias)
     lprofSetRuntimeCounterRelocation(1);
 
   EnvFilenamePat = getFilenamePatFromEnv();

compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c

@@ -116,10 +116,9 @@ void __llvm_profile_initialize(void) {
     return;
   }
 
-  /* This symbol is defined as weak and initialized to -1 by the runtimer, but
-   * compiler will generate a strong definition initialized to 0 when runtime
-   * counter relocation is used. */
-  if (__llvm_profile_counter_bias == -1) {
+  /* This symbol is defined by the compiler when runtime counter relocation is
+   * used and runtime provides a weak external reference so we can check it. */
+  if (!&__llvm_profile_counter_bias) {
     lprofWrite("LLVM Profile: counter relocation at runtime is required\n");
     return;
   }

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

@@ -690,10 +690,19 @@ void InstrProfiling::lowerIncrement(InstrProfIncrementInst *Inc) {
       Type *Int64Ty = Type::getInt64Ty(M->getContext());
       GlobalVariable *Bias = M->getGlobalVariable(getInstrProfCounterBiasVarName());
       if (!Bias) {
+        // Compiler must define this variable when runtime counter relocation
+        // is being used. Runtime has a weak external reference that is used
+        // to check whether that's the case or not.
         Bias = new GlobalVariable(*M, Int64Ty, false, GlobalValue::LinkOnceODRLinkage,
                                   Constant::getNullValue(Int64Ty),
                                   getInstrProfCounterBiasVarName());
         Bias->setVisibility(GlobalVariable::HiddenVisibility);
+        // A definition that's weak (linkonce_odr) without being in a COMDAT
+        // section wouldn't lead to link errors, but it would lead to a dead
+        // data word from every TU but one. Putting it in COMDAT ensures there
+        // will be exactly one data slot in the link.
+        if (TT.supportsCOMDAT())
+            Bias->setComdat(M->getOrInsertComdat(Bias->getName()));
       }
       LI = Builder.CreateLoad(Int64Ty, Bias);
     }

llvm/test/Instrumentation/InstrProfiling/runtime-counter-relocation.ll

@@ -4,7 +4,8 @@
 target triple = "x86_64-unknown-linux-gnu"
 
 @__profn_foo = private constant [3 x i8] c"foo"
-; RELOC: @__llvm_profile_counter_bias = linkonce_odr hidden global i64 0
+; RELOC: $__llvm_profile_counter_bias = comdat any
+; RELOC: @__llvm_profile_counter_bias = linkonce_odr hidden global i64 0, comdat
 
 ; CHECK-LABEL: define void @foo
 ; CHECK-NEXT: %pgocount = load i64, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc_foo, i64 0, i64 0)