[PATCH] D70910: Add a critical section when flushing gcov counters

[calixte via Phabricator via llvm-commits]

calixte created this revision.
calixte added reviewers: marco-c, froydnj.
Herald added subscribers: llvm-commits, Sanitizers, jfb.
Herald added projects: Sanitizers, LLVM.

Counters can be flushed in a multi-threaded context for example when the process is forked in different threads (https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp#L632-L663).
In order to avoid pretty bad things, a critical section is needed around the flush.
We had a lot of crashes in this code in Firefox CI when we switched to clang for linux ccov builds and those crashes disappeared with this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70910

Files:
  compiler-rt/lib/profile/GCDAProfiling.c


Index: compiler-rt/lib/profile/GCDAProfiling.c
===================================================================
--- compiler-rt/lib/profile/GCDAProfiling.c
+++ compiler-rt/lib/profile/GCDAProfiling.c
@@ -62,8 +62,39 @@
 #include "InstrProfiling.h"
 #include "InstrProfilingUtil.h"
 
-/* #define DEBUG_GCDAPROFILING */
+#ifndef _WIN32
+#include <pthread.h>
+pthread_mutex_t gcov_flush_mutex = PTHREAD_MUTEX_INITIALIZER;
+static __inline void gcov_flush_lock() {
+  pthread_mutex_lock(&gcov_flush_mutex);
+}
+static __inline void gcov_flush_unlock() {
+  pthread_mutex_unlock(&gcov_flush_mutex);
+}
+static __inline void gcov_flush_init_once(void) {}
+#else
+#include <windows.h>
+static CRITICAL_SECTION gcov_flush_mutex;
+static __inline void gcov_flush_lock() {
+  EnterCriticalSection(&gcov_flush_mutex);
+}
+static __inline void gcov_flush_unlock() {
+  LeaveCriticalSection(&gcov_flush_mutex);
+}
+static __inline void gcov_flush_exit(void) {
+  DeleteCriticalSection(&gcov_flush_mutex);
+}
+static __inline void gcov_flush_init(void) {
+  InitializeCriticalSection(&gcov_flush_mutex);
+  atexit(&gcov_flush_exit);
+}
+static __inline void gcov_flush_init_once(void) {
+  static INIT_ONCE once;
+  InitOnceExecuteOnce(&once, gcov_flush_init, NULL, NULL);
+}
+#endif
 
+/* #define DEBUG_GCDAPROFILING */
 /*
  * --- GCOV file format I/O primitives ---
  */
@@ -620,12 +651,17 @@
 }
 
 void __gcov_flush() {
+  gcov_flush_init_once();
+  gcov_flush_lock();
+
   struct fn_node* curr = flush_fn_list.head;
 
   while (curr) {
     curr->fn();
     curr = curr->next;
   }
+
+  gcov_flush_unlock();
 }
 
 COMPILER_RT_VISIBILITY


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70910.231731.patch
Type: text/x-patch
Size: 1631 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191202/76262f97/attachment.bin>