[esan|wset] Intercept and chain signal handlers

[bruening]

Summary

In preparation for fault-based shadow memory iteration, we add support for
our own signal handler by adding app signal handler interception as well as
chaining for SIGSEGV. This is done in a simple manner: we do not honor the
app's alternate stack nor any sigaction flags for SIGSEGV.

Adds a new test of transparency in app signal handling.

[aizatsky]

lib/esan/working_set_posix.cpp

39

s/skip/skip real/

54

ditto

100

Could a signal happen before this? AppSigAct would be uninitialized in process* functions.

test/esan/TestCases/workingset-fault.cpp

1 ↗

(On Diff #58262)

This test as it is doesn't test faults, but tests signal handling propagation, right? Maybe rename it?

(Optionally include "posix" in the name?)

[filcab]

lib/esan/esan.h

47

"Platform dependent"?

lib/esan/working_set.h

28

Minor nit: dependent? They're not changing the platform.

lib/esan/working_set_posix.cpp

68

handleSEGV for now. Possibly handleSignal later.
This will be called for any SEGV, and ShadowFault isn't descriptive.

85

registerSEGVHandler

test/esan/TestCases/workingset-fault.cpp

32 ↗

(On Diff #58262)

Would kill(SIGSEGV, getpid()) work?

[bruening]

lib/esan/working_set_posix.cpp

68

The purpose of this routine is to handle shadow page write faults.

85

This routine will have the same name on every platform. Using a UNIX-specific name is not ideal: "fault" will translate to Windows.

100

I think what you mean is could a call to signal() or sigaction() happen before this? Yes, that is a general issue with all interceptors, and it will be fixed in a separate CL that ensures our whole runtime lib is initialized at the first interceptor that is encountered.

test/esan/TestCases/workingset-fault.cpp

1 ↗

(On Diff #58262)

It's both, testing propagation of faults (SIGSEGV) as no other signals need propagation. Sure, posix sounds good.

32 ↗

(On Diff #58262)

If POSIX guarantees synchronous delivery before kill returns (and platforms implement that), which I believe is true for a single-threaded app, yes, but a true fault just feels like a more authentic test.

[bruening]

Address various reviewer comments.

[filcab]

lib/esan/working_set_posix.cpp

68

But it handles any and all SEGV, right? Not just in the shadow memory.
A "shadow fault" doesn't really mean much by itself either (unless you're familiar with this code, of course).
Maybe we can agree on handleMemoryFault? I'm ok with not adding the POSIX-related names, since it's generic. But I'd rather not mention Shadow like that in a function that will be called for things outside the shadow memory.

85

Fair enough. registerMemoryFaultHandler?

test/esan/TestCases/workingset-fault.cpp

32 ↗

(On Diff #58262)

Fair enough. No need casting argc twice, btw.
I cringe looking at that line, but couldn't get you an easier way to trigger the fault off the top of my head.

[aizatsky]

LG
Please make sure Filipe is happy as well :)

[bruening]

lib/esan/working_set_posix.cpp

68

But it handles any and all SEGV, right? Not just in the shadow memory.

It doesn't take its own action on non-shadow faults: it passes them through to the app. Thus, a non-shadow fault remains un-handled: we let the app handle it. It really depends on how you think of "handle".

A "shadow fault" doesn't really mean much by itself either (unless you're familiar with this code, of course).

I'll assume that what you mean is "I myself have never seen this phrase before". That does not make it a poor term. It is descriptive and self-explanatory in the context of a shadow memory tool.

I am ok changing these names, as it is true that other faults will pass through these routines, but the original names nicely captured what the code is trying to accomplish: to take action on shadow memory faults. We would really prefer to ignore all other faults, but the signal mechanism forces us to chain a general handler with the app's and to pass through the non-shadow faults.

test/esan/TestCases/workingset-fault.cpp

32 ↗

(On Diff #58262)

Two casts are needed to avoid a warning about casting a non-pointer-sized integer to a pointer.

[bruening]

Rename fault handling routines.

[bruening]

Good to go?

lib/esan/CMakeLists.txt

 # Build for the EfficiencySanitizer runtime support library.
 
 add_custom_target(esan)
 
 set(ESAN_RTL_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF ESAN_RTL_CFLAGS)
 
 include_directories(..)
 
 set(ESAN_SOURCES
   esan.cpp
   esan_flags.cpp
   esan_interface.cpp
   esan_interceptors.cpp
-  working_set.cpp)
+  working_set.cpp
+  working_set_posix.cpp)
 
 foreach (arch ${ESAN_SUPPORTED_ARCH})
   add_compiler_rt_runtime(clang_rt.esan
     STATIC
     ARCHS ${arch}
     SOURCES ${ESAN_SOURCES}
             $<TARGET_OBJECTS:RTInterception.${arch}>
             $<TARGET_OBJECTS:RTSanitizerCommon.${arch}>

lib/esan/esan.h

 int finalizeLibrary();
 // Esan creates the variable per tool per compilation unit at compile time
 // and passes its pointer Ptr to the runtime library.
 void processCompilationUnitInit(void *Ptr);
 void processCompilationUnitExit(void *Ptr);
 void processRangeAccess(uptr PC, uptr Addr, int Size, bool IsWrite);
 void initializeInterceptors();
 
+// Platform-dependent routines.
+// The return value indicates whether to call the real version or not.
+bool processSignal(int SigNum, void (*Handler)(int), void (**Result)(int));
+bool processSigaction(int SigNum, const void *Act, void *OldAct);
+
 } // namespace __esan
 
 #endif // ESAN_H

lib/esan/esan.cpp

void processRangeAccess(uptr PC, uptr Addr, int Size, bool IsWrite) {
   if (WhichTool == ESAN_CacheFrag) {
     // TODO(bruening): add shadow mapping and update shadow bits here.
     // We'll move this to cache_frag.cpp once we have something.
   } else if (WhichTool == ESAN_WorkingSet) {
     processRangeAccessWorkingSet(PC, Addr, Size, IsWrite);
   }
 }
 
+bool processSignal(int SigNum, void (*Handler)(int), void (**Result)(int)) {
+  if (WhichTool == ESAN_WorkingSet)
+    return processWorkingSetSignal(SigNum, Handler, Result);
+  return true;
+}
+
+bool processSigaction(int SigNum, const void *Act, void *OldAct) {
+  if (WhichTool == ESAN_WorkingSet)
+    return processWorkingSetSigaction(SigNum, Act, OldAct);
+  return true;
+}
+
 #if SANITIZER_DEBUG
 static bool verifyShadowScheme() {
   // Sanity checks for our shadow mapping scheme.
   uptr AppStart, AppEnd;
   if (Verbosity() >= 3) {
     for (int i = 0; getAppRegion(i, &AppStart, &AppEnd); ++i) {
       VPrintf(3, "App #%d: [%zx-%zx) (%zuGB)\n", i, AppStart, AppEnd,
               (AppEnd - AppStart) >> 30);

lib/esan/esan_interceptors.cpp

if (!fixMmapAddr(&addr, sz, flags))
     return (void *)-1;
   return REAL(mmap64)(addr, sz, prot, flags, fd, off);
 }
 #define ESAN_MAYBE_INTERCEPT_MMAP64 INTERCEPT_FUNCTION(mmap64)
 #else
 #define ESAN_MAYBE_INTERCEPT_MMAP64
 #endif
 
+//===----------------------------------------------------------------------===//
+// Signal-related interceptors
+//===----------------------------------------------------------------------===//
+
+#if SANITIZER_LINUX
+typedef void (*signal_handler_t)(int);
+INTERCEPTOR(signal_handler_t, signal, int signum, signal_handler_t handler) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, signal, signum, handler);
+  signal_handler_t result;
+  if (!processSignal(signum, handler, &result))
+    return result;
+  else
+    return REAL(signal)(signum, handler);
+}
+#define ESAN_MAYBE_INTERCEPT_SIGNAL INTERCEPT_FUNCTION(signal)
+#else
+#error Platform not supported
+#define ESAN_MAYBE_INTERCEPT_SIGNAL
+#endif
+
+#if SANITIZER_LINUX
+INTERCEPTOR(int, sigaction, int signum, const struct sigaction *act,
+            struct sigaction *oldact) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, sigaction, signum, act, oldact);
+  if (!processSigaction(signum, act, oldact))
+    return 0;
+  else
+    return REAL(sigaction)(signum, act, oldact);
+}
+#define ESAN_MAYBE_INTERCEPT_SIGACTION INTERCEPT_FUNCTION(sigaction)
+#else
+#error Platform not supported
+#define ESAN_MAYBE_INTERCEPT_SIGACTION
+#endif
+
 namespace __esan {
 
 void initializeInterceptors() {
   InitializeCommonInterceptors();
 
   INTERCEPT_FUNCTION(strcpy); // NOLINT
   INTERCEPT_FUNCTION(strncpy);
 
   INTERCEPT_FUNCTION(open);
   ESAN_MAYBE_INTERCEPT_OPEN64;
   INTERCEPT_FUNCTION(creat);
   ESAN_MAYBE_INTERCEPT_CREAT64;
   INTERCEPT_FUNCTION(unlink);
   INTERCEPT_FUNCTION(fread);
   INTERCEPT_FUNCTION(fwrite);
   INTERCEPT_FUNCTION(puts);
   INTERCEPT_FUNCTION(rmdir);
 
   INTERCEPT_FUNCTION(mmap);
   ESAN_MAYBE_INTERCEPT_MMAP64;
 
+  ESAN_MAYBE_INTERCEPT_SIGNAL;
+  ESAN_MAYBE_INTERCEPT_SIGACTION;
+
   // TODO(bruening): we should intercept calloc() and other memory allocation
   // routines that zero memory and update our shadow memory appropriately.
 
   // TODO(bruening): intercept routines that other sanitizers intercept that
   // are not in the common pool or here yet, ideally by adding to the common
   // pool.  Examples include wcslen and bcopy.
 
   // TODO(bruening): there are many more libc routines that read or write data
   // structures that no sanitizer is intercepting: sigaction, strtol, etc.
 }
 
 } // namespace __esan

