[sanitizer] Fix a crash in FP unwinder on ARM.

This change fixes 2 issues in the fast unwinder from r217079:
* A crash if a frame pointer points below current stack head, but
  inside the current thread stack limits. That memory may be
  unmapped. A check for this was lost in r217079.
* The last valid stack frame (the first one with an invalid next
  frame pointer) is always interpreted as a GCC layout frame. This
  results in garbled last PC in the (expected) case when the last
  frame has LLVM layout.

llvm-svn: 219683

2 files changed, 47 insertions, 11 deletions

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cc b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cc
index 7a00e3ad8f7..2c83185219e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cc
@@ -51,7 +51,15 @@ static inline uhwptr *GetCanonicFrame(uptr bp,
   if (!IsValidFrame(bp, stack_top, stack_bottom)) return 0;
   uhwptr *bp_prev = (uhwptr *)bp;
   if (IsValidFrame((uptr)bp_prev[0], stack_top, stack_bottom)) return bp_prev;
-  return bp_prev - 1;
+  // The next frame pointer does not look right. This could be a GCC frame, step
+  // back by 1 word and try again.
+  if (IsValidFrame((uptr)bp_prev[-1], stack_top, stack_bottom))
+    return bp_prev - 1;
+  // Nope, this does not look right either. This means the frame after next does
+  // not have a valid frame pointer, but we can still extract the caller PC.
+  // Unfortunately, there is no way to decide between GCC and LLVM frame
+  // layouts. Assume LLVM.
+  return bp_prev;
 #else
   return (uhwptr*)bp;
 #endif
@@ -65,18 +73,19 @@ void StackTrace::FastUnwindStack(uptr pc, uptr bp,
   size = 1;
   if (stack_top < 4096) return;  // Sanity check for stack top.
   uhwptr *frame = GetCanonicFrame(bp, stack_top, stack_bottom);
-  uhwptr *prev_frame = 0;
+  // 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.
-  while (frame > prev_frame &&
-         IsValidFrame((uptr)frame, stack_top, stack_bottom) &&
+  while (IsValidFrame((uptr)frame, stack_top, bottom) &&
          IsAligned((uptr)frame, sizeof(*frame)) &&
          size < max_depth) {
     uhwptr pc1 = frame[1];
     if (pc1 != pc) {
       trace[size++] = (uptr) pc1;
     }
-    prev_frame = frame;
-    frame = GetCanonicFrame((uptr)frame[0], stack_top, stack_bottom);
+    bottom = (uptr)frame;
+    frame = GetCanonicFrame((uptr)frame[0], stack_top, bottom);
   }
 }
 
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cc b/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cc
index cfd6ee599a6..2fb8fa6d98e 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cc
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cc
@@ -20,6 +20,7 @@ namespace __sanitizer {
 class FastUnwindTest : public ::testing::Test {
  protected:
   virtual void SetUp();
+  virtual void TearDown();
   bool TryFastUnwind(uptr max_depth) {
     if (!StackTrace::WillUseFastUnwind(true))
       return false;
@@ -28,7 +29,9 @@ class FastUnwindTest : public ::testing::Test {
     return true;
   }
 
-  uptr fake_stack[10];
+  void *mapping;
+  uptr *fake_stack;
+  const uptr fake_stack_size = 10;
   uptr start_pc;
   uptr fake_top;
   uptr fake_bottom;
@@ -40,22 +43,34 @@ static uptr PC(uptr idx) {
 }
 
 void FastUnwindTest::SetUp() {
+  size_t ps = GetPageSize();
+  mapping = MmapOrDie(2 * ps, "FastUnwindTest");
+  Mprotect((uptr)mapping, ps);
+
+  // Unwinder may peek 1 word down from the starting FP.
+  fake_stack = (uptr *)((uptr)mapping + ps + sizeof(uptr));
+
   // Fill an array of pointers with fake fp+retaddr pairs.  Frame pointers have
   // even indices.
-  for (uptr i = 0; i+1 < ARRAY_SIZE(fake_stack); i += 2) {
+  for (uptr i = 0; i + 1 < fake_stack_size; i += 2) {
     fake_stack[i] = (uptr)&fake_stack[i+2];  // fp
     fake_stack[i+1] = PC(i + 1); // retaddr
   }
   // Mark the last fp point back up to terminate the stack trace.
-  fake_stack[RoundDownTo(ARRAY_SIZE(fake_stack) - 1, 2)] = (uptr)&fake_stack[0];
+  fake_stack[RoundDownTo(fake_stack_size - 1, 2)] = (uptr)&fake_stack[0];
 
   // Top is two slots past the end because FastUnwindStack subtracts two.
-  fake_top = (uptr)&fake_stack[ARRAY_SIZE(fake_stack) + 2];
+  fake_top = (uptr)&fake_stack[fake_stack_size + 2];
   // Bottom is one slot before the start because FastUnwindStack uses >.
-  fake_bottom = (uptr)&fake_stack[-1];
+  fake_bottom = (uptr)mapping;
   start_pc = PC(0);
 }
 
+void FastUnwindTest::TearDown() {
+  size_t ps = GetPageSize();
+  UnmapOrDie(mapping, 2 * ps);
+}
+
 TEST_F(FastUnwindTest, Basic) {
   if (!TryFastUnwind(kStackTraceMax))
     return;
@@ -109,6 +124,18 @@ TEST_F(FastUnwindTest, ZeroFramesStackTrace) {
   EXPECT_EQ(0U, trace.top_frame_bp);
 }
 
+TEST_F(FastUnwindTest, FPBelowPrevFP) {
+  // The next FP points to unreadable memory inside the stack limits, but below
+  // current FP.
+  fake_stack[0] = (uptr)&fake_stack[-50];
+  fake_stack[1] = PC(1);
+  if (!TryFastUnwind(3))
+    return;
+  EXPECT_EQ(2U, trace.size);
+  EXPECT_EQ(PC(0), trace.trace[0]);
+  EXPECT_EQ(PC(1), trace.trace[1]);
+}
+
 TEST(SlowUnwindTest, ShortStackTrace) {
   if (StackTrace::WillUseFastUnwind(false))
     return;