Expose __gcov_flush for parity with libgcov in the gcc project

[ngie]

Summary

Exposing __gcov_flush increases parity with libgcov project, and exposes
the symbol for use with applications that need to flush gcov information,
counters, etc, when exiting via calls which don't invoke __gcov_flush
indirectly, like _exit(2).

Update the tests to use the new header, gcov.h, and fix the tests so
they compile cleanly on [more] platforms by including the appropriate
headers.

Bug: 32555

[ngie]

Ping?

[ngie]

I guess no one cares. Oh well, at least the gcc folks respond to reviews/bugs in a <6 month timeframe.

[davide]

@ngie, Vedant can probably take a look.
Sometimes code reviews get lost, and it's generally good to ping people here or on IRC.

[davide]

With that in mind:

1. You may want to consider adding context to your patch, i.e. via git diff -U500000 &> blah.patch
2. You may want not jump to conclusions so quickly. I'm pretty sure people care about your contributions, it's just that nobody got around to review your patch.

[vsk]

Sorry this slipped through the cracks. It's OK to ping patches weekly to make sure they get attention.

This patch generally looks good. Could you add a test and source context?

[vsk]

Here's a good example of how to write a test for this sort of thing: test/profile/instrprof-reset-counters.c

[ngie]

In D34499#957901, @davide wrote:

With that in mind:

1. You may want to consider adding context to your patch, i.e. via git diff -U500000 &> blah.patch
2. You may want not jump to conclusions so quickly. I'm pretty sure people care about your contributions, it's just that nobody got around to review your patch.

When I wrote my previous comment, I was really angry and disheartened at a number of things. This was just the straw that broke the camel's back :/.

I'll try to do what I was doing before, again, taking your advice.

[ngie]

Update tests to reference gcov.h

[vsk]

compiler-rt/include/CMakeLists.txt

56

I've noticed that the sanitizer headers fall under a dedicated directory. Is it worth doing that for gcov (i.e. have <gcov/gcov.h>)?

compiler-rt/include/gcov.h

8

There's a new llvm license now, and I think it applies here:

|* 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

[ngie]

compiler-rt/include/CMakeLists.txt

56

For compatibility reasons, this header should be installed under a toolchain visible path (which compiler-rt provides).

Here's the path for gcc 8.2.0:

$ find /usr/local/ -name gcov.h
/usr/local/lib/gcc8/gcc/x86_64-portbld-freebsd13.0/8.2.0/include/gcov.h

The compiler-rt headers will be installed in /usr/local/llvm80/lib/clang/8.0.0/include/ on FreeBSD for the third-party package.

compiler-rt/include/gcov.h

8

*nod!*

20–22

This seems like a mis-paste.

[ngie]

Update licensing header for gcov.h and fix a mispaste with the comment block for __gcov_flush(..)

[vsk]

Thanks, LGTM. If you don't have commit access, let me know and I can land this on your behalf.

[ngie]

In D34499#1406167, @vsk wrote:

Thanks, LGTM. If you don't have commit access, let me know and I can land this on your behalf.

Hi @vsk! I don't have commit access -- could you please land this for me?

[vsk]

Hi @ngie,

Sorry for the delay here. I am hitting two test failures with this patch applied on Darwin. Could you take a look?

/Users/vsk/src/llvm-project-master/compiler-rt/test/profile/Posix/../Inputs/instrprof-gcov-fork.c.gcov:10:15: error: CHECK-NEXT: expected string not found in input
// CHECK-NEXT:function func2 called 2 returned 100% blocks executed 100%
              ^
instrprof-gcov-fork.c.gcov:10:1: note: scanning from here
function func2 called 1 returned 100% blocks executed 100%

...

