From: Alex Shlyapnikov Date: Mon, 9 Jul 2018 17:54:55 +0000 (+0000) Subject: [ASan] Minor ASan error reporting cleanup X-Git-Tag: llvmorg-7.0.0-rc1~1941 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=63af91574f2015651fdf3a2ea9482466e353a010;p=platform%2Fupstream%2Fllvm.git [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 llvm-svn: 336573 --- diff --git a/compiler-rt/lib/asan/asan_descriptions.cc b/compiler-rt/lib/asan/asan_descriptions.cc index 1c70fed..cdb562d 100644 --- a/compiler-rt/lib/asan/asan_descriptions.cc +++ b/compiler-rt/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/compiler-rt/lib/asan/asan_descriptions.h b/compiler-rt/lib/asan/asan_descriptions.h index 1a1b01c..5c2d766 100644 --- a/compiler-rt/lib/asan/asan_descriptions.h +++ b/compiler-rt/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/compiler-rt/lib/asan/asan_errors.cc b/compiler-rt/lib/asan/asan_errors.cc index 6943e67..33d0613 100644 --- a/compiler-rt/lib/asan/asan_errors.cc +++ b/compiler-rt/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);