From: KAWASHIMA Takahiro <t-kawashima@fujitsu.com>
Date: Fri, 13 Mar 2020 12:54:06 +0000 (+0900)
Subject: Fix simultaneous .gcda creation
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9180c14fe4d7f9d6f65da8bc593cd61796fdc50a;p=platform%2Fupstream%2Fllvm.git

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

diff --git a/compiler-rt/lib/profile/GCDAProfiling.c b/compiler-rt/lib/profile/GCDAProfiling.c
index 498c059..5ff1e9c 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 0000000..6ce12d3
--- /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 0000000..ae6e60f
--- /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 0000000..0c7198e3
--- /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++;