Flush counters before forking to avoid counting the execution before fork twice

[marco-c]

Summary

If we don't flush the counters before fork, the counters of the execution up to the fork call will be added twice (by both processes).

This fixes https://bugs.llvm.org/show_bug.cgi?id=38180.

There's also another problem that probably should be fixed on the Clang/LLVM side but is not directly related to this patch (even though you can see its effects in the test): after fork there should be a separate block (right now everything in main in the test is in the same block).

[davidxl]

test/profile/Inputs/instrprof-gcov-fork.c.gcov

9

func1 is executed once not twice.

11

func2 is executed twice not 4 times.

20

this line should be executed twice.

22

same here.

[davidxl]

One of the real issues is that func1, fork, and func2 are in the same BB and share the same profile counter. The BB ending with fork should be split so that func2 's block has its own counter.

[llvm-commits]

Il 18/07/2018 20:52, David Li via Phabricator ha scritto:

davidxl added inline comments.

Comment at: test/profile/Inputs/instrprof-gcov-fork.c.gcov:9
+ CHECK-NEXT:function func1 called 1 returned 100% blocks executed 100%
+ CHECK-NEXT: 2: 3:void func1() {}

+// CHECK-NEXT:function func2 called 2 returned 100% blocks executed 100%

func1 is executed once not twice.

