[compiler-rt] 900d8a9 - [profile] Fix file contention causing dropped counts on Windows under -fprofile-generate
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 27 06:55:34 PST 2019
Author: Hans Wennborg
Date: 2019-11-27T15:55:13+01:00
New Revision: 900d8a9a3b4efeefddd310e92219741d98e7270b
URL: https://github.com/llvm/llvm-project/commit/900d8a9a3b4efeefddd310e92219741d98e7270b
DIFF: https://github.com/llvm/llvm-project/commit/900d8a9a3b4efeefddd310e92219741d98e7270b.diff
LOG: [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
Added:
compiler-rt/test/profile/Windows/Inputs/instrprof-multiprocess.c
compiler-rt/test/profile/Windows/instrprof-multiprocess.test
compiler-rt/test/profile/Windows/lit.local.cfg.py
Modified:
compiler-rt/lib/profile/InstrProfilingUtil.c
Removed:
################################################################################
diff --git a/compiler-rt/lib/profile/InstrProfilingUtil.c b/compiler-rt/lib/profile/InstrProfilingUtil.c
index 13301f341fc5..bf5a9670fe18 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 000000000000..774712d39738
--- /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 <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;
+ }
+}
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 000000000000..ae5ebd45bec9
--- /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 000000000000..e924d91c4493
--- /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
More information about the llvm-commits
mailing list