[compiler-rt] 9180c14 - Fix simultaneous .gcda creation

[KAWASHIMA Takahiro via llvm-commits]

Author: KAWASHIMA Takahiro
Date: 2020-04-01T10:29:50+09:00
New Revision: 9180c14fe4d7f9d6f65da8bc593cd61796fdc50a

URL: https://github.com/llvm/llvm-project/commit/9180c14fe4d7f9d6f65da8bc593cd61796fdc50a
DIFF: https://github.com/llvm/llvm-project/commit/9180c14fe4d7f9d6f65da8bc593cd61796fdc50a.diff

LOG: Fix simultaneous .gcda creation

The intent of the `llvm_gcda_start_file` function is that only
one process create the .gcda file and initialize it to be updated
by other processes later.

Before this change, if multiple processes are started simultaneously,
some of them may initialize the file because both the first and
second `open` calls may succeed in a race condition and `new_file`
becomes 1 in those processes. This leads incorrect coverage counter
values. This often happens in MPI (Message Passing Interface) programs.
The test program added in this change is a simple reproducer.

This change ensures only one process creates/initializes the file by
using the `O_EXCL` flag.

Differential Revision: https://reviews.llvm.org/D76206

Added: 
    compiler-rt/test/profile/Inputs/instrprof-gcov-parallel.driver.c
    compiler-rt/test/profile/Inputs/instrprof-gcov-parallel.target.c
    compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test

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

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/profile/GCDAProfiling.c b/compiler-rt/lib/profile/GCDAProfiling.c
index 498c05900bf2..5ff1e9cd8070 100644
--- a/compiler-rt/lib/profile/GCDAProfiling.c
+++ b/compiler-rt/lib/profile/GCDAProfiling.c
@@ -348,20 +348,29 @@ void llvm_gcda_start_file(const char *orig_filename, const char version[4],
   fd = open(filename, O_RDWR | O_BINARY);
 
   if (fd == -1) {
-    /* Try opening the file, creating it if necessary. */
-    new_file = 1;
-    mode = "w+b";
-    fd = open(filename, O_RDWR | O_CREAT | O_BINARY, 0644);
-    if (fd == -1) {
+    /* Try creating the file. */
+    fd = open(filename, O_RDWR | O_CREAT | O_EXCL | O_BINARY, 0644);
+    if (fd != -1) {
+      new_file = 1;
+      mode = "w+b";
+    } else {
       /* Try creating the directories first then opening the file. */
       __llvm_profile_recursive_mkdir(filename);
-      fd = open(filename, O_RDWR | O_CREAT | O_BINARY, 0644);
-      if (fd == -1) {
-        /* Bah! It's hopeless. */
-        int errnum = errno;
-        fprintf(stderr, "profiling: %s: cannot open: %s\n", filename,
-                strerror(errnum));
-        return;
+      fd = open(filename, O_RDWR | O_CREAT | O_EXCL | O_BINARY, 0644);
+      if (fd != -1) {
+        new_file = 1;
+        mode = "w+b";
+      } else {
+        /* Another process may have created the file just now.
+         * Try opening it without O_CREAT and O_EXCL. */
+        fd = open(filename, O_RDWR | O_BINARY);
+        if (fd == -1) {
+          /* Bah! It's hopeless. */
+          int errnum = errno;
+          fprintf(stderr, "profiling: %s: cannot open: %s\n", filename,
+                  strerror(errnum));
+          return;
+        }
       }
     }
   }

diff  --git a/compiler-rt/test/profile/Inputs/instrprof-gcov-parallel.driver.c b/compiler-rt/test/profile/Inputs/instrprof-gcov-parallel.driver.c
new file mode 100644
index 000000000000..6ce12d35772f
--- /dev/null
+++ b/compiler-rt/test/profile/Inputs/instrprof-gcov-parallel.driver.c
@@ -0,0 +1,36 @@
+#include <stdatomic.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#define CHILDREN 7
+
+int main(int argc, char *argv[]) {
+  _Atomic int *sync = mmap(NULL, sizeof(int), PROT_READ | PROT_WRITE,
+                           MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+  if (sync == MAP_FAILED)
+    return 1;
+  *sync = 0;
+
+  for (int i = 0; i < CHILDREN; i++) {
+    pid_t pid = fork();
+    if (!pid) {
+      // child
+      while (*sync == 0)
+        ; // wait the parent in order to call execl simultaneously
+      execl(argv[1], argv[1], NULL);
+    } else if (pid == -1) {
+      *sync = 1; // release all children
+      return 1;
+    }
+  }
+
+  // parent
+  *sync = 1; // start the program in all children simultaneously
+  for (int i = 0; i < CHILDREN; i++)
+    wait(NULL);
+
+  return 0;
+}

diff  --git a/compiler-rt/test/profile/Inputs/instrprof-gcov-parallel.target.c b/compiler-rt/test/profile/Inputs/instrprof-gcov-parallel.target.c
new file mode 100644
index 000000000000..ae6e60fb2190
--- /dev/null
+++ b/compiler-rt/test/profile/Inputs/instrprof-gcov-parallel.target.c
@@ -0,0 +1,9 @@
+#define COUNT 101
+
+static volatile int aaa;
+
+int main(int argc, char *argv[]) {
+  for (int i = 0; i < COUNT; i++)
+    aaa++;
+  return 0;
+}

diff  --git a/compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test b/compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test
new file mode 100644
index 000000000000..0c7198e3c4e9
--- /dev/null
+++ b/compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test
@@ -0,0 +1,16 @@
+RUN: mkdir -p %t.d
+RUN: cd %t.d
+
+RUN: %clang -o %t.driver %S/../Inputs/instrprof-gcov-parallel.driver.c
+RUN: %clang --coverage -o %t.target %S/../Inputs/instrprof-gcov-parallel.target.c
+RUN: test -f instrprof-gcov-parallel.target.gcno
+
+RUN: rm -f instrprof-gcov-parallel.target.gcda
+RUN: %run %t.driver %t.target
+RUN: llvm-cov gcov instrprof-gcov-parallel.target.gcda
+RUN: FileCheck --input-file instrprof-gcov-parallel.target.c.gcov %s
+
+# Test if the .gcda file is correctly created from one of child processes
+# and counters of all processes are recorded correctly.
+# 707 = CHILDREN * COUNT
+CHECK: 707: {{[0-9]+}}: aaa++;