r173825 - [driver] Refactor the driver so that a failing commands doesn't prevent
Chad Rosier
mcrosier at apple.com
Tue Feb 26 13:27:48 PST 2013
On Feb 26, 2013, at 1:12 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Tue, Feb 26, 2013 at 9:50 AM, Chad Rosier <mcrosier at apple.com> wrote:
> Matthew,
>
> On Feb 26, 2013, at 6:11 AM, Matthew Curtis <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 of rdar://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
>>> 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
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130226/2481fe20/attachment.html>
More information about the cfe-commits
mailing list