[dsymutil] Simplify temporary file handling.

Dsymutil's update functionality was broken on Windows because we tried
to rename a file while we're holding open handles to that file. TempFile
provides a solution for this through its keep(Twine) method. This patch
changes dsymutil to make use of that functionality.

Differential revision: https://reviews.llvm.org/D49860

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@338216 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 53 insertions, 46 deletions

diff --git a/tools/dsymutil/MachOUtils.cpp b/tools/dsymutil/MachOUtils.cpp
index eda530b810c..fc498cc49c1 100644
--- a/tools/dsymutil/MachOUtils.cpp
+++ b/tools/dsymutil/MachOUtils.cpp
@@ -27,6 +27,27 @@ namespace llvm {
 namespace dsymutil {
 namespace MachOUtils {
 
+llvm::Error ArchAndFile::createTempFile() {
+  llvm::SmallString<128> TmpModel;
+  llvm::sys::path::system_temp_directory(true, TmpModel);
+  llvm::sys::path::append(TmpModel, "dsym.tmp%%%%%.dwarf");
+  Expected<sys::fs::TempFile> T = sys::fs::TempFile::create(TmpModel);
+
+  if (!T)
+    return T.takeError();
+
+  File = llvm::Optional<sys::fs::TempFile>(std::move(*T));
+  return Error::success();
+}
+
+llvm::StringRef ArchAndFile::path() const { return File->TmpName; }
+
+ArchAndFile::~ArchAndFile() {
+  if (File)
+    if (auto E = File->discard())
+      llvm::consumeError(std::move(E));
+}
+
 std::string getArchName(StringRef Arch) {
   if (Arch.startswith("thumb"))
     return (llvm::Twine("arm") + Arch.drop_front(5)).str();
@@ -53,21 +74,16 @@ static bool runLipo(StringRef SDKPath, SmallVectorImpl<StringRef> &Args) {
   return true;
 }
 
-bool generateUniversalBinary(SmallVectorImpl<ArchAndFilename> &ArchFiles,
+bool generateUniversalBinary(SmallVectorImpl<ArchAndFile> &ArchFiles,
                              StringRef OutputFileName,
                              const LinkOptions &Options, StringRef SDKPath) {
-  // No need to merge one file into a universal fat binary. First, try
-  // to move it (rename) to the final location. If that fails because
-  // of cross-device link issues then copy and delete.
+  // No need to merge one file into a universal fat binary.
   if (ArchFiles.size() == 1) {
-    StringRef From(ArchFiles.front().Path);
-    if (sys::fs::rename(From, OutputFileName)) {
-      if (std::error_code EC = sys::fs::copy_file(From, OutputFileName)) {
-        WithColor::error() << "while copying " << From << " to "
-                           << OutputFileName << ": " << EC.message() << "\n";
-        return false;
-      }
-      sys::fs::remove(From);
+    if (auto E = ArchFiles.front().File->keep(OutputFileName)) {
+      WithColor::error() << "while keeping " << ArchFiles.front().path()
+                         << " as " << OutputFileName << ": "
+                         << toString(std::move(E)) << "\n";
+      return false;
     }
     return true;
   }
@@ -77,7 +93,7 @@ bool generateUniversalBinary(SmallVectorImpl<ArchAndFilename> &ArchFiles,
   Args.push_back("-create");
 
   for (auto &Thin : ArchFiles)
-    Args.push_back(Thin.Path);
+    Args.push_back(Thin.path());
 
   // Align segments to match dsymutil-classic alignment
   for (auto &Thin : ArchFiles) {
diff --git a/tools/dsymutil/MachOUtils.h b/tools/dsymutil/MachOUtils.h
index 0db8ed1a1e3..a8be89e906b 100644
--- a/tools/dsymutil/MachOUtils.h
+++ b/tools/dsymutil/MachOUtils.h
@@ -10,6 +10,7 @@
 #define LLVM_TOOLS_DSYMUTIL_MACHOUTILS_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FileSystem.h"
 #include <string>
 
 namespace llvm {
@@ -20,12 +21,20 @@ class DebugMap;
 struct LinkOptions;
 namespace MachOUtils {
 
-struct ArchAndFilename {
-  std::string Arch, Path;
-  ArchAndFilename(StringRef Arch, StringRef Path) : Arch(Arch), Path(Path) {}
+struct ArchAndFile {
+  std::string Arch;
+  // Optional because TempFile has no default constructor.
+  Optional<llvm::sys::fs::TempFile> File;
+
+  llvm::Error createTempFile();
+  llvm::StringRef path() const;
+
+  ArchAndFile(StringRef Arch) : Arch(Arch) {}
+  ArchAndFile(ArchAndFile &&A) = default;
+  ~ArchAndFile();
 };
 
-bool generateUniversalBinary(SmallVectorImpl<ArchAndFilename> &ArchFiles,
+bool generateUniversalBinary(SmallVectorImpl<ArchAndFile> &ArchFiles,
                              StringRef OutputFileName, const LinkOptions &,
                              StringRef SDKPath);
 
diff --git a/tools/dsymutil/dsymutil.cpp b/tools/dsymutil/dsymutil.cpp
index fc447b30be9..c0e6d505941 100644
--- a/tools/dsymutil/dsymutil.cpp
+++ b/tools/dsymutil/dsymutil.cpp
@@ -313,13 +313,6 @@ static std::string getOutputFileName(llvm::StringRef InputFile) {
   return BundleDir.str();
 }
 
-static Expected<sys::fs::TempFile> createTempFile() {
-  llvm::SmallString<128> TmpModel;
-  llvm::sys::path::system_temp_directory(true, TmpModel);
-  llvm::sys::path::append(TmpModel, "dsym.tmp%%%%%.dwarf");
-  return sys::fs::TempFile::create(TmpModel);
-}
-
 /// Parses the command line options into the LinkOptions struct and performs
 /// some sanity checking. Returns an error in case the latter fails.
 static Expected<LinkOptions> getOptions() {
@@ -400,18 +393,6 @@ static Expected<std::vector<std::string>> getInputs(bool DsymAsInput) {
   return Inputs;
 }
 
-namespace {
-struct TempFileVector {
-  std::vector<sys::fs::TempFile> Files;
-  ~TempFileVector() {
-    for (sys::fs::TempFile &Tmp : Files) {
-      if (Error E = Tmp.discard())
-        errs() << toString(std::move(E));
-    }
-  }
-};
-} // namespace
-
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
 
@@ -523,8 +504,7 @@ int main(int argc, char **argv) {
         !DumpDebugMap && (OutputFileOpt != "-") &&
         (DebugMapPtrsOrErr->size() != 1 || OptionsOrErr->Update);
 
-    llvm::SmallVector<MachOUtils::ArchAndFilename, 4> TempFiles;
-    TempFileVector TempFileStore;
+    llvm::SmallVector<MachOUtils::ArchAndFile, 4> TempFiles;
     std::atomic_char AllOK(1);
     for (auto &Map : *DebugMapPtrsOrErr) {
       if (Verbose || DumpDebugMap)
@@ -543,16 +523,18 @@ int main(int argc, char **argv) {
       std::shared_ptr<raw_fd_ostream> OS;
       std::string OutputFile = getOutputFileName(InputFile);
       if (NeedsTempFiles) {
-        Expected<sys::fs::TempFile> T = createTempFile();
-        if (!T) {
-          errs() << toString(T.takeError());
+        TempFiles.emplace_back(Map->getTriple().getArchName().str());
+
+        auto E = TempFiles.back().createTempFile();
+        if (E) {
+          errs() << toString(std::move(E));
           return 1;
         }
-        OS = std::make_shared<raw_fd_ostream>(T->FD, /*shouldClose*/ false);
-        OutputFile = T->TmpName;
-        TempFileStore.Files.push_back(std::move(*T));
-        TempFiles.emplace_back(Map->getTriple().getArchName().str(),
-                               OutputFile);
+
+        auto &TempFile = *(TempFiles.back().File);
+        OS = std::make_shared<raw_fd_ostream>(TempFile.FD,
+                                              /*shouldClose*/ false);
+        OutputFile = TempFile.TmpName;
       } else {
         std::error_code EC;
         OS = std::make_shared<raw_fd_ostream>(NoOutput ? "-" : OutputFile, EC,