[InstrProfiling] Use external weak reference for bias variable

[phosek]

Summary

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 in which case the symbol selected
by the linker is going to depend on the order of inputs which is far too
fragile.

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.

We also disable the use of runtime counter relocation on Darwin since
Mach-O doesn't support weak external references, but Darwin already uses
a different continous mode that relies on overmapping so runtime counter
relocation isn't needed there.

[mcgrathr]

It makes sense that the contract be that the runtime must define the variable.

I don't understand how this change is accomplishing that, however. From the IR in the test case, it's become a hidden global definition in a COMDAT group. AFAIK, that's what linkonce_odr actually translates to in ELF backends anyway. What's the difference in the .s or .o file generated from that IR? I would expect to just use a vanilla undefined external symbol here.

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

693

This merits a comment about the contract, e.g. "The runtime must define this variable."

706

What's the meaning of comdat on an undefined external?
There is no such concept at the object file level, at least not in ELF.

[davidxl]

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

706

The variable is defined (it has an initialization).

The related question is that without COMDAT support, would this leads to multiple definition linker error?

[phosek]

After an offline discussion with @mcgrathr we've decided to switch to external weak reference for the bias variable which is a cleaner solution.

[davidxl]

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

705

Is this change still needed?

[phosek]

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

705

It's not necessary for correctness, but should enable GC in the case this variable ends up unused.

[davidxl]

lgtm

[mcgrathr]

lgtm modulo some comment cleanups

compiler-rt/lib/profile/InstrProfiling.h

322–323

I don't understand this sentence.
The runtime has no "copy" of this symbol, it only refers to it.
The salient point here is that this is a weak undefined reference so the runtime can detect whether or not the compiler defined the symbol.

compiler-rt/lib/profile/InstrProfilingFile.c

1036–1037

Any check of a weak reference like this merits a comment since it's always a subtle matter.

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

706

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.

[thakis]

This breaks 52 libprofile tests on macOS: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8842844620347296624/+/u/package_clang/stdout?format=raw

Undefined symbols for architecture x86_64:
  "___llvm_profile_counter_bias", referenced from:
      ___llvm_profile_initialize_file in libclang_rt.profile_osx.a(InstrProfilingFile.c.o)
      ___llvm_profile_initialize in libclang_rt.profile_osx.a(InstrProfilingFile.c.o)
      _relocateCounters in libclang_rt.profile_osx.a(InstrProfilingFile.c.o)
ld: symbol(s) not found for architecture x86_64
clang-13: error: linker command failed with exit code 1 (use -v to see invocation)

I reverted this in a92964779cb5fa59e832816b14a30bc8dbf927a9 for now.

[thakis]

(I believe @MaskRay did a lot of cool work in this general area recently, maybe he knows how to make this work.)

[MaskRay]

In D105176#2855238, @thakis wrote:

(I believe @MaskRay did a lot of cool work in this general area recently, maybe he knows how to make this work.)

This is the normal problem that ld64 disallows undefined symbols by default, even in dylib mode.
To make the linker happy, we can use a dynamic lookup symbol, i.e. -Wl,-U,__llvm_profile_counter_bias.
However, this just defers the error to the runtime dyld: Symbol not found.

Am I right that Mach-O doesn't really support undefined weak symbols?
(The binary format is unfortunate in this case: the hidden state of an undefined weak is not represented, so the linker cannot know the symbol needs to be resolved to zero)

Perhaps disallowing this for Darwin or making this Fuchsia specific is the simplest solution.

For

extern int var;
void *addr() { return &var; }

-fprofile-instr-generate -fcoverage-mapping a.c -S -O2 -mllvm -runtime-counter-relocation -fno-asynchronous-unwind-tables -fno-exceptions produces:

addr:                                   # @addr
# %bb.0:                                # %entry
        movq    __llvm_profile_counter_bias(%rip), %rax
        addq    $1, .L__profc_addr(%rax)
        movl    $var, %eax
        retq

The indirection is unfortunate. I don't think Linux users may want to have this.

[mcgrathr]

MaskRay's code generation example shows the optimal code possible for implementing this feature and how that code is generated is unaffected by this change, so I don't understand why that subject is being raised here.

As to the linking issues, it may make sense to define things differently for different object formats. Using the approach taken here for ELF platforms and leaving other platforms to their own devices seems like a sensible plan to me.

[MaskRay]

If an undefined weak symbol is kept, restricting to ELF only looks fine to me.

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.

But I don't understand this sentence from the description. Are you linking libclang_rt.profile-* before other input files?
If there may be conflicting definitions, LinkOnceODRLinkage is not suitable, but you can use LinkOnceAnyLinkage/WeakAnyLinkage

[phosek]

