[ASan] Minor ASan error reporting cleanup

Summary:
- use proper Error() decorator for error messages
- refactor ASan thread id and name reporting

Reviewers: eugenis

Subscribers: kubamracek, delcypher, #sanitizers, llvm-commits

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

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@336573 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 84 insertions, 100 deletions

diff --git a/lib/asan/asan_descriptions.cc b/lib/asan/asan_descriptions.cc
index 1c70fed6f..cdb562d97 100644
--- a/lib/asan/asan_descriptions.cc
+++ b/lib/asan/asan_descriptions.cc
@@ -20,23 +20,25 @@
 
 namespace __asan {
 
-// Return " (thread_name) " or an empty string if the name is empty.
-const char *ThreadNameWithParenthesis(AsanThreadContext *t, char buff[],
-                                      uptr buff_len) {
-  const char *name = t->name;
-  if (name[0] == '\0') return "";
-  buff[0] = 0;
-  internal_strncat(buff, " (", 3);
-  internal_strncat(buff, name, buff_len - 4);
-  internal_strncat(buff, ")", 2);
-  return buff;
+AsanThreadIdAndName::AsanThreadIdAndName(AsanThreadContext *t) {
+  Init(t->tid, t->name);
 }
 
-const char *ThreadNameWithParenthesis(u32 tid, char buff[], uptr buff_len) {
-  if (tid == kInvalidTid) return "";
-  asanThreadRegistry().CheckLocked();
-  AsanThreadContext *t = GetThreadContextByTidLocked(tid);
-  return ThreadNameWithParenthesis(t, buff, buff_len);
+AsanThreadIdAndName::AsanThreadIdAndName(u32 tid) {
+  if (tid == kInvalidTid) {
+    Init(tid, "");
+  } else {
+    asanThreadRegistry().CheckLocked();
+    AsanThreadContext *t = GetThreadContextByTidLocked(tid);
+    Init(tid, t->name);
+  }
+}
+
+void AsanThreadIdAndName::Init(u32 tid, const char *tname) {
+  int len = internal_snprintf(name, sizeof(name), "T%d", tid);
+  CHECK(((unsigned int)len) < sizeof(name));
+  if (tname[0] != '\0')
+    internal_snprintf(&name[len], sizeof(name) - len, " (%s)", tname);
 }
 
 void DescribeThread(AsanThreadContext *context) {
@@ -47,18 +49,15 @@ void DescribeThread(AsanThreadContext *context) {
     return;
   }
   context->announced = true;
-  char tname[128];
   InternalScopedString str(1024);
-  str.append("Thread T%d%s", context->tid,
-             ThreadNameWithParenthesis(context->tid, tname, sizeof(tname)));
+  str.append("Thread %s", AsanThreadIdAndName(context).c_str());
   if (context->parent_tid == kInvalidTid) {
     str.append(" created by unknown thread\n");
     Printf("%s", str.data());
     return;
   }
-  str.append(
-      " created by T%d%s here:\n", context->parent_tid,
-      ThreadNameWithParenthesis(context->parent_tid, tname, sizeof(tname)));
+  str.append(" created by %s here:\n",
+             AsanThreadIdAndName(context->parent_tid).c_str());
   Printf("%s", str.data());
   StackDepotGet(context->stack_id).Print();
   // Recursively described parent thread if needed.
@@ -358,10 +357,9 @@ bool GlobalAddressDescription::PointsInsideTheSameVariable(
 
 void StackAddressDescription::Print() const {
   Decorator d;
-  char tname[128];
   Printf("%s", d.Location());
-  Printf("Address %p is located in stack of thread T%d%s", addr, tid,
-         ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+  Printf("Address %p is located in stack of thread %s", addr,
+         AsanThreadIdAndName(tid).c_str());
 
   if (!frame_descr) {
     Printf("%s\n", d.Default());
@@ -419,26 +417,19 @@ void HeapAddressDescription::Print() const {
   AsanThreadContext *alloc_thread = GetThreadContextByTidLocked(alloc_tid);
   StackTrace alloc_stack = GetStackTraceFromId(alloc_stack_id);
 
-  char tname[128];
   Decorator d;
   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.Default());
+    Printf("%sfreed by thread %s here:%s\n", d.Allocation(),
+           AsanThreadIdAndName(free_thread).c_str(), d.Default());
     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.Default());
+    Printf("%spreviously allocated by thread %s here:%s\n", d.Allocation(),
+           AsanThreadIdAndName(alloc_thread).c_str(), d.Default());
   } else {
-    Printf("%sallocated by thread T%d%s here:%s\n", d.Allocation(),
-           alloc_thread->tid,
-           ThreadNameWithParenthesis(alloc_thread, tname, sizeof(tname)),
-           d.Default());
+    Printf("%sallocated by thread %s here:%s\n", d.Allocation(),
+           AsanThreadIdAndName(alloc_thread).c_str(), d.Default());
   }
   alloc_stack.Print();
   DescribeThread(GetCurrentThread());
diff --git a/lib/asan/asan_descriptions.h b/lib/asan/asan_descriptions.h
index 1a1b01cf2..5c2d7662a 100644
--- a/lib/asan/asan_descriptions.h
+++ b/lib/asan/asan_descriptions.h
@@ -26,9 +26,20 @@ void DescribeThread(AsanThreadContext *context);
 static inline void DescribeThread(AsanThread *t) {
   if (t) DescribeThread(t->context());
 }
-const char *ThreadNameWithParenthesis(AsanThreadContext *t, char buff[],
-                                      uptr buff_len);
-const char *ThreadNameWithParenthesis(u32 tid, char buff[], uptr buff_len);
+
+class AsanThreadIdAndName {
+ public:
+  explicit AsanThreadIdAndName(AsanThreadContext *t);
+  explicit AsanThreadIdAndName(u32 tid);
+
+  // Contains "T%tid (%name)" or "T%tid" if the name is empty.
+  const char *c_str() const { return &name[0]; }
+
+ private:
+  void Init(u32 tid, const char *tname);
+
+  char name[128];
+};
 
 class Decorator : public __sanitizer::SanitizerCommonDecorator {
  public:
diff --git a/lib/asan/asan_errors.cc b/lib/asan/asan_errors.cc
index 6943e67c5..33d0613f7 100644
--- a/lib/asan/asan_errors.cc
+++ b/lib/asan/asan_errors.cc
@@ -45,13 +45,11 @@ void ErrorDeadlySignal::Print() {
 
 void ErrorDoubleFree::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
-  char tname[128];
+  Printf("%s", d.Error());
   Report(
-      "ERROR: AddressSanitizer: attempting %s on %p in "
-      "thread T%d%s:\n",
-      scariness.GetDescription(), addr_description.addr, tid,
-      ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+      "ERROR: AddressSanitizer: attempting %s on %p in thread %s:\n",
+      scariness.GetDescription(), addr_description.addr,
+      AsanThreadIdAndName(tid).c_str());
   Printf("%s", d.Default());
   scariness.Print();
   GET_STACK_TRACE_FATAL(second_free_stack->trace[0],
@@ -63,13 +61,11 @@ void ErrorDoubleFree::Print() {
 
 void ErrorNewDeleteTypeMismatch::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
-  char tname[128];
+  Printf("%s", d.Error());
   Report(
-      "ERROR: AddressSanitizer: %s on %p in thread "
-      "T%d%s:\n",
-      scariness.GetDescription(), addr_description.addr, tid,
-      ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+      "ERROR: AddressSanitizer: %s on %p in thread %s:\n",
+      scariness.GetDescription(), addr_description.addr,
+      AsanThreadIdAndName(tid).c_str());
   Printf("%s  object passed to delete has wrong type:\n", d.Default());
   if (delete_size != 0) {
     Printf(
@@ -106,13 +102,11 @@ void ErrorNewDeleteTypeMismatch::Print() {
 
 void ErrorFreeNotMalloced::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
-  char tname[128];
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: attempting free on address "
-      "which was not malloc()-ed: %p in thread T%d%s\n",
-      addr_description.Address(), tid,
-      ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+      "which was not malloc()-ed: %p in thread %s\n",
+      addr_description.Address(), AsanThreadIdAndName(tid).c_str());
   Printf("%s", d.Default());
   CHECK_GT(free_stack->size, 0);
   scariness.Print();
@@ -129,7 +123,7 @@ void ErrorAllocTypeMismatch::Print() {
                                         "operator delete []"};
   CHECK_NE(alloc_type, dealloc_type);
   Decorator d;
-  Printf("%s", d.Warning());
+  Printf("%s", d.Error());
   Report("ERROR: AddressSanitizer: %s (%s vs %s) on %p\n",
          scariness.GetDescription(),
          alloc_names[alloc_type], dealloc_names[dealloc_type],
@@ -148,7 +142,7 @@ void ErrorAllocTypeMismatch::Print() {
 
 void ErrorMallocUsableSizeNotOwned::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: attempting to call malloc_usable_size() for "
       "pointer which is not owned: %p\n",
@@ -161,7 +155,7 @@ void ErrorMallocUsableSizeNotOwned::Print() {
 
 void ErrorSanitizerGetAllocatedSizeNotOwned::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: attempting to call "
       "__sanitizer_get_allocated_size() for pointer which is not owned: %p\n",
@@ -174,12 +168,11 @@ void ErrorSanitizerGetAllocatedSizeNotOwned::Print() {
 
 void ErrorCallocOverflow::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
-  char tname[128];
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: calloc parameters overflow: count * size "
-      "(%zd * %zd) cannot be represented in type size_t (thread T%d%s)\n",
-      count, size, tid, ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+      "(%zd * %zd) cannot be represented in type size_t (thread %s)\n",
+      count, size, AsanThreadIdAndName(tid).c_str());
   Printf("%s", d.Default());
   stack->Print();
   PrintHintAllocatorCannotReturnNull();
@@ -188,14 +181,12 @@ void ErrorCallocOverflow::Print() {
 
 void ErrorPvallocOverflow::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
-  char tname[128];
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: pvalloc parameters overflow: size 0x%zx "
       "rounded up to system page size 0x%zx cannot be represented in type "
-      "size_t (thread T%d%s)\n",
-      size, GetPageSizeCached(), tid,
-      ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+      "size_t (thread %s)\n",
+      size, GetPageSizeCached(), AsanThreadIdAndName(tid).c_str());
   Printf("%s", d.Default());
   stack->Print();
   PrintHintAllocatorCannotReturnNull();
@@ -204,12 +195,11 @@ void ErrorPvallocOverflow::Print() {
 
 void ErrorInvalidAllocationAlignment::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
-  char tname[128];
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: invalid allocation alignment: %zd, "
-      "alignment must be a power of two (thread T%d%s)\n",
-      alignment, tid, ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+      "alignment must be a power of two (thread %s)\n",
+      alignment, AsanThreadIdAndName(tid).c_str());
   Printf("%s", d.Default());
   stack->Print();
   PrintHintAllocatorCannotReturnNull();
@@ -218,19 +208,17 @@ void ErrorInvalidAllocationAlignment::Print() {
 
 void ErrorInvalidAlignedAllocAlignment::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
-  char tname[128];
+  Printf("%s", d.Error());
 #if SANITIZER_POSIX
   Report("ERROR: AddressSanitizer: invalid alignment requested in "
          "aligned_alloc: %zd, alignment must be a power of two and the "
          "requested size 0x%zx must be a multiple of alignment "
-         "(thread T%d%s)\n", alignment, size, tid,
-         ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+         "(thread %s)\n", alignment, size, AsanThreadIdAndName(tid).c_str());
 #else
   Report("ERROR: AddressSanitizer: invalid alignment requested in "
          "aligned_alloc: %zd, the requested size 0x%zx must be a multiple of "
-         "alignment (thread T%d%s)\n", alignment, size, tid,
-         ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+         "alignment (thread %s)\n", alignment, size,
+         AsanThreadIdAndName(tid).c_str());
 #endif
   Printf("%s", d.Default());
   stack->Print();
@@ -240,14 +228,12 @@ void ErrorInvalidAlignedAllocAlignment::Print() {
 
 void ErrorInvalidPosixMemalignAlignment::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
-  char tname[128];
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: invalid alignment requested in posix_memalign: "
       "%zd, alignment must be a power of two and a multiple of sizeof(void*) "
-      "== %zd (thread T%d%s)\n",
-      alignment, sizeof(void*), tid,  // NOLINT
-      ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+      "== %zd (thread %s)\n",
+      alignment, sizeof(void*), AsanThreadIdAndName(tid).c_str());  // NOLINT
   Printf("%s", d.Default());
   stack->Print();
   PrintHintAllocatorCannotReturnNull();
@@ -256,14 +242,12 @@ void ErrorInvalidPosixMemalignAlignment::Print() {
 
 void ErrorAllocationSizeTooBig::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
-  char tname[128];
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: requested allocation size 0x%zx (0x%zx after "
       "adjustments for alignment, red zones etc.) exceeds maximum supported "
-      "size of 0x%zx (thread T%d%s)\n",
-      user_size, total_size, max_size, tid,
-      ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+      "size of 0x%zx (thread %s)\n",
+      user_size, total_size, max_size, AsanThreadIdAndName(tid).c_str());
   Printf("%s", d.Default());
   stack->Print();
   PrintHintAllocatorCannotReturnNull();
@@ -272,7 +256,7 @@ void ErrorAllocationSizeTooBig::Print() {
 
 void ErrorRssLimitExceeded::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: specified RSS limit exceeded, currently set to "
       "soft_rss_limit_mb=%zd\n", common_flags()->soft_rss_limit_mb);
@@ -284,7 +268,7 @@ void ErrorRssLimitExceeded::Print() {
 
 void ErrorOutOfMemory::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: allocator is out of memory trying to allocate "
       "0x%zx bytes\n", requested_size);
@@ -298,7 +282,7 @@ void ErrorStringFunctionMemoryRangesOverlap::Print() {
   Decorator d;
   char bug_type[100];
   internal_snprintf(bug_type, sizeof(bug_type), "%s-param-overlap", function);
-  Printf("%s", d.Warning());
+  Printf("%s", d.Error());
   Report(
       "ERROR: AddressSanitizer: %s: memory ranges [%p,%p) and [%p, %p) "
       "overlap\n",
@@ -315,7 +299,7 @@ void ErrorStringFunctionMemoryRangesOverlap::Print() {
 
 void ErrorStringFunctionSizeOverflow::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
+  Printf("%s", d.Error());
   Report("ERROR: AddressSanitizer: %s: (size=%zd)\n",
          scariness.GetDescription(), size);
   Printf("%s", d.Default());
@@ -343,7 +327,7 @@ void ErrorBadParamsToAnnotateContiguousContainer::Print() {
 
 void ErrorODRViolation::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
+  Printf("%s", d.Error());
   Report("ERROR: AddressSanitizer: %s (%p):\n", scariness.GetDescription(),
          global1.beg);
   Printf("%s", d.Default());
@@ -372,7 +356,7 @@ void ErrorODRViolation::Print() {
 
 void ErrorInvalidPointerPair::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
+  Printf("%s", d.Error());
   Report("ERROR: AddressSanitizer: %s: %p %p\n", scariness.GetDescription(),
          addr1_description.Address(), addr2_description.Address());
   Printf("%s", d.Default());
@@ -576,17 +560,15 @@ static void PrintShadowMemoryForAddress(uptr addr) {
 
 void ErrorGeneric::Print() {
   Decorator d;
-  Printf("%s", d.Warning());
+  Printf("%s", d.Error());
   uptr addr = addr_description.Address();
   Report("ERROR: AddressSanitizer: %s on address %p at pc %p bp %p sp %p\n",
          bug_descr, (void *)addr, pc, bp, sp);
   Printf("%s", d.Default());
 
-  char tname[128];
-  Printf("%s%s of size %zu at %p thread T%d%s%s\n", d.Access(),
+  Printf("%s%s of size %zu at %p thread %s%s\n", d.Access(),
          access_size ? (is_write ? "WRITE" : "READ") : "ACCESS", access_size,
-         (void *)addr, tid,
-         ThreadNameWithParenthesis(tid, tname, sizeof(tname)), d.Default());
+         (void *)addr, AsanThreadIdAndName(tid).c_str(), d.Default());
 
   scariness.Print();
   GET_STACK_TRACE_FATAL(pc, bp);