Add zero size dummy data to ensure section symbol is always created

[davidxl]

Summary

There is a scenario where a user does not specify -fprofile-instr-generate on compilation command but on the linker command only for various reasons. In such cases, since no object is instrumented, the llvm_prof_xxx sections won't exist and linker won't generate __start__.. and _stop__.. symbols leading to linker time unsat.

Making start_../stop_.. weak extern symbols solve the problem when building executable, but PC relative dynamic reloc is not allowed so shared library case will still fail.

THis simple patch fixes the problem. It won't have any effect on regular use cases.

It has another added benefit -- it allows us to conditionally turn off generation of name sections completely (depending on user option) to reduce binary/profile data size -- that feature will come later.

[davidxl]

Update patch:
do not dump a header when there is no data to be dumped.

[vsk]

I think the same idea works for InstrProfilingPlatformDarwin.c. Do you mind adding it there?

This could also use a test. Can we compile an input from test/profile without -fprofile-instr-generate, pass it to the linker, and check for the sections we expect? If that requires too many platform-specific tools, is there some other way we can test this?

[davidxl]

The change does not apply to Darwin as on Darwin, we still pay the overhead of emitting the runtime hook user function and does not use -u,symbol trick -- so when no module is instrumented, the runtime library won't linked in.

An extensive test case is added for shared library PGO -- it covers all possible scenarios -- it is better to have a black box testing without need to check the implementation detail.

Also suppressed the pedantic warning for zero size array.

[vsk]

Nice!

test/profile/instrprof-shared.test

5

Could you rename instrprof-shared-lib.o to instrprof-shared-lib-noinstr.o?

8

Could you add a comment explaining your approach? Something like:

'''
This test produces three shared objects.

• libt-instr.so is instrumented
• libt-no-instr1.so is not instrumented
• libt-no-instr2.so is compiled with instrumentation enabled, but is formed out of an uninstrumented object

Verify that compiling against these shared objects with and without instrumentation enabled behaves as expected.
'''

10

%t-instr-no-instr -> %t-instr-no-instr1?

13

%t-no-instr-no-instr -> %t-no-instr-no-instr1?

31

Can you add negative tests (not RUN?) showing that the uninstrumented binaries produce no profraw files?

[davidxl]

test/profile/instrprof-shared.test

31

I have added two. There are two cases no-instr*-no-instr2 where linux version produces zero size prof data vs non-existing file (making changes to unlink file can be tricky) -- so adding a test here can be tricky. As this is not the expected using case anyway, existing testings already cover the expected behavior (build, run etc), I will leave this as it is .

[davidxl]

Updating test case according to vsk's comments.

[vsk]

Thanks, lgtm

test/profile/instrprof-shared.test

6

nit: extra 'but is' here

lib/profile/CMakeLists.txt

 add_custom_target(profile)
 
 set(PROFILE_SOURCES
   GCDAProfiling.c
   InstrProfiling.c
   InstrProfilingBuffer.c
   InstrProfilingFile.c
   InstrProfilingPlatformDarwin.c
   InstrProfilingPlatformLinux.c
   InstrProfilingPlatformOther.c
   InstrProfilingRuntime.cc
   InstrProfilingUtil.c)
 
+if(UNIX)
+ set(EXTRA_FLAGS
+     -fPIC
+     -Wno-pedantic)
+else()
+ set(EXTRA_FLAGS
+     -fPIC)
+endif()
+
 if(APPLE)
   add_compiler_rt_runtime(clang_rt.profile
     STATIC
     OS ${PROFILE_SUPPORTED_OS}
     ARCHS ${PROFILE_SUPPORTED_ARCH}
     SOURCES ${PROFILE_SOURCES}
     PARENT_TARGET profile)
 else()
   add_compiler_rt_runtime(clang_rt.profile
     STATIC
     ARCHS ${PROFILE_SUPPORTED_ARCH}
-    CFLAGS -fPIC
+    CFLAGS ${EXTRA_FLAGS}
     SOURCES ${PROFILE_SOURCES}
     PARENT_TARGET profile)
 endif()
 
 add_dependencies(compiler-rt profile)

lib/profile/InstrProfilingBuffer.c

