From 633dcdc52d0832cd756db350103d457e5448ddf8 Mon Sep 17 00:00:00 2001 From: Chad Rosier Date: Thu, 24 Jan 2013 19:14:47 +0000 Subject: [PATCH] [driver] Associate a JobAction with each result file. This enables the driver to delete result files for only those commands that fail. Part of rdar://12984531 llvm-svn: 173361 --- clang/include/clang/Driver/Compilation.h | 35 +++++++++---- clang/include/clang/Driver/Util.h | 5 ++ clang/lib/Driver/Compilation.cpp | 67 +++++++++++++++--------- clang/lib/Driver/Driver.cpp | 14 ++--- clang/lib/Driver/Tools.cpp | 7 +-- clang/lib/Driver/Tools.h | 1 + clang/test/Driver/output-file-cleanup.c | 19 +++++-- 7 files changed, 102 insertions(+), 46 deletions(-) diff --git a/clang/include/clang/Driver/Compilation.h b/clang/include/clang/Driver/Compilation.h index 5f63aa768857..1872050522f0 100644 --- a/clang/include/clang/Driver/Compilation.h +++ b/clang/include/clang/Driver/Compilation.h @@ -20,6 +20,7 @@ namespace driver { class DerivedArgList; class Driver; class InputArgList; + class JobAction; class JobList; class ToolChain; @@ -54,11 +55,11 @@ class Compilation { ArgStringList TempFiles; /// Result files which should be removed on failure. - ArgStringList ResultFiles; + ArgStringMap ResultFiles; /// Result files which are generated correctly on failure, and which should /// only be removed if we crash. - ArgStringList FailureResultFiles; + ArgStringMap FailureResultFiles; /// Redirection for stdout, stderr, etc. const llvm::sys::Path **Redirects; @@ -88,9 +89,9 @@ public: const ArgStringList &getTempFiles() const { return TempFiles; } - const ArgStringList &getResultFiles() const { return ResultFiles; } + const ArgStringMap &getResultFiles() const { return ResultFiles; } - const ArgStringList &getFailureResultFiles() const { + const ArgStringMap &getFailureResultFiles() const { return FailureResultFiles; } @@ -113,24 +114,40 @@ public: /// addResultFile - Add a file to remove on failure, and returns its /// argument. - const char *addResultFile(const char *Name) { - ResultFiles.push_back(Name); + const char *addResultFile(const char *Name, const JobAction *JA) { + ResultFiles[JA] = Name; return Name; } /// addFailureResultFile - Add a file to remove if we crash, and returns its /// argument. - const char *addFailureResultFile(const char *Name) { - FailureResultFiles.push_back(Name); + const char *addFailureResultFile(const char *Name, const JobAction *JA) { + FailureResultFiles[JA] = Name; return Name; } + /// CleanupFile - Delete a given file. + /// + /// \param IssueErrors - Report failures as errors. + /// \return Whether the file was removed successfully. + bool CleanupFile(const char *File, bool IssueErrors = false) const; + /// CleanupFileList - Remove the files in the given list. /// /// \param IssueErrors - Report failures as errors. /// \return Whether all files were removed successfully. bool CleanupFileList(const ArgStringList &Files, - bool IssueErrors=false) const; + bool IssueErrors = false) const; + + /// CleanupFileMap - Remove the files in the given map. + /// + /// \param JA - If specified, only delete the files associated with this + /// JobAction. Otherwise, delete all files in the map. + /// \param IssueErrors - Report failures as errors. + /// \return Whether all files were removed successfully. + bool CleanupFileMap(const ArgStringMap &Files, + const JobAction *JA, + bool IssueErrors = false) const; /// PrintJob - Print one job in -### format. /// diff --git a/clang/include/clang/Driver/Util.h b/clang/include/clang/Driver/Util.h index 65aef4b31025..06b82b977fe0 100644 --- a/clang/include/clang/Driver/Util.h +++ b/clang/include/clang/Driver/Util.h @@ -11,14 +11,19 @@ #define CLANG_DRIVER_UTIL_H_ #include "clang/Basic/LLVM.h" +#include "llvm/ADT/DenseMap.h" namespace clang { namespace driver { class Action; + class JobAction; /// ArgStringList - Type used for constructing argv lists for subprocesses. typedef SmallVector ArgStringList; + /// ArgStringMap - Type used to map a JobAction to its result file. + typedef llvm::DenseMap ArgStringMap; + /// ActionList - Type used for lists of actions. typedef SmallVector ActionList; diff --git a/clang/lib/Driver/Compilation.cpp b/clang/lib/Driver/Compilation.cpp index 9c7100fb80c4..96e6048551ce 100644 --- a/clang/lib/Driver/Compilation.cpp +++ b/clang/lib/Driver/Compilation.cpp @@ -199,39 +199,56 @@ void Compilation::PrintDiagnosticJob(raw_ostream &OS, const Job &J) const { } } +bool Compilation::CleanupFile(const char *File, bool IssueErrors) const { + llvm::sys::Path P(File); + std::string Error; + + // Don't try to remove files which we don't have write access to (but may be + // able to remove), or non-regular files. Underlying tools may have + // intentionally not overwritten them. + if (!P.canWrite() || !P.isRegularFile()) + return true; + + if (P.eraseFromDisk(false, &Error)) { + // Failure is only failure if the file exists and is "regular". There is + // a race condition here due to the limited interface of + // llvm::sys::Path, we want to know if the removal gave ENOENT. + + // FIXME: Grumble, P.exists() is broken. PR3837. + struct stat buf; + if (::stat(P.c_str(), &buf) == 0 ? (buf.st_mode & S_IFMT) == S_IFREG : + (errno != ENOENT)) { + if (IssueErrors) + getDriver().Diag(clang::diag::err_drv_unable_to_remove_file) + << Error; + return false; + } + } + return true; +} + bool Compilation::CleanupFileList(const ArgStringList &Files, bool IssueErrors) const { bool Success = true; - for (ArgStringList::const_iterator + it = Files.begin(), ie = Files.end(); it != ie; ++it) + Success &= CleanupFile(*it, IssueErrors); + return Success; +} + +bool Compilation::CleanupFileMap(const ArgStringMap &Files, + const JobAction *JA, + bool IssueErrors) const { + bool Success = true; + for (ArgStringMap::const_iterator it = Files.begin(), ie = Files.end(); it != ie; ++it) { - llvm::sys::Path P(*it); - std::string Error; - - // Don't try to remove files which we don't have write access to (but may be - // able to remove), or non-regular files. Underlying tools may have - // intentionally not overwritten them. - if (!P.canWrite() || !P.isRegularFile()) + // If specified, only delete the files associated with the JobAction. + // Otherwise, delete all files in the map. + if (JA && it->first != JA) continue; - - if (P.eraseFromDisk(false, &Error)) { - // Failure is only failure if the file exists and is "regular". There is - // a race condition here due to the limited interface of - // llvm::sys::Path, we want to know if the removal gave ENOENT. - - // FIXME: Grumble, P.exists() is broken. PR3837. - struct stat buf; - if (::stat(P.c_str(), &buf) == 0 ? (buf.st_mode & S_IFMT) == S_IFREG : - (errno != ENOENT)) { - if (IssueErrors) - getDriver().Diag(clang::diag::err_drv_unable_to_remove_file) - << Error; - Success = false; - } - } + Success &= CleanupFile(it->second, IssueErrors); } - return Success; } diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index dcb3c5fd0987..dda7099a2fc6 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -485,8 +485,9 @@ void Driver::generateCompilationDiagnostics(Compilation &C, << "\n\n********************"; } else { // Failure, remove preprocessed files. - if (!C.getArgs().hasArg(options::OPT_save_temps)) + if (!C.getArgs().hasArg(options::OPT_save_temps)) { C.CleanupFileList(C.getTempFiles(), true); + } Diag(clang::diag::note_drv_command_failed_diag_msg) << "Error generating preprocessed source(s)."; @@ -516,11 +517,12 @@ int Driver::ExecuteCompilation(const Compilation &C, // Otherwise, remove result files as well. if (!C.getArgs().hasArg(options::OPT_save_temps)) { - C.CleanupFileList(C.getResultFiles(), true); + const JobAction *JA = cast(&FailingCommand->getSource()); + C.CleanupFileMap(C.getResultFiles(), JA, true); // Failure result files are valid unless we crashed. if (Res < 0) - C.CleanupFileList(C.getFailureResultFiles(), true); + C.CleanupFileMap(C.getFailureResultFiles(), JA, true); } // Print extra information about abnormal failures, if possible. @@ -1417,7 +1419,7 @@ const char *Driver::GetNamedOutputPath(Compilation &C, if (AtTopLevel && !isa(JA) && !isa(JA)) { if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o)) - return C.addResultFile(FinalOutput->getValue()); + return C.addResultFile(FinalOutput->getValue(), &JA); } // Default to writing to stdout? @@ -1487,9 +1489,9 @@ const char *Driver::GetNamedOutputPath(Compilation &C, BasePath = NamedOutput; else llvm::sys::path::append(BasePath, NamedOutput); - return C.addResultFile(C.getArgs().MakeArgString(BasePath.c_str())); + return C.addResultFile(C.getArgs().MakeArgString(BasePath.c_str()), &JA); } else { - return C.addResultFile(NamedOutput); + return C.addResultFile(NamedOutput, &JA); } } diff --git a/clang/lib/Driver/Tools.cpp b/clang/lib/Driver/Tools.cpp index 309e149ef121..69340e62aed6 100644 --- a/clang/lib/Driver/Tools.cpp +++ b/clang/lib/Driver/Tools.cpp @@ -228,6 +228,7 @@ static bool forwardToGCC(const Option &O) { } void Clang::AddPreprocessingOptions(Compilation &C, + const JobAction &JA, const Driver &D, const ArgList &Args, ArgStringList &CmdArgs, @@ -248,7 +249,7 @@ void Clang::AddPreprocessingOptions(Compilation &C, const char *DepFile; if (Arg *MF = Args.getLastArg(options::OPT_MF)) { DepFile = MF->getValue(); - C.addFailureResultFile(DepFile); + C.addFailureResultFile(DepFile, &JA); } else if (Output.getType() == types::TY_Dependencies) { DepFile = Output.getFilename(); } else if (A->getOption().matches(options::OPT_M) || @@ -256,7 +257,7 @@ void Clang::AddPreprocessingOptions(Compilation &C, DepFile = "-"; } else { DepFile = getDependencyFileName(Args, Inputs); - C.addFailureResultFile(DepFile); + C.addFailureResultFile(DepFile, &JA); } CmdArgs.push_back("-dependency-file"); CmdArgs.push_back(DepFile); @@ -2300,7 +2301,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, // // FIXME: Support -fpreprocessed if (types::getPreprocessedType(InputType) != types::TY_INVALID) - AddPreprocessingOptions(C, D, Args, CmdArgs, Output, Inputs); + AddPreprocessingOptions(C, JA, D, Args, CmdArgs, Output, Inputs); // Don't warn about "clang -c -DPIC -fPIC test.i" because libtool.m4 assumes // that "The compiler can only warn and ignore the option if not recognized". diff --git a/clang/lib/Driver/Tools.h b/clang/lib/Driver/Tools.h index dcfd3114ddbb..a2b3c07b0c66 100644 --- a/clang/lib/Driver/Tools.h +++ b/clang/lib/Driver/Tools.h @@ -40,6 +40,7 @@ namespace tools { private: void AddPreprocessingOptions(Compilation &C, + const JobAction &JA, const Driver &D, const ArgList &Args, ArgStringList &CmdArgs, diff --git a/clang/test/Driver/output-file-cleanup.c b/clang/test/Driver/output-file-cleanup.c index 0a0c96001b59..19c0badfdbe5 100644 --- a/clang/test/Driver/output-file-cleanup.c +++ b/clang/test/Driver/output-file-cleanup.c @@ -1,15 +1,15 @@ // RUN: touch %t.o -// RUN: not %clang -DCRASH -o %t.o -MMD -MF %t.d %s +// RUN: not %clang -c -DCRASH -o %t.o -MMD -MF %t.d %s // RUN: test ! -f %t.o // RUN: test ! -f %t.d // RUN: touch %t.o -// RUN: not %clang -DMISSING -o %t.o -MMD -MF %t.d %s +// RUN: not %clang -c -DMISSING -o %t.o -MMD -MF %t.d %s // RUN: test ! -f %t.o // RUN: test ! -f %t.d // RUN: touch %t.o -// RUN: not %clang -o %t.o -MMD -MF %t.d %s +// RUN: not %clang -c -o %t.o -MMD -MF %t.d %s // RUN: test ! -f %t.o // RUN: test -f %t.d @@ -23,3 +23,16 @@ #else invalid C code #endif + +// RUN: touch %t1.c +// RUN: echo "invalid C code" > %t2.c +// RUN: cd %T && not %clang -c %t1.c %t2.c +// RUN: test -f %t1.o +// RUN: test ! -f %t2.o + +// RUN: touch %t1.c +// RUN: touch %t2.c +// RUN: chmod -r %t2.c +// RUN: cd %T && not %clang -c %t1.c %t2.c +// RUN: test -f %t1.o +// RUN: test ! -f %t2.o