Cleanup: Move the *AddressDescription printing code to Print() members inside those structs.

[filcab]

Summary

The Print() members might take optional access_size and bug_type
parameters to still be able to provide the same information

[kcc]

LGTM

compiler-rt/trunk/lib/asan/asan_descriptions.h

 };
 static const char *const ShadowNames[] = {"low shadow", "shadow gap",
                                           "high shadow"};
 
 struct ShadowAddressDescription {
   uptr addr;
   ShadowKind kind;
   u8 shadow_byte;
+
+  void Print();
 };
 
 bool GetShadowAddressInformation(uptr addr, ShadowAddressDescription *descr);
 bool DescribeAddressIfShadow(uptr addr);
 
 enum AccessType {
   kAccessTypeLeft,
   kAccessTypeRight,

 
 struct HeapAddressDescription {
   uptr addr;
   uptr alloc_tid;
   uptr free_tid;
   u32 alloc_stack_id;
   u32 free_stack_id;
   ChunkAccess chunk_access;
+
+  void Print();
 };
 
 bool GetHeapAddressInformation(uptr addr, uptr access_size,
                                HeapAddressDescription *descr);
 bool DescribeAddressIfHeap(uptr addr, uptr access_size = 1);
 
 struct StackAddressDescription {
   uptr addr;
   uptr tid;
   uptr offset;
   uptr frame_pc;
   const char *frame_descr;
+
+  void Print(uptr access_size = 1);
 };
+
 bool GetStackAddressInformation(uptr addr, StackAddressDescription *descr);
 bool DescribeAddressIfStack(uptr addr, uptr access_size);
 
 struct GlobalAddressDescription {
   uptr addr;
   // Assume address is close to at most four globals.
   static const int kMaxGlobals = 4;
   __asan_global globals[kMaxGlobals];
   u32 reg_sites[kMaxGlobals];
   u8 size;
+
+  void Print(uptr access_size = 1, const char *bug_type = "");
 };
 
 bool GetGlobalAddressInformation(uptr addr, GlobalAddressDescription *descr);
 bool DescribeAddressIfGlobal(uptr addr, uptr access_size, const char *bug_type);
 
+// General function to describe an address. Will try to describe the address as
+// a shadow, global (variable), stack, or heap address.
+// bug_type is optional and is used for checking if we're reporting an
+// initialization-order-fiasco
+// The proper access_size should be passed for stack, global, and heap
+// addresses. Defaults to 1.
+// Each of the *AddressDescription functions has its own Print() member, which
+// may take access_size and bug_type parameters if needed.
+void PrintAddressDescription(uptr addr, uptr access_size = 1,
+                             const char *bug_type = "");
+
 }  // namespace __asan
 
 #endif  // ASAN_DESCRIPTIONS_H

compiler-rt/trunk/lib/asan/asan_descriptions.cc

