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

4 ↗

(On Diff #40079)

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

7 ↗

(On Diff #40079)

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.
'''

9 ↗

(On Diff #40079)

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

12 ↗

(On Diff #40079)

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

30 ↗

(On Diff #40079)

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

[davidxl]

test/profile/instrprof-shared.test

30 ↗

(On Diff #40079)

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 ↗

(On Diff #40167)

nit: extra 'but is' here

lib/profile/InstrProfilingPlatformLinux.c

 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;