From 900d8a9a3b4efeefddd310e92219741d98e7270b Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Wed, 27 Nov 2019 15:47:44 +0100 Subject: [PATCH] [profile] Fix file contention causing dropped counts on Windows under -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 --- compiler-rt/lib/profile/InstrProfilingUtil.c | 9 ++- .../Windows/Inputs/instrprof-multiprocess.c | 89 ++++++++++++++++++++++ .../profile/Windows/instrprof-multiprocess.test | 10 +++ compiler-rt/test/profile/Windows/lit.local.cfg.py | 9 +++ 4 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 compiler-rt/test/profile/Windows/Inputs/instrprof-multiprocess.c create mode 100644 compiler-rt/test/profile/Windows/instrprof-multiprocess.test create mode 100644 compiler-rt/test/profile/Windows/lit.local.cfg.py diff --git a/compiler-rt/lib/profile/InstrProfilingUtil.c b/compiler-rt/lib/profile/InstrProfilingUtil.c index 13301f3..bf5a967 100644 --- a/compiler-rt/lib/profile/InstrProfilingUtil.c +++ b/compiler-rt/lib/profile/InstrProfilingUtil.c @@ -207,8 +207,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; @@ -218,6 +219,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); diff --git a/compiler-rt/test/profile/Windows/Inputs/instrprof-multiprocess.c b/compiler-rt/test/profile/Windows/Inputs/instrprof-multiprocess.c new file mode 100644 index 0000000..774712d --- /dev/null +++ b/compiler-rt/test/profile/Windows/Inputs/instrprof-multiprocess.c @@ -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 +#include +#include + +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; + } +} diff --git a/compiler-rt/test/profile/Windows/instrprof-multiprocess.test b/compiler-rt/test/profile/Windows/instrprof-multiprocess.test new file mode 100644 index 0000000..ae5ebd4 --- /dev/null +++ b/compiler-rt/test/profile/Windows/instrprof-multiprocess.test @@ -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 diff --git a/compiler-rt/test/profile/Windows/lit.local.cfg.py b/compiler-rt/test/profile/Windows/lit.local.cfg.py new file mode 100644 index 0000000..e924d91 --- /dev/null +++ b/compiler-rt/test/profile/Windows/lit.local.cfg.py @@ -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 -- 2.7.4