[PATCH] D54599: [Profile] Avoid race condition when dumping GCDA files.

[Marco Castelluccio via Phabricator via llvm-commits]

marco-c added inline comments.


================
Comment at: lib/profile/GCDAProfiling.c:361
       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_BINARY);
+        if (fd == -1) {
----------------
vsk wrote:
> calixte wrote:
> > vsk wrote:
> > > Why try to create the file again?
> > The first attempt (line 355) can fail because the parent directory is not here or because the file already exists.
> > So we create the parent dirs  (if required) and then try again to create the file and if it fails again then the file exists and then get the fd with fd = open(filename, O_RDWR | O_BINARY) (which shouldn't return -1)
> I'm a bit suspicious of the need to have 4 calls to open(). Could one of them be avoided by calling '__llvm_profile_recursive_mkdir' unconditionally?
> 
> How do InstrProfiling{Util,File}.c avoid the same race?
I suppose __llvm_profile_recursive_mkdir was added as an optimization (if GCOV_PREFIX is not set, then the directories should exist already).

InstrProfilingFile.c is always creating the directory first and it seems to directly open the file with O_CREAT, but it doesn't seem to handle the race any differently than GCDAProfiling.c (that is, it is not handling the race).


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D54599