[GWP-ASan] Update backtrace function signature.

Summary:
Updates the function signature and comments for backtracing (and printing
backtraces). This update brings GWP-ASan in line with future requirements for
stack frame compression, wherein the length of the trace is provided
explicitly, rather than relying on nullptr-termination.

Reviewers: vlad.tsyrklevich

Reviewed By: vlad.tsyrklevich

Subscribers: #sanitizers, llvm-commits, morehouse

Tags: #sanitizers, #llvm

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

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@368619 91177308-0d34-0410-b5e6-96231b3b80d8

6 files changed, 89 insertions, 57 deletions

diff --git a/lib/gwp_asan/guarded_pool_allocator.cpp b/lib/gwp_asan/guarded_pool_allocator.cpp
index 7e3628eba..9b49aa288 100644
--- a/lib/gwp_asan/guarded_pool_allocator.cpp
+++ b/lib/gwp_asan/guarded_pool_allocator.cpp
@@ -44,11 +44,12 @@ private:
   bool &Bool;
 };
 
-void defaultPrintStackTrace(uintptr_t *Trace, options::Printf_t Printf) {
-  if (Trace[0] == 0)
+void defaultPrintStackTrace(uintptr_t *Trace, size_t TraceLength,
+                            options::Printf_t Printf) {
+  if (TraceLength == 0)
     Printf("  <unknown (does your allocator support backtracing?)>\n");
 
-  for (size_t i = 0; Trace[i] != 0; ++i) {
+  for (size_t i = 0; i < TraceLength; ++i) {
     Printf("  #%zu 0x%zx in <unknown>\n", i, Trace[i]);
   }
   Printf("\n");
@@ -68,12 +69,12 @@ void GuardedPoolAllocator::AllocationMetadata::RecordAllocation(
   // TODO(hctim): Ask the caller to provide the thread ID, so we don't waste
   // other thread's time getting the thread ID under lock.
   AllocationTrace.ThreadID = getThreadID();
+  AllocationTrace.TraceLength = 0;
+  DeallocationTrace.TraceLength = 0;
   DeallocationTrace.ThreadID = kInvalidThreadID;
   if (Backtrace)
-    Backtrace(AllocationTrace.Trace, kMaximumStackFrames);
-  else
-    AllocationTrace.Trace[0] = 0;
-  DeallocationTrace.Trace[0] = 0;
+    AllocationTrace.TraceLength =
+        Backtrace(AllocationTrace.Trace, kMaximumStackFrames);
 }
 
 void GuardedPoolAllocator::AllocationMetadata::RecordDeallocation(
@@ -81,11 +82,11 @@ void GuardedPoolAllocator::AllocationMetadata::RecordDeallocation(
   IsDeallocated = true;
   // Ensure that the unwinder is not called if the recursive flag is set,
   // otherwise non-reentrant unwinders may deadlock.
+  DeallocationTrace.TraceLength = 0;
   if (Backtrace && !ThreadLocals.RecursiveGuard) {
     ScopedBoolean B(ThreadLocals.RecursiveGuard);
-    Backtrace(DeallocationTrace.Trace, kMaximumStackFrames);
-  } else {
-    DeallocationTrace.Trace[0] = 0;
+    DeallocationTrace.TraceLength =
+        Backtrace(DeallocationTrace.Trace, kMaximumStackFrames);
   }
   DeallocationTrace.ThreadID = getThreadID();
 }
@@ -442,7 +443,8 @@ void printAllocDeallocTraces(uintptr_t AccessPtr, AllocationMetadata *Meta,
       Printf("0x%zx was deallocated by thread %zu here:\n", AccessPtr,
              Meta->DeallocationTrace.ThreadID);
 
-    PrintBacktrace(Meta->DeallocationTrace.Trace, Printf);
+    PrintBacktrace(Meta->DeallocationTrace.Trace,
+                   Meta->DeallocationTrace.TraceLength, Printf);
   }
 
   if (Meta->AllocationTrace.ThreadID == GuardedPoolAllocator::kInvalidThreadID)
@@ -451,7 +453,8 @@ void printAllocDeallocTraces(uintptr_t AccessPtr, AllocationMetadata *Meta,
     Printf("0x%zx was allocated by thread %zu here:\n", Meta->Addr,
            Meta->AllocationTrace.ThreadID);
 
-  PrintBacktrace(Meta->AllocationTrace.Trace, Printf);
+  PrintBacktrace(Meta->AllocationTrace.Trace, Meta->AllocationTrace.TraceLength,
+                 Printf);
 }
 
 struct ScopedEndOfReportDecorator {
@@ -491,11 +494,11 @@ void GuardedPoolAllocator::reportErrorInternal(uintptr_t AccessPtr, Error E) {
   uint64_t ThreadID = getThreadID();
   printErrorType(E, AccessPtr, Meta, Printf, ThreadID);
   if (Backtrace) {
-    static constexpr unsigned kMaximumStackFramesForCrashTrace = 128;
+    static constexpr unsigned kMaximumStackFramesForCrashTrace = 512;
     uintptr_t Trace[kMaximumStackFramesForCrashTrace];
-    Backtrace(Trace, kMaximumStackFramesForCrashTrace);
+    size_t TraceLength = Backtrace(Trace, kMaximumStackFramesForCrashTrace);
 
-    PrintBacktrace(Trace, Printf);
+    PrintBacktrace(Trace, TraceLength, Printf);
   } else {
     Printf("  <unknown (does your allocator support backtracing?)>\n\n");
   }
diff --git a/lib/gwp_asan/guarded_pool_allocator.h b/lib/gwp_asan/guarded_pool_allocator.h
index 28a41110f..76a3cc6dc 100644
--- a/lib/gwp_asan/guarded_pool_allocator.h
+++ b/lib/gwp_asan/guarded_pool_allocator.h
@@ -51,11 +51,12 @@ public:
     void RecordDeallocation(options::Backtrace_t Backtrace);
 
     struct CallSiteInfo {
-      // The backtrace to the allocation/deallocation. If the first value is
-      // zero, we did not collect a trace.
+      // The backtrace to the allocation/deallocation.
       uintptr_t Trace[kMaximumStackFrames] = {};
       // The thread ID for this trace, or kInvalidThreadID if not available.
       uint64_t ThreadID = kInvalidThreadID;
+      // The length of the trace. Zero indicates that no trace was collected.
+      size_t TraceLength = 0;
     };
 
     // The address of this allocation.
diff --git a/lib/gwp_asan/optional/backtrace.h b/lib/gwp_asan/optional/backtrace.h
index 2700970e5..3aad9ead4 100644
--- a/lib/gwp_asan/optional/backtrace.h
+++ b/lib/gwp_asan/optional/backtrace.h
@@ -14,7 +14,10 @@
 namespace gwp_asan {
 namespace options {
 // Functions to get the platform-specific and implementation-specific backtrace
-// and backtrace printing functions.
+// and backtrace printing functions when RTGwpAsanBacktraceLibc or
+// RTGwpAsanBacktraceSanitizerCommon are linked. Use these functions to get the
+// backtrace function for populating the Options::Backtrace and
+// Options::PrintBacktrace when initialising the GuardedPoolAllocator.
 Backtrace_t getBacktraceFunction();
 PrintBacktrace_t getPrintBacktraceFunction();
 } // namespace options
diff --git a/lib/gwp_asan/optional/backtrace_linux_libc.cpp b/lib/gwp_asan/optional/backtrace_linux_libc.cpp
index f20a31009..a656c9b41 100644
--- a/lib/gwp_asan/optional/backtrace_linux_libc.cpp
+++ b/lib/gwp_asan/optional/backtrace_linux_libc.cpp
@@ -17,33 +17,23 @@
 #include "gwp_asan/options.h"
 
 namespace {
-void Backtrace(uintptr_t *TraceBuffer, size_t Size) {
-  // Grab (what seems to be) one more trace than we need. TraceBuffer needs to
-  // be null-terminated, but we wish to remove the frame of this function call.
+size_t Backtrace(uintptr_t *TraceBuffer, size_t Size) {
   static_assert(sizeof(uintptr_t) == sizeof(void *), "uintptr_t is not void*");
-  int NumTraces =
-      backtrace(reinterpret_cast<void **>(TraceBuffer), Size);
 
-  // Now shift the entire trace one place to the left and null-terminate.
-  memmove(TraceBuffer, TraceBuffer + 1, NumTraces * sizeof(void *));
-  TraceBuffer[NumTraces - 1] = 0;
+  return backtrace(reinterpret_cast<void **>(TraceBuffer), Size);
 }
 
-static void PrintBacktrace(uintptr_t *Trace,
+static void PrintBacktrace(uintptr_t *Trace, size_t TraceLength,
                            gwp_asan::options::Printf_t Printf) {
-  size_t NumTraces = 0;
-  for (; Trace[NumTraces] != 0; ++NumTraces) {
-  }
-
-  if (NumTraces == 0) {
+  if (TraceLength == 0) {
     Printf("  <not found (does your allocator support backtracing?)>\n\n");
     return;
   }
 
   char **BacktraceSymbols =
-      backtrace_symbols(reinterpret_cast<void **>(Trace), NumTraces);
+      backtrace_symbols(reinterpret_cast<void **>(Trace), TraceLength);
 
-  for (size_t i = 0; i < NumTraces; ++i) {
+  for (size_t i = 0; i < TraceLength; ++i) {
     if (!BacktraceSymbols)
       Printf("  #%zu %p\n", i, Trace[i]);
     else
diff --git a/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp b/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp
index 7d17eec0d..427ac77b6 100644
--- a/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp
+++ b/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp
@@ -26,7 +26,7 @@ void __sanitizer::BufferedStackTrace::UnwindImpl(uptr pc, uptr bp,
 }
 
 namespace {
-void Backtrace(uintptr_t *TraceBuffer, size_t Size) {
+size_t Backtrace(uintptr_t *TraceBuffer, size_t Size) {
   __sanitizer::BufferedStackTrace Trace;
   Trace.Reset();
   if (Size > __sanitizer::kStackTraceMax)
@@ -38,19 +38,14 @@ void Backtrace(uintptr_t *TraceBuffer, size_t Size) {
                /* fast unwind */ true, Size - 1);
 
   memcpy(TraceBuffer, Trace.trace, Trace.size * sizeof(uintptr_t));
-  TraceBuffer[Trace.size] = 0;
+  return Trace.size;
 }
 
-static void PrintBacktrace(uintptr_t *Trace,
+static void PrintBacktrace(uintptr_t *Trace, size_t TraceLength,
                            gwp_asan::options::Printf_t Printf) {
   __sanitizer::StackTrace StackTrace;
   StackTrace.trace = reinterpret_cast<__sanitizer::uptr *>(Trace);
-
-  for (StackTrace.size = 0; StackTrace.size < __sanitizer::kStackTraceMax;
-       ++StackTrace.size) {
-    if (Trace[StackTrace.size] == 0)
-      break;
-  }
+  StackTrace.size = TraceLength;
 
   if (StackTrace.size == 0) {
     Printf("  <unknown (does your allocator support backtracing?)>\n\n");
diff --git a/lib/gwp_asan/options.h b/lib/gwp_asan/options.h
index 6423e1652..67a612947 100644
--- a/lib/gwp_asan/options.h
+++ b/lib/gwp_asan/options.h
@@ -14,22 +14,62 @@
 
 namespace gwp_asan {
 namespace options {
-// The function pointer type for printf(). Follows the standard format from the
-// sanitizers library. If the supported allocator exposes printing via a
-// different function signature, please provide a wrapper which has this
-// printf() signature, and pass the wrapper instead.
+// ================================ Requirements ===============================
+// This function is required to be implemented by the supporting allocator. The
+// sanitizer::Printf() function can be simply used here.
+// ================================ Description ================================
+// This function shall produce output according to a strict subset of the C
+// standard library's printf() family. This function must support printing the
+// following formats:
+//   1. integers: "%([0-9]*)?(z|ll)?{d,u,x,X}"
+//   2. pointers: "%p"
+//   3. strings:  "%[-]([0-9]*)?(\\.\\*)?s"
+//   4. chars:    "%c"
+// =================================== Notes ===================================
+// This function has a slightly different signature than the C standard
+// library's printf(). Notably, it returns 'void' rather than 'int'.
 typedef void (*Printf_t)(const char *Format, ...);
 
-// The function pointer type for backtrace information. Required to be
-// implemented by the supporting allocator. The callee should elide itself and
-// all frames below itself from TraceBuffer, i.e. the caller's frame should be
-// in TraceBuffer[0], and subsequent frames 1..n into TraceBuffer[1..n], where a
-// maximum of `MaximumDepth - 1` frames are stored. TraceBuffer should be
-// nullptr-terminated (i.e. if there are 5 frames; TraceBuffer[5] == nullptr).
-// If the allocator cannot supply backtrace information, it should set
-// TraceBuffer[0] == nullptr.
-typedef void (*Backtrace_t)(uintptr_t *TraceBuffer, size_t Size);
-typedef void (*PrintBacktrace_t)(uintptr_t *TraceBuffer, Printf_t Print);
+// ================================ Requirements ===============================
+// This function is required to be either implemented by the supporting
+// allocator, or one of the two provided implementations may be used
+// (RTGwpAsanBacktraceLibc or RTGwpAsanBacktraceSanitizerCommon).
+// ================================ Description ================================
+// This function shall collect the backtrace for the calling thread and place
+// the result in `TraceBuffer`. This function should elide itself and all frames
+// below itself from `TraceBuffer`, i.e. the caller's frame should be in
+// TraceBuffer[0], and subsequent frames 1..n into TraceBuffer[1..n], where a
+// maximum of `Size` frames are stored. Returns the number of frames stored into
+// `TraceBuffer`, and zero on failure. If the return value of this function is
+// equal to `Size`, it may indicate that the backtrace is truncated.
+// =================================== Notes ===================================
+// This function may directly or indirectly call malloc(), as the
+// GuardedPoolAllocator contains a reentrancy barrier to prevent infinite
+// recursion. Any allocation made inside this function will be served by the
+// supporting allocator, and will not have GWP-ASan protections.
+typedef size_t (*Backtrace_t)(uintptr_t *TraceBuffer, size_t Size);
+
+// ================================ Requirements ===============================
+// This function is optional for the supporting allocator, but one of the two
+// provided implementations may be used (RTGwpAsanBacktraceLibc or
+// RTGwpAsanBacktraceSanitizerCommon). If not provided, a default implementation
+// is used which prints the raw pointers only.
+// ================================ Description ================================
+// This function shall take the backtrace provided in `TraceBuffer`, and print
+// it in a human-readable format using `Print`. Generally, this function shall
+// resolve raw pointers to section offsets and print them with the following
+// sanitizer-common format:
+//      "  #{frame_number} {pointer} in {function name} ({binary name}+{offset}"
+// e.g. "  #5 0x420459 in _start (/tmp/uaf+0x420459)"
+// This format allows the backtrace to be symbolized offline successfully using
+// llvm-symbolizer.
+// =================================== Notes ===================================
+// This function may directly or indirectly call malloc(), as the
+// GuardedPoolAllocator contains a reentrancy barrier to prevent infinite
+// recursion. Any allocation made inside this function will be served by the
+// supporting allocator, and will not have GWP-ASan protections.
+typedef void (*PrintBacktrace_t)(uintptr_t *TraceBuffer, size_t TraceLength,
+                                 Printf_t Print);
 
 struct Options {
   Printf_t Printf = nullptr;