r173825 - [driver] Refactor the driver so that a failing commands doesn't prevent
Matthew Curtis
mcurtis at codeaurora.org
Wed Feb 27 05:35:46 PST 2013
How about the attached patch as a possible solution?
It uses the dependency information in the Action associated with a
Command to prevent the execution of any Commands downstream from a
failing Command.
On 2/26/2013 3:27 PM, Chad Rosier wrote:
>
> On Feb 26, 2013, at 1:12 PM, Richard Smith <richard at metafoo.co.uk
> <mailto:richard at metafoo.co.uk>> wrote:
>
>> On Tue, Feb 26, 2013 at 9:50 AM, Chad Rosier<mcrosier at apple.com
>> <mailto:mcrosier at apple.com>>wrote:
>>
>> Matthew,
>>
>> On Feb 26, 2013, at 6:11 AM, Matthew Curtis
>> <mcurtis at codeaurora.org <mailto:mcurtis at codeaurora.org>> wrote:
>>
>>> Hello Chad,
>>>
>>> Could you provide a little more background on why it's important
>>> to execute subsequent commands after one fails?
>>
>> It is required for Unix conformance testing. :)
>>
>>>
>>> This change is currently causing some noise for failed
>>> compilations on hexagon:
>>>
>>> hello-world.c:17:26: error: expected ';' after expression
>>> printf("hello world\n")
>>> ^
>>> ;
>>> 1 error generated.
>>> Assembler messages:
>>> Error: can't open /tmp/hello-world-ZhHzJ1.s for reading: No
>>> such file or directory
>>> <...>/qc/../gnu/bin/hexagon-ld: cannot find
>>> /tmp/hello-world-KKCeEQ.o: No such file or directory
>>> clang: error: hexagon-as command failed with exit code 1
>>> (use -v to see invocation)
>>> clang: error: hexagon-ld command failed with exit code 1
>>> (use -v to see invocation)
>>>
>>>
>>> Should it only apply to commands that do not depend on the
>>> output of a failing command?
>>
>> It would be nice to detect such dependencies, but I don't plan on
>> implementing it any time soon.
>>
>
> Change of plan; I'll see if I can't get this fixed in a day or two.
>
>>
>>>
>>> I'm just trying to understand if this feature is perhaps not
>>> appropriate for the hexagon target or if I should try to do
>>> something to quiet or inhibit downstream commands.
>>
>> If it's a huge distraction, we might consider adding a driver
>> flag that would cause the previous behavior.
>>
>>
>> This has made all failing multi-stage compiles ugly. Do we have a
>> good reason not to restore the previous behavior by default? This
>> just looks stupid:
>>
>> $ clang++ my_simple_program.cc
>> my_simple_program.cc:1:10: error: expected ';' after top level declarator
>> int error error;
>> ^
>> ;
>> 1 error generated.
>> /usr/bin/ld: cannot find /tmp/--gWf0D2.o: No such file or directory
>> clang-3: error: linker command failed with exit code 1 (use -v to see
>> invocation)FWIW, the original idea was to prevent the failure of one
>> translation unit not effect
>
> FWIW, the original idea was to prevent the failure of one translation
> unit from preventing the compilation of another. Unfortunately, I
> missed the fact that the failure of any phase (e.g., preprocess,
> compile, assemble) for a single translation unit should prevent later
> phases from executing.
>
> Chad
>
>> Chad
>>
>>>
>>> Thanks,
>>> Matthew Curtis
>>>
>>> On 1/29/2013 2:15 PM, Chad Rosier wrote:
>>>> Author: mcrosier
>>>> Date: Tue Jan 29 14:15:05 2013
>>>> New Revision: 173825
>>>>
>>>> URL:http://llvm.org/viewvc/llvm-project?rev=173825&view=rev
>>>> Log:
>>>> [driver] Refactor the driver so that a failing commands doesn't prevent
>>>> subsequent commands from being executed.
>>>>
>>>> The diagnostics generation isn't designed for this use case, so add a note to
>>>> fix this in the very near future. For now, just generated the diagnostics for
>>>> the first failing command.
>>>> Part ofrdar://12984531
>>>>
>>>> Modified:
>>>> cfe/trunk/include/clang/Driver/Compilation.h
>>>> cfe/trunk/include/clang/Driver/Driver.h
>>>> cfe/trunk/lib/Driver/Compilation.cpp
>>>> cfe/trunk/lib/Driver/Driver.cpp
>>>> cfe/trunk/test/Driver/output-file-cleanup.c
>>>> cfe/trunk/tools/driver/driver.cpp
>>>>
>>>> Modified: cfe/trunk/include/clang/Driver/Compilation.h
>>>> URL:http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=173825&r1=173824&r2=173825&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/include/clang/Driver/Compilation.h (original)
>>>> +++ cfe/trunk/include/clang/Driver/Compilation.h Tue Jan 29 14:15:05 2013
>>>> @@ -175,10 +175,10 @@ public:
>>>>
>>>> /// ExecuteJob - Execute a single job.
>>>> ///
>>>> - /// \param FailingCommand - For non-zero results, this will be set to the
>>>> - /// Command which failed.
>>>> - /// \return The accumulated result code of the job.
>>>> - int ExecuteJob(const Job &J, const Command *&FailingCommand) const;
>>>> + /// \param FailingCommands - For non-zero results, this will be a vector of
>>>> + /// failing commands and their associated result code.
>>>> + void ExecuteJob(const Job &J,
>>>> + SmallVectorImpl< std::pair<int, const Command *> > &FailingCommands) const;
>>>>
>>>> /// initCompilationForDiagnostics - Remove stale state and suppress output
>>>> /// so compilation can be reexecuted to generate additional diagnostic
>>>>
>>>> Modified: cfe/trunk/include/clang/Driver/Driver.h
>>>> URL:http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=173825&r1=173824&r2=173825&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/include/clang/Driver/Driver.h (original)
>>>> +++ cfe/trunk/include/clang/Driver/Driver.h Tue Jan 29 14:15:05 2013
>>>> @@ -272,7 +272,7 @@ public:
>>>> /// to just running the subprocesses, for example reporting errors, removing
>>>> /// temporary files, etc.
>>>> int ExecuteCompilation(const Compilation &C,
>>>> - const Command *&FailingCommand) const;
>>>> + SmallVectorImpl< std::pair<int, const Command *> > &FailingCommands) const;
>>>>
>>>> /// generateCompilationDiagnostics - Generate diagnostics information
>>>> /// including preprocessed source file(s).
>>>>
>>>> Modified: cfe/trunk/lib/Driver/Compilation.cpp
>>>> URL:http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=173825&r1=173824&r2=173825&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/Driver/Compilation.cpp (original)
>>>> +++ cfe/trunk/lib/Driver/Compilation.cpp Tue Jan 29 14:15:05 2013
>>>> @@ -307,17 +307,17 @@ int Compilation::ExecuteCommand(const Co
>>>> return Res;
>>>> }
>>>>
>>>> -int Compilation::ExecuteJob(const Job &J,
>>>> - const Command *&FailingCommand) const {
>>>> +void Compilation::ExecuteJob(const Job &J,
>>>> + SmallVectorImpl< std::pair<int, const Command *> > &FailingCommands) const {
>>>> if (const Command *C = dyn_cast<Command>(&J)) {
>>>> - return ExecuteCommand(*C, FailingCommand);
>>>> + const Command *FailingCommand = 0;
>>>> + if (int Res = ExecuteCommand(*C, FailingCommand))
>>>> + FailingCommands.push_back(std::make_pair(Res, FailingCommand));
>>>> } else {
>>>> const JobList *Jobs = cast<JobList>(&J);
>>>> - for (JobList::const_iterator
>>>> - it = Jobs->begin(), ie = Jobs->end(); it != ie; ++it)
>>>> - if (int Res = ExecuteJob(**it, FailingCommand))
>>>> - return Res;
>>>> - return 0;
>>>> + for (JobList::const_iterator it = Jobs->begin(), ie = Jobs->end();
>>>> + it != ie; ++it)
>>>> + ExecuteJob(**it, FailingCommands);
>>>> }
>>>> }
>>>>
>>>>
>>>> Modified: cfe/trunk/lib/Driver/Driver.cpp
>>>> URL:http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=173825&r1=173824&r2=173825&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/Driver/Driver.cpp (original)
>>>> +++ cfe/trunk/lib/Driver/Driver.cpp Tue Jan 29 14:15:05 2013
>>>> @@ -440,11 +440,11 @@ void Driver::generateCompilationDiagnost
>>>> }
>>>>
>>>> // Generate preprocessed output.
>>>> - FailingCommand = 0;
>>>> - int Res = C.ExecuteJob(C.getJobs(), FailingCommand);
>>>> + SmallVector<std::pair<int, const Command *>, 4> FailingCommands;
>>>> + C.ExecuteJob(C.getJobs(), FailingCommands);
>>>>
>>>> // If the command succeeded, we are done.
>>>> - if (Res == 0) {
>>>> + if (FailingCommands.empty()) {
>>>> Diag(clang::diag::note_drv_command_failed_diag_msg)
>>>> << "\n********************\n\n"
>>>> "PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:\n"
>>>> @@ -495,7 +495,7 @@ void Driver::generateCompilationDiagnost
>>>> }
>>>>
>>>> int Driver::ExecuteCompilation(const Compilation &C,
>>>> - const Command *&FailingCommand) const {
>>>> + SmallVectorImpl< std::pair<int, const Command *> > &FailingCommands) const {
>>>> // Just print if -### was present.
>>>> if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH)) {
>>>> C.PrintJob(llvm::errs(), C.getJobs(), "\n", true);
>>>> @@ -506,45 +506,52 @@ int Driver::ExecuteCompilation(const Com
>>>> if (Diags.hasErrorOccurred())
>>>> return 1;
>>>>
>>>> - int Res = C.ExecuteJob(C.getJobs(), FailingCommand);
>>>> + C.ExecuteJob(C.getJobs(), FailingCommands);
>>>>
>>>> // Remove temp files.
>>>> C.CleanupFileList(C.getTempFiles());
>>>>
>>>> // If the command succeeded, we are done.
>>>> - if (Res == 0)
>>>> - return Res;
>>>> + if (FailingCommands.empty())
>>>> + return 0;
>>>>
>>>> - // Otherwise, remove result files as well.
>>>> - if (!C.getArgs().hasArg(options::OPT_save_temps)) {
>>>> - const JobAction *JA = cast<JobAction>(&FailingCommand->getSource());
>>>> - C.CleanupFileMap(C.getResultFiles(), JA, true);
>>>> + // Otherwise, remove result files and print extra information about abnormal
>>>> + // failures.
>>>> + for (SmallVectorImpl< std::pair<int, const Command *> >::iterator it =
>>>> + FailingCommands.begin(), ie = FailingCommands.end(); it != ie; ++it) {
>>>> + int Res = it->first;
>>>> + const Command *FailingCommand = it->second;
>>>>
>>>> - // Failure result files are valid unless we crashed.
>>>> - if (Res < 0)
>>>> - C.CleanupFileMap(C.getFailureResultFiles(), JA, true);
>>>> - }
>>>> + // Remove result files if we're not saving temps.
>>>> + if (!C.getArgs().hasArg(options::OPT_save_temps)) {
>>>> + const JobAction *JA = cast<JobAction>(&FailingCommand->getSource());
>>>> + C.CleanupFileMap(C.getResultFiles(), JA, true);
>>>>
>>>> - // Print extra information about abnormal failures, if possible.
>>>> - //
>>>> - // This is ad-hoc, but we don't want to be excessively noisy. If the result
>>>> - // status was 1, assume the command failed normally. In particular, if it was
>>>> - // the compiler then assume it gave a reasonable error code. Failures in other
>>>> - // tools are less common, and they generally have worse diagnostics, so always
>>>> - // print the diagnostic there.
>>>> - const Tool &FailingTool = FailingCommand->getCreator();
>>>> -
>>>> - if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) {
>>>> - // FIXME: See FIXME above regarding result code interpretation.
>>>> - if (Res < 0)
>>>> - Diag(clang::diag::err_drv_command_signalled)
>>>> - << FailingTool.getShortName();
>>>> - else
>>>> - Diag(clang::diag::err_drv_command_failed)
>>>> - << FailingTool.getShortName() << Res;
>>>> - }
>>>> + // Failure result files are valid unless we crashed.
>>>> + if (Res < 0)
>>>> + C.CleanupFileMap(C.getFailureResultFiles(), JA, true);
>>>> + }
>>>>
>>>> - return Res;
>>>> + // Print extra information about abnormal failures, if possible.
>>>> + //
>>>> + // This is ad-hoc, but we don't want to be excessively noisy. If the result
>>>> + // status was 1, assume the command failed normally. In particular, if it
>>>> + // was the compiler then assume it gave a reasonable error code. Failures
>>>> + // in other tools are less common, and they generally have worse
>>>> + // diagnostics, so always print the diagnostic there.
>>>> + const Tool &FailingTool = FailingCommand->getCreator();
>>>> +
>>>> + if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) {
>>>> + // FIXME: See FIXME above regarding result code interpretation.
>>>> + if (Res < 0)
>>>> + Diag(clang::diag::err_drv_command_signalled)
>>>> + << FailingTool.getShortName();
>>>> + else
>>>> + Diag(clang::diag::err_drv_command_failed)
>>>> + << FailingTool.getShortName() << Res;
>>>> + }
>>>> + }
>>>> + return 0;
>>>> }
>>>>
>>>> void Driver::PrintOptions(const ArgList &Args) const {
>>>>
>>>> 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=173825&r1=173824&r2=173825&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/test/Driver/output-file-cleanup.c (original)
>>>> +++ cfe/trunk/test/Driver/output-file-cleanup.c Tue Jan 29 14:15:05 2013
>>>> @@ -36,3 +36,15 @@ invalid C code
>>>> // RUN: cd %T && not %clang -S %t1.c %t2.c
>>>> // RUN: test -f %t1.s
>>>> // RUN: test ! -f %t2.s
>>>> +
>>>> +// RUN: touch %t1.c
>>>> +// RUN: echo "invalid C code" > %t2.c
>>>> +// RUN: touch %t3.c
>>>> +// RUN: echo "invalid C code" > %t4.c
>>>> +// RUN: touch %t5.c
>>>> +// RUN: cd %T && not %clang -S %t1.c %t2.c %t3.c %t4.c %t5.c
>>>> +// RUN: test -f %t1.s
>>>> +// RUN: test ! -f %t2.s
>>>> +// RUN: test -f %t3.s
>>>> +// RUN: test ! -f %t4.s
>>>> +// RUN: test -f %t5.s
>>>>
>>>> Modified: cfe/trunk/tools/driver/driver.cpp
>>>> URL:http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/driver.cpp?rev=173825&r1=173824&r2=173825&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/tools/driver/driver.cpp (original)
>>>> +++ cfe/trunk/tools/driver/driver.cpp Tue Jan 29 14:15:05 2013
>>>> @@ -466,21 +466,35 @@ int main(int argc_, const char **argv_)
>>>>
>>>> OwningPtr<Compilation> C(TheDriver.BuildCompilation(argv));
>>>> int Res = 0;
>>>> - const Command *FailingCommand = 0;
>>>> + SmallVector<std::pair<int, const Command *>, 4> FailingCommands;
>>>> if (C.get())
>>>> - Res = TheDriver.ExecuteCompilation(*C, FailingCommand);
>>>> + Res = TheDriver.ExecuteCompilation(*C, FailingCommands);
>>>>
>>>> // Force a crash to test the diagnostics.
>>>> if (::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH")) {
>>>> Diags.Report(diag::err_drv_force_crash) << "FORCE_CLANG_DIAGNOSTICS_CRASH";
>>>> - Res = -1;
>>>> + const Command *FailingCommand = 0;
>>>> + FailingCommands.push_back(std::make_pair(-1, FailingCommand));
>>>> }
>>>>
>>>> - // If result status is < 0, then the driver command signalled an error.
>>>> - // If result status is 70, then the driver command reported a fatal error.
>>>> - // In these cases, generate additional diagnostic information if possible.
>>>> - if (Res < 0 || Res == 70)
>>>> - TheDriver.generateCompilationDiagnostics(*C, FailingCommand);
>>>> + for (SmallVectorImpl< std::pair<int, const Command *> >::iterator it =
>>>> + FailingCommands.begin(), ie = FailingCommands.end(); it != ie; ++it) {
>>>> + int CommandRes = it->first;
>>>> + const Command *FailingCommand = it->second;
>>>> + if (!Res)
>>>> + Res = CommandRes;
>>>> +
>>>> + // If result status is < 0, then the driver command signalled an error.
>>>> + // If result status is 70, then the driver command reported a fatal error.
>>>> + // In these cases, generate additional diagnostic information if possible.
>>>> + if (CommandRes < 0 || CommandRes == 70) {
>>>> + TheDriver.generateCompilationDiagnostics(*C, FailingCommand);
>>>> +
>>>> + // FIXME: generateCompilationDiagnostics() needs to be tested when there
>>>> + // are multiple failing commands.
>>>> + break;
>>>> + }
>>>> + }
>>>>
>>>> // If any timers were active but haven't been destroyed yet, print their
>>>> // results now. This happens in -disable-free mode.
>>>> @@ -496,5 +510,7 @@ int main(int argc_, const char **argv_)
>>>> Res = 1;
>>>> #endif
>>>>
>>>> + // If we have multiple failing commands, we return the result of the first
>>>> + // failing command.
>>>> return Res;
>>>> }
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>> --
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130227/908c3f94/attachment.html>
-------------- next part --------------
>From ab16be648f7f43e5aa5f128d349dec85b02afe80 Mon Sep 17 00:00:00 2001
From: Matthew Curtis <mcurtis at codeaurora.org>
Date: Tue, 26 Feb 2013 16:07:29 -0600
Subject: [PATCH] Inhibit downstream commands when a command fails.
---
lib/Driver/Compilation.cpp | 34 ++++++++++++++++++++++++++++-
test/Driver/inhibit-downstream-commands.c | 4 +++
2 files changed, 37 insertions(+), 1 deletions(-)
create mode 100644 test/Driver/inhibit-downstream-commands.c
diff --git a/lib/Driver/Compilation.cpp b/lib/Driver/Compilation.cpp
index df904f0..929b54f 100644
--- a/lib/Driver/Compilation.cpp
+++ b/lib/Driver/Compilation.cpp
@@ -307,9 +307,41 @@ int Compilation::ExecuteCommand(const Command &C,
return Res;
}
+typedef SmallVectorImpl< std::pair<int, const Command *> > FailingCommandList;
+
+static bool ActionFailed(const Action *A,
+ const FailingCommandList &FailingCommands) {
+
+ if (FailingCommands.empty())
+ return false;
+
+ FailingCommandList::const_iterator CI;
+ const FailingCommandList::const_iterator CE = FailingCommands.end();
+ for (CI = FailingCommands.begin(); CI != CE; ++CI) {
+ if (A == &(CI->second->getSource()))
+ return true;
+ }
+
+ Action::const_iterator AI;
+ const Action::const_iterator AE = A->end();
+ for (AI = A->begin(); AI != AE; ++AI) {
+ if (ActionFailed(*AI, FailingCommands))
+ return true;
+ }
+
+ return false;
+}
+
+static bool InputsOk(const Command &C,
+ const FailingCommandList &FailingCommands) {
+ return !ActionFailed(&C.getSource(), FailingCommands);
+}
+
void Compilation::ExecuteJob(const Job &J,
- SmallVectorImpl< std::pair<int, const Command *> > &FailingCommands) const {
+ FailingCommandList &FailingCommands) const {
if (const Command *C = dyn_cast<Command>(&J)) {
+ if (!InputsOk(*C, FailingCommands))
+ return;
const Command *FailingCommand = 0;
if (int Res = ExecuteCommand(*C, FailingCommand))
FailingCommands.push_back(std::make_pair(Res, FailingCommand));
diff --git a/test/Driver/inhibit-downstream-commands.c b/test/Driver/inhibit-downstream-commands.c
new file mode 100644
index 0000000..522a4bc
--- /dev/null
+++ b/test/Driver/inhibit-downstream-commands.c
@@ -0,0 +1,4 @@
+// RUN: %clang %s 2>&1 | FileCheck %s
+// CHECK: error: unknown type name 'invalid'
+// CHECK-NOT: clang: error: linker command failed
+invalid C code!
--
1.7.8.3
More information about the cfe-commits
mailing list