Skip to content

Commit

Permalink
[profile] Fix file contention causing dropped counts on Windows under…
Browse files Browse the repository at this point in the history
… -fprofile-generate

See PR43425:
https://bugs.llvm.org/show_bug.cgi?id=43425

When writing profile data on Windows we were opening profile file with
exclusive read/write access.

In case we are trying to write to the file from multiple processes
simultaneously, subsequent calls to CreateFileA would return
INVALID_HANDLE_VALUE.

To fix this, I changed to open without exclusive access and then take a
lock.

Patch by Michael Holman!

Differential revision: https://reviews.llvm.org/D70330

(cherry picked from commit 900d8a9)
  • Loading branch information
zmodem authored and tstellar committed Dec 5, 2019
1 parent b9297dc commit 432bf48
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 2 deletions.
9 changes: 7 additions & 2 deletions compiler-rt/lib/profile/InstrProfilingUtil.c
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,9 @@ COMPILER_RT_VISIBILITY FILE *lprofOpenFileEx(const char *ProfileName) {
f = fdopen(fd, "r+b");
#elif defined(_WIN32)
// FIXME: Use the wide variants to handle Unicode filenames.
HANDLE h = CreateFileA(ProfileName, GENERIC_READ | GENERIC_WRITE, 0, 0,
OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0);
HANDLE h = CreateFileA(ProfileName, GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE, 0, OPEN_ALWAYS,
FILE_ATTRIBUTE_NORMAL, 0);
if (h == INVALID_HANDLE_VALUE)
return NULL;

Expand All @@ -200,6 +201,10 @@ COMPILER_RT_VISIBILITY FILE *lprofOpenFileEx(const char *ProfileName) {
return NULL;
}

if (lprofLockFd(fd) != 0)
PROF_WARN("Data may be corrupted during profile merging : %s\n",
"Fail to obtain file lock due to system limit.");

f = _fdopen(fd, "r+b");
if (f == 0) {
CloseHandle(h);
Expand Down
89 changes: 89 additions & 0 deletions compiler-rt/test/profile/Windows/Inputs/instrprof-multiprocess.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/* This is a test case where the parent process forks 10 children
* which contend to merge profile data to the same file. With
* file locking support, the data from each child should not
* be lost.
*/
#include <stdio.h>
#include <stdlib.h>
#include <windows.h>

void spawn_child(PROCESS_INFORMATION *pi, int child_num) {
wchar_t child_str[10];
_itow(child_num, child_str, 10);
if (!SetEnvironmentVariableW(L"CHILD_NUM", child_str)) {
printf("SetEnvironmentVariableW failed (0x%8lx).\n", GetLastError());
fflush(stdout);
exit(1);
}

STARTUPINFOW si;
memset(&si, 0, sizeof(si));
si.cb = sizeof(si);

memset(pi, 0, sizeof(PROCESS_INFORMATION));

if (!CreateProcessW(NULL, // No module name (use command line)
GetCommandLineW(), // Command line
NULL, // Process handle not inheritable
NULL, // Thread handle not inheritable
TRUE, // Set handle inheritance to TRUE
0, // No flags
NULL, // Use parent's environment block
NULL, // Use parent's starting directory
&si, pi)) {
printf("CreateProcess failed (0x%08lx).\n", GetLastError());
fflush(stdout);
exit(1);
}
}

int wait_child(PROCESS_INFORMATION *pi) {
WaitForSingleObject(pi->hProcess, INFINITE);

DWORD exit_code;
if (!GetExitCodeProcess(pi->hProcess, &exit_code)) {
printf("GetExitCodeProcess failed (0x%08lx).\n", GetLastError());
fflush(stdout);
exit(1);
}

CloseHandle(pi->hProcess);
CloseHandle(pi->hThread);

return exit_code;
}

#define NUM_CHILDREN 10

int foo(int num) {
if (num < (NUM_CHILDREN / 2)) {
return 1;
} else if (num < NUM_CHILDREN) {
return 2;
}
return 3;
}

int main(int argc, char *argv[]) {
char *child_str = getenv("CHILD_NUM");
if (!child_str) {
PROCESS_INFORMATION child[NUM_CHILDREN];
// In parent
for (int i = 0; i < NUM_CHILDREN; i++) {
spawn_child(&child[i], i);
}
for (int i = 0; i < NUM_CHILDREN; i++) {
wait_child(&child[i]);
}
return 0;
} else {
// In child
int child_num = atoi(child_str);
int result = foo(child_num);
if (result == 3) {
fprintf(stderr, "Invalid child count!");
return 1;
}
return 0;
}
}
10 changes: 10 additions & 0 deletions compiler-rt/test/profile/Windows/instrprof-multiprocess.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
RUN: %clang_profgen %S/Inputs/instrprof-multiprocess.c -o %t
RUN: rm -f %t_*.profraw
RUN: env LLVM_PROFILE_FILE=%t_%m.profraw %run %t
RUN: llvm-profdata show --counts -function=foo %t_*.profraw | FileCheck %s

CHECK: Counters:
CHECK: foo:
CHECK: Function count: 10
CHECK: Block counts: [5, 5]
CHECK: Functions shown: 1
9 changes: 9 additions & 0 deletions compiler-rt/test/profile/Windows/lit.local.cfg.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
def getRoot(config):
if not config.parent:
return config
return getRoot(config.parent)

root = getRoot(config)

if root.host_os not in ['Windows']:
config.unsupported = True

0 comments on commit 432bf48

Please sign in to comment.