[Driver] check for exit code from SIGPIPE

[nickdesaulniers]

Summary

D53000 adds a special exit code for SIGPIPE (writing to a closed
reader), and rather than print a fatal warning, skips printing the
error. This can be seen commonly from piping into head, tee, or
split.

Fixes PR25349, rdar://problem/14285346, b/77310947.

[jfb]

What's the return code of the driver when the pipe is broken that way?

lib/Driver/Driver.cpp

1406 ↗

(On Diff #168716)

Ditto on magical number in a header.

1406 ↗

(On Diff #168716)

I think you want to create a new diagnostic here for "broken pipe" before the continue.

[nickdesaulniers]

lib/Driver/Driver.cpp

1406 ↗

(On Diff #168716)

Won't the diagnostic always be printed then? I thought we were trying to silence all related warnings?

[jfb]

lib/Driver/Driver.cpp

1406 ↗

(On Diff #168716)

It is an error that I think we want to report, but we don't want to ask for a bug report. So I think we want to avoid printing the long bug report error message, but we want to say "Broken pipe" which seems to be the standard Unix thing to do.

Not my area of expertise though, maybe someone has a different opinion?

[nickdesaulniers]

• prefer EX_IOERR from sysexits.h

[jfb]

Just to be sure: what's the exit code from the driver? If we don't diagnose I'm fine with it... but the exit code still needs to reflect the failure!

[nickdesaulniers]

• return error code 74 (EX_IOERR) if observed

[jfb]

Digging through tools/driver/driver.cpp this seems to be the right thing. Maybe leave it open for a bit so others can chime in (if say they have other drivers or whatever?). Thanks for fixing!

[nickdesaulniers]

@rnk has reverted this due to breaking the windows build (sysexits.h) is not available there.

[nickdesaulniers]

cfe/trunk/lib/Driver/Driver.cpp

81–85

@rnk || @majnemer thoughts on making this:

#if LLVM_ON_UNIX
​#include <unistd.h> // getpid
#include <sysexits.h> // EX_IOERR
#else
#define EX_IOERR 74
​#endif

vs also guarding the check I added on L1409 with #if LLVM_ON_UNIX?

[nickdesaulniers]

cfe/trunk/lib/Driver/Driver.cpp

81–85

posted to: https://reviews.llvm.org/D53210
(let's do code review there).

cfe/trunk/lib/Driver/Driver.cpp

 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include <map>
 #include <memory>
+#include <sysexits.h>
 #include <utility>
 #if LLVM_ON_UNIX
 #include <unistd.h> // getpid
 #endif
 
 using namespace clang::driver;
 using namespace clang;
 using namespace llvm::opt;
 
 Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
                DiagnosticsEngine &Diags,
                IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS)
int Driver::ExecuteCompilation(
   C.ExecuteJobs(C.getJobs(), FailingCommands);
 
   // If the command succeeded, we are done.
   if (FailingCommands.empty())
     return 0;
 
   // Otherwise, remove result files and print extra information about abnormal
   // failures.
+  int Res = 0;
   for (const auto &CmdPair : FailingCommands) {
-    int Res = CmdPair.first;
+    int CommandRes = CmdPair.first;
     const Command *FailingCommand = CmdPair.second;
 
     // Remove result files if we're not saving temps.
     if (!isSaveTempsEnabled()) {
       const JobAction *JA = cast<JobAction>(&FailingCommand->getSource());
       C.CleanupFileMap(C.getResultFiles(), JA, true);
 
       // Failure result files are valid unless we crashed.
-      if (Res < 0)
+      if (CommandRes < 0)
         C.CleanupFileMap(C.getFailureResultFiles(), JA, true);
     }
 
+    // llvm/lib/Support/Unix/Signals.inc will exit with a special return code
+    // for SIGPIPE. Do not print diagnostics for this case.
+    if (CommandRes == EX_IOERR) {
+      Res = CommandRes;
+      continue;
+    }
+
     // 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) {
+    if (!FailingCommand->getCreator().hasGoodDiagnostics() || CommandRes != 1) {
       // FIXME: See FIXME above regarding result code interpretation.
-      if (Res < 0)
+      if (CommandRes < 0)
         Diag(clang::diag::err_drv_command_signalled)
             << FailingTool.getShortName();
       else
-        Diag(clang::diag::err_drv_command_failed) << FailingTool.getShortName()
-                                                  << Res;
+        Diag(clang::diag::err_drv_command_failed)
+            << FailingTool.getShortName() << CommandRes;
     }
   }
-  return 0;
+  return Res;
 }
 
 void Driver::PrintHelp(bool ShowHidden) const {
   unsigned IncludedFlagsBitmask;
   unsigned ExcludedFlagsBitmask;
   std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
       getIncludeExcludeOptionFlagMasks();