[GWP-ASan] Split the unwinder into segv/non-segv.

Summary:
Splits the unwinder into a non-segv (for allocation/deallocation traces) and a
segv unwinder. This ensures that implementations can select an accurate, slower
unwinder in the segv handler (if they choose to use the GWP-ASan provided one).
This is important as fast frame-pointer unwinders (like the sanitizer unwinder)
don't like unwinding through signal handlers.

Reviewers: morehouse, cryptoad

Reviewed By: morehouse, cryptoad

Subscribers: cryptoad, mgorny, eugenis, pcc, #sanitizers

Tags: #sanitizers

Differential Revision: https://reviews.llvm.org/D83994

compiler-rt/lib/gwp_asan/optional/backtrace_linux_libc.cpp

@@ -23,6 +23,14 @@ size_t Backtrace(uintptr_t *TraceBuffer, size_t Size) {
   return backtrace(reinterpret_cast<void **>(TraceBuffer), Size);
 }
 
+// We don't need any custom handling for the Segv backtrace - the libc unwinder
+// has no problems with unwinding through a signal handler. Force inlining here
+// to avoid the additional frame.
+GWP_ASAN_ALWAYS_INLINE size_t SegvBacktrace(uintptr_t *TraceBuffer, size_t Size,
+                                            void * /*Context*/) {
+  return Backtrace(TraceBuffer, Size);
+}
+
 static void PrintBacktrace(uintptr_t *Trace, size_t TraceLength,
                            gwp_asan::crash_handler::Printf_t Printf) {
   if (TraceLength == 0) {
@@ -53,4 +61,8 @@ crash_handler::PrintBacktrace_t getPrintBacktraceFunction() {
   return PrintBacktrace;
 }
 } // namespace options
+
+namespace crash_handler {
+SegvBacktrace_t getSegvBacktraceFunction() { return SegvBacktrace; }
+} // namespace crash_handler
 } // namespace gwp_asan

compiler-rt/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp

@@ -22,28 +22,45 @@ void __sanitizer::BufferedStackTrace::UnwindImpl(uptr pc, uptr bp,
                                                  void *context,
                                                  bool request_fast,
                                                  u32 max_depth) {
-  if (!StackTrace::WillUseFastUnwind(request_fast)) {
-    return Unwind(max_depth, pc, bp, context, 0, 0, request_fast);
-  }
-  Unwind(max_depth, pc, 0, context, 0, 0, false);
+  if (!StackTrace::WillUseFastUnwind(request_fast))
+    return Unwind(max_depth, pc, 0, context, 0, 0, false);
+
+  uptr top = 0;
+  uptr bottom = 0;
+  GetThreadStackTopAndBottom(/*at_initialization*/ false, &top, &bottom);
+
+  return Unwind(max_depth, pc, bp, context, top, bottom, request_fast);
 }
 
 namespace {
-size_t Backtrace(uintptr_t *TraceBuffer, size_t Size) {
+size_t BacktraceCommon(uintptr_t *TraceBuffer, size_t Size, void *Context) {
+  // Use the slow sanitizer unwinder in the segv handler. Fast frame pointer
+  // unwinders can end up dropping frames because the kernel sigreturn() frame's
+  // return address is the return address at time of fault. This has the result
+  // of never actually capturing the PC where the signal was raised.
+  bool UseFastUnwind = (Context == nullptr);
+
   __sanitizer::BufferedStackTrace Trace;
   Trace.Reset();
   if (Size > __sanitizer::kStackTraceMax)
     Size = __sanitizer::kStackTraceMax;
 
   Trace.Unwind((__sanitizer::uptr)__builtin_return_address(0),
-               (__sanitizer::uptr)__builtin_frame_address(0),
-               /* ucontext */ nullptr,
-               /* fast unwind */ true, Size - 1);
+               (__sanitizer::uptr)__builtin_frame_address(0), Context,
+               UseFastUnwind, Size - 1);
 
   memcpy(TraceBuffer, Trace.trace, Trace.size * sizeof(uintptr_t));
   return Trace.size;
 }
 
+size_t Backtrace(uintptr_t *TraceBuffer, size_t Size) {
+  return BacktraceCommon(TraceBuffer, Size, nullptr);
+}
+
+size_t SegvBacktrace(uintptr_t *TraceBuffer, size_t Size, void *Context) {
+  return BacktraceCommon(TraceBuffer, Size, Context);
+}
+
 static void PrintBacktrace(uintptr_t *Trace, size_t TraceLength,
                            gwp_asan::crash_handler::Printf_t Printf) {
   __sanitizer::StackTrace StackTrace;
@@ -77,4 +94,8 @@ crash_handler::PrintBacktrace_t getPrintBacktraceFunction() {
   return PrintBacktrace;
 }
 } // namespace options
+
+namespace crash_handler {
+SegvBacktrace_t getSegvBacktraceFunction() { return SegvBacktrace; }
+} // namespace crash_handler
 } // namespace gwp_asan