lib/esan/working_set.h

 
 namespace __esan {
 
 void initializeWorkingSet();
 int finalizeWorkingSet();
 void processRangeAccessWorkingSet(uptr PC, uptr Addr, SIZE_T Size,
                                   bool IsWrite);
 
+// Platform-dependent.
+void registerShadowFault();
+bool processWorkingSetSignal(int SigNum, void (*Handler)(int),
+                             void (**Result)(int));
+bool processWorkingSetSigaction(int SigNum, const void *Act, void *OldAct);
+
 } // namespace __esan
 
 #endif // WORKING_SET_H

lib/esan/working_set.cpp

while (I < NumLines) {
     ++Shadow;
     ++I;
   }
 }
 
 void initializeWorkingSet() {
   // The shadow mapping assumes 64 so this cannot be changed.
   CHECK(getFlags()->cache_line_size == 64);
+  registerShadowFault();
 }
 
 int finalizeWorkingSet() {
   // FIXME NYI: we need to add memory scanning to report the total lines
   // touched, and later add sampling to get intermediate values.
   Report("%s is not finished: nothing yet to report\n", SanitizerToolName);
   return 0;
 }
 
 } // namespace __esan

lib/esan/working_set_posix.cpp

+//===-- working_set_posix.cpp -----------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is a part of EfficiencySanitizer, a family of performance tuners.
+//
+// POSIX-specific working set tool code.
+//===----------------------------------------------------------------------===//
+
+#include "working_set.h"
+#include "esan_flags.h"
+#include "esan_shadow.h"
+#include "sanitizer_common/sanitizer_common.h"
+#include "sanitizer_common/sanitizer_linux.h"
+#include <signal.h>
+#include <sys/mman.h>
+
+namespace __esan {
+
+// We only support regular POSIX threads with a single signal handler
+// for the whole process == thread group.
+// Thus we only need to store one app signal handler.
+// FIXME: Store and use any alternate stack and signal flags set by
+// the app.  For now we just call the app handler from our handler.
+static __sanitizer_sigaction AppSigAct;
+
+bool processWorkingSetSignal(int SigNum, void (*Handler)(int),
+                             void (**Result)(int)) {
+  VPrintf(2, "%s: %d\n", __FUNCTION__, SigNum);
+  if (SigNum == SIGSEGV) {
+    *Result = AppSigAct.handler;
+    AppSigAct.sigaction = (void (*)(int, void*, void*))Handler;
+    return false; // Skip real call.
+  }
+  return true;
+}
+
+bool processWorkingSetSigaction(int SigNum, const void *ActVoid,
+                                void *OldActVoid) {
+  VPrintf(2, "%s: %d\n", __FUNCTION__, SigNum);
+  if (SigNum == SIGSEGV) {
+    const struct sigaction *Act = (const struct sigaction *) ActVoid;
+    struct sigaction *OldAct = (struct sigaction *) OldActVoid;
+    if (OldAct)
+      internal_memcpy(OldAct, &AppSigAct, sizeof(OldAct));
+    if (Act)
+      internal_memcpy(&AppSigAct, Act, sizeof(AppSigAct));
+    return false; // Skip real call.
+  }
+  return true;
+}
+
+static void reinstateDefaultHandler(int SigNum) {
+  __sanitizer_sigaction SigAct;
+  internal_memset(&SigAct, 0, sizeof(SigAct));
+  SigAct.sigaction = (void (*)(int, void*, void*)) SIG_DFL;
+  int Res = internal_sigaction(SigNum, &SigAct, nullptr);
+  CHECK(Res == 0);
+  VPrintf(1, "Unregistered for %d handler\n", SigNum);
+}
+
+static void handleShadowFault(int SigNum, void *Info, void *Ctx) {
+  if (SigNum == SIGSEGV) {
+
+    // TODO: Add shadow memory fault detection and handling.
+
+    if (AppSigAct.sigaction) {
+      // FIXME: For simplicity we ignore app options including its signal stack
+      // (we just use ours) and all the delivery flags.
+      AppSigAct.sigaction(SigNum, Info, Ctx);
+    } else {
+      // Crash instead of spinning with infinite faults.
+      reinstateDefaultHandler(SigNum);
+    }
+  } else
+    UNREACHABLE("signal not registered");
+}
+
+void registerShadowFault() {
+  // We do not use an alternate signal stack, as doing so would require
+  // setting it up for each app thread.
+  // FIXME: This could result in problems with emulating the app's signal
+  // handling if the app relies on an alternate stack for SIGSEGV.
+
+  // We assume SIGSEGV is not blocked and won't be blocked by the app, so
+  // we leave the mask alone.
+
+  __sanitizer_sigaction SigAct;
+  internal_memset(&SigAct, 0, sizeof(SigAct));
+  SigAct.sigaction = handleShadowFault;
+  // We want to handle nested signals b/c we need to handle a
+  // shadow fault in an app signal handler.
+  SigAct.sa_flags = SA_SIGINFO | SA_NODEFER;
+  int Res = internal_sigaction(SIGSEGV, &SigAct, &AppSigAct);
+  CHECK(Res == 0);
+  VPrintf(1, "Registered for SIGSEGV handler\n");
+}
+
+} // namespace __esan

