That means the test is bad. I'm not at a place where I can do much about this now. Feel free to fail the test and I'll work on a new test case tomorrow morning.
Chad On Jan 24, 2013, at 7:40 PM, NAKAMURA Takumi <[email protected]> wrote: > It doesn't work as expected if the driver is running with -no-integrated-as. > %.o (assembler) is not touched when compiler is crashed. > > 2013/1/25 Chad Rosier <[email protected]>: >> Author: mcrosier >> Date: Thu Jan 24 13:14:47 2013 >> New Revision: 173361 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=173361&view=rev >> Log: >> [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 >> >> Modified: >> cfe/trunk/include/clang/Driver/Compilation.h >> cfe/trunk/include/clang/Driver/Util.h >> cfe/trunk/lib/Driver/Compilation.cpp >> cfe/trunk/lib/Driver/Driver.cpp >> cfe/trunk/lib/Driver/Tools.cpp >> cfe/trunk/lib/Driver/Tools.h >> cfe/trunk/test/Driver/output-file-cleanup.c >> >> Modified: cfe/trunk/include/clang/Driver/Compilation.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=173361&r1=173360&r2=173361&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Driver/Compilation.h (original) >> +++ cfe/trunk/include/clang/Driver/Compilation.h Thu Jan 24 13:14:47 2013 >> @@ -20,6 +20,7 @@ >> class DerivedArgList; >> class Driver; >> class InputArgList; >> + class JobAction; >> class JobList; >> class ToolChain; >> >> @@ -54,11 +55,11 @@ >> 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 @@ >> >> 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 @@ >> >> /// 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. >> /// >> >> Modified: cfe/trunk/include/clang/Driver/Util.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Util.h?rev=173361&r1=173360&r2=173361&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Driver/Util.h (original) >> +++ cfe/trunk/include/clang/Driver/Util.h Thu Jan 24 13:14:47 2013 >> @@ -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<const char*, 16> ArgStringList; >> >> + /// ArgStringMap - Type used to map a JobAction to its result file. >> + typedef llvm::DenseMap<const JobAction*, const char*> ArgStringMap; >> + >> /// ActionList - Type used for lists of actions. >> typedef SmallVector<Action*, 3> ActionList; >> >> >> Modified: cfe/trunk/lib/Driver/Compilation.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=173361&r1=173360&r2=173361&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Driver/Compilation.cpp (original) >> +++ cfe/trunk/lib/Driver/Compilation.cpp Thu Jan 24 13:14:47 2013 >> @@ -199,39 +199,56 @@ >> } >> } >> >> +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) { >> + it = Files.begin(), ie = Files.end(); it != ie; ++it) >> + Success &= CleanupFile(*it, IssueErrors); >> + return Success; >> +} >> >> - llvm::sys::Path P(*it); >> - std::string Error; >> +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) { >> >> - // 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; >> } >> >> >> Modified: cfe/trunk/lib/Driver/Driver.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=173361&r1=173360&r2=173361&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Driver/Driver.cpp (original) >> +++ cfe/trunk/lib/Driver/Driver.cpp Thu Jan 24 13:14:47 2013 >> @@ -485,8 +485,9 @@ >> << "\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 @@ >> >> // Otherwise, remove result files as well. >> if (!C.getArgs().hasArg(options::OPT_save_temps)) { >> - C.CleanupFileList(C.getResultFiles(), true); >> + const JobAction *JA = cast<JobAction>(&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 @@ >> if (AtTopLevel && !isa<DsymutilJobAction>(JA) && >> !isa<VerifyJobAction>(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 @@ >> 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); >> } >> } >> >> >> Modified: cfe/trunk/lib/Driver/Tools.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=173361&r1=173360&r2=173361&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Driver/Tools.cpp (original) >> +++ cfe/trunk/lib/Driver/Tools.cpp Thu Jan 24 13:14:47 2013 >> @@ -228,6 +228,7 @@ >> } >> >> void Clang::AddPreprocessingOptions(Compilation &C, >> + const JobAction &JA, >> const Driver &D, >> const ArgList &Args, >> ArgStringList &CmdArgs, >> @@ -248,7 +249,7 @@ >> 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 @@ >> 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 @@ >> // >> // 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". >> >> Modified: cfe/trunk/lib/Driver/Tools.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.h?rev=173361&r1=173360&r2=173361&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Driver/Tools.h (original) >> +++ cfe/trunk/lib/Driver/Tools.h Thu Jan 24 13:14:47 2013 >> @@ -40,6 +40,7 @@ >> >> private: >> void AddPreprocessingOptions(Compilation &C, >> + const JobAction &JA, >> const Driver &D, >> const ArgList &Args, >> ArgStringList &CmdArgs, >> >> Modified: cfe/trunk/test/Driver/output-file-cleanup.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/output-file-cleanup.c?rev=173361&r1=173360&r2=173361&view=diff >> ============================================================================== >> --- cfe/trunk/test/Driver/output-file-cleanup.c (original) >> +++ cfe/trunk/test/Driver/output-file-cleanup.c Thu Jan 24 13:14:47 2013 >> @@ -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 >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
