Close file mapping handle on Windows, so flushed gcda files can be removed while the process is in execution

[marco-c]

Summary

We were leaking it and it was "fine", but this way flushed gcda files can't be removed while the process is in execution.
This makes it impossible to collect coverage information for a subset of the execution.

[dmajor]

lib/profile/GCDAProfiling.c

271

When CreateFileMapping fails, it returns NULL rather than INVALID_HANDLE_VALUE. For consistency, the sentinel value for mmap_handle should probably be NULL throughout (e.g. in the initializer and unmap_file assignment).

[marco-c]

Made mmap_handle NULL by default (as CreateFileMapping returns NULL on failure and not INVALID_HANDLE_VALUE).

[marco-c]

Add calls to FlushViewOfFile and FlushFileBuffers when unmapping to be sure that all the data is written to disk.

[calixte]

lib/profile/GCDAProfiling.c

34

this header is probably useless now

[marco-c]

lib/profile/GCDAProfiling.c

34

It's needed for flock.

[marco-c]

Moved DWORD_HI and DWORD_LO to WindowsMMap.h, and removed FlushFileBuffers as it fails with ERROR_INVALID_HANDLE.

[calixte]

Looks good to me.

[marco-c]

Landed in rCRT346300 (sorry, forgot to land it via arc).

lib/profile/GCDAProfiling.c

 
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
 #if defined(_WIN32)
+#define WIN32_LEAN_AND_MEAN
+#include <windows.h>
 #include "WindowsMMap.h"
 #else
 #include <sys/mman.h>
 #include <sys/file.h>
 #endif
 
 #if defined(__FreeBSD__) && defined(__i386__)
 #define I386_FREEBSD 1
 #else

  * Buffer that we write things into.
  */
 #define WRITE_BUFFER_SIZE (128 * 1024)
 static unsigned char *write_buffer = NULL;
 static uint64_t cur_buffer_size = 0;
 static uint64_t cur_pos = 0;
 static uint64_t file_size = 0;
 static int new_file = 0;
+#if defined(_WIN32)
+static HANDLE mmap_handle = NULL;
+#endif
 static int fd = -1;
 
 typedef void (*fn_ptr)();
 
 typedef void* dynamic_object_id;
 // The address of this variable identifies a given dynamic object.
 static dynamic_object_id current_id;
 #define CURRENT_ID (&current_id)
static int map_file() {
   fseek(output_file, 0L, SEEK_END);
   file_size = ftell(output_file);
 
   /* A size of 0 is invalid to `mmap'. Return a fail here, but don't issue an
    * error message because it should "just work" for the user. */
   if (file_size == 0)
     return -1;
 
+#if defined(_WIN32)
+  HANDLE mmap_fd;
+  if (fd == -1)
+    mmap_fd = INVALID_HANDLE_VALUE;
+  else
+    mmap_fd = (HANDLE)_get_osfhandle(fd);
+
+  mmap_handle = CreateFileMapping(mmap_fd, NULL, PAGE_READWRITE, DWORD_HI(file_size), DWORD_LO(file_size), NULL);
+  if (mmap_handle == NULL) {
+    fprintf(stderr, "profiling: %s: cannot create file mapping: %d\n", filename,
+            GetLastError());
+    return -1;
+  }
+
+  write_buffer = MapViewOfFile(mmap_handle, FILE_MAP_WRITE, 0, 0, file_size);
+  if (write_buffer == NULL) {
+    fprintf(stderr, "profiling: %s: cannot map: %d\n", filename,
+            GetLastError());
+    CloseHandle(mmap_handle);
+    return -1;
+  }
+#else
   write_buffer = mmap(0, file_size, PROT_READ | PROT_WRITE,
                       MAP_FILE | MAP_SHARED, fd, 0);
   if (write_buffer == (void *)-1) {
     int errnum = errno;
     fprintf(stderr, "profiling: %s: cannot map: %s\n", filename,
             strerror(errnum));
     return -1;
   }
+#endif
+
   return 0;
 }
 
 static void unmap_file() {
+#if defined(_WIN32)
+  if (!FlushViewOfFile(write_buffer, file_size)) {
+    fprintf(stderr, "profiling: %s: cannot flush mapped view: %d\n", filename,
+            GetLastError());
+  }
+
+  if (!UnmapViewOfFile(write_buffer)) {
+    fprintf(stderr, "profiling: %s: cannot unmap mapped view: %d\n", filename,
+            GetLastError());
+  }
+
+  if (!CloseHandle(mmap_handle)) {
+    fprintf(stderr, "profiling: %s: cannot close file mapping handle: %d\n", filename,
+            GetLastError());
+  }
+
+  mmap_handle = NULL;
+#else
   if (msync(write_buffer, file_size, MS_SYNC) == -1) {
     int errnum = errno;
     fprintf(stderr, "profiling: %s: cannot msync: %s\n", filename,
             strerror(errnum));
   }
 
   /* We explicitly ignore errors from unmapping because at this point the data
    * is written and we don't care.
    */
   (void)munmap(write_buffer, file_size);
+#endif
+
   write_buffer = NULL;
   file_size = 0;
 }
 
 /*
  * --- LLVM line counter API ---
  */
 

