Skip to content

Commit

Permalink
[profile] Move __llvm_profile_filename into a separate object
Browse files Browse the repository at this point in the history
Users can specify the path a raw profile is written to by passing
-fprofile-instr-generate=<path>, but this functionality broke on Darwin
after __llvm_profile_filename was made weak [1], resulting in profiles
being written to "default.profraw" even when <path> is specified.

The situation is that instrumented programs provide a weak definition of
__llvm_profile_filename, which conflicts with a weak redefinition
provided by the profiling runtime.

The linker appears to pick the 'winning' definition arbitrarily: on
Darwin, it usually prefers the larger definition, which is probably why
the instrprof-override-filename.c test has been passing.

The fix is to move the runtime's definition into a separate object file
within the archive. This means that the linker won't "see" the runtime's
definition unless the user program has not provided one. I couldn't
think of a great way to test this other than to mimic the Darwin
failure: use -fprofile-instr-generate=<some-small-path>.

Testing: check-{clang,profile}, modified instrprof-override-filename.c.

[1] [Profile] deprecate __llvm_profile_override_default_filename
https://reviews.llvm.org/D22613
https://reviews.llvm.org/D22614

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

llvm-svn: 306710
  • Loading branch information
vedantk committed Jun 29, 2017
1 parent 2434025 commit ff3227e
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 8 deletions.
1 change: 1 addition & 0 deletions compiler-rt/lib/profile/CMakeLists.txt
Expand Up @@ -48,6 +48,7 @@ set(PROFILE_SOURCES
InstrProfilingFile.c
InstrProfilingMerge.c
InstrProfilingMergeFile.c
InstrProfilingNameVar.c
InstrProfilingWriter.c
InstrProfilingPlatformDarwin.c
InstrProfilingPlatformLinux.c
Expand Down
2 changes: 0 additions & 2 deletions compiler-rt/lib/profile/InstrProfiling.c
Expand Up @@ -19,8 +19,6 @@

COMPILER_RT_WEAK uint64_t INSTR_PROF_RAW_VERSION_VAR = INSTR_PROF_RAW_VERSION;

COMPILER_RT_WEAK char INSTR_PROF_PROFILE_NAME_VAR[1] = {0};

COMPILER_RT_VISIBILITY uint64_t __llvm_profile_get_magic(void) {
return sizeof(void *) == sizeof(uint64_t) ? (INSTR_PROF_RAW_MAGIC_64)
: (INSTR_PROF_RAW_MAGIC_32);
Expand Down
18 changes: 18 additions & 0 deletions compiler-rt/lib/profile/InstrProfilingNameVar.c
@@ -0,0 +1,18 @@
//===- InstrProfilingNameVar.c - profile name variable setup --------------===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//

#include "InstrProfiling.h"

/* char __llvm_profile_filename[1]
*
* 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 char INSTR_PROF_PROFILE_NAME_VAR[1] = {0};
22 changes: 16 additions & 6 deletions compiler-rt/test/profile/instrprof-override-filename.c
@@ -1,14 +1,24 @@
// RUN: %clang_profgen=%t.profraw -o %t -O3 %s
// RUN: %run %t %t.profraw
// RUN: llvm-profdata merge -o %t.profdata %t.profraw
// RUN: %clang_profuse=%t.profdata -o - -S -emit-llvm %s | FileCheck %s
// RUN: rm -rf %t.dir && mkdir -p %t.dir
// RUN: cd %t.dir
//
// RUN: %clang_profgen=P_RAW -o %t -O3 %s
// RUN: %run %t P_RAW
// RUN: llvm-profdata merge -o %t.profdata P_RAW
// RUN: %clang_profuse=%t.profdata -o - -S -emit-llvm %s | FileCheck %s --check-prefix=FE
//
// RUN: %clang_pgogen=I_RAW -o %t.2 %s
// RUN: %run %t.2 I_RAW
// RUN: llvm-profdata merge -o %t2.profdata I_RAW
// RUN: %clang_profuse=%t2.profdata -o - -S -emit-llvm %s | FileCheck %s --check-prefix=IR

void bar() {}
int main(int argc, const char *argv[]) {
// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD1:[0-9]+]]
// FE: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD1:[0-9]+]]
// IR: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD1:[0-9]+]]
if (argc < 2)
return 1;
bar();
return 0;
}
// CHECK: ![[PD1]] = !{!"branch_weights", i32 1, i32 2}
// FE: ![[PD1]] = !{!"branch_weights", i32 1, i32 2}
// IR: ![[PD1]] = !{!"branch_weights", i32 0, i32 1}

0 comments on commit ff3227e

Please sign in to comment.