if (AddrIsInShadowGap(addr)) {
     return false;
   }
   return true;
 }
 
 bool DescribeAddressIfShadow(uptr addr) {
   ShadowAddressDescription descr;
   if (!GetShadowAddressInformation(addr, &descr)) return false;
-  Printf("Address %p is located in the %s area.\n", addr,
-         ShadowNames[descr.kind]);
+  descr.Print();
   return true;
 }
 
 bool GetShadowAddressInformation(uptr addr, ShadowAddressDescription *descr) {
   if (AddrIsInMem(addr)) return false;
   ShadowKind shadow_kind;
   if (!GetShadowKind(addr, &shadow_kind)) return false;
   if (shadow_kind != kShadowKindGap) descr->shadow_byte = *(u8 *)addr;

 bool DescribeAddressIfHeap(uptr addr, uptr access_size) {
   HeapAddressDescription descr;
   if (!GetHeapAddressInformation(addr, access_size, &descr)) {
     Printf(
         "AddressSanitizer can not describe address in more detail "
         "(wild memory access suspected).\n");
     return false;
   }
-  PrintHeapChunkAccess(addr, descr.chunk_access);
-
-  asanThreadRegistry().CheckLocked();
-  AsanThreadContext *alloc_thread =
-      GetThreadContextByTidLocked(descr.alloc_tid);
-  StackTrace alloc_stack = GetStackTraceFromId(descr.alloc_stack_id);
-
-  char tname[128];
-  Decorator d;
-  AsanThreadContext *free_thread = nullptr;
-  if (descr.free_tid != kInvalidTid) {
-    free_thread = GetThreadContextByTidLocked(descr.free_tid);
-    Printf("%sfreed by thread T%d%s here:%s\n", d.Allocation(),
-           free_thread->tid,
-           ThreadNameWithParenthesis(free_thread, tname, sizeof(tname)),
-           d.EndAllocation());
-    StackTrace free_stack = GetStackTraceFromId(descr.free_stack_id);
-    free_stack.Print();
-    Printf("%spreviously allocated by thread T%d%s here:%s\n", d.Allocation(),
-           alloc_thread->tid,
-           ThreadNameWithParenthesis(alloc_thread, tname, sizeof(tname)),
-           d.EndAllocation());
-  } else {
-    Printf("%sallocated by thread T%d%s here:%s\n", d.Allocation(),
-           alloc_thread->tid,
-           ThreadNameWithParenthesis(alloc_thread, tname, sizeof(tname)),
-           d.EndAllocation());
-  }
-  alloc_stack.Print();
-  DescribeThread(GetCurrentThread());
-  if (free_thread) DescribeThread(free_thread);
-  DescribeThread(alloc_thread);
+  descr.Print();
   return true;
 }
 
 // Stack descriptions
 bool GetStackAddressInformation(uptr addr, StackAddressDescription *descr) {
   AsanThread *t = FindThreadByStackAddress(addr);
   if (!t) return false;
 
if (pos_descr) {
     str.append("\n");
   }
   Printf("%s", str.data());
 }
 
 bool DescribeAddressIfStack(uptr addr, uptr access_size) {
   StackAddressDescription descr;
   if (!GetStackAddressInformation(addr, &descr)) return false;
+  descr.Print(access_size);
+  return true;
+}
+
+// Global descriptions
+static void DescribeAddressRelativeToGlobal(uptr addr, uptr access_size,
+                                            const __asan_global &g) {
+  InternalScopedString str(4096);
+  Decorator d;
+  str.append("%s", d.Location());
+  if (addr < g.beg) {
+    str.append("%p is located %zd bytes to the left", (void *)addr,
+               g.beg - addr);
+  } else if (addr + access_size > g.beg + g.size) {
+    if (addr < g.beg + g.size) addr = g.beg + g.size;
+    str.append("%p is located %zd bytes to the right", (void *)addr,
+               addr - (g.beg + g.size));
+  } else {
+    // Can it happen?
+    str.append("%p is located %zd bytes inside", (void *)addr, addr - g.beg);
+  }
+  str.append(" of global variable '%s' defined in '",
+             MaybeDemangleGlobalName(g.name));
+  PrintGlobalLocation(&str, g);
+  str.append("' (0x%zx) of size %zu\n", g.beg, g.size);
+  str.append("%s", d.EndLocation());
+  PrintGlobalNameIfASCII(&str, g);
+  Printf("%s", str.data());
+}
+
+bool GetGlobalAddressInformation(uptr addr, GlobalAddressDescription *descr) {
+  descr->addr = addr;
+  int globals_num = GetGlobalsForAddress(addr, descr->globals, descr->reg_sites,
+                                         ARRAY_SIZE(descr->globals));
+  descr->size = globals_num;
+  return globals_num != 0;
+}
+
+bool DescribeAddressIfGlobal(uptr addr, uptr access_size,
+                             const char *bug_type) {
+  GlobalAddressDescription descr;
+  if (!GetGlobalAddressInformation(addr, &descr)) return false;
+
+  descr.Print(access_size, bug_type);
+  return true;
+}
+
+void ShadowAddressDescription::Print() {
+  Printf("Address %p is located in the %s area.\n", addr, ShadowNames[kind]);
+}
+
+void GlobalAddressDescription::Print(uptr access_size, const char *bug_type) {
+  for (int i = 0; i < size; i++) {
+    DescribeAddressRelativeToGlobal(addr, access_size, globals[i]);
+    if (0 == internal_strcmp(bug_type, "initialization-order-fiasco") &&
+        reg_sites[i]) {
+      Printf("  registered at:\n");
+      StackDepotGet(reg_sites[i]).Print();
+    }
+  }
+}
 
+void StackAddressDescription::Print(uptr access_size) {
   Decorator d;
   char tname[128];
   Printf("%s", d.Location());
-  Printf("Address %p is located in stack of thread T%d%s", addr, descr.tid,
-         ThreadNameWithParenthesis(descr.tid, tname, sizeof(tname)));
+  Printf("Address %p is located in stack of thread T%d%s", addr, tid,
+         ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
 
-  if (!descr.frame_descr) {
+  if (!frame_descr) {
     Printf("%s\n", d.EndLocation());
-    return true;
+    return;
   }
-  Printf(" at offset %zu in frame%s\n", descr.offset, d.EndLocation());
+  Printf(" at offset %zu in frame%s\n", 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).
   Printf("%s", d.EndLocation());
-  StackTrace alloca_stack(&descr.frame_pc, 1);
+  StackTrace alloca_stack(&frame_pc, 1);
   alloca_stack.Print();
 
   InternalMmapVector<StackVarDescr> vars(16);
-  if (!ParseFrameDescription(descr.frame_descr, &vars)) {
+  if (!ParseFrameDescription(frame_descr, &vars)) {
     Printf(
         "AddressSanitizer can't parse the stack frame "
         "descriptor: |%s|\n",
-        descr.frame_descr);
+        frame_descr);
     // 'addr' is a stack address, so return true even if we can't parse frame
-    return true;
+    return;
   }
   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.
   for (uptr i = 0; i < n_objects; i++) {
     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(vars[i], descr.offset, access_size,
-                                  prev_var_end, next_var_beg);
+    PrintAccessAndVarIntersection(vars[i], 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");
 
-  DescribeThread(GetThreadContextByTidLocked(descr.tid));
-  return true;
+  DescribeThread(GetThreadContextByTidLocked(tid));
 }
 
-// Global descriptions
-static void DescribeAddressRelativeToGlobal(uptr addr, uptr access_size,
-                                            const __asan_global &g) {
-  InternalScopedString str(4096);
+void HeapAddressDescription::Print() {
+  PrintHeapChunkAccess(addr, chunk_access);
+
+  asanThreadRegistry().CheckLocked();
+  AsanThreadContext *alloc_thread = GetThreadContextByTidLocked(alloc_tid);
+  StackTrace alloc_stack = GetStackTraceFromId(alloc_stack_id);
+
+  char tname[128];
   Decorator d;
-  str.append("%s", d.Location());
-  if (addr < g.beg) {
-    str.append("%p is located %zd bytes to the left", (void *)addr,
-               g.beg - addr);
-  } else if (addr + access_size > g.beg + g.size) {
-    if (addr < g.beg + g.size) addr = g.beg + g.size;
-    str.append("%p is located %zd bytes to the right", (void *)addr,
-               addr - (g.beg + g.size));
+  AsanThreadContext *free_thread = nullptr;
+  if (free_tid != kInvalidTid) {
+    free_thread = GetThreadContextByTidLocked(free_tid);
+    Printf("%sfreed by thread T%d%s here:%s\n", d.Allocation(),
+           free_thread->tid,
+           ThreadNameWithParenthesis(free_thread, tname, sizeof(tname)),
+           d.EndAllocation());
+    StackTrace free_stack = GetStackTraceFromId(free_stack_id);
+    free_stack.Print();
+    Printf("%spreviously allocated by thread T%d%s here:%s\n", d.Allocation(),
+           alloc_thread->tid,
+           ThreadNameWithParenthesis(alloc_thread, tname, sizeof(tname)),
+           d.EndAllocation());
   } else {
-    // Can it happen?
-    str.append("%p is located %zd bytes inside", (void *)addr, addr - g.beg);
-  }
-  str.append(" of global variable '%s' defined in '",
-             MaybeDemangleGlobalName(g.name));
-  PrintGlobalLocation(&str, g);
-  str.append("' (0x%zx) of size %zu\n", g.beg, g.size);
-  str.append("%s", d.EndLocation());
-  PrintGlobalNameIfASCII(&str, g);
-  Printf("%s", str.data());
+    Printf("%sallocated by thread T%d%s here:%s\n", d.Allocation(),
+           alloc_thread->tid,
+           ThreadNameWithParenthesis(alloc_thread, tname, sizeof(tname)),
+           d.EndAllocation());
   }
-
-bool GetGlobalAddressInformation(uptr addr, GlobalAddressDescription *descr) {
-  descr->addr = addr;
-  int globals_num = GetGlobalsForAddress(addr, descr->globals, descr->reg_sites,
-                                         ARRAY_SIZE(descr->globals));
-  descr->size = globals_num;
-  return globals_num != 0;
+  alloc_stack.Print();
+  DescribeThread(GetCurrentThread());
+  if (free_thread) DescribeThread(free_thread);
+  DescribeThread(alloc_thread);
 }
 
-bool DescribeAddressIfGlobal(uptr addr, uptr access_size,
+void PrintAddressDescription(uptr addr, uptr access_size,
                              const char *bug_type) {
-  GlobalAddressDescription descr;
-  if (!GetGlobalAddressInformation(addr, &descr)) return false;
+  ShadowAddressDescription shadow_descr;
+  if (GetShadowAddressInformation(addr, &shadow_descr)) {
+    shadow_descr.Print();
+    return;
+  }
 
-  for (int i = 0; i < descr.size; i++) {
-    DescribeAddressRelativeToGlobal(descr.addr, access_size, descr.globals[i]);
-    if (0 == internal_strcmp(bug_type, "initialization-order-fiasco") &&
-        descr.reg_sites[i]) {
-      Printf("  registered at:\n");
-      StackDepotGet(descr.reg_sites[i]).Print();
-    }
+  GlobalAddressDescription global_descr;
+  if (GetGlobalAddressInformation(addr, &global_descr)) {
+    global_descr.Print(access_size, bug_type);
+    return;
+
+  StackAddressDescription stack_descr;
+  if (GetStackAddressInformation(addr, &stack_descr)) {
+    stack_descr.Print(access_size);
+    return;
   }
-  return true;
+
+  HeapAddressDescription heap_descr;
+  if (GetHeapAddressInformation(addr, access_size, &heap_descr)) {
+    heap_descr.Print();
+    return;
   }
 
+  // We exhausted our possibilities. Bail out.
+  Printf(
+      "AddressSanitizer can not describe address in more detail "
+      "(wild memory access suspected).\n");
+}
 }  // namespace __asan

compiler-rt/trunk/lib/asan/asan_report.cc

for (uptr i = 0; i < n_objects; i++) {
     StackVarDescr var = {beg, size, p, len};
     vars->push_back(var);
     p += len;
   }
 
   return true;
 }
 
-static void DescribeAddress(uptr addr, uptr access_size, const char *bug_type) {
-  // Check if this is shadow or shadow gap.
-  if (DescribeAddressIfShadow(addr))
-    return;
-  CHECK(AddrIsInMem(addr));
-  if (DescribeAddressIfGlobal(addr, access_size, bug_type))
-    return;
-  if (DescribeAddressIfStack(addr, access_size))
-    return;
-  // Assume it is a heap address.
-  DescribeAddressIfHeap(addr, access_size);
-}
-
 // -------------------- Different kinds of reports ----------------- {{{1
 
 // Use ScopedInErrorReport to run common actions just before and
 // immediately after printing error report.
 class ScopedInErrorReport {
  public:
   explicit ScopedInErrorReport(ReportData *report = nullptr,
                                bool fatal = false) {
void ReportStringFunctionMemoryRangesOverlap(const char *function,
   internal_snprintf(bug_type, sizeof(bug_type), "%s-param-overlap", function);
   Printf("%s", d.Warning());
   Report("ERROR: AddressSanitizer: %s: "
              "memory ranges [%p,%p) and [%p, %p) overlap\n", \
              bug_type, offset1, offset1 + length1, offset2, offset2 + length2);
   Printf("%s", d.EndWarning());
   ScarinessScore::PrintSimple(10, bug_type);
   stack->Print();
-  DescribeAddress((uptr)offset1, length1, bug_type);
-  DescribeAddress((uptr)offset2, length2, bug_type);
+  PrintAddressDescription((uptr)offset1, length1, bug_type);
+  PrintAddressDescription((uptr)offset2, length2, bug_type);
   ReportErrorSummary(bug_type, stack);
 }
 
 void ReportStringFunctionSizeOverflow(uptr offset, uptr size,
                                       BufferedStackTrace *stack) {
   ScopedInErrorReport in_report;
   Decorator d;
   const char *bug_type = "negative-size-param";
   Printf("%s", d.Warning());
   Report("ERROR: AddressSanitizer: %s: (size=%zd)\n", bug_type, size);
   Printf("%s", d.EndWarning());
   ScarinessScore::PrintSimple(10, bug_type);
   stack->Print();
-  DescribeAddress(offset, size, bug_type);
+  PrintAddressDescription(offset, size, bug_type);
   ReportErrorSummary(bug_type, stack);
 }
 
 void ReportBadParamsToAnnotateContiguousContainer(uptr beg, uptr end,
                                                   uptr old_mid, uptr new_mid,
                                                   BufferedStackTrace *stack) {
   ScopedInErrorReport in_report;
   Report("ERROR: AddressSanitizer: bad parameters to "
ReportInvalidPointerPair(uptr pc, uptr bp, uptr sp, uptr a1, uptr a2) {
   ScopedInErrorReport in_report;
   const char *bug_type = "invalid-pointer-pair";
   Decorator d;
   Printf("%s", d.Warning());
   Report("ERROR: AddressSanitizer: invalid-pointer-pair: %p %p\n", a1, a2);
   Printf("%s", d.EndWarning());
   GET_STACK_TRACE_FATAL(pc, bp);
   stack.Print();
-  DescribeAddress(a1, 1, bug_type);
-  DescribeAddress(a2, 1, bug_type);
+  PrintAddressDescription(a1, 1, bug_type);
+  PrintAddressDescription(a2, 1, bug_type);
   ReportErrorSummary(bug_type, &stack);
 }
 
 static INLINE void CheckForInvalidPointerPair(void *p1, void *p2) {
   if (!flags()->detect_invalid_pointer_pairs) return;
   uptr a1 = reinterpret_cast<uptr>(p1);
   uptr a2 = reinterpret_cast<uptr>(p2);
   AsanChunkView chunk1 = FindHeapChunkByAddress(a1);
Printf("%s%s of size %zu at %p thread T%d%s%s\n",
          access_size, (void*)addr, curr_tid,
          ThreadNameWithParenthesis(curr_tid, tname, sizeof(tname)),
          d.EndAccess());
 
   SS.Print();
   GET_STACK_TRACE_FATAL(pc, bp);
   stack.Print();
 
-  DescribeAddress(addr, access_size, bug_descr);
+  PrintAddressDescription(addr, access_size, bug_descr);
   if (shadow_val == kAsanContiguousContainerOOBMagic)
     PrintContainerOverflowHint();
   ReportErrorSummary(bug_descr, &stack);
   PrintShadowMemoryForAddress(addr);
 }
 
 }  // namespace __asan
 

 void NOINLINE __asan_set_error_report_callback(void (*callback)(const char*)) {
   BlockingMutexLock l(&error_message_buf_mutex);
   error_report_callback = callback;
 }
 
 void __asan_describe_address(uptr addr) {
   // Thread registry must be locked while we're describing an address.
   asanThreadRegistry().Lock();
-  DescribeAddress(addr, 1, "");
+  PrintAddressDescription(addr, 1, "");
   asanThreadRegistry().Unlock();
 }
 
 int __asan_report_present() {
   return report_happened ? 1 : 0;
 }
 
 uptr __asan_get_report_pc() {