[Profile] Disable continuous mode when reset to default.profraw due to malformed LLVM_PROFILE_FILE.

[ZequanWu]

When LLVM_PROFILE_FILE is set incorrectly (e.g. multiple %c) and it falls back to use default.profraw name, but continuous mode is still set. This might cause signal bus in the following scenario.

LLVM_PROFILE_FILE is set incorrectly (with "%c%c") for process A and B. Suppose A starts first and falls back to use default.profraw and mmaped its file content to memory. Later B starts and also falls back to use default.profraw, but it will truncate the file because online merging is disable when reseting to default.profraw. When A tries to update counter via mmaped memory, signal bus will occur.

This fixes it by disabling continuous mode when reset to default.profraw.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Zequan Wu (ZequanWu)

Changes

When LLVM_PROFILE_FILE is set incorrectly (e.g. multiple %c) and it falls back to use default.profraw name, but continuous mode is still set. This might cause signal bus in the following scenario.

LLVM_PROFILE_FILE is set incorrectly (with "%c%c") for process A and B. Suppose A starts first and falls back to use default.profraw and mmaped its file content to memory. Later B starts and also falls back to use default.profraw, but it will truncate the file because online merging is disable when reseting to to default.profraw. When A tries to update counter via mmaped memory, signal bus will occur.

This fixes it by disabling continuous mode when reset to default.profraw.

---

Full diff: https://github.com/llvm/llvm-project/pull/74879.diff

4 Files Affected:

• (modified) compiler-rt/lib/profile/InstrProfiling.h (+6)
• (modified) compiler-rt/lib/profile/InstrProfilingBuffer.c (+4)
• (modified) compiler-rt/lib/profile/InstrProfilingFile.c (+1)
• (added) compiler-rt/test/profile/ContinuousSyncMode/reset-default-profile.c (+21)

diff --git a/compiler-rt/lib/profile/InstrProfiling.h b/compiler-rt/lib/profile/InstrProfiling.h
index c5b0b34f2d8af0..813545a29a8e86 100644
--- a/compiler-rt/lib/profile/InstrProfiling.h
+++ b/compiler-rt/lib/profile/InstrProfiling.h
@@ -54,6 +54,12 @@ int __llvm_profile_is_continuous_mode_enabled(void);
  */
 void __llvm_profile_enable_continuous_mode(void);
 
+/*!
+ * \brief Enable continuous mode.
+ *
+ */
+void __llvm_profile_disable_continuous_mode(void);
+
 /*!
  * \brief Set the page size.
  *
diff --git a/compiler-rt/lib/profile/InstrProfilingBuffer.c b/compiler-rt/lib/profile/InstrProfilingBuffer.c
index cd1f067bd188e4..5657bf5bacf225 100644
--- a/compiler-rt/lib/profile/InstrProfilingBuffer.c
+++ b/compiler-rt/lib/profile/InstrProfilingBuffer.c
@@ -33,6 +33,10 @@ COMPILER_RT_VISIBILITY void __llvm_profile_enable_continuous_mode(void) {
   ContinuouslySyncProfile = 1;
 }
 
+void __llvm_profile_disable_continuous_mode(void) {
+  ContinuouslySyncProfile = 0;
+}
+
 COMPILER_RT_VISIBILITY void __llvm_profile_set_page_size(unsigned PS) {
   PageSize = PS;
 }
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index 1685b30b9492a6..4fea835f3332da 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -711,6 +711,7 @@ static void resetFilenameToDefault(void) {
   memset(&lprofCurFilename, 0, sizeof(lprofCurFilename));
   lprofCurFilename.FilenamePat = DefaultProfileName;
   lprofCurFilename.PNS = PNS_default;
+  __llvm_profile_disable_continuous_mode();
 }
 
 static unsigned getMergePoolSize(const char *FilenamePat, int *I) {
diff --git a/compiler-rt/test/profile/ContinuousSyncMode/reset-default-profile.c b/compiler-rt/test/profile/ContinuousSyncMode/reset-default-profile.c
new file mode 100644
index 00000000000000..f09ab40f4b0352
--- /dev/null
+++ b/compiler-rt/test/profile/ContinuousSyncMode/reset-default-profile.c
@@ -0,0 +1,21 @@
+// REQUIRES: darwin || linux
+
+// Test when LLVM_PROFILE_FILE is set incorrectly, it should fall backs to use default.profraw without runtime error.
+
+// Create & cd into a temporary directory.
+// RUN: rm -rf %t.dir && mkdir -p %t.dir && cd %t.dir
+// RUN: %clang -fprofile-instr-generate -fcoverage-mapping -mllvm -runtime-counter-relocation=true -o %t.exe %s
+// RUN: env LLVM_PROFILE_FILE="incorrect-profile-name%m%c%c.profraw" %run %t.exe
+// RUN: ls -l default.profraw | FileCheck %s
+
+// CHECK:     default.profraw
+// CEHCK-NOT: incorrect-profile-name.profraw
+
+#include <stdio.h>
+int f() { return 0; }
+
+int main(int argc, char **argv) {
+  FILE* File = fopen("default.profraw", "w");
+  f();
+  return 0;
+}

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[alanzhao1]

nit: fix comment?

[ZequanWu]

Done.

[alanzhao1]

nit: CHECK

[ZequanWu]

Done.

[aeubanks]

what about printing an error and immediately aborting when LLVM_PROFILE_FILE is invalid?

[david-xl]

The disabling can be done earlier at around line 808.

[ZequanWu]

what about printing an error and immediately aborting when LLVM_PROFILE_FILE is invalid?

I think this is better than falls back to default.profraw with a warning, which is hard to notice.
Or we can simply allow multiple %c and enable continuous mode as this shouldn't cause any ambiguity. But multiple %m might cause ambiguity because it can have %1m and %2m at the same time and cause ambiguity.

[ZequanWu]

The disabling can be done earlier at around line 808.

Done.

[david-xl]

what about printing an error and immediately aborting when LLVM_PROFILE_FILE is invalid?

I think this is better than falls back to default.profraw with a warning, which is hard to notice. Or we can simply allow multiple %c and enable continuous mode as this shouldn't cause any ambiguity. But multiple %m might cause ambiguity because it can have %1m and %2m at the same time and cause ambiguity.

Currently all parsing errors are treated as warnings. Perhaps a runtime option can be added in the future to turn them into hard errors.