diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -10,6 +10,7 @@ let Component = "Frontend" in { +def err_fe_error_removing : Error<"error removing '%0': %1">; def err_fe_error_opening : Error<"error opening '%0': %1">; def err_fe_error_reading : Error<"error reading '%0'">; def err_fe_error_reading_stdin : Error<"error reading stdin: %0">; diff --git a/clang/include/clang/Frontend/PrecompiledPreamble.h b/clang/include/clang/Frontend/PrecompiledPreamble.h --- a/clang/include/clang/Frontend/PrecompiledPreamble.h +++ b/clang/include/clang/Frontend/PrecompiledPreamble.h @@ -17,6 +17,7 @@ #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/Support/AlignOf.h" +#include "llvm/Support/Error.h" #include "llvm/Support/MD5.h" #include #include @@ -156,7 +157,7 @@ llvm::StringRef getFilePath() const; private: - void RemoveFileIfPresent(); + llvm::Error RemoveFileIfPresent(); private: llvm::Optional FilePath; diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -2314,7 +2314,8 @@ } if (llvm::sys::fs::rename(TempPath, File)) { - llvm::sys::fs::remove(TempPath); + // Ignore failure to remove if we already fail to rename. + (void)llvm::sys::fs::remove(TempPath); return true; } diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -630,7 +630,9 @@ for (OutputFile &OF : OutputFiles) { if (!OF.TempFilename.empty()) { if (EraseFiles) { - llvm::sys::fs::remove(OF.TempFilename); + if (std::error_code EC = llvm::sys::fs::remove(OF.TempFilename)) + getDiagnostics().Report(diag::err_fe_error_removing) + << OF.TempFilename << EC.message(); } else { SmallString<128> NewOutFile(OF.Filename); @@ -642,16 +644,22 @@ getDiagnostics().Report(diag::err_unable_to_rename_temp) << OF.TempFilename << OF.Filename << ec.message(); - llvm::sys::fs::remove(OF.TempFilename); + if (std::error_code EC = llvm::sys::fs::remove(OF.TempFilename)) + getDiagnostics().Report(diag::err_fe_error_removing) + << OF.TempFilename << EC.message(); } } } else if (!OF.Filename.empty() && EraseFiles) - llvm::sys::fs::remove(OF.Filename); + if (std::error_code EC = llvm::sys::fs::remove(OF.Filename)) + getDiagnostics().Report(diag::err_fe_error_removing) + << OF.Filename << EC.message(); } OutputFiles.clear(); if (DeleteBuiltModules) { for (auto &Module : BuiltModules) - llvm::sys::fs::remove(Module.second); + if (std::error_code EC = llvm::sys::fs::remove(Module.second)) + getDiagnostics().Report(diag::err_fe_error_removing) + << Module.second << EC.message(); BuiltModules.clear(); } NonSeekStream.reset(); @@ -1433,18 +1441,20 @@ } // Remove the file. - llvm::sys::fs::remove(File->path()); + if ((EC = llvm::sys::fs::remove(File->path()))) + break; - // Remove the timestamp file. - std::string TimpestampFilename = File->path() + ".timestamp"; - llvm::sys::fs::remove(TimpestampFilename); + std::string TimestampFilename = File->path() + ".timestamp"; + if ((EC = llvm::sys::fs::remove(TimestampFilename))) + break; } // If we removed all of the files in the directory, remove the directory // itself. if (llvm::sys::fs::directory_iterator(Dir->path(), EC) == llvm::sys::fs::directory_iterator() && !EC) - llvm::sys::fs::remove(Dir->path()); + if ((EC = llvm::sys::fs::remove(Dir->path()))) + break; } } diff --git a/clang/lib/Frontend/DependencyFile.cpp b/clang/lib/Frontend/DependencyFile.cpp --- a/clang/lib/Frontend/DependencyFile.cpp +++ b/clang/lib/Frontend/DependencyFile.cpp @@ -311,7 +311,8 @@ void DependencyFileGenerator::outputDependencyFile(DiagnosticsEngine &Diags) { if (SeenMissingHeader) { - llvm::sys::fs::remove(OutputFile); + if (std::error_code EC = llvm::sys::fs::remove(OutputFile)) + Diags.Report(diag::err_fe_error_removing) << OutputFile << EC.message(); return; } diff --git a/clang/lib/Frontend/PrecompiledPreamble.cpp b/clang/lib/Frontend/PrecompiledPreamble.cpp --- a/clang/lib/Frontend/PrecompiledPreamble.cpp +++ b/clang/lib/Frontend/PrecompiledPreamble.cpp @@ -92,7 +92,7 @@ void addFile(StringRef File); /// Remove \p File from disk and from the set of tracked files. - void removeFile(StringRef File); + llvm::Error removeFile(StringRef File); private: llvm::sys::SmartMutex Mutex; @@ -107,7 +107,8 @@ TemporaryFiles::~TemporaryFiles() { llvm::MutexGuard Guard(Mutex); for (const auto &File : Files) - llvm::sys::fs::remove(File.getKey()); + if (std::error_code EC = llvm::sys::fs::remove(File.getKey())) + llvm::report_fatal_error("failed removing file \"" + File.getKey() + "\": " + EC.message()); } void TemporaryFiles::addFile(StringRef File) { @@ -117,12 +118,14 @@ assert(IsInserted && "File has already been added"); } -void TemporaryFiles::removeFile(StringRef File) { +llvm::Error TemporaryFiles::removeFile(StringRef File) { llvm::MutexGuard Guard(Mutex); auto WasPresent = Files.erase(File); (void)WasPresent; assert(WasPresent && "File was not tracked"); - llvm::sys::fs::remove(File); + if (std::error_code EC = llvm::sys::fs::remove(File)) + return llvm::createStringError(EC, "failed removing \"" + File + "\""); + return llvm::Error::success(); } class PrecompilePreambleAction : public ASTFrontendAction { @@ -572,20 +575,26 @@ PrecompiledPreamble::TempPCHFile &PrecompiledPreamble::TempPCHFile:: operator=(TempPCHFile &&Other) { - RemoveFileIfPresent(); + if (llvm::Error E = RemoveFileIfPresent()) + llvm::report_fatal_error("failed assigning temporary PCH file: " + toString(std::move(E))); FilePath = std::move(Other.FilePath); Other.FilePath = None; return *this; } -PrecompiledPreamble::TempPCHFile::~TempPCHFile() { RemoveFileIfPresent(); } +PrecompiledPreamble::TempPCHFile::~TempPCHFile() { + if (llvm::Error E = RemoveFileIfPresent()) + llvm::report_fatal_error("failed destroying temporary PCH file: " + toString(std::move(E))); +} -void PrecompiledPreamble::TempPCHFile::RemoveFileIfPresent() { +llvm::Error PrecompiledPreamble::TempPCHFile::RemoveFileIfPresent() { if (FilePath) { - TemporaryFiles::getInstance().removeFile(*FilePath); + if (llvm::Error E = TemporaryFiles::getInstance().removeFile(*FilePath)) + return E; FilePath = None; } + return llvm::Error::success(); } llvm::StringRef PrecompiledPreamble::TempPCHFile::getFilePath() const { diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp --- a/clang/lib/Rewrite/Rewriter.cpp +++ b/clang/lib/Rewrite/Rewriter.cpp @@ -434,7 +434,7 @@ << TempFilename << Filename << ec.message(); // If the remove fails, there's not a lot we can do - this is already an // error. - llvm::sys::fs::remove(TempFilename); + (void)llvm::sys::fs::remove(TempFilename); } } diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp --- a/clang/lib/Serialization/GlobalModuleIndex.cpp +++ b/clang/lib/Serialization/GlobalModuleIndex.cpp @@ -932,12 +932,13 @@ return llvm::createStringError(Out.error(), "failed writing to stream"); // Remove the old index file. It isn't relevant any more. - llvm::sys::fs::remove(IndexPath); + if (std::error_code EC = llvm::sys::fs::remove(IndexPath)) + return llvm::createStringError(EC, "failed removing \"" + IndexPath + "\""); // Rename the newly-written index file to the proper name. if (std::error_code Err = llvm::sys::fs::rename(IndexTmpPath, IndexPath)) { // Remove the file on failure, don't check whether removal succeeded. - llvm::sys::fs::remove(IndexTmpPath); + (void)llvm::sys::fs::remove(IndexTmpPath); return llvm::createStringError(Err, "failed renaming file \"%s\" to \"%s\"", IndexTmpPath.c_str(), IndexPath.c_str()); } diff --git a/clang/tools/driver/cc1gen_reproducer_main.cpp b/clang/tools/driver/cc1gen_reproducer_main.cpp --- a/clang/tools/driver/cc1gen_reproducer_main.cpp +++ b/clang/tools/driver/cc1gen_reproducer_main.cpp @@ -190,6 +190,11 @@ } // Remove the input file. - llvm::sys::fs::remove(Input); + if (std::error_code EC = llvm::sys::fs::remove(Input)) { + llvm::errs() << "error: failed to remove file " << Input << ": " + << EC.message() << "\n"; + return 1; + } + return Result; } diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h --- a/llvm/include/llvm/Support/Error.h +++ b/llvm/include/llvm/Support/Error.h @@ -1170,6 +1170,10 @@ Error createStringError(std::error_code EC, char const *Msg); +static inline Error createStringError(std::error_code EC, const Twine &S) { + return createStringError(EC, S.str().c_str()); +} + template inline Error createStringError(std::errc EC, char const *Fmt, const Ts &... Vals) { diff --git a/llvm/include/llvm/Support/FileUtilities.h b/llvm/include/llvm/Support/FileUtilities.h --- a/llvm/include/llvm/Support/FileUtilities.h +++ b/llvm/include/llvm/Support/FileUtilities.h @@ -49,8 +49,8 @@ ~FileRemover() { if (DeleteIt) { - // Ignore problems deleting the file. - sys::fs::remove(Filename); + if (std::error_code EC = sys::fs::remove(Filename)) + report_fatal_error("failed removing file \"" + Filename + "\": " + EC.message()); } } @@ -59,8 +59,8 @@ /// had ownership of a file, remove it first. void setFile(const Twine& filename, bool deleteIt = true) { if (DeleteIt) { - // Ignore problems deleting the file. - sys::fs::remove(Filename); + if (std::error_code EC = sys::fs::remove(Filename)) + report_fatal_error("failed removing file \"" + Filename + "\": " + EC.message()); } Filename.clear(); diff --git a/llvm/lib/Support/GraphWriter.cpp b/llvm/lib/Support/GraphWriter.cpp --- a/llvm/lib/Support/GraphWriter.cpp +++ b/llvm/lib/Support/GraphWriter.cpp @@ -98,7 +98,8 @@ errs() << "Error: " << ErrMsg << "\n"; return true; } - sys::fs::remove(Filename); + if (std::error_code EC = sys::fs::remove(Filename)) + errs() << "Failed removing file\"" << Filename << "\": " << EC.message() << '\n'; errs() << " done. \n"; } else { sys::ExecuteNoWait(ExecPath, args, None, {}, 0, &ErrMsg); diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp --- a/llvm/lib/Support/LockFileManager.cpp +++ b/llvm/lib/Support/LockFileManager.cpp @@ -47,14 +47,15 @@ /// \param LockFileName The name of the lock file to read. /// /// \returns The process ID of the process that owns this lock file -Optional > +Optional> LockFileManager::readLockFile(StringRef LockFileName) { // Read the owning host and PID out of the lock file. If it appears that the // owning process is dead, the lock file is invalid. ErrorOr> MBOrErr = MemoryBuffer::getFile(LockFileName); if (!MBOrErr) { - sys::fs::remove(LockFileName); + if (std::error_code EC = sys::fs::remove(LockFileName)) + report_fatal_error("Unable to remove invalid lock file \"" + LockFileName + "\": " + EC.message()); return None; } MemoryBuffer &MB = *MBOrErr.get(); @@ -71,7 +72,8 @@ } // Delete the lock file. It's invalid anyway. - sys::fs::remove(LockFileName); + if (std::error_code EC = sys::fs::remove(LockFileName)) + report_fatal_error("Unable to remove invalid lock file \"" + LockFileName + "\": " + EC.message()); return None; } @@ -144,7 +146,8 @@ // released. return; } - sys::fs::remove(Filename); + if (std::error_code EC = sys::fs::remove(Filename)) + report_fatal_error("Unable to remove unique lock file on signal \"" + Filename + "\": " + EC.message()); sys::DontRemoveFileOnSignal(Filename); } @@ -204,8 +207,9 @@ // unique lock file, and fail. std::string S("failed to write to "); S.append(UniqueLockFileName.str()); + if (std::error_code EC = sys::fs::remove(UniqueLockFileName)) + S.append("also failed to remove the lockfile"); setError(Out.error(), S); - sys::fs::remove(UniqueLockFileName); return; } } @@ -234,8 +238,12 @@ // Someone else managed to create the lock file first. Read the process ID // from the lock file. if ((Owner = readLockFile(LockFileName))) { - // Wipe out our unique lock file (it's useless now) - sys::fs::remove(UniqueLockFileName); + // Wipe out our unique lock file (it's useless now). + if ((EC = sys::fs::remove(UniqueLockFileName))) { + std::string S("failed to remove useless lockfile "); + S.append(UniqueLockFileName.str()); + setError(EC, S); + } return; } @@ -283,8 +291,10 @@ return; // Since we own the lock, remove the lock file and our own unique lock file. - sys::fs::remove(LockFileName); - sys::fs::remove(UniqueLockFileName); + if (std::error_code EC = sys::fs::remove(LockFileName)) + report_fatal_error("failed removing file\"" + LockFileName + "\": " + EC.message()); + if (std::error_code EC = sys::fs::remove(UniqueLockFileName)) + report_fatal_error("failed removing file\"" + UniqueLockFileName + "\": " + EC.message()); // The unique file is now gone, so remove it from the signal handler. This // matches a sys::RemoveFileOnSignal() in LockFileManager(). sys::DontRemoveFileOnSignal(UniqueLockFileName); diff --git a/llvm/lib/Support/Path.cpp b/llvm/lib/Support/Path.cpp --- a/llvm/lib/Support/Path.cpp +++ b/llvm/lib/Support/Path.cpp @@ -1182,9 +1182,10 @@ 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 we can't rename or copy, discard the temporary file, ignoring any + // further failure. if (RenameEC) - remove(TmpName); + (void)remove(TmpName); } sys::DontRemoveFileOnSignal(TmpName); #endif diff --git a/llvm/lib/Support/ToolOutputFile.cpp b/llvm/lib/Support/ToolOutputFile.cpp --- a/llvm/lib/Support/ToolOutputFile.cpp +++ b/llvm/lib/Support/ToolOutputFile.cpp @@ -25,7 +25,8 @@ ToolOutputFile::CleanupInstaller::~CleanupInstaller() { // Delete the file if the client hasn't told us not to. if (!Keep && Filename != "-") - sys::fs::remove(Filename); + if (std::error_code EC = sys::fs::remove(Filename)) + report_fatal_error("Failed removing file \"" + Filename + "\": " + EC.message()); // Ok, the file is successfully written and closed, or deleted. There's no // further need to clean it up on signals.