This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by marco-c on Nov 1 2018, 11:11 AM.

Details

Reviewers
calixte
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.

Diff Detail

Event Timeline

marco-c created this revision.Nov 1 2018, 11:11 AM
Herald added subscribers: Restricted Project, llvm-commits. · View Herald TranscriptNov 1 2018, 11:11 AM
marco-c retitled this revision from Close file mapping handle on Windows, so flushed gcda files can be removed while th to Close file mapping handle on Windows, so flushed gcda files can be removed while the process is in execution.Nov 1 2018, 11:11 AM
dmajor added a subscriber: dmajor.Nov 2 2018, 12:59 PM
dmajor added inline comments.
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 updated this revision to Diff 172682.Nov 5 2018, 4:37 PM

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

marco-c marked an inline comment as done.Nov 5 2018, 4:38 PM
marco-c updated this revision to Diff 172727.Nov 6 2018, 2:21 AM

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

calixte added inline comments.Nov 6 2018, 2:35 AM
lib/profile/GCDAProfiling.c
34

this header is probably useless now

marco-c marked an inline comment as done.Nov 6 2018, 4:20 AM
marco-c added inline comments.
lib/profile/GCDAProfiling.c
34

It's needed for flock.

marco-c updated this revision to Diff 172907.Nov 7 2018, 1:17 AM
marco-c marked an inline comment as done.

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

calixte accepted this revision.Nov 7 2018, 1:30 AM

Looks good to me.

This revision is now accepted and ready to land.Nov 7 2018, 1:30 AM
marco-c closed this revision.Nov 7 2018, 1:43 AM

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