Fix sigaction interceptor to always correctly populate oldact

[IanPudney]

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=47118. Before this change, when the sigaction interceptor prevented a signal from being changed, it also prevented the oldact output parameter from being written to. This resulted in a use-of-uninitialized-variable by any program that used sigaction for the purpose of reading signals.

This change fixes this: the regular sigaction implementation is still called, but with the act parameter nullified, preventing any changes.

[morehouse]

compiler-rt/test/msan/interception_sigaction_test.cpp

13

Do we need this wrapper? Maybe set MSAN_OPTIONS=handle_segv=2 to force the mode we need, and then attempt to install your own handler and ensure it is never called.

17

Nit: s/11/SIGSEGV

18

Why do we need a loop?

20

Nit: Can we use fprintf?

[IanPudney]

compiler-rt/test/msan/interception_sigaction_test.cpp

13

So, I copied this pattern from other tests that test interceptors. I believe the purpose of the wrapper is to make sure that the interceptor is being called rather than the original one; that way, we know the test is actually testing what it's supposed to. I suppose your proposal would also achieve that, but this seems simpler and matches what other tests do.

18

Oops, I was planning to iterate over all signals and make sure they all worked, but then decided SIGSEGV was enough. Removed.

20

Done. Can't on the other one though, because it's happening in a signal handler.

[morehouse]

LGTM

compiler-rt/lib/sanitizer_common/sanitizer_signal_interceptors.inc

INTERCEPTOR(uptr, signal, int signum, uptr handler) {
   if (GetHandleSignalMode(signum) == kHandleSignalExclusive)
     return (uptr) nullptr;
   SIGNAL_INTERCEPTOR_SIGNAL_IMPL(signal, signum, handler);
 }
 #define INIT_SIGNAL COMMON_INTERCEPT_FUNCTION(signal)
 
 INTERCEPTOR(int, sigaction_symname, int signum,
             const __sanitizer_sigaction *act, __sanitizer_sigaction *oldact) {
-  if (GetHandleSignalMode(signum) == kHandleSignalExclusive) return 0;
+  if (GetHandleSignalMode(signum) == kHandleSignalExclusive) {
+    if (oldact) {
+      act = nullptr;
+    } else {
+      return 0;
+    }
+  }
   SIGNAL_INTERCEPTOR_SIGACTION_IMPL(signum, act, oldact);
 }
 #define INIT_SIGACTION COMMON_INTERCEPT_FUNCTION(sigaction_symname)
 
 namespace __sanitizer {
 int real_sigaction(int signum, const void *act, void *oldact) {
   return REAL(sigaction_symname)(signum, (const __sanitizer_sigaction *)act,
                          (__sanitizer_sigaction *)oldact);

 #endif  // SANITIZER_INTERCEPT_SIGNAL_AND_SIGACTION
 
 static void InitializeSignalInterceptors() {
   static bool was_called_once;
   CHECK(!was_called_once);
   was_called_once = true;
 
   INIT_BSD_SIGNAL;
   INIT_SIGNAL;
   INIT_SIGACTION;
 }

compiler-rt/test/msan/interception_sigaction_test.cpp

+// RUN: %clangxx_msan -O0 -g %s -o %t
+// RUN: %t 2>&1 | FileCheck %s
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <signal.h>
+#include <string.h>
+
+extern "C" int __interceptor_sigaction(int signum, const struct sigaction *act, struct sigaction *oldact);
+extern "C" int sigaction(int signum, const struct sigaction *act, struct sigaction *oldact) {
+  write(2, "sigaction call\n", sizeof("sigaction call\n") - 1);
+  return __interceptor_sigaction(signum, act, oldact);
+}
+
+int main() {
+  struct sigaction oldact;
+  sigaction(11, nullptr, &oldact);
+  for (int i = 0; i < sizeof(oldact); ++i) {
+    if (oldact.sa_handler || oldact.sa_sigaction) {
+      write(2, "oldact filled\n", sizeof("oldact filled\n") - 1);
+    }
+  }
+  return 0;
+  // CHECK: sigaction call
+  // CHECK: oldact filled
+}