I'm fine disabling this for Mach-O which uses the continuous mode that relies on overmapping (see https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#running-the-instrumented-program for more details).

I'll clarify the commit message, the issue is that relying on link order is fragile and can lead to hard-to-notice issues which we observed in practice.

[MaskRay]

We also disable the use of runtime counter relocation on Darwin since Mach-O doesn't support weak external references, but Darwin already uses a different continous mode that relies on overmapping so runtime counter relocation isn't needed there.

I think you can restrict undefined weak to ELF now. PE-COFF doesn't have undefined weak, either.

[phosek]

In D105176#2865370, @MaskRay wrote:

We also disable the use of runtime counter relocation on Darwin since Mach-O doesn't support weak external references, but Darwin already uses a different continous mode that relies on overmapping so runtime counter relocation isn't needed there.

I think you can restrict undefined weak to ELF now. PE-COFF doesn't have undefined weak, either.

Done, do you know if there's any alternative we could use for PE-COFF?

[MaskRay]

In D105176#2865477, @phosek wrote:

In D105176#2865370, @MaskRay wrote:

We also disable the use of runtime counter relocation on Darwin since Mach-O doesn't support weak external references, but Darwin already uses a different continous mode that relies on overmapping so runtime counter relocation isn't needed there.

I think you can restrict undefined weak to ELF now. PE-COFF doesn't have undefined weak, either.

Done, do you know if there's any alternative we could use for PE-COFF?

A comdat any is the easiest. The runtime defines a comdat any variable with value 0 which can be overridden by the program.

It does rely on the link order but is the easiest.

Alternatives include IMAGE_SYM_CLASS_WEAK_EXTERNAL (weak definition) and linker directives /alternative: both of which are quite different/inconvenient.

So I want to re-ask how the order difference is a problem. Using a comdat any can be straightforwardly ported to PE/COFF.

I'll clarify the commit message, the issue is that relying on link order is fragile and can lead to hard-to-notice issues which we observed in practice.

[phosek]

In D105176#2865512, @MaskRay wrote:

In D105176#2865477, @phosek wrote:

In D105176#2865370, @MaskRay wrote:

We also disable the use of runtime counter relocation on Darwin since Mach-O doesn't support weak external references, but Darwin already uses a different continous mode that relies on overmapping so runtime counter relocation isn't needed there.

I think you can restrict undefined weak to ELF now. PE-COFF doesn't have undefined weak, either.

Done, do you know if there's any alternative we could use for PE-COFF?

A comdat any is the easiest. The runtime defines a comdat any variable with value 0 which can be overridden by the program.

We need to distinguish the case where the variable is defined by the TU (in which case it has to be initialized with value 0) and where the variable is defined by the runtime.

Previously, the variable defined by the runtime would have the value -1 and the variable defined by the TU would have 0. After this change, runtime has an undefined reference and variable defined by TU is still initialized to 0.

Given that requirement, I'm not sure comdat any is going to work.

It does rely on the link order but is the easiest.

Alternatives include IMAGE_SYM_CLASS_WEAK_EXTERNAL (weak definition) and linker directives /alternative: both of which are quite different/inconvenient.

That might work, I'll look into it.

So I want to re-ask how the order difference is a problem. Using a comdat any can be straightforwardly ported to PE/COFF.

I noticed that in some of our instrumented binaries, the runtime definition initialized to -1 prevails in the binary. I haven't had pinpoint exactly why it's happening, in some cases we do mix C++ and Rust and manually link the profile runtime and I suspect that this might be one of the culprits.

I can dig into this a bit more and try to find, but I still think that relying on link order is fragile (especially since you won't notice the issue until you run the binary) and so I'd like to avoid it if possible.

I'll clarify the commit message, the issue is that relying on link order is fragile and can lead to hard-to-notice issues which we observed in practice.

[phosek]

@MaskRay it looks like we could use the following pattern:

extern intptr_t __llvm_profile_counter_bias;

#if defined(_WIN32)
intptr_t __llvm_profile_counter_bias_default = 0;
#pragma comment(linker, "/alternatename:" \
                SYMBOL_NAME("__llvm_profile_counter_bias") "=" \
                SYMBOL_NAME("__llvm_profile_counter_bias_default"))
#endif

bool usesRuntimeCounterRelocation() {
  void *s1 = &__llvm_profile_counter_bias;
  void *s2 = &__llvm_profile_counter_bias_default;
  return s1 != s2;
}

This does seem to work with clang-cl but when I tried this with cl I still get an error:

error LNK2019: unresolved external symbol __llvm_profile_counter_bias referenced in function usesRuntimeCounterRelocation

[phosek]

In D105176#2868626, @phosek wrote:

@MaskRay it looks like we could use the following pattern:

extern intptr_t __llvm_profile_counter_bias;

