From 58d4a39f630a4caf6faed2d4ce034bb27d88586a Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Sun, 29 Jul 2018 14:56:15 +0000 Subject: [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 --- tools/dsymutil/MachOUtils.cpp | 42 +++++++++++++++++++++++++++++------------- tools/dsymutil/MachOUtils.h | 17 +++++++++++++---- tools/dsymutil/dsymutil.cpp | 40 +++++++++++----------------------------- 3 files changed, 53 insertions(+), 46 deletions(-) (limited to 'tools') 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 T = sys::fs::TempFile::create(TmpModel); + + if (!T) + return T.takeError(); + + File = llvm::Optional(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 &Args) { return true; } -bool generateUniversalBinary(SmallVectorImpl &ArchFiles, +bool generateUniversalBinary(SmallVectorImpl &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 &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 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 File; + + llvm::Error createTempFile(); + llvm::StringRef path() const; + + ArchAndFile(StringRef Arch) : Arch(Arch) {} + ArchAndFile(ArchAndFile &&A) = default; + ~ArchAndFile(); }; -bool generateUniversalBinary(SmallVectorImpl &ArchFiles, +bool generateUniversalBinary(SmallVectorImpl &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 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 getOptions() { @@ -400,18 +393,6 @@ static Expected> getInputs(bool DsymAsInput) { return Inputs; } -namespace { -struct TempFileVector { - std::vector 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 TempFiles; - TempFileVector TempFileStore; + llvm::SmallVector 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 OS; std::string OutputFile = getOutputFileName(InputFile); if (NeedsTempFiles) { - Expected 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(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(TempFile.FD, + /*shouldClose*/ false); + OutputFile = TempFile.TmpName; } else { std::error_code EC; OS = std::make_shared(NoOutput ? "-" : OutputFile, EC, -- cgit v1.2.3