[compiler-rt] r279237 - Cleanup: Move the *AddressDescription printing code to Print() members inside those structs.

Filipe Cabecinhas via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 19 06:07:24 PDT 2016


Author: filcab
Date: Fri Aug 19 08:07:23 2016
New Revision: 279237

URL: http://llvm.org/viewvc/llvm-project?rev=279237&view=rev
Log:
Cleanup: Move the *AddressDescription printing code to Print() members inside those structs.

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

Reviewers: kcc, samsonov

Subscribers: kubabrecka, llvm-commits

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

Modified:
    compiler-rt/trunk/lib/asan/asan_descriptions.cc
    compiler-rt/trunk/lib/asan/asan_descriptions.h
    compiler-rt/trunk/lib/asan/asan_report.cc

Modified: compiler-rt/trunk/lib/asan/asan_descriptions.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_descriptions.cc?rev=279237&r1=279236&r2=279237&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_descriptions.cc (original)
+++ compiler-rt/trunk/lib/asan/asan_descriptions.cc Fri Aug 19 08:07:23 2016
@@ -88,8 +88,7 @@ static bool GetShadowKind(uptr addr, Sha
 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;
 }
 
@@ -188,38 +187,7 @@ bool DescribeAddressIfHeap(uptr addr, up
         "(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;
 }
 
@@ -297,18 +265,80 @@ static void PrintAccessAndVarIntersectio
 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.
@@ -318,17 +348,17 @@ bool DescribeAddressIfStack(uptr addr, u
   // 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.
@@ -338,8 +368,8 @@ bool DescribeAddressIfStack(uptr addr, u
   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 "
@@ -349,58 +379,72 @@ bool DescribeAddressIfStack(uptr addr, u
   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);
+    Printf("%sallocated by thread T%d%s here:%s\n", d.Allocation(),
+           alloc_thread->tid,
+           ThreadNameWithParenthesis(alloc_thread, tname, sizeof(tname)),
+           d.EndAllocation());
   }
-  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;
+  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

Modified: compiler-rt/trunk/lib/asan/asan_descriptions.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_descriptions.h?rev=279237&r1=279236&r2=279237&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_descriptions.h (original)
+++ compiler-rt/trunk/lib/asan/asan_descriptions.h Fri Aug 19 08:07:23 2016
@@ -90,6 +90,8 @@ struct ShadowAddressDescription {
   uptr addr;
   ShadowKind kind;
   u8 shadow_byte;
+
+  void Print();
 };
 
 bool GetShadowAddressInformation(uptr addr, ShadowAddressDescription *descr);
@@ -118,6 +120,8 @@ struct HeapAddressDescription {
   u32 alloc_stack_id;
   u32 free_stack_id;
   ChunkAccess chunk_access;
+
+  void Print();
 };
 
 bool GetHeapAddressInformation(uptr addr, uptr access_size,
@@ -130,7 +134,10 @@ struct StackAddressDescription {
   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);
 
@@ -141,11 +148,24 @@ struct GlobalAddressDescription {
   __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

Modified: compiler-rt/trunk/lib/asan/asan_report.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_report.cc?rev=279237&r1=279236&r2=279237&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_report.cc (original)
+++ compiler-rt/trunk/lib/asan/asan_report.cc Fri Aug 19 08:07:23 2016
@@ -212,19 +212,6 @@ bool ParseFrameDescription(const char *f
   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
@@ -526,8 +513,8 @@ void ReportStringFunctionMemoryRangesOve
   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);
 }
 
@@ -541,7 +528,7 @@ void ReportStringFunctionSizeOverflow(up
   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);
 }
 
@@ -603,8 +590,8 @@ ReportInvalidPointerPair(uptr pc, uptr b
   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);
 }
 
@@ -792,7 +779,7 @@ void ReportGenericError(uptr pc, uptr bp
   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);
@@ -819,7 +806,7 @@ void NOINLINE __asan_set_error_report_ca
 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();
 }
 




More information about the llvm-commits mailing list