int __llvm_profile_write_buffer_internal(
   const uint64_t NamesSize = NamesEnd - NamesBegin;
   const uint64_t Padding = sizeof(uint64_t) - NamesSize % sizeof(uint64_t);
 
   /* Enough zeroes for padding. */
   const char Zeroes[sizeof(uint64_t)] = {0};
 
   /* Create the header. */
   __llvm_profile_header Header;
+
+  if (!DataSize)
+    return 0;
+
   Header.Magic = __llvm_profile_get_magic();
   Header.Version = __llvm_profile_get_version();
   Header.DataSize = DataSize;
   Header.CountersSize = CountersSize;
   Header.NamesSize = NamesSize;
   Header.CountersDelta = (uintptr_t)CountersBegin;
   Header.NamesDelta = (uintptr_t)NamesBegin;
 

lib/profile/InstrProfilingFile.c

static int writeFile(FILE *File) {
   const uint64_t NamesSize = NamesEnd - NamesBegin;
   const uint64_t Padding = sizeof(uint64_t) - NamesSize % sizeof(uint64_t);
 
   /* Enough zeroes for padding. */
   const char Zeroes[sizeof(uint64_t)] = {0};
 
   /* Create the header. */
   __llvm_profile_header Header;
+
+  if (!DataSize)
+    return 0;
+
   Header.Magic = __llvm_profile_get_magic();
   Header.Version = __llvm_profile_get_version();
   Header.DataSize = DataSize;
   Header.CountersSize = CountersSize;
   Header.NamesSize = NamesSize;
   Header.CountersDelta = (uintptr_t)CountersBegin;
   Header.NamesDelta = (uintptr_t)NamesBegin;
 

lib/profile/InstrProfilingPlatformLinux.c

extern __llvm_profile_data __start___llvm_prf_data
     __attribute__((visibility("hidden")));
 extern __llvm_profile_data __stop___llvm_prf_data
     __attribute__((visibility("hidden")));
 extern uint64_t __start___llvm_prf_cnts __attribute__((visibility("hidden")));
 extern uint64_t __stop___llvm_prf_cnts __attribute__((visibility("hidden")));
 extern char __start___llvm_prf_names __attribute__((visibility("hidden")));
 extern char __stop___llvm_prf_names __attribute__((visibility("hidden")));
 
+/* Add dummy data to ensure the section is always created. */
+__llvm_profile_data __llvm_prof_sect_data[0]
+    __attribute__((section("__llvm_prf_data")));
+uint64_t __llvm_prof_cnts_sect_data[0]
+    __attribute__((section("__llvm_prf_cnts")));
+char __llvm_prof_nms_sect_data[0] __attribute__((section("__llvm_prf_names")));
+
 __attribute__((visibility("hidden"))) const __llvm_profile_data *
 __llvm_profile_begin_data(void) {
   return &__start___llvm_prf_data;
 }
 __attribute__((visibility("hidden"))) const __llvm_profile_data *
 __llvm_profile_end_data(void) {
   return &__stop___llvm_prf_data;
 }

test/profile/Inputs/instrprof-shared-lib.c

+int g1 = 0;
+int g2 = 1;
+
+void foo(int n) {
+  if (n % 5 == 0)
+    g1++;
+  else
+    g2++;
+}

test/profile/Inputs/instrprof-shared-main.c

+extern int g1, g2;
+extern void foo(int n);
+
+int main() {
+  int i, j;
+  for (i = 0; i < 1000; i++)
+    for (j = 0; j < 1000; j++)
+      foo(i * j);
+
+  if (g2 - g1 == 280001)
+    return 0;
+  return 1;
+}

test/profile/instrprof-shared.test

+"""
+This test produces three shared libraries:
+
+1. libt-instr.so is instrumented
+2. libt-no-instr1.so is not instrumented
+3. libt-no-instr2.so is compiled with instrumentation enabled, but is but the object file is built
+   with instrumentation turned off.
+
+After the libraries are built, the main program is then built with/without instrumentation and linked
+against 3 libraries above.
+
+The test is to verify that compiling against these shared objects with and without instrumentation
+enabled behaves as expected.
+"""
+
+RUN: mkdir -p %t.d
+RUN: %clang_profgen -o %t.d/libt-instr.so -fPIC -shared %S/Inputs/instrprof-shared-lib.c
+RUN: %clang -o %t.d/libt-no-instr1.so -fPIC -shared %S/Inputs/instrprof-shared-lib.c
+RUN: %clang -c -o %t.d/instrprof-shared-lib-no-instr2.o -fPIC  %S/Inputs/instrprof-shared-lib.c
+RUN: %clang_profgen -o %t.d/libt-no-instr2.so -fPIC -shared %t.d/instrprof-shared-lib-no-instr2.o
+
+RUN: %clang_profgen -o %t-instr-instr -L%t.d -rpath %t.d -lt-instr  %S/Inputs/instrprof-shared-main.c
+RUN: %clang_profgen -o %t-instr-no-instr1 -L%t.d -rpath %t.d -lt-no-instr1  %S/Inputs/instrprof-shared-main.c
+RUN: %clang_profgen -o %t-instr-no-instr2 -L%t.d -rpath %t.d -lt-no-instr2  %S/Inputs/instrprof-shared-main.c
+RUN: %clang -o %t-no-instr1-instr -L%t.d -rpath %t.d -lt-instr  %S/Inputs/instrprof-shared-main.c
+RUN: %clang -o %t-no-instr1-no-instr1 -L%t.d -rpath %t.d -lt-no-instr1  %S/Inputs/instrprof-shared-main.c
+RUN: %clang -o %t-no-instr1-no-instr2 -L%t.d -rpath %t.d -lt-no-instr2  %S/Inputs/instrprof-shared-main.c
+RUN: %clang -c -o %t.d/instrprof-shared-main-no-instr2.o  %S/Inputs/instrprof-shared-main.c
+RUN: %clang -o %t-no-instr2-instr -L%t.d -rpath %t.d -lt-instr  %t.d/instrprof-shared-main-no-instr2.o
+RUN: %clang -o %t-no-instr2-no-instr1 -L%t.d -rpath %t.d -lt-no-instr1  %t.d/instrprof-shared-main-no-instr2.o
+RUN: %clang -o %t-no-instr2-no-instr2 -L%t.d -rpath %t.d -lt-no-instr2  %t.d/instrprof-shared-main-no-instr2.o
+
+RUN: env LLVM_PROFILE_FILE=%t-instr-instr.profraw %run %t-instr-instr
+RUN: env LLVM_PROFILE_FILE=%t-instr-no-instr1.profraw %run %t-instr-no-instr1
+RUN: env LLVM_PROFILE_FILE=%t-instr-no-instr2.profraw %run %t-instr-no-instr2
+RUN: env LLVM_PROFILE_FILE=%t-no-instr1-instr.profraw %run %t-no-instr1-instr
+RUN: env LLVM_PROFILE_FILE=%t-no-instr2-instr.profraw %run %t-no-instr2-instr
+RUN: env LLVM_PROFILE_FILE=%t-no-instr1-no-instr1.profraw %run %t-no-instr1-no-instr1
+RUN: env LLVM_PROFILE_FILE=%t-no-instr1-no-instr2.profraw %run %t-no-instr1-no-instr2
+RUN: env LLVM_PROFILE_FILE=%t-no-instr2-no-instr1.profraw %run %t-no-instr2-no-instr1
+RUN: env LLVM_PROFILE_FILE=%t-no-instr2-no-instr2.profraw %run %t-no-instr2-no-instr2
+
+RUN: llvm-profdata merge -o %t-instr-instr.profdata %t-instr-instr.profraw
+RUN: llvm-profdata merge -o %t-instr-no-instr1.profdata %t-instr-no-instr1.profraw
+RUN: llvm-profdata merge -o %t-instr-no-instr2.profdata %t-instr-no-instr2.profraw
+RUN: llvm-profdata merge -o %t-no-instr1-instr.profdata %t-no-instr1-instr.profraw
+RUN: llvm-profdata merge -o %t-no-instr2-instr.profdata %t-no-instr2-instr.profraw
+
+RUN: not llvm-profdata merge -o %t-no-instr1-no-instr1.profdata %t-no-instr1-no-instr1.profraw 2>&1 | FileCheck %s --check-prefix=MISSING-FILE
+RUN: not llvm-profdata merge -o %t-no-instr2-no-instr1.profdata %t-no-instr2-no-instr1.profraw 2>&1 | FileCheck %s --check-prefix=MISSING-FILE
+MISSING-FILE: profraw
+
+RUN: llvm-profdata show -counts --function main %t-instr-instr.profdata | grep -v 'Total\|Maximum' > %t-main-1
+RUN: llvm-profdata show -counts --function main %t-instr-no-instr1.profdata | grep -v 'Total\|Maximum' > %t-main-2
+RUN: llvm-profdata show -counts --function main %t-instr-no-instr2.profdata | grep -v 'Total\|Maximum' > %t-main-3
+RUN: llvm-profdata show -counts --function foo %t-instr-instr.profdata | grep -v 'Total\|Maximum' > %t-foo-1
+RUN: llvm-profdata show -counts --function foo %t-no-instr1-instr.profdata | grep -v 'Total\|Maximum' > %t-foo-2
+RUN: llvm-profdata show -counts --function foo %t-no-instr2-instr.profdata | grep -v 'Total\|Maximum'  > %t-foo-3
+
+RUN: %clang_profuse=%t-instr-instr.profdata -o %t-main-instr-instr.ll -S -emit-llvm %S/Inputs/instrprof-shared-main.c
+RUN: %clang_profuse=%t-instr-no-instr1.profdata -o %t-main-instr-no-instr1.ll -S -emit-llvm %S/Inputs/instrprof-shared-main.c
+RUN: %clang_profuse=%t-instr-no-instr2.profdata -o %t-main-instr-no-instr2.ll -S -emit-llvm %S/Inputs/instrprof-shared-main.c
+RUN: %clang_profuse=%t-instr-instr.profdata -o %t-lib-instr-instr.ll -S -emit-llvm %S/Inputs/instrprof-shared-lib.c
+RUN: %clang_profuse=%t-no-instr1-instr.profdata -o %t-lib-no-instr1-instr.ll -S -emit-llvm %S/Inputs/instrprof-shared-lib.c
+RUN: %clang_profuse=%t-no-instr2-instr.profdata -o %t-lib-no-instr2-instr.ll -S -emit-llvm %S/Inputs/instrprof-shared-lib.c
+RUN: %clang_profuse=%t-instr-instr.profdata -o %t-lib-instr-instr.ll -S -emit-llvm %S/Inputs/instrprof-shared-lib.c
+
+RUN: diff %t-main-instr-no-instr1.ll %t-main-instr-no-instr2.ll
+RUN: diff %t-lib-no-instr1-instr.ll %t-lib-no-instr2-instr.ll
+
+RUN: diff %t-main-1 %t-main-2
+RUN: diff %t-main-1 %t-main-3
+RUN: diff %t-foo-1 %t-foo-2
+RUN: diff %t-foo-1 %t-foo-3
+