#if defined(_WIN32)
intptr_t __llvm_profile_counter_bias_default = 0;
#pragma comment(linker, "/alternatename:" \
                SYMBOL_NAME("__llvm_profile_counter_bias") "=" \
                SYMBOL_NAME("__llvm_profile_counter_bias_default"))
#endif

bool usesRuntimeCounterRelocation() {
  void *s1 = &__llvm_profile_counter_bias;
  void *s2 = &__llvm_profile_counter_bias_default;
  return s1 != s2;
}

This does seem to work with clang-cl but when I tried this with cl I still get an error:

error LNK2019: unresolved external symbol __llvm_profile_counter_bias referenced in function usesRuntimeCounterRelocation

Never mind, I have figured it out. This should now handle COFF as well.

[thakis]

compiler-rt/lib/profile/InstrProfilingFile.c

456

../../compiler-rt/lib/profile/InstrProfilingFile.c:429:12: warning: unused function 'writeMMappedFile' [-Wunused-function]
static int writeMMappedFile(FILE *OutputFile, char **Profile) {
           ^

I guess this should now just move into the elif?

compiler-rt/include/profile/InstrProfData.inc

  */
 #define VARIANT_MASKS_ALL 0xff00000000000000ULL
 #define GET_VERSION(V) ((V) & ~VARIANT_MASKS_ALL)
 #define VARIANT_MASK_IR_PROF (0x1ULL << 56)
 #define VARIANT_MASK_CSIR_PROF (0x1ULL << 57)
 #define VARIANT_MASK_INSTR_ENTRY (0x1ULL << 58)
 #define INSTR_PROF_RAW_VERSION_VAR __llvm_profile_raw_version
 #define INSTR_PROF_PROFILE_RUNTIME_VAR __llvm_profile_runtime
+#define INSTR_PROF_PROFILE_COUNTER_BIAS_VAR __llvm_profile_counter_bias
 
 /* The variable that holds the name of the profile data
  * specified via command line. */
 #define INSTR_PROF_PROFILE_NAME_VAR __llvm_profile_filename
 
 /* section name strings common to all targets other
    than WIN32 */
 #define INSTR_PROF_DATA_COMMON __llvm_prf_data

compiler-rt/lib/profile/CMakeLists.txt

 
 add_compiler_rt_component(profile)
 
 set(PROFILE_SOURCES
   GCDAProfiling.c
   InstrProfiling.c
   InstrProfilingInternal.c
   InstrProfilingValue.c
-  InstrProfilingBiasVar.c
   InstrProfilingBuffer.c
   InstrProfilingFile.c
   InstrProfilingMerge.c
   InstrProfilingMergeFile.c
   InstrProfilingNameVar.c
   InstrProfilingVersionVar.c
   InstrProfilingWriter.c
   InstrProfilingPlatformDarwin.c

compiler-rt/lib/profile/InstrProfiling.h

 
 /*!
  * This variable is a weak symbol defined in InstrProfiling.c. It allows
  * compiler instrumentation to provide overriding definition with value
  * from compiler command line. This variable has default visibility.
  */
 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.
- */
-COMPILER_RT_VISIBILITY extern intptr_t __llvm_profile_counter_bias;
-
 #endif /* PROFILE_INSTRPROFILING_H_ */

compiler-rt/lib/profile/InstrProfilingBiasVar.c

-/*===- 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/InstrProfilingBuffer.c

 /// for (\p Offset + Padding) to be page-aligned.
 static uint64_t calculateBytesNeededToPageAlign(uint64_t Offset) {
   uint64_t OffsetModPage = Offset % PageSize;
   if (OffsetModPage > 0)
     return PageSize - OffsetModPage;
   return 0;
 }
 
+static int needsCounterPadding(void) {
+#if defined(__APPLE__)
+  return __llvm_profile_is_continuous_mode_enabled();
+#else
+  return 0;
+#endif
+}
+
 COMPILER_RT_VISIBILITY
 void __llvm_profile_get_padding_sizes_for_counters(
     uint64_t DataSize, uint64_t CountersSize, uint64_t NamesSize,
     uint64_t *PaddingBytesBeforeCounters, uint64_t *PaddingBytesAfterCounters,
     uint64_t *PaddingBytesAfterNames) {
-  if (!__llvm_profile_is_continuous_mode_enabled() ||
-      lprofRuntimeCounterRelocation()) {
+  if (!needsCounterPadding()) {
     *PaddingBytesBeforeCounters = 0;
     *PaddingBytesAfterCounters = 0;
     *PaddingBytesAfterNames = __llvm_profile_get_num_padding_bytes(NamesSize);
     return;
   }
 
   // In continuous mode, the file offsets for headers and for the start of
   // counter sections need to be page-aligned.

compiler-rt/lib/profile/InstrProfilingFile.c

#endif
 
   /* Truncate the file.  Later we'll reopen and append. */
   File = fopen(Filename, "w");
   if (!File)
     return;
   fclose(File);
 }
 
-#if !defined(__Fuchsia__) && !defined(_WIN32)
-static void assertIsZero(int *i) {
-  if (*i)
-    PROF_WARN("Expected flag to be 0, but got: %d\n", *i);
-}
-
-/* Write a partial profile to \p Filename, which is required to be backed by
- * the open file object \p File. */
-static int writeProfileWithFileObject(const char *Filename, FILE *File) {
-  setProfileFile(File);
-  int rc = writeFile(Filename);
-  if (rc)
-    PROF_ERR("Failed to write file \"%s\": %s\n", Filename, strerror(errno));
-  setProfileFile(NULL);
-  return rc;
-}
-
-/* Unlock the profile \p File and clear the unlock flag. */
-static void unlockProfile(int *ProfileRequiresUnlock, FILE *File) {
-  if (!*ProfileRequiresUnlock) {
-    PROF_WARN("%s", "Expected to require profile unlock\n");
-  }
-  lprofUnlockFileHandle(File);
-  *ProfileRequiresUnlock = 0;
-}
-#endif // !defined(__Fuchsia__) && !defined(_WIN32)
-
 static int writeMMappedFile(FILE *OutputFile, char **Profile) {
   if (!OutputFile)
     return -1;
 
   /* Write the data into a file. */
   setupIOBuffer();
   ProfDataWriter fileWriter;
   initFileWriter(&fileWriter, OutputFile);
   if (lprofWriteData(&fileWriter, NULL, 0)) {
static int writeMMappedFile(FILE *OutputFile, char **Profile) {
   if (*Profile == MAP_FAILED) {
     PROF_ERR("Unable to mmap profile: %s\n", strerror(errno));
     return -1;
   }
 
   return 0;
 }
 
-static void relocateCounters(void) {
-  if (!__llvm_profile_is_continuous_mode_enabled() ||
-      !lprofRuntimeCounterRelocation())
-    return;
-
-  /* Get the sizes of various profile data sections. Taken from
-   * __llvm_profile_get_size_for_buffer(). */
-  const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
-  const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
-  const uint64_t *CountersBegin = __llvm_profile_begin_counters();
-  const uint64_t *CountersEnd = __llvm_profile_end_counters();
-  uint64_t DataSize = __llvm_profile_get_data_size(DataBegin, DataEnd);
-  const uint64_t CountersOffset = sizeof(__llvm_profile_header) +
-      (DataSize * sizeof(__llvm_profile_data));
-
-  int Length = getCurFilenameLength();
-  char *FilenameBuf = (char *)COMPILER_RT_ALLOCA(Length + 1);
-  const char *Filename = getCurFilename(FilenameBuf, 0);
-  if (!Filename)
-    return;
-
-  FILE *File = NULL;
-  char *Profile = NULL;
-
-  if (!doMerging()) {
-    File = fopen(Filename, "w+b");
-    if (!File)
-      return;
-
-    if (writeMMappedFile(File, &Profile) == -1) {
-      fclose(File);
-      return;
-    }
-  } else {
-    File = lprofOpenFileEx(Filename);
-    if (!File)
-      return;
+// TODO: Move these functions into InstrProfilingPlatform* files.
+#if defined(__APPLE__)
+static void assertIsZero(int *i) {
+  if (*i)
+    PROF_WARN("Expected flag to be 0, but got: %d\n", *i);
 
-    uint64_t ProfileFileSize = 0;
-    if (getProfileFileSizeForMerging(File, &ProfileFileSize) == -1) {
-      lprofUnlockFileHandle(File);
-      fclose(File);
-      return;
-    }
+/* Write a partial profile to \p Filename, which is required to be backed by
+ * the open file object \p File. */
+static int writeProfileWithFileObject(const char *Filename, FILE *File) {
+  setProfileFile(File);
+  int rc = writeFile(Filename);
+  if (rc)
+    PROF_ERR("Failed to write file \"%s\": %s\n", Filename, strerror(errno));
+  setProfileFile(NULL);
+  return rc;
 
-    if (!ProfileFileSize) {
-      if (writeMMappedFile(File, &Profile) == -1) {
-        fclose(File);
-        return;
-      }
-    } else {
-      /* The merged profile has a non-zero length. Check that it is compatible
-       * with the data in this process. */
-      if (mmapProfileForMerging(File, ProfileFileSize, &Profile) == -1) {
-        fclose(File);
-        return;
-      }
-    }
+/* Unlock the profile \p File and clear the unlock flag. */
+static void unlockProfile(int *ProfileRequiresUnlock, FILE *File) {
+  if (!*ProfileRequiresUnlock) {
+    PROF_WARN("%s", "Expected to require profile unlock\n");
 
-    lprofUnlockFileHandle(File);
-  }
-
-  /* Update the profile fields based on the current mapping. */
-  __llvm_profile_counter_bias =
-      (intptr_t)Profile - (uintptr_t)CountersBegin + CountersOffset;
-
-  /* Return the memory allocated for counters to OS. */
-  lprofReleaseMemoryPagesToOS((uintptr_t)CountersBegin, (uintptr_t)CountersEnd);
+  *ProfileRequiresUnlock = 0;
 }
 
 static void initializeProfileForContinuousMode(void) {
   if (!__llvm_profile_is_continuous_mode_enabled())
     return;
 
-#if defined(__Fuchsia__) || defined(_WIN32)
-  PROF_ERR("%s\n", "Continuous mode not yet supported on Fuchsia or Windows.");
-#else // defined(__Fuchsia__) || defined(_WIN32)
   /* Get the sizes of various profile data sections. Taken from
    * __llvm_profile_get_size_for_buffer(). */
   const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
   const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
   const uint64_t *CountersBegin = __llvm_profile_begin_counters();
   const uint64_t *CountersEnd = __llvm_profile_end_counters();
   const char *NamesBegin = __llvm_profile_begin_names();
   const char *NamesEnd = __llvm_profile_end_names();
if (CounterMmap != CountersBegin) {
           "  - FileOffsetToCounters: %" PRIu64 "\n",
           strerror(errno), CountersBegin, PageAlignedCountersLength, Fileno,
           FileOffsetToCounters);
     }
   }
 
   if (ProfileRequiresUnlock)
     unlockProfile(&ProfileRequiresUnlock, File);
-#endif // defined(__Fuchsia__) || defined(_WIN32)
 }
+#elif defined(__ELF__) || defined(_WIN32)
+
+#define INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR                            \
+  INSTR_PROF_CONCAT(INSTR_PROF_PROFILE_COUNTER_BIAS_VAR, _default)
+intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR = 0;
+
+/* This variable is a weak external reference which could be used to detect
+ * whether or not the compiler defined this symbol. */
+#if defined(_WIN32)
+COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
+#pragma comment(linker, "/alternatename:"                                      \
+      INSTR_PROF_QUOTE(INSTR_PROF_PROFILE_COUNTER_BIAS_VAR) "="                \
+      INSTR_PROF_QUOTE(INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR))
+#else
+COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR
+    __attribute__((weak, alias(INSTR_PROF_QUOTE(
+                             INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR))));
+#endif
+
+static void initializeProfileForContinuousMode(void) {
+  if (!__llvm_profile_is_continuous_mode_enabled())
+    return;
+
+  /* This symbol is defined by the compiler when runtime counter relocation is
+   * used and runtime provides a weak alias so we can check if it's defined. */
+  void *BiasAddr = &INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
+  void *BiasDefaultAddr = &INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR;
+  if (BiasAddr == BiasDefaultAddr) {
+    PROF_ERR("%s\n", "__llvm_profile_counter_bias is undefined");
+    return;
+  }
+
+  /* Get the sizes of various profile data sections. Taken from
+   * __llvm_profile_get_size_for_buffer(). */
+  const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
+  const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
+  const uint64_t *CountersBegin = __llvm_profile_begin_counters();
+  const uint64_t *CountersEnd = __llvm_profile_end_counters();
+  uint64_t DataSize = __llvm_profile_get_data_size(DataBegin, DataEnd);
+  const uint64_t CountersOffset =
+      sizeof(__llvm_profile_header) + (DataSize * sizeof(__llvm_profile_data));
+
+  int Length = getCurFilenameLength();
+  char *FilenameBuf = (char *)COMPILER_RT_ALLOCA(Length + 1);
+  const char *Filename = getCurFilename(FilenameBuf, 0);
+  if (!Filename)
+    return;
+
+  FILE *File = NULL;
+  char *Profile = NULL;
+
+  if (!doMerging()) {
+    File = fopen(Filename, "w+b");
+    if (!File)
+      return;
+
+    if (writeMMappedFile(File, &Profile) == -1) {
+      fclose(File);
+      return;
+    }
+  } else {
+    File = lprofOpenFileEx(Filename);
+    if (!File)
+      return;
+
+    uint64_t ProfileFileSize = 0;
+    if (getProfileFileSizeForMerging(File, &ProfileFileSize) == -1) {
+      lprofUnlockFileHandle(File);
+      fclose(File);
+      return;
+    }
+
+    if (!ProfileFileSize) {
+      if (writeMMappedFile(File, &Profile) == -1) {
+        fclose(File);
+        return;
+      }
+    } else {
+      /* The merged profile has a non-zero length. Check that it is compatible
+       * with the data in this process. */
+      if (mmapProfileForMerging(File, ProfileFileSize, &Profile) == -1) {
+        fclose(File);
+        return;
+      }
+    }
+
+    lprofUnlockFileHandle(File);
+  }
+
+  /* Update the profile fields based on the current mapping. */
+  INSTR_PROF_PROFILE_COUNTER_BIAS_VAR =
+      (intptr_t)Profile - (uintptr_t)CountersBegin +
+      CountersOffset;
+
+  /* Return the memory allocated for counters to OS. */
+  lprofReleaseMemoryPagesToOS((uintptr_t)CountersBegin, (uintptr_t)CountersEnd);
+}
+#else
+static void initializeProfileForContinuousMode(void) {
+  PROF_ERR("%s\n", "continuous mode is unsupported on this platform");
+}
+#endif
 
 static const char *DefaultProfileName = "default.profraw";
 static void resetFilenameToDefault(void) {
   if (lprofCurFilename.FilenamePat && lprofCurFilename.OwnsFilenamePat) {
     free((void *)lprofCurFilename.FilenamePat);
   }
   memset(&lprofCurFilename, 0, sizeof(lprofCurFilename));
   lprofCurFilename.FilenamePat = DefaultProfileName;
if (FilenamePat[I] == '%') {
           return -1;
         }
       } else if (FilenamePat[I] == 'c') {
         if (__llvm_profile_is_continuous_mode_enabled()) {
           PROF_WARN("%%c specifier can only be specified once in %s.\n",
                     FilenamePat);
           return -1;
         }
-
+#if defined(__APPLE__) || defined(__ELF__) || defined(_WIN32)
         __llvm_profile_set_page_size(getpagesize());
         __llvm_profile_enable_continuous_mode();
+#else
+        PROF_WARN("%s", "Continous mode is currently only supported for Mach-O,"
+                        " ELF and COFF formats.");
+        return -1;
+#endif
       } else {
         unsigned MergePoolSize = getMergePoolSize(FilenamePat, &I);
         if (!MergePoolSize)
           continue;
         if (MergingEnabled) {
           PROF_WARN("%%m specifier can only be specified once in %s.\n",
                     FilenamePat);
           return -1;
static void parseAndSetFilename(const char *FilenamePat,
   } else {
     if (getenv("LLVM_PROFILE_VERBOSE"))
       PROF_NOTE("Override old profile path \"%s\" via %s to \"%s\" via %s.\n",
                 OldFilenamePat, getPNSStr(OldPNS), lprofCurFilename.FilenamePat,
                 getPNSStr(PNS));
   }
 
   truncateCurrentFile();
-  if (__llvm_profile_is_continuous_mode_enabled()) {
-    if (lprofRuntimeCounterRelocation())
-      relocateCounters();
-    else
-      initializeProfileForContinuousMode();
-  }
-}
+  if (__llvm_profile_is_continuous_mode_enabled())
 
 /* Return buffer length that is required to store the current profile
  * filename with PID and hostname substitutions. */
 /* The length to hold uint64_t followed by 3 digits pool id including '_' */
 #define SIGLEN 24
 static int getCurFilenameLength() {
   int Len;
   if (!lprofCurFilename.FilenamePat || !lprofCurFilename.FilenamePat[0])

  * environment variable can override this default value.
  */
 COMPILER_RT_VISIBILITY
 void __llvm_profile_initialize_file(void) {
   const char *EnvFilenamePat;
   const char *SelectedPat = NULL;
   ProfileNameSpecifier PNS = PNS_unknown;
   int hasCommandLineOverrider = (INSTR_PROF_PROFILE_NAME_VAR[0] != 0);
 
-  if (__llvm_profile_counter_bias != -1)
-    lprofSetRuntimeCounterRelocation(1);
-
   EnvFilenamePat = getFilenamePatFromEnv();
   if (EnvFilenamePat) {
     /* Pass CopyFilenamePat = 1, to ensure that the filename would be valid
        at the  moment when __llvm_profile_write_file() gets executed. */
     parseAndSetFilename(EnvFilenamePat, PNS_environment, 1);
     return;
   } else if (hasCommandLineOverrider) {
     SelectedPat = INSTR_PROF_PROFILE_NAME_VAR;
     PNS = PNS_command_line;

compiler-rt/lib/profile/InstrProfilingInternal.h

 
 /* 
  * Return non zero value if the profile data has already been
  * dumped to the file.
  */
 unsigned lprofProfileDumped(void);
 void lprofSetProfileDumped(unsigned);
 
-/* Return non zero value if counters are being relocated at runtime. */
-unsigned lprofRuntimeCounterRelocation(void);
-void lprofSetRuntimeCounterRelocation(unsigned);
-
 COMPILER_RT_VISIBILITY extern void (*FreeHook)(void *);
 COMPILER_RT_VISIBILITY extern uint8_t *DynamicBufferIOBuffer;
 COMPILER_RT_VISIBILITY extern uint32_t VPBufferSize;
 COMPILER_RT_VISIBILITY extern uint32_t VPMaxNumValsPerSite;
 /* Pointer to the start of static value counters to be allocted. */
 COMPILER_RT_VISIBILITY extern ValueProfNode *CurrentVNode;
 COMPILER_RT_VISIBILITY extern ValueProfNode *EndVNode;
 extern void (*VPMergeHook)(struct ValueProfData *, __llvm_profile_data *);
 
 #endif

compiler-rt/lib/profile/InstrProfilingInternal.c

 COMPILER_RT_VISIBILITY unsigned lprofProfileDumped() {
   return ProfileDumped;
 }
 
 COMPILER_RT_VISIBILITY void lprofSetProfileDumped(unsigned Value) {
   ProfileDumped = Value;
 }
 
-static unsigned RuntimeCounterRelocation = 0;
-
-COMPILER_RT_VISIBILITY unsigned lprofRuntimeCounterRelocation(void) {
-  return RuntimeCounterRelocation;
-}
-
-COMPILER_RT_VISIBILITY void lprofSetRuntimeCounterRelocation(unsigned Value) {
-  RuntimeCounterRelocation = Value;
-}
-
 #endif

compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c

 #include <zircon/sanitizer.h>
 #include <zircon/status.h>
 #include <zircon/syscalls.h>
 
 #include "InstrProfiling.h"
 #include "InstrProfilingInternal.h"
 #include "InstrProfilingUtil.h"
 
+/* This variable is an external reference to symbol defined by the compiler. */
+COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
+
 COMPILER_RT_VISIBILITY unsigned lprofProfileDumped() {
   return 1;
 }
 COMPILER_RT_VISIBILITY void lprofSetProfileDumped(unsigned Value) {}
 
-COMPILER_RT_VISIBILITY unsigned lprofRuntimeCounterRelocation(void) {
-  return 1;
-}
-COMPILER_RT_VISIBILITY void lprofSetRuntimeCounterRelocation(unsigned Value) {}
-
 static const char ProfileSinkName[] = "llvm-profile";
 
 static inline void lprofWrite(const char *fmt, ...) {
   char s[256];
 
   va_list ap;
   va_start(ap, fmt);
   int ret = vsnprintf(s, sizeof(s), fmt, ap);
void __llvm_profile_initialize(void) {
   if (GET_VERSION(__llvm_profile_get_version()) != INSTR_PROF_RAW_VERSION) {
     lprofWrite("LLVM Profile: runtime and instrumentation version mismatch: "
                "expected %d, but got %d\n",
                INSTR_PROF_RAW_VERSION,
                (int)GET_VERSION(__llvm_profile_get_version()));
     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) {
-    lprofWrite("LLVM Profile: counter relocation at runtime is required\n");
-    return;
-  }
-
   const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
   const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
   const uint64_t *CountersBegin = __llvm_profile_begin_counters();
   const uint64_t *CountersEnd = __llvm_profile_end_counters();
   const uint64_t DataSize = __llvm_profile_get_data_size(DataBegin, DataEnd);
   const uint64_t CountersOffset =
       sizeof(__llvm_profile_header) + (DataSize * sizeof(__llvm_profile_data));
   uint64_t CountersSize = CountersEnd - CountersBegin;
void __llvm_profile_initialize(void) {
   /* Publish the VMO which contains profile data to the system. Note that this
    * also consumes the VMO handle. */
   __sanitizer_publish_data(ProfileSinkName, Vmo);
 
   /* Use the dumpfile symbolizer markup element to write the name of VMO. */
   lprofWrite("LLVM Profile: {{{dumpfile:%s:%s}}}\n", ProfileSinkName, VmoName);
 
   /* Update the profile fields based on the current mapping. */
-  __llvm_profile_counter_bias =
+  INSTR_PROF_PROFILE_COUNTER_BIAS_VAR =
       (intptr_t)Mapping - (uintptr_t)CountersBegin + CountersOffset;
 
   /* Return the memory allocated for counters to OS. */
   lprofReleaseMemoryPagesToOS((uintptr_t)CountersBegin, (uintptr_t)CountersEnd);
 }
 
 #endif

compiler-rt/lib/profile/InstrProfilingUtil.h

 /* Temporarily suspend SIGKILL. Return value of 1 means a restore is needed.
  * Other return values mean no restore is needed.
  */
 int lprofSuspendSigKill();
 
 /* Restore previously suspended SIGKILL. */
 void lprofRestoreSigKill();
 
-inline size_t lprofRoundUpTo(size_t x, size_t boundary) {
+static inline size_t lprofRoundUpTo(size_t x, size_t boundary) {
   return (x + boundary - 1) & ~(boundary - 1);
 }
 
-inline size_t lprofRoundDownTo(size_t x, size_t boundary) {
+static inline size_t lprofRoundDownTo(size_t x, size_t boundary) {
   return x & ~(boundary - 1);
 }
 
 int lprofReleaseMemoryPagesToOS(uintptr_t Begin, uintptr_t End);
 
 #endif /* PROFILE_INSTRPROFILINGUTIL_H */

llvm/include/llvm/ProfileData/InstrProf.h

 
 /// Return the name of the compiler generated function that references the
 /// runtime hook variable. The function is a weak global.
 inline StringRef getInstrProfRuntimeHookVarUseFuncName() {
   return "__llvm_profile_runtime_user";
 }
 
 inline StringRef getInstrProfCounterBiasVarName() {
-  return "__llvm_profile_counter_bias";
+  return INSTR_PROF_QUOTE(INSTR_PROF_PROFILE_COUNTER_BIAS_VAR);
 }
 
 /// Return the marker used to separate PGO names during serialization.
 inline StringRef getInstrProfNameSeparator() { return "\01"; }
 
 /// Return the modified name for function \c F suitable to be
 /// used the key for profile lookup. Variable \c InLTO indicates if this
 /// is called in LTO optimization passes.

llvm/include/llvm/ProfileData/InstrProfData.inc

  */
 #define VARIANT_MASKS_ALL 0xff00000000000000ULL
 #define GET_VERSION(V) ((V) & ~VARIANT_MASKS_ALL)
 #define VARIANT_MASK_IR_PROF (0x1ULL << 56)
 #define VARIANT_MASK_CSIR_PROF (0x1ULL << 57)
 #define VARIANT_MASK_INSTR_ENTRY (0x1ULL << 58)
 #define INSTR_PROF_RAW_VERSION_VAR __llvm_profile_raw_version
 #define INSTR_PROF_PROFILE_RUNTIME_VAR __llvm_profile_runtime
+#define INSTR_PROF_PROFILE_COUNTER_BIAS_VAR __llvm_profile_counter_bias
 
 /* The variable that holds the name of the profile data
  * specified via command line. */
 #define INSTR_PROF_PROFILE_NAME_VAR __llvm_profile_filename
 
 /* section name strings common to all targets other
    than WIN32 */
 #define INSTR_PROF_DATA_COMMON __llvm_prf_data

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

bool InstrProfiling::lowerIntrinsics(Function *F) {
   if (!MadeChange)
     return false;
 
   promoteCounterLoadStores(F);
   return true;
 }
 
 bool InstrProfiling::isRuntimeCounterRelocationEnabled() const {
+  // Mach-O don't support weak external references.
+  if (TT.isOSBinFormatMachO())
+    return false;
+
   if (RuntimeCounterRelocation.getNumOccurrences() > 0)
     return RuntimeCounterRelocation;
 
+  // Fuchsia uses runtime counter relocation by default.
   return TT.isOSFuchsia();
 }
 
 bool InstrProfiling::isCounterPromotionEnabled() const {
   if (DoCounterPromotion.getNumOccurrences() > 0)
     return DoCounterPromotion;
 
   return Options.DoCounterPromotion;
if (isRuntimeCounterRelocationEnabled()) {
     Function *Fn = Inc->getParent()->getParent();
     Instruction &I = Fn->getEntryBlock().front();
     LoadInst *LI = dyn_cast<LoadInst>(&I);
     if (!LI) {
       IRBuilder<> Builder(&I);
       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);
     }
     auto *Add = Builder.CreateAdd(Builder.CreatePtrToInt(Addr, Int64Ty), LI);
     Addr = Builder.CreateIntToPtr(Add, Int64PtrTy);
   }
 
   if (Options.Atomic || AtomicCounterUpdateAll ||

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

 ; RUN: opt < %s -S -instrprof | FileCheck %s
 ; RUN: opt < %s -S -instrprof -runtime-counter-relocation | FileCheck -check-prefixes=RELOC %s
 
 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)
 ; CHECK-NEXT: %1 = add i64 %pgocount, 1
 ; CHECK-NEXT: store i64 %1, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc_foo, i64 0, i64 0)
 ; RELOC-LABEL: define void @foo
 ; RELOC-NEXT: %1 = load i64, i64* @__llvm_profile_counter_bias
 ; RELOC-NEXT: %2 = add i64 ptrtoint ([1 x i64]* @__profc_foo to i64), %1