[LinkerWrapper] Fix memory issues due to unguarded accesses to global state

[jhuber6]

Summary

There were intemittent errors in the linker wrapper when using the
sanitizers in parallel. First, this is because the TempFiles global
was not guarded when creating a new file. Second, even though the Args
list is passed as const, the internal state is mutable when adding a
string. So that needs to be guarded too.

Fixes https://github.com/llvm/llvm-project/issues/60437

[jhuber6]

If this fixes the issues on your side, please open a bug so it can be backported.

[uabelho]

@jhuber6
I really have no idea about the fix, but I've run the linker-wrapper.c testcase with asan built binaries with this fix 500 times now without seeing any failures.
Without the fix it fails in like 1 out of 10 runs so it surely looks like the fix does something good!

Nice!

[uabelho]

In D142985#4093783, @jhuber6 wrote:

If this fixes the issues on your side, please open a bug so it can be backported.

I wrote
https://github.com/llvm/llvm-project/issues/60437

[jhuber6]

In D142985#4095701, @uabelho wrote:

I wrote
https://github.com/llvm/llvm-project/issues/60437

Great, I'll land it once the patch is accepted.

[uabelho]

In D142985#4096297, @jhuber6 wrote:

In D142985#4095701, @uabelho wrote:

I wrote
https://github.com/llvm/llvm-project/issues/60437

Great, I'll land it once the patch is accepted.

Thanks! Thumbs up from me, unfortunately I don't feel confident to review the actual code change.

[tianshilei1992]

LG

clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

 static bool Verbose = false;
 
 /// Filename of the executable being created.
 static StringRef ExecutableName;
 
 /// Binary path for the CUDA installation.
 static std::string CudaBinaryPath;
 
+/// Mutex lock to protect writes to shared TempFiles in parallel.
+static std::mutex TempFilesMutex;
+
 /// Temporary files created by the linker wrapper.
 static std::list<SmallString<128>> TempFiles;
 
 /// Codegen flags for LTO backend.
 static codegen::RegisterCodeGenFlags CodeGenFlags;
 
 /// Global flag to indicate that the LTO pipeline threw an error.
 static std::atomic<bool> LTOError;

 std::string getMainExecutable(const char *Name) {
   void *Ptr = (void *)(intptr_t)&getMainExecutable;
   auto COWPath = sys::fs::getMainExecutable(Name, Ptr);
   return sys::path::parent_path(COWPath).str();
 }
 
 /// Get a temporary filename suitable for output.
 Expected<StringRef> createOutputFile(const Twine &Prefix, StringRef Extension) {
+  std::scoped_lock<decltype(TempFilesMutex)> Lock(TempFilesMutex);
   SmallString<128> OutputFile;
   if (SaveTemps) {
     (Prefix + "." + Extension).toNullTerminatedStringRef(OutputFile);
   } else {
     if (std::error_code EC =
             sys::fs::createTemporaryFile(Prefix, Extension, OutputFile))
       return createFileError(OutputFile, EC);
   }
for (OffloadKind Kind : ActiveOffloadKinds) {
           llvm::MemoryBuffer::getFileOrSTDIN(*OutputOrErr);
       if (std::error_code EC = FileOrErr.getError()) {
         if (DryRun)
           FileOrErr = MemoryBuffer::getMemBuffer("");
         else
           return createFileError(*OutputOrErr, EC);
       }
 
+      std::scoped_lock<decltype(ImageMtx)> Guard(ImageMtx);
       OffloadingImage TheImage{};
       TheImage.TheImageKind =
           Args.hasArg(OPT_embed_bitcode) ? IMG_Bitcode : IMG_Object;
       TheImage.TheOffloadKind = Kind;
       TheImage.StringData = {
           {"triple",
            Args.MakeArgString(LinkerArgs.getLastArgValue(OPT_triple_EQ))},
           {"arch",
            Args.MakeArgString(LinkerArgs.getLastArgValue(OPT_arch_EQ))}};
       TheImage.Image = std::move(*FileOrErr);
 
-      std::lock_guard<decltype(ImageMtx)> Guard(ImageMtx);
       Images[Kind].emplace_back(std::move(TheImage));
     }
     return Error::success();
   });
   if (Err)
     return std::move(Err);
 
   // Create a binary image of each offloading image and embed it into a new