[clang] 9c4591d - [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2023-02-01T08:12:10-06:00
New Revision: 9c4591d7f3acaa00318900bdba4b4ba26c99c666

URL: https://github.com/llvm/llvm-project/commit/9c4591d7f3acaa00318900bdba4b4ba26c99c666
DIFF: https://github.com/llvm/llvm-project/commit/9c4591d7f3acaa00318900bdba4b4ba26c99c666.diff

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

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

Reviewed By: tianshilei1992

Differential Revision: https://reviews.llvm.org/D142985

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 3c92d34571044..6981e124b1062 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -77,6 +77,9 @@ 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;
 
@@ -200,6 +203,7 @@ std::string getMainExecutable(const char *Name) {
 
 /// 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);
@@ -1047,6 +1051,7 @@ linkAndWrapDeviceFiles(SmallVectorImpl<OffloadFile> &LinkerInputFiles,
           return createFileError(*OutputOrErr, EC);
       }
 
+      std::scoped_lock<decltype(ImageMtx)> Guard(ImageMtx);
       OffloadingImage TheImage{};
       TheImage.TheImageKind =
           Args.hasArg(OPT_embed_bitcode) ? IMG_Bitcode : IMG_Object;
@@ -1058,7 +1063,6 @@ linkAndWrapDeviceFiles(SmallVectorImpl<OffloadFile> &LinkerInputFiles,
            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();