llvm-profgen: Fix race condition

[MatzeB]

Fix race condition when multiple instances of llvm-progen read from the same
inputs.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Matthias Braun (MatzeB)

Changes

Fix race condition when multiple instances of llvm-progen read from the same
inputs.

---

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

1 Files Affected:

• (modified) llvm/tools/llvm-profgen/PerfReader.cpp (+8-3)

diff --git a/llvm/tools/llvm-profgen/PerfReader.cpp b/llvm/tools/llvm-profgen/PerfReader.cpp
index 313d40483a2591..df952734fbf939 100644
--- a/llvm/tools/llvm-profgen/PerfReader.cpp
+++ b/llvm/tools/llvm-profgen/PerfReader.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 #include "PerfReader.h"
 #include "ProfileGenerator.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/DebugInfo/Symbolize/SymbolizableModule.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Process.h"
@@ -361,8 +362,11 @@ PerfScriptReader::convertPerfDataToTrace(ProfiledBinary *Binary,
     exitWithError("Perf not found.");
   }
   std::string PerfPath = *PerfExecutable;
-  std::string PerfTraceFile = PerfData.str() + ".script.tmp";
-  std::string ErrorFile = PerfData.str() + ".script.err.tmp";
+
+  SmallString<128> PerfTraceFile;
+  sys::fs::createUniquePath("perf-script-%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%.tmp",
+                            PerfTraceFile, /*MakeAbsolute=*/true);
+  std::string ErrorFile = std::string(PerfTraceFile) + ".script.err.tmp";
   StringRef ScriptMMapArgs[] = {PerfPath, "script",   "--show-mmap-events",
                                 "-F",     "comm,pid", "-i",
                                 PerfData};
@@ -400,7 +404,8 @@ PerfScriptReader::convertPerfDataToTrace(ProfiledBinary *Binary,
                                   PIDs,     "-i",         PerfData};
   sys::ExecuteAndWait(PerfPath, ScriptSampleArgs, std::nullopt, Redirects);
 
-  return {PerfTraceFile, PerfFormat::PerfScript, PerfContent::UnknownContent};
+  return {std::string(PerfTraceFile), PerfFormat::PerfScript,
+          PerfContent::UnknownContent};
 }
 
 void PerfScriptReader::updateBinaryAddress(const MMapEvent &Event) {

[wlei-llvm]

LGTM, thanks!

[MatzeB]

test failure on windows bot seems unrelated, merging

[HaohaiWen]

@MatzeB , why do we need to create a uniq temp perf-script for each llvm-profgen read?

[MatzeB]

@MatzeB , why do we need to create a uniq temp perf-script for each llvm-profgen read?

Without this change, when you have two llvm-profgen instances running on the same input at the same time they will overwrite each others intermediate files and break (i.e. first caller starts conversion; at the same time 2nd caller starts conversion writing to the same file; then if first caller starts reading it may read a half-finished conversion from the 2nd caller).
The previous code also had downsides like needing the directory containing the profile file to be writable which isn't necessarily true or expected by the user.

[MatzeB]

Are you having trouble with temporary files accumulating and you running out of disk space? You could probably submit a patch then that the temporary file is deleted after it was read...

[HaohaiWen]

Are you having trouble with temporary files accumulating and you running out of disk space? You could probably submit a patch then that the temporary file is deleted after it was read...

Yeah, I run out of disk for large project.