[ASan] Make stack-buffer-overflow reports more robust

[samsonov]

Summary

Fix the function that gets stack frame description by address in
thread stack, so that it clearly indicates failures. Make this error non-fatal,
and print as much information as we can in this case. Make all errors in
ParseFrameDescription non-fatal.

[samsonov]

Upload correct set of files.

[kcc]

Is this change testable?

lib/asan/asan_report.cc

458

Isn't this buffer too large? (we may get stack overflow while reporting)

[samsonov]

Testing this is hard. I've tested this change on a large program with racy use-after-return, and failed to reproduce this in a small test case.

lib/asan/asan_report.cc

458

Yeah, we may get rid of this buffer completely. I'll address this in a separate change.

[kcc]

LGTM

[samsonov]

lib/asan/asan_report.cc

458

Done in r218827.

lib/asan/asan_debugging.cc

 namespace __asan {
 
 void GetInfoForStackVar(uptr addr, AddressDescription *descr, AsanThread *t) {
   descr->name[0] = 0;
   descr->region_address = 0;
   descr->region_size = 0;
   descr->region_kind = "stack";
 
-  uptr offset = 0;
-  uptr frame_pc = 0;
-  const char *frame_descr = t->GetFrameNameByAddr(addr, &offset, &frame_pc);
+  AsanThread::StackFrameAccess access;
+  if (!t->GetStackFrameAccessByAddr(addr, &access))
+    return;
   InternalMmapVector<StackVarDescr> vars(16);
-  if (!ParseFrameDescription(frame_descr, &vars)) {
+  if (!ParseFrameDescription(access.frame_descr, &vars)) {
     return;
   }
 
   for (uptr i = 0; i < vars.size(); i++) {
-    if (offset <= vars[i].beg + vars[i].size) {
+    if (access.offset <= vars[i].beg + vars[i].size) {
       internal_strncat(descr->name, vars[i].name_pos,
                        Min(descr->name_size, vars[i].name_len));
-      descr->region_address = addr - (offset - vars[i].beg);
+      descr->region_address = addr - (access.offset - vars[i].beg);
       descr->region_size = vars[i].size;
       return;
     }
   }
 }
 
 void GetInfoForHeapAddress(uptr addr, AddressDescription *descr) {
   AsanChunkView chunk = FindHeapChunkByAddress(addr);

lib/asan/asan_report.cc

void PrintAccessAndVarIntersection(const char *var_name,
   } else {
     str.append("\n");
   }
   Printf("%s", str.data());
 }
 
 bool ParseFrameDescription(const char *frame_descr,
                            InternalMmapVector<StackVarDescr> *vars) {
+  CHECK(frame_descr);
   char *p;
+  // This string is created by the compiler and has the following form:
+  // "n alloc_1 alloc_2 ... alloc_n"
+  // where alloc_i looks like "offset size len ObjectName".
   uptr n_objects = (uptr)internal_simple_strtoll(frame_descr, &p, 10);
-  CHECK_GT(n_objects, 0);
+  if (n_objects == 0)
+    return false;
 
   for (uptr i = 0; i < n_objects; i++) {
     uptr beg  = (uptr)internal_simple_strtoll(p, &p, 10);
     uptr size = (uptr)internal_simple_strtoll(p, &p, 10);
     uptr len  = (uptr)internal_simple_strtoll(p, &p, 10);
     if (beg == 0 || size == 0 || *p != ' ') {
       return false;
     }
     p++;
     StackVarDescr var = {beg, size, p, len};
     vars->push_back(var);
     p += len;
   }
 
   return true;
 }
 
 bool DescribeAddressIfStack(uptr addr, uptr access_size) {
   AsanThread *t = FindThreadByStackAddress(addr);
   if (!t) return false;
-  const uptr kBufSize = 4095;
-  char buf[kBufSize];
-  uptr offset = 0;
-  uptr frame_pc = 0;
-  char tname[128];
-  const char *frame_descr = t->GetFrameNameByAddr(addr, &offset, &frame_pc);
 
-#ifdef __powerpc64__
-  // On PowerPC64, the address of a function actually points to a
-  // three-doubleword data structure with the first field containing
-  // the address of the function's code.
-  frame_pc = *reinterpret_cast<uptr *>(frame_pc);
-#endif
-
-  // This string is created by the compiler and has the following form:
-  // "n alloc_1 alloc_2 ... alloc_n"
-  // where alloc_i looks like "offset size len ObjectName ".
-  CHECK(frame_descr);
   Decorator d;
+  char tname[128];
   Printf("%s", d.Location());
-  Printf("Address %p is located in stack of thread T%d%s "
-         "at offset %zu in frame\n",
-         addr, t->tid(),
-         ThreadNameWithParenthesis(t->tid(), tname, sizeof(tname)),
-         offset);
+  Printf("Address %p is located in stack of thread T%d%s", addr, t->tid(),
+         ThreadNameWithParenthesis(t->tid(), tname, sizeof(tname)));
+
+  // Try to fetch precise stack frame for this access.
+  AsanThread::StackFrameAccess access;
+  if (!t->GetStackFrameAccessByAddr(addr, &access)) {
+    Printf("%s\n", d.EndLocation());
+    return true;
+  }
+  Printf(" at offset %zu in frame%s\n", access.offset, d.EndLocation());
+
   // Now we print the frame where the alloca has happened.
   // We print this frame as a stack trace with one element.
   // The symbolizer may print more than one frame if inlining was involved.
   // The frame numbers may be different than those in the stack trace printed
   // previously. That's unfortunate, but I have no better solution,
   // especially given that the alloca may be from entirely different place
   // (e.g. use-after-scope, or different thread's stack).
   StackTrace alloca_stack;
-  alloca_stack.trace[0] = frame_pc + 16;
+#ifdef __powerpc64__
+  // On PowerPC64, the address of a function actually points to a
+  // three-doubleword data structure with the first field containing
+  // the address of the function's code.
+  access.frame_pc = *reinterpret_cast<uptr *>(access.frame_pc);
+#endif
+  alloca_stack.trace[0] = access.frame_pc + 16;
   alloca_stack.size = 1;
   Printf("%s", d.EndLocation());
   alloca_stack.Print();
 
   InternalMmapVector<StackVarDescr> vars(16);
-  if (!ParseFrameDescription(frame_descr, &vars)) {
+  if (!ParseFrameDescription(access.frame_descr, &vars)) {
     Printf("AddressSanitizer can't parse the stack frame "
-           "descriptor: |%s|\n", frame_descr);
+           "descriptor: |%s|\n", access.frame_descr);
     // 'addr' is a stack address, so return true even if we can't parse frame
     return true;
   }
   uptr n_objects = vars.size();
   // Report the number of stack objects.
   Printf("  This frame has %zu object(s):\n", n_objects);
 
   // Report all objects in this frame.
+  const uptr kBufSize = 4095;
+  char buf[kBufSize];
   for (uptr i = 0; i < n_objects; i++) {
     buf[0] = 0;
     internal_strncat(buf, vars[i].name_pos,
                      static_cast<uptr>(Min(kBufSize, vars[i].name_len)));
     uptr prev_var_end = i ? vars[i - 1].beg + vars[i - 1].size : 0;
     uptr next_var_beg = i + 1 < n_objects ? vars[i + 1].beg : ~(0UL);
-    PrintAccessAndVarIntersection(buf, vars[i].beg, vars[i].size,
-                                  offset, access_size,
-                                  prev_var_end, next_var_beg);
+    PrintAccessAndVarIntersection(buf, vars[i].beg, vars[i].size, access.offset,
+                                  access_size, prev_var_end, next_var_beg);
   }
   Printf("HINT: this may be a false positive if your program uses "
          "some custom stack unwind mechanism or swapcontext\n");
   if (SANITIZER_WINDOWS)
     Printf("      (longjmp, SEH and C++ exceptions *are* supported)\n");
   else
     Printf("      (longjmp and C++ exceptions *are* supported)\n");
 

lib/asan/asan_thread.h

public:
   uptr stack_bottom() { return stack_bottom_; }
   uptr stack_size() { return stack_size_; }
   uptr tls_begin() { return tls_begin_; }
   uptr tls_end() { return tls_end_; }
   u32 tid() { return context_->tid; }
   AsanThreadContext *context() { return context_; }
   void set_context(AsanThreadContext *context) { context_ = context; }
 
-  const char *GetFrameNameByAddr(uptr addr, uptr *offset, uptr *frame_pc);
+  struct StackFrameAccess {
+    uptr offset;
+    uptr frame_pc;
+    const char *frame_descr;
+  };
+  bool GetStackFrameAccessByAddr(uptr addr, StackFrameAccess *access);
 
   bool AddrIsInStack(uptr addr) {
     return addr >= stack_bottom_ && addr < stack_top_;
   }
 
   void DeleteFakeStack(int tid) {
     if (!fake_stack_) return;
     FakeStack *t = fake_stack_;

lib/asan/asan_thread.cc

 }
 
 void AsanThread::ClearShadowForThreadStackAndTLS() {
   PoisonShadow(stack_bottom_, stack_top_ - stack_bottom_, 0);
   if (tls_begin_ != tls_end_)
     PoisonShadow(tls_begin_, tls_end_ - tls_begin_, 0);
 }
 
-const char *AsanThread::GetFrameNameByAddr(uptr addr, uptr *offset,
-                                           uptr *frame_pc) {
+bool AsanThread::GetStackFrameAccessByAddr(uptr addr,
+                                           StackFrameAccess *access) {
   uptr bottom = 0;
   if (AddrIsInStack(addr)) {
     bottom = stack_bottom();
   } else if (has_fake_stack()) {
     bottom = fake_stack()->AddrIsInFakeStack(addr);
     CHECK(bottom);
-    *offset = addr - bottom;
-    *frame_pc = ((uptr*)bottom)[2];
-    return  (const char *)((uptr*)bottom)[1];
+    access->offset = addr - bottom;
+    access->frame_pc = ((uptr*)bottom)[2];
+    access->frame_descr = (const char *)((uptr*)bottom)[1];
+    return true;
   }
   uptr aligned_addr = addr & ~(SANITIZER_WORDSIZE/8 - 1);  // align addr.
   u8 *shadow_ptr = (u8*)MemToShadow(aligned_addr);
   u8 *shadow_bottom = (u8*)MemToShadow(bottom);
 
   while (shadow_ptr >= shadow_bottom &&
          *shadow_ptr != kAsanStackLeftRedzoneMagic) {
     shadow_ptr--;
   }
 
   while (shadow_ptr >= shadow_bottom &&
          *shadow_ptr == kAsanStackLeftRedzoneMagic) {
     shadow_ptr--;
   }
 
   if (shadow_ptr < shadow_bottom) {
-    *offset = 0;
-    return "UNKNOWN";
+    return false;
   }
 
   uptr* ptr = (uptr*)SHADOW_TO_MEM((uptr)(shadow_ptr + 1));
   CHECK(ptr[0] == kCurrentStackFrameMagic);
-  *offset = addr - (uptr)ptr;
-  *frame_pc = ptr[2];
-  return (const char*)ptr[1];
+  access->offset = addr - (uptr)ptr;
+  access->frame_pc = ptr[2];
+  access->frame_descr = (const char*)ptr[1];
+  return true;
 }
 
 static bool ThreadStackContainsAddress(ThreadContextBase *tctx_base,
                                        void *addr) {
   AsanThreadContext *tctx = static_cast<AsanThreadContext*>(tctx_base);
   AsanThread *t = tctx->thread;
   if (!t) return false;
   if (t->AddrIsInStack((uptr)addr)) return true;