From 69852a843c65492c932ea55470f623ca471353d1 Mon Sep 17 00:00:00 2001 From: Kostya Serebryany Date: Wed, 30 Jul 2014 09:48:23 +0000 Subject: [PATCH] [asan] add a feature to detect new-delete-size-mismatch (when used with -Xclang -fsized-deallocation). Not yet on Mac. Also, remove some unused code. llvm-svn: 214296 --- compiler-rt/lib/asan/asan_allocator.h | 2 + compiler-rt/lib/asan/asan_allocator2.cc | 35 ++++++------- compiler-rt/lib/asan/asan_flags.h | 1 + compiler-rt/lib/asan/asan_new_delete.cc | 5 ++ compiler-rt/lib/asan/asan_report.cc | 22 ++++++++ compiler-rt/lib/asan/asan_report.h | 2 + compiler-rt/lib/asan/asan_rtl.cc | 5 ++ .../test/asan/TestCases/Linux/sized_delete_test.cc | 58 ++++++++++++++++++++++ 8 files changed, 109 insertions(+), 21 deletions(-) create mode 100644 compiler-rt/test/asan/TestCases/Linux/sized_delete_test.cc diff --git a/compiler-rt/lib/asan/asan_allocator.h b/compiler-rt/lib/asan/asan_allocator.h index 6b2324a..d4e3daa 100644 --- a/compiler-rt/lib/asan/asan_allocator.h +++ b/compiler-rt/lib/asan/asan_allocator.h @@ -142,6 +142,8 @@ struct AsanThreadLocalMallocStorage { void *asan_memalign(uptr alignment, uptr size, StackTrace *stack, AllocType alloc_type); void asan_free(void *ptr, StackTrace *stack, AllocType alloc_type); +void asan_sized_free(void *ptr, uptr size, StackTrace *stack, + AllocType alloc_type); void *asan_malloc(uptr size, StackTrace *stack); void *asan_calloc(uptr nmemb, uptr size, StackTrace *stack); diff --git a/compiler-rt/lib/asan/asan_allocator2.cc b/compiler-rt/lib/asan/asan_allocator2.cc index f07b0f0..969b895 100644 --- a/compiler-rt/lib/asan/asan_allocator2.cc +++ b/compiler-rt/lib/asan/asan_allocator2.cc @@ -168,23 +168,6 @@ struct AsanChunk: ChunkBase { } return reinterpret_cast(Beg() - RZLog2Size(rz_log)); } - // If we don't use stack depot, we store the alloc/free stack traces - // in the chunk itself. - u32 *AllocStackBeg() { - return (u32*)(Beg() - RZLog2Size(rz_log)); - } - uptr AllocStackSize() { - CHECK_LE(RZLog2Size(rz_log), kChunkHeaderSize); - return (RZLog2Size(rz_log) - kChunkHeaderSize) / sizeof(u32); - } - u32 *FreeStackBeg() { - return (u32*)(Beg() + kChunkHeader2Size); - } - uptr FreeStackSize() { - if (user_requested_size < kChunkHeader2Size) return 0; - uptr available = RoundUpTo(user_requested_size, SHADOW_GRANULARITY); - return (available - kChunkHeader2Size) / sizeof(u32); - } bool AddrIsInside(uptr addr, bool locked_version = false) { return (addr >= Beg()) && (addr < Beg() + UsedSize(locked_version)); } @@ -464,12 +447,17 @@ static void QuarantineChunk(AsanChunk *m, void *ptr, } } -static void Deallocate(void *ptr, StackTrace *stack, AllocType alloc_type) { +static void Deallocate(void *ptr, uptr delete_size, StackTrace *stack, + AllocType alloc_type) { uptr p = reinterpret_cast(ptr); if (p == 0) return; uptr chunk_beg = p - kChunkHeaderSize; AsanChunk *m = reinterpret_cast(chunk_beg); + if (delete_size && flags()->new_delete_size_mismatch && + delete_size != m->UsedSize()) { + ReportNewDeleteSizeMismatch(p, delete_size, stack); + } ASAN_FREE_HOOK(ptr); // Must mark the chunk as quarantined before any changes to its metadata. AtomicallySetQuarantineFlag(m, ptr, stack); @@ -496,7 +484,7 @@ static void *Reallocate(void *old_ptr, uptr new_size, StackTrace *stack) { // If realloc() races with free(), we may start copying freed memory. // However, we will report racy double-free later anyway. REAL(memcpy)(new_ptr, old_ptr, memcpy_size); - Deallocate(old_ptr, stack, FROM_MALLOC); + Deallocate(old_ptr, 0, stack, FROM_MALLOC); } return new_ptr; } @@ -595,7 +583,12 @@ void *asan_memalign(uptr alignment, uptr size, StackTrace *stack, } void asan_free(void *ptr, StackTrace *stack, AllocType alloc_type) { - Deallocate(ptr, stack, alloc_type); + Deallocate(ptr, 0, stack, alloc_type); +} + +void asan_sized_free(void *ptr, uptr size, StackTrace *stack, + AllocType alloc_type) { + Deallocate(ptr, size, stack, alloc_type); } void *asan_malloc(uptr size, StackTrace *stack) { @@ -617,7 +610,7 @@ void *asan_realloc(void *p, uptr size, StackTrace *stack) { if (p == 0) return Allocate(size, 8, stack, FROM_MALLOC, true); if (size == 0) { - Deallocate(p, stack, FROM_MALLOC); + Deallocate(p, 0, stack, FROM_MALLOC); return 0; } return Reallocate(p, size, stack); diff --git a/compiler-rt/lib/asan/asan_flags.h b/compiler-rt/lib/asan/asan_flags.h index 1139204..8424104 100644 --- a/compiler-rt/lib/asan/asan_flags.h +++ b/compiler-rt/lib/asan/asan_flags.h @@ -58,6 +58,7 @@ struct Flags { bool poison_heap; bool poison_partial; bool alloc_dealloc_mismatch; + bool new_delete_size_mismatch; bool strict_memcmp; bool strict_init_order; bool start_deactivated; diff --git a/compiler-rt/lib/asan/asan_new_delete.cc b/compiler-rt/lib/asan/asan_new_delete.cc index 86b9f28..859d86c 100644 --- a/compiler-rt/lib/asan/asan_new_delete.cc +++ b/compiler-rt/lib/asan/asan_new_delete.cc @@ -105,6 +105,11 @@ CXX_OPERATOR_ATTRIBUTE void operator delete[](void *ptr, std::nothrow_t const&) { OPERATOR_DELETE_BODY(FROM_NEW_BR); } +CXX_OPERATOR_ATTRIBUTE +void operator delete(void *ptr, size_t size) throw() { + GET_STACK_TRACE_FREE; + asan_sized_free(ptr, size, &stack, FROM_NEW); +} #else // SANITIZER_MAC INTERCEPTOR(void, _ZdlPv, void *ptr) { diff --git a/compiler-rt/lib/asan/asan_report.cc b/compiler-rt/lib/asan/asan_report.cc index 19bd79d..4f162fa 100644 --- a/compiler-rt/lib/asan/asan_report.cc +++ b/compiler-rt/lib/asan/asan_report.cc @@ -652,6 +652,28 @@ void ReportDoubleFree(uptr addr, StackTrace *free_stack) { ReportErrorSummary("double-free", &stack); } +void ReportNewDeleteSizeMismatch(uptr addr, uptr delete_size, + StackTrace *free_stack) { + ScopedInErrorReport in_report; + Decorator d; + Printf("%s", d.Warning()); + char tname[128]; + u32 curr_tid = GetCurrentTidOrInvalid(); + Report("ERROR: AddressSanitizer: new-delete-size-mismatch on %p in " + "thread T%d%s:\n", + addr, curr_tid, + ThreadNameWithParenthesis(curr_tid, tname, sizeof(tname))); + Printf("%s sized operator delete called with size %zd\n", d.EndWarning(), + delete_size); + CHECK_GT(free_stack->size, 0); + GET_STACK_TRACE_FATAL(free_stack->trace[0], free_stack->top_frame_bp); + stack.Print(); + DescribeHeapAddress(addr, 1); + ReportErrorSummary("new-delete-size-mismatch", &stack); + Report("HINT: if you don't care about these warnings you may set " + "ASAN_OPTIONS=new_delete_size_mismatch=0\n"); +} + void ReportFreeNotMalloced(uptr addr, StackTrace *free_stack) { ScopedInErrorReport in_report; Decorator d; diff --git a/compiler-rt/lib/asan/asan_report.h b/compiler-rt/lib/asan/asan_report.h index 481401b..55d26a7 100644 --- a/compiler-rt/lib/asan/asan_report.h +++ b/compiler-rt/lib/asan/asan_report.h @@ -45,6 +45,8 @@ void NORETURN ReportStackOverflow(uptr pc, uptr sp, uptr bp, void *context, uptr addr); void NORETURN ReportSIGSEGV(const char *description, uptr pc, uptr sp, uptr bp, void *context, uptr addr); +void NORETURN ReportNewDeleteSizeMismatch(uptr addr, uptr delete_size, + StackTrace *free_stack); void NORETURN ReportDoubleFree(uptr addr, StackTrace *free_stack); void NORETURN ReportFreeNotMalloced(uptr addr, StackTrace *free_stack); void NORETURN ReportAllocTypeMismatch(uptr addr, StackTrace *free_stack, diff --git a/compiler-rt/lib/asan/asan_rtl.cc b/compiler-rt/lib/asan/asan_rtl.cc index ce799fc..809cc7c 100644 --- a/compiler-rt/lib/asan/asan_rtl.cc +++ b/compiler-rt/lib/asan/asan_rtl.cc @@ -198,6 +198,10 @@ static void ParseFlagsFromString(Flags *f, const char *str) { ParseFlag(str, &f->alloc_dealloc_mismatch, "alloc_dealloc_mismatch", "Report errors on malloc/delete, new/free, new/delete[], etc."); + + ParseFlag(str, &f->new_delete_size_mismatch, "new_delete_size_mismatch", + "Report errors on mismatch betwen size of new and delete."); + ParseFlag(str, &f->strict_memcmp, "strict_memcmp", "If true, assume that memcmp(p1, p2, n) always reads n bytes before " "comparing p1 and p2."); @@ -274,6 +278,7 @@ void InitializeFlags(Flags *f, const char *env) { // https://code.google.com/p/address-sanitizer/issues/detail?id=309 // TODO(glider,timurrrr): Fix known issues and enable this back. f->alloc_dealloc_mismatch = (SANITIZER_MAC == 0) && (SANITIZER_WINDOWS == 0); + f->new_delete_size_mismatch = true; f->strict_memcmp = true; f->strict_init_order = false; f->start_deactivated = false; diff --git a/compiler-rt/test/asan/TestCases/Linux/sized_delete_test.cc b/compiler-rt/test/asan/TestCases/Linux/sized_delete_test.cc new file mode 100644 index 0000000..822e057 --- /dev/null +++ b/compiler-rt/test/asan/TestCases/Linux/sized_delete_test.cc @@ -0,0 +1,58 @@ +// RUN: %clangxx_asan -Xclang -fsized-deallocation -O0 %s -o %t +// RUN: not %run %t 2>&1 | FileCheck %s +// RUN: ASAN_OPTIONS=new_delete_size_mismatch=1 not %run %t 2>&1 | FileCheck %s +// RUN: ASAN_OPTIONS=new_delete_size_mismatch=0 %run %t +#include +#include + +inline void break_optimization(void *arg) { + __asm__ __volatile__("" : : "r" (arg) : "memory"); +} + +struct S12 { + int a, b, c; +}; + +struct S20 { + int a, b, c, d, e; +}; + +void Del12(S12 *x) { + break_optimization(x); + delete x; +} +void Del12NoThrow(S12 *x) { + break_optimization(x); + operator delete(x, std::nothrow); +} +void Del12Ar(S12 *x) { + break_optimization(x); + delete [] x; +} +void Del12ArNoThrow(S12 *x) { + break_optimization(x); + operator delete[](x, std::nothrow); +} + +int main() { + // These are correct. + Del12(new S12); + Del12NoThrow(new S12); + Del12Ar(new S12[100]); + Del12ArNoThrow(new S12[100]); + + // Here we pass wrong type of pointer to delete, + // but [] and nothrow variants of delete are not sized. + Del12Ar(reinterpret_cast(new S20[100])); + Del12NoThrow(reinterpret_cast(new S20)); + Del12ArNoThrow(reinterpret_cast(new S20[100])); + fprintf(stderr, "OK SO FAR\n"); + // CHECK: OK SO FAR + // Here asan should bark as we are passing a wrong type of pointer + // to sized delete. + Del12(reinterpret_cast(new S20)); + // CHECK: AddressSanitizer: new-delete-size-mismatch + // CHECK: sized operator delete called with size + // CHECK: is located 0 bytes inside of 20-byte region + // CHECK: SUMMARY: AddressSanitizer: new-delete-size-mismatch +} -- 2.7.4