compiler-rt/lib/gwp_asan/optional/segv_handler.h

@@ -59,6 +59,15 @@ typedef void (*PrintBacktrace_t)(uintptr_t *TraceBuffer, size_t TraceLength,
 // without any symbolization.
 PrintBacktrace_t getBasicPrintBacktraceFunction();
 
+// Returns a function pointer to a backtrace function that's suitable for
+// unwinding through a signal handler. This is important primarily for frame-
+// pointer based unwinders, DWARF or other unwinders can simply provide the
+// normal backtrace function as the implementation here. On POSIX, SignalContext
+// should be the `ucontext_t` from the signal handler.
+typedef size_t (*SegvBacktrace_t)(uintptr_t *TraceBuffer, size_t Size,
+                                  void *SignalContext);
+SegvBacktrace_t getSegvBacktraceFunction();
+
 // Install the SIGSEGV crash handler for printing use-after-free and heap-
 // buffer-{under|over}flow exceptions if the user asked for it. This is platform
 // specific as even though POSIX and Windows both support registering handlers
@@ -67,14 +76,14 @@ PrintBacktrace_t getBasicPrintBacktraceFunction();
 // before this function.
 void installSignalHandlers(gwp_asan::GuardedPoolAllocator *GPA, Printf_t Printf,
                            PrintBacktrace_t PrintBacktrace,
-                           options::Backtrace_t Backtrace);
+                           SegvBacktrace_t SegvBacktrace);
 
 void uninstallSignalHandlers();
 
 void dumpReport(uintptr_t ErrorPtr, const gwp_asan::AllocatorState *State,
                 const gwp_asan::AllocationMetadata *Metadata,
-                options::Backtrace_t Backtrace, Printf_t Printf,
-                PrintBacktrace_t PrintBacktrace);
+                SegvBacktrace_t SegvBacktrace, Printf_t Printf,
+                PrintBacktrace_t PrintBacktrace, void *Context);
 } // namespace crash_handler
 } // namespace gwp_asan
 

compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp

@@ -23,14 +23,14 @@ using gwp_asan::Error;
 using gwp_asan::GuardedPoolAllocator;
 using gwp_asan::crash_handler::PrintBacktrace_t;
 using gwp_asan::crash_handler::Printf_t;
-using gwp_asan::options::Backtrace_t;
+using gwp_asan::crash_handler::SegvBacktrace_t;
 
 struct sigaction PreviousHandler;
 bool SignalHandlerInstalled;
 gwp_asan::GuardedPoolAllocator *GPAForSignalHandler;
 Printf_t PrintfForSignalHandler;
 PrintBacktrace_t PrintBacktraceForSignalHandler;
-Backtrace_t BacktraceForSignalHandler;
+SegvBacktrace_t BacktraceForSignalHandler;
 
 static void sigSegvHandler(int sig, siginfo_t *info, void *ucontext) {
   if (GPAForSignalHandler) {
@@ -40,7 +40,7 @@ static void sigSegvHandler(int sig, siginfo_t *info, void *ucontext) {
         reinterpret_cast<uintptr_t>(info->si_addr),
         GPAForSignalHandler->getAllocatorState(),
         GPAForSignalHandler->getMetadataRegion(), BacktraceForSignalHandler,
-        PrintfForSignalHandler, PrintBacktraceForSignalHandler);
+        PrintfForSignalHandler, PrintBacktraceForSignalHandler, ucontext);
   }
 
   // Process any previous handlers.