/Users/vsk/src/llvm-project-master/compiler-rt/test/profile/Inputs/instrprof-gcov-__gcov_flush-multiple.c:6:7: warning: implicit declaration of function 'remove' is invalid in C99 [-Wimplicit-function-declaration]
  if (remove("instrprof-gcov-__gcov_flush-multiple.gcda") != 0) {
      ^
1 warning generated.
/Users/vsk/src/llvm-project-master/compiler-rt/test/profile/Inputs/instrprof-gcov-__gcov_flush-multiple.c.gcov:6:15: error: CHECK-NEXT: expected string not found in input
// CHECK-NEXT:    #####:    1:int main(void) {
              ^
instrprof-gcov-__gcov_flush-multiple.c.gcov:6:9: note: scanning from here
        -:    1:#include <gcov.h>
        ^
instrprof-gcov-__gcov_flush-multiple.c.gcov:8:5: note: possible intended match here
    #####:    3:int main(void) {

[ngie]

In D34499#1451696, @vsk wrote:

Hi @ngie,

Sorry for the delay here. I am hitting two test failures with this patch applied on Darwin. Could you take a look?

/Users/vsk/src/llvm-project-master/compiler-rt/test/profile/Posix/../Inputs/instrprof-gcov-fork.c.gcov:10:15: error: CHECK-NEXT: expected string not found in input
// CHECK-NEXT:function func2 called 2 returned 100% blocks executed 100%
              ^
instrprof-gcov-fork.c.gcov:10:1: note: scanning from here
function func2 called 1 returned 100% blocks executed 100%

...

/Users/vsk/src/llvm-project-master/compiler-rt/test/profile/Inputs/instrprof-gcov-__gcov_flush-multiple.c:6:7: warning: implicit declaration of function 'remove' is invalid in C99 [-Wimplicit-function-declaration]
  if (remove("instrprof-gcov-__gcov_flush-multiple.gcda") != 0) {
      ^
1 warning generated.
/Users/vsk/src/llvm-project-master/compiler-rt/test/profile/Inputs/instrprof-gcov-__gcov_flush-multiple.c.gcov:6:15: error: CHECK-NEXT: expected string not found in input
// CHECK-NEXT:    #####:    1:int main(void) {
              ^
instrprof-gcov-__gcov_flush-multiple.c.gcov:6:9: note: scanning from here
        -:    1:#include <gcov.h>
        ^
instrprof-gcov-__gcov_flush-multiple.c.gcov:8:5: note: possible intended match here
    #####:    3:int main(void) {

@vsk: I will try and figure out how to do this in the meantime, but just in case I'm slow, I was wondering how to run the tests.

The compile-time warning on Darwin with remove() is because a header is likely missing (stdio.h?) from the test program and isn't being pulled in via header pollution.

[ngie]

Add stdio.h to test to handle reference to posix API, remove(3)

[ngie]

stdio.h should come before gcov.h for style reasons

[ngie]

@vsk: ping?

[vsk]

Thanks for fixing the warning. To run the tests, try 'ninja check-profile' from your build directory. The failure from earlier just meant that the CHECK lines in a few *.gcov files needed to be updated. Here's a version of this patch which fixes up the tests which run on Darwin:

gcov-flush-fix-up-darwin-tests.diff7 KBDownload

There are a number of tests which reference instrprof-shared-main-gcov-flush.c which still need to be fixed up -- these aren't supported on Darwin, and they weren't as easy for me to get to.

compiler-rt/include/CMakeLists.txt

+set(GCOV_COMPAT_HEADERS
+  gcov.h)
+
 if (COMPILER_RT_BUILD_SANITIZERS)
   set(SANITIZER_HEADERS
     sanitizer/allocator_interface.h
     sanitizer/asan_interface.h
     sanitizer/common_interface_defs.h
     sanitizer/coverage_interface.h
     sanitizer/dfsan_interface.h
     sanitizer/hwasan_interface.h
if (COMPILER_RT_BUILD_XRAY)
   set(XRAY_HEADERS
     xray/xray_interface.h
     xray/xray_log_interface.h
     xray/xray_records.h
     )
 endif(COMPILER_RT_BUILD_XRAY)
 
 set(COMPILER_RT_HEADERS
+  ${GCOV_COMPAT_HEADERS}
   ${SANITIZER_HEADERS}
   ${XRAY_HEADERS})
 
 set(output_dir ${COMPILER_RT_OUTPUT_DIR}/include)
 
 # Copy compiler-rt headers to the build tree.
 set(out_files)
 foreach( f ${COMPILER_RT_HEADERS} )
   set( src ${CMAKE_CURRENT_SOURCE_DIR}/${f} )
   set( dst ${output_dir}/${f} )
   add_custom_command(OUTPUT ${dst}
     DEPENDS ${src}
     COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
     COMMENT "Copying compiler-rt's ${f}...")
   list(APPEND out_files ${dst})
 endforeach( f )
 
 add_custom_target(compiler-rt-headers ALL DEPENDS ${out_files})
 add_dependencies(compiler-rt compiler-rt-headers)
 set_target_properties(compiler-rt-headers PROPERTIES FOLDER "Compiler-RT Misc")
 
+# Install gcov compat headers.
+install(FILES ${GCOV_COMPAT_HEADERS}
+  PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
+  DESTINATION ${COMPILER_RT_INSTALL_PATH}/include)
+
 # Install sanitizer headers.
 install(FILES ${SANITIZER_HEADERS}
   COMPONENT compiler-rt-headers
   PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
   DESTINATION ${COMPILER_RT_INSTALL_PATH}/include/sanitizer)
 # Install xray headers.
 install(FILES ${XRAY_HEADERS}
   COMPONENT compiler-rt-headers

compiler-rt/include/gcov.h

+//===-- gcov.h ------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides the public interface for functions which are available to
+// end-users when using gcov(3).
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef COMPILER_RT_GCOV_H
+#define COMPILER_RT_GCOV_H
+
+/*
+ * - Write profile information/counters out to their appropriate GCDA files.
+ * - Reset profile counters to zero.
+ */
+extern void __gcov_flush(void);
+
+#endif

compiler-rt/lib/profile/CMakeLists.txt

 set(PROFILE_HEADERS
   InstrProfData.inc
   InstrProfiling.h
   InstrProfilingInternal.h
   InstrProfilingPort.h
   InstrProfilingUtil.h
   WindowsMMap.h)
 
+include_directories(../../include)
+
 if(WIN32)
     list(APPEND PROFILE_SOURCES WindowsMMap.c)
 endif()
 
 if(FUCHSIA OR UNIX)
  set(EXTRA_FLAGS
      -fPIC
      -Wno-pedantic)

compiler-rt/lib/profile/GCDAProfiling.c

 #else
 #define I386_FREEBSD 0
 #endif
 
 #if !defined(_MSC_VER) && !I386_FREEBSD
 #include <stdint.h>
 #endif
 
+#include <gcov.h>
+
 #if defined(_MSC_VER)
 typedef unsigned char uint8_t;
 typedef unsigned int uint32_t;
 typedef unsigned long long uint64_t;
 #elif I386_FREEBSD
 /* System headers define 'size_t' incorrectly on x64 FreeBSD (prior to
  * FreeBSD 10, r232261) when compiled in 32-bit mode.
  */
void llvm_delete_writeout_function_list(void) {
   fn_list_remove(&writeout_fn_list);
 }
 
 COMPILER_RT_VISIBILITY
 void llvm_register_flush_function(fn_ptr fn) {
   fn_list_insert(&flush_fn_list, fn);
 }
 
-void __gcov_flush() {
+void __gcov_flush(void) {
   struct fn_node* curr = flush_fn_list.head;
 
   while (curr) {
     curr->fn();
     curr = curr->next;
   }
 }
 

compiler-rt/test/profile/Inputs/instrprof-gcov-__gcov_flush-multiple.c

+#include <stdio.h>
+#include <gcov.h>
+
 int main(void) {
   __gcov_flush();
 
   if (remove("instrprof-gcov-__gcov_flush-multiple.gcda") != 0) {
     return 1;
   }
 
   __gcov_flush();
   __gcov_flush();
 
   if (remove("instrprof-gcov-__gcov_flush-multiple.gcda") != 0) {
     return 1;
   }
 
   return 0;
 }

compiler-rt/test/profile/Inputs/instrprof-gcov-__gcov_flush-terminate.c

+#include <gcov.h>
+
 int main(void) {
   int i = 22;
 
   __gcov_flush();
 
   i = 42;
 
   asm("int $3");
 
   i = 84;
 
   return 0;
 }

compiler-rt/test/profile/Inputs/instrprof-shared-main-gcov-flush.c

+#include <gcov.h>
+
 extern void foo(int n);
-extern void __gcov_flush(void);
 
 int bar1 = 0;
 int bar2 = 1;
 
 void bar(int n) {
   if (n % 5 == 0)
     bar1++;
   else