[cfe-commits] r173361 - in /cfe/trunk: include/clang/Driver/Compilation.h include/clang/Driver/Util.h lib/Driver/Compilation.cpp lib/Driver/Driver.cpp lib/Driver/Tools.cpp lib/Driver/Tools.h test/Driver/output-file-cleanup.c
Chad Rosier
mcrosier at apple.com
Thu Jan 24 20:45:36 PST 2013
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 <geek4civic at gmail.com> 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 <mcrosier at apple.com>:
>> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list