@@ -138,11 +138,11 @@ PrintBacktrace_t getBasicPrintBacktraceFunction() {
 
 void installSignalHandlers(gwp_asan::GuardedPoolAllocator *GPA, Printf_t Printf,
                            PrintBacktrace_t PrintBacktrace,
-                           options::Backtrace_t Backtrace) {
+                           SegvBacktrace_t SegvBacktrace) {
   GPAForSignalHandler = GPA;
   PrintfForSignalHandler = Printf;
   PrintBacktraceForSignalHandler = PrintBacktrace;
-  BacktraceForSignalHandler = Backtrace;
+  BacktraceForSignalHandler = SegvBacktrace;
 
   struct sigaction Action;
   Action.sa_sigaction = sigSegvHandler;
@@ -160,8 +160,8 @@ void uninstallSignalHandlers() {
 
 void dumpReport(uintptr_t ErrorPtr, const gwp_asan::AllocatorState *State,
                 const gwp_asan::AllocationMetadata *Metadata,
-                options::Backtrace_t Backtrace, Printf_t Printf,
-                PrintBacktrace_t PrintBacktrace) {
+                SegvBacktrace_t SegvBacktrace, Printf_t Printf,
+                PrintBacktrace_t PrintBacktrace, void *Context) {
   assert(State && "dumpReport missing Allocator State.");
   assert(Metadata && "dumpReport missing Metadata.");
   assert(Printf && "dumpReport missing Printf.");
@@ -194,7 +194,8 @@ void dumpReport(uintptr_t ErrorPtr, const gwp_asan::AllocatorState *State,
   // Print the fault backtrace.
   static constexpr unsigned kMaximumStackFramesForCrashTrace = 512;
   uintptr_t Trace[kMaximumStackFramesForCrashTrace];
-  size_t TraceLength = Backtrace(Trace, kMaximumStackFramesForCrashTrace);
+  size_t TraceLength =
+      SegvBacktrace(Trace, kMaximumStackFramesForCrashTrace, Context);
 
   PrintBacktrace(Trace, TraceLength, Printf);
 

compiler-rt/lib/gwp_asan/tests/CMakeLists.txt

@@ -5,7 +5,8 @@ set(GWP_ASAN_UNITTEST_CFLAGS
   ${COMPILER_RT_GTEST_CFLAGS}
   -I${COMPILER_RT_SOURCE_DIR}/lib/
   -O2
-  -g)
+  -g
+  -fno-omit-frame-pointer)
 
 file(GLOB GWP_ASAN_HEADERS ../*.h)
 set(GWP_ASAN_UNITTESTS

compiler-rt/lib/gwp_asan/tests/harness.h

@@ -86,7 +86,8 @@ public:
 
     gwp_asan::crash_handler::installSignalHandlers(
         &GPA, gwp_asan::test::getPrintfFunction(),
-        gwp_asan::options::getPrintBacktraceFunction(), Opts.Backtrace);
+        gwp_asan::options::getPrintBacktraceFunction(),
+        gwp_asan::crash_handler::getSegvBacktraceFunction());
   }
 
   void TearDown() override {

compiler-rt/lib/scudo/scudo_allocator.cpp

@@ -29,6 +29,7 @@
 # include "gwp_asan/guarded_pool_allocator.h"
 # include "gwp_asan/optional/backtrace.h"
 # include "gwp_asan/optional/options_parser.h"
+#include "gwp_asan/optional/segv_handler.h"
 #endif // GWP_ASAN_HOOKS
 
 #include <errno.h>
@@ -679,7 +680,8 @@ void initScudo() {
   if (Opts.InstallSignalHandlers)
     gwp_asan::crash_handler::installSignalHandlers(
         &GuardedAlloc, __sanitizer::Printf,
-        gwp_asan::options::getPrintBacktraceFunction(), Opts.Backtrace);
+        gwp_asan::options::getPrintBacktraceFunction(),
+        gwp_asan::crash_handler::getSegvBacktraceFunction());
 #endif // GWP_ASAN_HOOKS
 }
 

compiler-rt/lib/scudo/standalone/combined.h

@@ -192,7 +192,7 @@ public:
     if (Opt.InstallSignalHandlers)
       gwp_asan::crash_handler::installSignalHandlers(
           &GuardedAlloc, Printf, gwp_asan::options::getPrintBacktraceFunction(),
-          Opt.Backtrace);
+          gwp_asan::crash_handler::getSegvBacktraceFunction());
 #endif // GWP_ASAN_HOOKS
   }
 

compiler-rt/test/gwp_asan/backtrace.c

@@ -0,0 +1,29 @@
+// REQUIRES: gwp_asan
+// RUN: %clang_gwp_asan %s -g -o %t
+// RUN: %expect_crash %t 2>&1 | FileCheck %s
+
+#include <stdlib.h>
+
+__attribute__((noinline)) void *allocate_mem() { return malloc(1); }
+
+__attribute__((noinline)) void free_mem(void *ptr) { free(ptr); }
+
+__attribute__((noinline)) void touch_mem(void *ptr) {
+  volatile char sink = *((volatile char *)ptr);
+}
+
+// CHECK: Use After Free
+// CHECK: touch_mem
+// CHECK: was deallocated
+// CHECK: free_mem
+// CHECK: was allocated
+// CHECK: allocate_mem
+
+int main() {
+  for (unsigned i = 0; i < 0x10000; ++i) {
+    void *ptr = allocate_mem();
+    free_mem(ptr);
+    touch_mem(ptr);
+  }
+  return 0;
+}