Index: llvm/trunk/lib/Support/Path.cpp =================================================================== --- llvm/trunk/lib/Support/Path.cpp +++ llvm/trunk/lib/Support/Path.cpp @@ -1157,9 +1157,13 @@ setDeleteDisposition(H, true); #else std::error_code RenameEC = fs::rename(TmpName, Name); - // If we can't rename, discard the temporary file. - if (RenameEC) - remove(TmpName); + if (RenameEC) { + // If we can't rename, try to copy to work around cross-device link issues. + RenameEC = sys::fs::copy_file(TmpName, Name); + // If we can't rename or copy, discard the temporary file. + if (RenameEC) + remove(TmpName); + } sys::DontRemoveFileOnSignal(TmpName); #endif Index: llvm/trunk/lib/Support/Windows/Path.inc =================================================================== --- llvm/trunk/lib/Support/Windows/Path.inc +++ llvm/trunk/lib/Support/Windows/Path.inc @@ -450,7 +450,7 @@ if (std::error_code EC2 = realPathFromHandle(FromHandle, WideFrom)) return EC2; if (::MoveFileExW(WideFrom.begin(), WideTo.begin(), - MOVEFILE_REPLACE_EXISTING)) + MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED)) return std::error_code(); return mapWindowsError(GetLastError()); } Index: llvm/trunk/test/tools/dsymutil/X86/accelerator.test =================================================================== --- llvm/trunk/test/tools/dsymutil/X86/accelerator.test +++ llvm/trunk/test/tools/dsymutil/X86/accelerator.test @@ -1,7 +1,3 @@ -UNSUPPORTED: system-windows -Windows does not like renaming files that have open handles to them. We -need to use TempFile::keep to move them in a portable way. - RUN: dsymutil -accelerator=Dwarf -oso-prepend-path=%p/.. %p/../Inputs/basic.macho.x86_64 -o %t.dwarf.dSYM RUN: dsymutil -accelerator=Apple -oso-prepend-path=%p/.. %p/../Inputs/basic.macho.x86_64 -o %t.apple.dSYM Index: llvm/trunk/test/tools/dsymutil/X86/update-one-CU.test =================================================================== --- llvm/trunk/test/tools/dsymutil/X86/update-one-CU.test +++ llvm/trunk/test/tools/dsymutil/X86/update-one-CU.test @@ -1,7 +1,3 @@ -UNSUPPORTED: system-windows -Windows does not like renaming files that have open handles to them. We -need to use TempFile::keep to move them in a portable way. - RUN: dsymutil -oso-prepend-path=%p/.. %p/../Inputs/objc.macho.x86_64 -o %t.dSYM RUN: dsymutil -update %t.dSYM RUN: llvm-dwarfdump -apple-types -apple-objc %t.dSYM | FileCheck %s Index: llvm/trunk/test/tools/dsymutil/X86/update.test =================================================================== --- llvm/trunk/test/tools/dsymutil/X86/update.test +++ llvm/trunk/test/tools/dsymutil/X86/update.test @@ -1,7 +1,3 @@ -UNSUPPORTED: system-windows -Windows does not like renaming files that have open handles to them. We -need to use TempFile::keep to move them in a portable way. - RUN: rm -rf %t.dir RUN: mkdir -p %t.dir RUN: cat %p/../Inputs/basic.macho.x86_64 > %t.dir/basic Index: llvm/trunk/tools/dsymutil/MachOUtils.h =================================================================== --- llvm/trunk/tools/dsymutil/MachOUtils.h +++ llvm/trunk/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 @@ 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); Index: llvm/trunk/tools/dsymutil/MachOUtils.cpp =================================================================== --- llvm/trunk/tools/dsymutil/MachOUtils.cpp +++ llvm/trunk/tools/dsymutil/MachOUtils.cpp @@ -27,6 +27,27 @@ 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 @@ 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 @@ 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) { Index: llvm/trunk/tools/dsymutil/dsymutil.cpp =================================================================== --- llvm/trunk/tools/dsymutil/dsymutil.cpp +++ llvm/trunk/tools/dsymutil/dsymutil.cpp @@ -313,13 +313,6 @@ 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 @@ 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 @@ !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 @@ 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,