This is a pre-existing problem
(https://bugs.llvm.org/show_bug.cgi?id=38065), but you can see that
after the patch it is actually executed once ("called 1").

Comment at: test/profile/Inputs/instrprof-gcov-fork.c.gcov:11
+ CHECK-NEXT:function func2 called 2 returned 100% blocks executed 100%
+ CHECK-NEXT: 4: 4:void func2() {}

+// CHECK-NEXT: -: 5:

func2 is executed twice not 4 times.

Same here.

Comment at: test/profile/Inputs/instrprof-gcov-fork.c.gcov:20
+ CHECK-NEXT: -: 11:
+ CHECK-NEXT: 1: 12: func2();

+// CHECK-NEXT: -: 13:

this line should be executed twice.

Comment at: test/profile/Inputs/instrprof-gcov-fork.c.gcov:22
+ CHECK-NEXT: -: 13:
+ CHECK-NEXT: 1: 14: return 0;

+// CHECK-NEXT: -: 15:}

same here.
davidxl added a comment.

One of the real issues is that func1, fork, and func2 are in the same BB and share the same profile counter. The BB ending with fork should be split so that func2 's block has its own counter.

Exactly, these are due to the problem I mentioned in the summary, which
probably has to be fixed in the Clang/LLVM side.
I think they are unrelated problems though, this patch is going to fix
the double counting, the other patch would fix the BBs in the function
which calls fork.

---

llvm-commits mailing list
[e-mail]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

[davidxl]

lgtm

[marco-c]

There's a problem that I hadn't noticed while running tests (as they all pass), but is noticeable when building a standalone source file.
The interception library is using functions from libdl (dlsym and dlvsym), which can't be resolved unless you build with "-ldl".

[davidxl]

Adding dependency on libdl is not desirable. I think you also need to change codegen (when coverage is on) to lower call to fork into call to the gcov version.

[marco-c]

Calixte is working on this in https://reviews.llvm.org/D53593.

lib/profile/CMakeLists.txt

+include_directories(..)
 
 CHECK_CXX_SOURCE_COMPILES("
 #ifdef _MSC_VER
 #include <Intrin.h> /* Workaround for PR19898. */
 #include <windows.h>
 #endif
 int main() {
 #ifdef _MSC_VER

 }
 
 " COMPILER_RT_TARGET_HAS_UNAME)
 
 add_compiler_rt_component(profile)
 
 set(PROFILE_SOURCES
   GCDAProfiling.c
+  GCDAProfilingInterceptors.cc
   InstrProfiling.c
   InstrProfilingValue.c
   InstrProfilingBuffer.c
   InstrProfilingFile.c
   InstrProfilingMerge.c
   InstrProfilingMergeFile.c
   InstrProfilingNameVar.c
   InstrProfilingWriter.c
   InstrProfilingPlatformDarwin.c
   InstrProfilingPlatformLinux.c
   InstrProfilingPlatformOther.c
   InstrProfilingRuntime.cc
   InstrProfilingUtil.c)
 
 set(PROFILE_HEADERS
+  GCDAProfiling.h
   InstrProfData.inc
   InstrProfiling.h
   InstrProfilingInternal.h
   InstrProfilingPort.h
   InstrProfilingUtil.h
   WindowsMMap.h)
 
 if(WIN32)

 # nonstandard extension used : 'identifier' : cannot be initialized using address of automatic variable
 append_list_if(COMPILER_RT_HAS_WD4221_FLAG /wd4221 EXTRA_FLAGS)
 
 if(APPLE)
   add_compiler_rt_runtime(clang_rt.profile
     STATIC
     OS ${PROFILE_SUPPORTED_OS}
     ARCHS ${PROFILE_SUPPORTED_ARCH}
+    OBJECT_LIBS RTInterception
     CFLAGS ${EXTRA_FLAGS}
     SOURCES ${PROFILE_SOURCES}
     ADDITIONAL_HEADERS ${PROFILE_HEADERS}
     PARENT_TARGET profile)
 else()
   add_compiler_rt_runtime(clang_rt.profile
     STATIC
     ARCHS ${PROFILE_SUPPORTED_ARCH}
+    OBJECT_LIBS RTInterception
     CFLAGS ${EXTRA_FLAGS}
     SOURCES ${PROFILE_SOURCES}
     ADDITIONAL_HEADERS ${PROFILE_HEADERS}
     PARENT_TARGET profile)
 endif()

lib/profile/GCDAProfiling.h

+/*===- GCDAProfiling.h ----------------------------------------------------===*\
+|*
+|*                     The LLVM Compiler Infrastructure
+|*
+|* This file is distributed under the University of Illinois Open Source
+|* License. See LICENSE.TXT for details.
+|*
+\*===----------------------------------------------------------------------===*/
+
+#ifndef PROFILE_INSTRPROFILING_GCDAPROFILING_H
+#define PROFILE_INSTRPROFILING_GCDAPROFILING_H
+
+void gcov_initialize_interceptors();
+void __gcov_flush();
+
+#endif /* PROFILE_INSTRPROFILING_GCDAPROFILING_H */

lib/profile/GCDAProfiling.c

 \*===----------------------------------------------------------------------===*/
 
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
+#include "GCDAProfiling.h"
 #if defined(_WIN32)
 #include "WindowsMMap.h"
 #else
 #include <sys/mman.h>
 #include <sys/file.h>
 #endif
 
 #if defined(__FreeBSD__) && defined(__i386__)

 void llvm_delete_flush_function_list(void) {
   fn_list_remove(&flush_fn_list);
 }
 
 COMPILER_RT_VISIBILITY
 void llvm_gcov_init(fn_ptr wfn, fn_ptr ffn) {
   static int atexit_ran = 0;
 
+  gcov_initialize_interceptors();
+
   if (wfn)
     llvm_register_writeout_function(wfn);
 
   if (ffn)
     llvm_register_flush_function(ffn);
 
   if (atexit_ran == 0) {
     atexit_ran = 1;
 
     /* Make sure we write out the data and delete the data structures. */
     atexit(llvm_delete_flush_function_list);
     atexit(llvm_delete_writeout_function_list);
     atexit(llvm_writeout_files);
   }
 }

lib/profile/GCDAProfilingInterceptors.cc

+//===-- GCDAProfilingInterceptors.c ---------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "interception/interception.h"
+extern "C" {
+#include "GCDAProfiling.h"
+}
+
+#include <stdio.h>
+
+// Before forking, we should flush the counters so that the execution so far is
+// not counted twice.
+INTERCEPTOR(int, fork, void) {
+  __gcov_flush();
+  return REAL(fork)();
+}
+
+extern "C" void gcov_initialize_interceptors() {
+  static int inited = 0;
+  if (inited != 0) {
+    fprintf(stderr, "llvmgcda: Interceptors already initialized\n");
+  }
+
+  INTERCEPT_FUNCTION(fork);
+
+  inited = 1;
+}

test/profile/Inputs/instrprof-gcov-fork.c

+#include <unistd.h>
+
+void func1() {}
+void func2() {}
+
+int main(void)
+{
+  func1();
+
+  fork();
+
+  func2();
+
+  return 0;
+}

test/profile/Inputs/instrprof-gcov-fork.c.gcov

+// CHECK:        -:    0:Source:{{.*}}Inputs/instrprof-gcov-fork.c
+// CHECK-NEXT:        -:    0:Graph:instrprof-gcov-fork.gcno
+// CHECK-NEXT:        -:    0:Data:instrprof-gcov-fork.gcda
+// CHECK-NEXT:        -:    0:Runs:1
+// CHECK-NEXT:        -:    0:Programs:1
+// CHECK-NEXT:        -:    1:#include <unistd.h>
+// CHECK-NEXT:        -:    2:
+// CHECK-NEXT:function func1 called 1 returned 100% blocks executed 100%
+// CHECK-NEXT:        2:    3:void func1() {}
+// CHECK-NEXT:function func2 called 2 returned 100% blocks executed 100%
+// CHECK-NEXT:        4:    4:void func2() {}
+// CHECK-NEXT:        -:    5:
+// CHECK-NEXT:function main called 1 returned 100% blocks executed 100%
+// CHECK-NEXT:        -:    6:int main(void)
+// CHECK-NEXT:        -:    7:{
+// CHECK-NEXT:        1:    8:  func1();
+// CHECK-NEXT:        -:    9:
+// CHECK-NEXT:        1:   10:  fork();
+// CHECK-NEXT:        -:   11:
+// CHECK-NEXT:        1:   12:  func2();
+// CHECK-NEXT:        -:   13:
+// CHECK-NEXT:        1:   14:  return 0;
+// CHECK-NEXT:        -:   15:}

test/profile/Posix/instrprof-gcov-fork.test

+RUN: mkdir -p %t.d
+RUN: cd %t.d
+
+RUN: %clang --coverage -o %t %S/../Inputs/instrprof-gcov-fork.c
+RUN: test -f instrprof-gcov-fork.gcno
+
+RUN: rm -f instrprof-gcov-fork.gcda
+RUN: %run %t
+RUN: llvm-cov gcov -b -c instrprof-gcov-fork.gcda
+RUN: FileCheck --match-full-lines --strict-whitespace --input-file instrprof-gcov-fork.c.gcov %S/../Inputs/instrprof-gcov-fork.c.gcov