lib/profile/WindowsMMap.h

 /*
  * flock() operations
  */
 #define   LOCK_SH   1    /* shared lock */
 #define   LOCK_EX   2    /* exclusive lock */
 #define   LOCK_NB   4    /* don't block when locking */
 #define   LOCK_UN   8    /* unlock */
 
+#ifdef __USE_FILE_OFFSET64
+# define DWORD_HI(x) (x >> 32)
+# define DWORD_LO(x) ((x) & 0xffffffff)
+#else
+# define DWORD_HI(x) (0)
+# define DWORD_LO(x) (x)
+#endif
+
 void *mmap(void *start, size_t length, int prot, int flags, int fd,
            off_t offset);
 
 void munmap(void *addr, size_t length);
 
 int msync(void *addr, size_t length, int flags);
 
 int flock(int fd, int operation);
 
 #endif /* _WIN32 */
 
 #endif /* PROFILE_INSTRPROFILING_WINDOWS_MMAP_H */

lib/profile/WindowsMMap.c

 
 #include "WindowsMMap.h"
 
 #define WIN32_LEAN_AND_MEAN
 #include <windows.h>
 
 #include "InstrProfiling.h"
 
-#ifdef __USE_FILE_OFFSET64
-# define DWORD_HI(x) (x >> 32)
-# define DWORD_LO(x) ((x) & 0xffffffff)
-#else
-# define DWORD_HI(x) (0)
-# define DWORD_LO(x) (x)
-#endif
-
 COMPILER_RT_VISIBILITY
 void *mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset)
 {
   if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC))
     return MAP_FAILED;
   if (fd == -1) {
     if (!(flags & MAP_ANON) || offset)
       return MAP_FAILED;

test/profile/Inputs/instrprof-gcov-__gcov_flush-multiple.c

+int main(void) {
+  __gcov_flush();
+
+  if (remove("instrprof-gcov-__gcov_flush-multiple.gcda") != 0) {
+    return 1;
+  }
+
+  __gcov_flush();
+  __gcov_flush();
+
+  if (remove("instrprof-gcov-__gcov_flush-multiple.gcda") != 0) {
+    return 1;
+  }
+
+  return 0;
+}

test/profile/Inputs/instrprof-gcov-__gcov_flush-multiple.c.gcov

+// CHECK:        -:    0:Source:{{.*}}Inputs/instrprof-gcov-__gcov_flush-multiple.c
+// CHECK-NEXT:        -:    0:Graph:instrprof-gcov-__gcov_flush-multiple.gcno
+// CHECK-NEXT:        -:    0:Data:instrprof-gcov-__gcov_flush-multiple.gcda
+// CHECK-NEXT:        -:    0:Runs:1
+// CHECK-NEXT:        -:    0:Programs:1
+// CHECK-NEXT:    #####:    1:int main(void) {
+// CHECK-NEXT:    #####:    2:  __gcov_flush();
+// CHECK-NEXT:        -:    3:
+// CHECK-NEXT:    #####:    4:  if (remove("instrprof-gcov-__gcov_flush-multiple.gcda") != 0) {
+// CHECK-NEXT:    #####:    5:    return 1;
+// CHECK-NEXT:        -:    6:  }
+// CHECK-NEXT:        -:    7:
+// CHECK-NEXT:    #####:    8:  __gcov_flush();
+// CHECK-NEXT:    #####:    9:  __gcov_flush();
+// CHECK-NEXT:        -:   10:
+// CHECK-NEXT:    #####:   11:  if (remove("instrprof-gcov-__gcov_flush-multiple.gcda") != 0) {
+// CHECK-NEXT:    #####:   12:    return 1;
+// CHECK-NEXT:        -:   13:  }
+// CHECK-NEXT:        -:   14:
+// CHECK-NEXT:        1:   15:  return 0;
+// CHECK-NEXT:        1:   16:}

test/profile/instrprof-gcov-__gcov_flush-multiple.test

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