test/esan/TestCases/workingset-signal-posix.cpp

+// RUN: %clang_esan_wset -O0 %s -o %t 2>&1
+// RUN: %run %t 2>&1 | FileCheck %s
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <setjmp.h>
+#include <assert.h>
+
+sigjmp_buf mark;
+
+static void SignalHandler(int Sig) {
+  if (Sig == SIGSEGV) {
+    fprintf(stderr, "Handling SIGSEGV for signal\n");
+    siglongjmp(mark, 1);
+  }
+  exit(1);
+}
+
+static void SigactionHandler(int Sig, siginfo_t *Info, void *Ctx) {
+  if (Sig == SIGSEGV) {
+    fprintf(stderr, "Handling SIGSEGV for sigaction\n");
+    siglongjmp(mark, 1);
+  }
+  exit(1);
+}
+
+int main(int argc, char **argv) {
+  __sighandler_t Prior = signal(SIGSEGV, SignalHandler);
+  assert(Prior == SIG_DFL);
+  if (sigsetjmp(mark, 1) == 0)
+    *((volatile int *)(ssize_t)argc) = 42; // Raise SIGSEGV
+  fprintf(stderr, "Past longjmp for signal\n");
+
+  Prior = signal(SIGSEGV, SIG_DFL);
+  assert(Prior == SignalHandler);
+
+  struct sigaction SigAct;
+  SigAct.sa_sigaction = SigactionHandler;
+  int Res = sigfillset(&SigAct.sa_mask);
+  assert(Res == 0);
+  SigAct.sa_flags = SA_SIGINFO;
+  Res = sigaction(SIGSEGV, &SigAct, NULL);
+  assert(Res == 0);
+
+  if (sigsetjmp(mark, 1) == 0)
+    *((volatile int *)(ssize_t)argc) = 42; // Raise SIGSEGV
+  fprintf(stderr, "Past longjmp for sigaction\n");
+
+  Res = sigaction(SIGSEGV, NULL, &SigAct);
+  assert(Res == 0);
+  assert(SigAct.sa_sigaction == SigactionHandler);
+
+  return 0;
+}
+// CHECK:      Handling SIGSEGV for signal
+// CHECK-NEXT: Past longjmp for signal
+// CHECK-NEXT: Handling SIGSEGV for sigaction
+// CHECK-NEXT: Past longjmp for sigaction