[NFC][Sanitizer] Add argument checks to BufferedStackTrace::Unwind* functions

[rtrieu]

compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace.h

59–60

Is this suppose to be a run time check or a compile time check? The location of this assert makes it look like a run time check, only checking when the function is called, but that would require a regular assert instead of a static assert. If this is meant to be a compile time check as it is now, it would be clearer that it appears right after the two macros are defined rather than inside this function.

I think a run-time check is better to prevent compile errors from merely including this file without calling this function.

[rtrieu]

compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace.h

59–60

I've removed this assert for the time being so it doesn't block anyone over the weekend. r354720

[yln]

compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace.h

59–60

This was meant to be a sanity check at compile time. I was completely unaware that there are platform/architectures that don't support any unwinder. How do we handle such cases?

compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace.h

struct StackTrace {
   StackTrace(const uptr *trace, u32 size) : trace(trace), size(size), tag(0) {}
   StackTrace(const uptr *trace, u32 size, u32 tag)
       : trace(trace), size(size), tag(tag) {}
 
   // Prints a symbolized stacktrace, followed by an empty line.
   void Print() const;
 
   static bool WillUseFastUnwind(bool request_fast_unwind) {
+    static_assert(SANITIZER_CAN_FAST_UNWIND || SANITIZER_CAN_SLOW_UNWIND,
+                  "Neither fast nor slow unwinder is supported");
     if (!SANITIZER_CAN_FAST_UNWIND)
       return false;
-    else if (!SANITIZER_CAN_SLOW_UNWIND)
+    if (!SANITIZER_CAN_SLOW_UNWIND)
       return true;
     return request_fast_unwind;
   }
 
   static uptr GetCurrentPc();
   static inline uptr GetPreviousInstructionPc(uptr pc);
   static uptr GetNextInstructionPc(uptr pc);
   typedef bool (*SymbolizeCallback)(const void *pc, char *out_buffer,

compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace.cc

#ifdef __arm__
   return bp_prev;
 #else
   return (uhwptr*)bp;
 #endif
 }
 
 void BufferedStackTrace::UnwindFast(uptr pc, uptr bp, uptr stack_top,
                                     uptr stack_bottom, u32 max_depth) {
-  const uptr kPageSize = GetPageSizeCached();
+  CHECK_NE(stack_bottom, 0);
+  CHECK_GT(stack_top, stack_bottom);
   CHECK_GE(max_depth, 2);
+  const uptr kPageSize = GetPageSizeCached();
   trace_buffer[0] = pc;
   size = 1;
   if (stack_top < 4096) return;  // Sanity check for stack top.
   uhwptr *frame = GetCanonicFrame(bp, stack_top, stack_bottom);
   // Lowest possible address that makes sense as the next frame pointer.
   // Goes up as we walk the stack.
   uptr bottom = stack_bottom;
   // Avoid infinite loop when frame == frame[0] by using frame > prev_frame.

compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_sparc.cc

 
 #include "sanitizer_common.h"
 #include "sanitizer_stacktrace.h"
 
 namespace __sanitizer {
 
 void BufferedStackTrace::UnwindFast(uptr pc, uptr bp, uptr stack_top,
                                     uptr stack_bottom, u32 max_depth) {
-  const uptr kPageSize = GetPageSizeCached();
+  CHECK_NE(stack_bottom, 0);
+  CHECK_GT(stack_top, stack_bottom);
   CHECK_GE(max_depth, 2);
+  const uptr kPageSize = GetPageSizeCached();
   trace_buffer[0] = pc;
   size = 1;
   if (stack_top < 4096) return;  // Sanity check for stack top.
   // Flush register windows to memory
   asm volatile("ta 3" ::: "memory");
   uhwptr *frame = (uhwptr*)bp;
   // Lowest possible address that makes sense as the next frame pointer.
   // Goes up as we walk the stack.

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_markup.cc

void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) {
   uptr to_pop = LocatePcInTrace(pc);
   // trace_buffer[0] belongs to the current function so we always pop it,
   // unless there is only 1 frame in the stack trace (1 frame is always better
   // than 0!).
   PopStackFrames(Min(to_pop, static_cast<uptr>(1)));
   trace_buffer[0] = pc;
 }
 
-void BufferedStackTrace::UnwindSlow(uptr pc, void *context,
-                                                    u32 max_depth) {
-  CHECK_NE(context, nullptr);
+void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
+  CHECK(context);
+  CHECK_GE(max_depth, 2);
   UNREACHABLE("signal context doesn't exist");
 }
 #endif  // SANITIZER_CAN_SLOW_UNWIND
 
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_SYMBOLIZER_MARKUP

compiler-rt/trunk/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cc

void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) {
   // unwinder implementation (libgcc, libunwind, etc) which is outside of our
   // control.
   if (to_pop == 0 && size > 1)
     to_pop = 1;
   PopStackFrames(to_pop);
   trace_buffer[0] = pc;
 }
 
-void BufferedStackTrace::UnwindSlow(uptr pc, void *context,
-                                                    u32 max_depth) {
+void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
+  CHECK(context);
   CHECK_GE(max_depth, 2);
   if (!unwind_backtrace_signal_arch) {
     UnwindSlow(pc, max_depth);
     return;
   }
 
   void *map = acquire_my_map_info_list();
   CHECK(map);

compiler-rt/trunk/lib/sanitizer_common/sanitizer_unwind_win.cc

void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) {
   if (size == 0)
     return;
 
   // Skip the RTL frames by searching for the PC in the stacktrace.
   uptr pc_location = LocatePcInTrace(pc);
   PopStackFrames(pc_location);
 }
 
-void BufferedStackTrace::UnwindSlow(uptr pc, void *context,
-  u32 max_depth) {
+void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
+  CHECK(context);
+  CHECK_GE(max_depth, 2);
   CONTEXT ctx = *(CONTEXT *)context;
   STACKFRAME64 stack_frame;
   memset(&stack_frame, 0, sizeof(stack_frame));
 
   InitializeDbgHelpIfNeeded();
 
   size = 0;
 #if defined(_WIN64)