Don't install signal handler if the disposition of a signal is SIG_IGN, resignal after the signal handler handles a signal so the exit code of the compiler contains the signal number.

Author: xingxue-ibm

clang-tools-extra/clangd/unittests/ThreadCrashReporterTests.cpp

@@ -13,6 +13,17 @@
 #include <csignal>
 #include <string>
 
+// According to the POSIX specification, if the inherited disposition of a
+// signal is the default action, the behavior of utilitys must be as if the
+// default action had been taken. When the required default action is to
+// terminate the process, such as for SIGUSR1, the utility may catch the
+// signal, perform additional processing, restore the default disposition,
+// and then re-signal itself. This causes the process to terminate as
+// required. Because of this behavior, the crash-reporter test here is not
+// suitable for Unix platforms.
+
+#ifndef LLVM_ON_UNIX
+
 namespace clang {
 namespace clangd {
 
@@ -76,3 +87,4 @@ TEST(ThreadCrashReporterTest, All) {
 } // namespace
 } // namespace clangd
 } // namespace clang
+#endif // !LLVM_ON_UNIX

clang/lib/Driver/Driver.cpp

@@ -2298,7 +2298,14 @@ int Driver::ExecuteCompilation(
 
     if (!FailingCommand->getCreator().hasGoodDiagnostics() || CommandRes != 1) {
       // FIXME: See FIXME above regarding result code interpretation.
+#if LLVM_ON_UNIX
+      // On Unix, signals are represented by return codes of 128 plus the
+      // signal number. Return code 255 is excluded because some tools,
+      // such as llvm-ifs, exit with code 255 (-1) on failure.
+      if (CommandRes > 128 && CommandRes != 255)
+#else
       if (CommandRes < 0)
+#endif
         Diag(clang::diag::err_drv_command_signalled)
             << FailingTool.getShortName();
       else

clang/tools/driver/driver.cpp

@@ -54,6 +54,9 @@
 #include <optional>
 #include <set>
 #include <system_error>
+#if LLVM_ON_UNIX
+#include <signal.h>
+#endif
 
 using namespace clang;
 using namespace clang::driver;
@@ -400,14 +403,15 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) {
   Driver::CommandStatus CommandStatus = Driver::CommandStatus::Ok;
   // Pretend the first command failed if ReproStatus is Always.
   const Command *FailingCommand = nullptr;
+  int CommandRes = 0;
   if (!C->getJobs().empty())
     FailingCommand = &*C->getJobs().begin();
   if (C && !C->containsError()) {
     SmallVector<std::pair<int, const Command *>, 4> FailingCommands;
     Res = TheDriver.ExecuteCompilation(*C, FailingCommands);
 
     for (const auto &P : FailingCommands) {
-      int CommandRes = P.first;
+      CommandRes = P.first;
       FailingCommand = P.second;
       if (!Res)
         Res = CommandRes;
@@ -464,6 +468,18 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) {
     Res = 1;
 #endif
 
+#if LLVM_ON_UNIX
+  // On Unix, signals are represented by return codes of 128 plus the signal
+  // number. If the return code indicates it was from a signal handler, raise
+  // the signal so that the exit code includes the signal number, as required
+  // by POSIX. Return code 255 is excluded because some tools, such as
+  // llvm-ifs, exit with code 255 (-1) on failure.
+  if (CommandRes > 128 && CommandRes != 255) {
+    llvm::sys::unregisterHandlers();
+    raise(CommandRes - 128);
+  }
+#endif
+
   // If we have multiple failing commands, we return the result of the first
   // failing command.
   return Res;

llvm/lib/Support/CrashRecoveryContext.cpp

@@ -398,7 +398,11 @@ static void installExceptionOrSignalHandlers() {
   sigemptyset(&Handler.sa_mask);
 
   for (unsigned i = 0; i != NumSignals; ++i) {
-    sigaction(Signals[i], &Handler, &PrevActions[i]);
+    struct sigaction act;
+    sigaction(Signals[i], NULL, &act);
+    // Don't install signal handler if the disposition of a signal is SIG_IGN.
+    if (act.sa_handler != SIG_IGN)
+      sigaction(Signals[i], &Handler, &PrevActions[i]);
   }
 }
 

llvm/lib/Support/Unix/Signals.inc

@@ -327,8 +327,12 @@ static void RegisterHandlers() { // Not signal-safe.
     }
     sigemptyset(&NewHandler.sa_mask);
 
-    // Install the new handler, save the old one in RegisteredSignalInfo.
-    sigaction(Signal, &NewHandler, &RegisteredSignalInfo[Index].SA);
+    // Install the new handler if the signal disposition isn't SIG_IGN,
+    // save the old one in RegisteredSignalInfo.
+    struct sigaction act;
+    sigaction(Signal, NULL, &act);
+    if (act.sa_handler != SIG_IGN)
+      sigaction(Signal, &NewHandler, &RegisteredSignalInfo[Index].SA);
     RegisteredSignalInfo[Index].SigNo = Signal;
     ++NumRegisteredSignals;
   };
@@ -408,14 +412,12 @@ static void SignalHandler(int Sig, siginfo_t *Info, void *) {
   // Otherwise if it is a fault (like SEGV) run any handler.
   llvm::sys::RunSignalHandlers();
 
-#ifdef __s390__
-  // On S/390, certain signals are delivered with PSW Address pointing to
-  // *after* the faulting instruction.  Simply returning from the signal
-  // handler would continue execution after that point, instead of
-  // re-raising the signal.  Raise the signal manually in those cases.
-  if (Sig == SIGILL || Sig == SIGFPE || Sig == SIGTRAP)
-    raise(Sig);
-#endif
+  // Resignal if it is a kill signal so that the exit code contains the
+  // terminating signal number.
+  if (llvm::is_contained(KillSigs, Sig)) {
+    raise(Sig); // Execute the default handler.
+    return;
+  }
 
 #if defined(__linux__)
   // Re-raising a signal via `raise` loses the original siginfo. Recent
@@ -438,6 +440,11 @@ static void InfoSignalHandler(int Sig) {
   SaveAndRestore SaveErrnoDuringASignalHandler(errno);
   if (SignalHandlerFunctionType CurrentInfoFunction = InfoSignalFunction)
     CurrentInfoFunction();
+
+  if (Sig == SIGUSR1) {
+    sys::unregisterHandlers();
+    raise(Sig);
+  }
 }
 
 void llvm::sys::RunInterruptHandlers() { RemoveFilesToRemove(); }