From 3741ab82baf9d6197a89fdfdd3ad0761238023f8 Mon Sep 17 00:00:00 2001 From: Alexey Samsonov Date: Sun, 26 Oct 2014 06:23:07 +0000 Subject: [PATCH] Change StackDepot interface to use StackTrace more extensively llvm-svn: 220637 --- compiler-rt/lib/asan/asan_allocator2.cc | 4 +- compiler-rt/lib/asan/asan_globals.cc | 2 +- compiler-rt/lib/asan/asan_thread.cc | 2 +- compiler-rt/lib/lsan/lsan_allocator.cc | 2 +- compiler-rt/lib/lsan/lsan_common.cc | 4 +- compiler-rt/lib/lsan/lsan_common_linux.cc | 7 +-- compiler-rt/lib/msan/msan.cc | 2 +- compiler-rt/lib/msan/msan_allocator.cc | 4 +- compiler-rt/lib/msan/msan_interceptors.cc | 2 +- .../lib/sanitizer_common/sanitizer_stackdepot.cc | 65 +++++----------------- .../lib/sanitizer_common/sanitizer_stackdepot.h | 9 +-- .../lib/sanitizer_common/sanitizer_stacktrace.h | 22 ++++++++ .../tests/sanitizer_stackdepot_test.cc | 60 +++++++++++--------- compiler-rt/lib/tsan/dd/dd_rtl.cc | 8 +-- compiler-rt/lib/tsan/rtl/tsan_rtl.cc | 4 +- 15 files changed, 93 insertions(+), 104 deletions(-) diff --git a/compiler-rt/lib/asan/asan_allocator2.cc b/compiler-rt/lib/asan/asan_allocator2.cc index 687d5ba..52bdcf6 100644 --- a/compiler-rt/lib/asan/asan_allocator2.cc +++ b/compiler-rt/lib/asan/asan_allocator2.cc @@ -354,7 +354,7 @@ static void *Allocate(uptr size, uptr alignment, BufferedStackTrace *stack, meta[1] = chunk_beg; } - m->alloc_context_id = StackDepotPut(stack->trace, stack->size); + m->alloc_context_id = StackDepotPut(*stack); uptr size_rounded_down_to_granularity = RoundDownTo(size, SHADOW_GRANULARITY); // Unpoison the bulk of the memory region. @@ -423,7 +423,7 @@ static void QuarantineChunk(AsanChunk *m, void *ptr, BufferedStackTrace *stack, CHECK_EQ(m->free_tid, kInvalidTid); AsanThread *t = GetCurrentThread(); m->free_tid = t ? t->tid() : 0; - m->free_context_id = StackDepotPut(stack->trace, stack->size); + m->free_context_id = StackDepotPut(*stack); // Poison the region. PoisonShadow(m->Beg(), RoundUpTo(m->UsedSize(), SHADOW_GRANULARITY), diff --git a/compiler-rt/lib/asan/asan_globals.cc b/compiler-rt/lib/asan/asan_globals.cc index b950641..be111d4 100644 --- a/compiler-rt/lib/asan/asan_globals.cc +++ b/compiler-rt/lib/asan/asan_globals.cc @@ -217,7 +217,7 @@ using namespace __asan; // NOLINT void __asan_register_globals(__asan_global *globals, uptr n) { if (!flags()->report_globals) return; GET_STACK_TRACE_FATAL_HERE; - u32 stack_id = StackDepotPut(stack.trace, stack.size); + u32 stack_id = StackDepotPut(stack); BlockingMutexLock lock(&mu_for_globals); if (!global_registration_site_vector) global_registration_site_vector = diff --git a/compiler-rt/lib/asan/asan_thread.cc b/compiler-rt/lib/asan/asan_thread.cc index bdb627a..ce53bea 100644 --- a/compiler-rt/lib/asan/asan_thread.cc +++ b/compiler-rt/lib/asan/asan_thread.cc @@ -30,7 +30,7 @@ namespace __asan { void AsanThreadContext::OnCreated(void *arg) { CreateThreadContextArgs *args = static_cast(arg); if (args->stack) - stack_id = StackDepotPut(args->stack->trace, args->stack->size); + stack_id = StackDepotPut(*args->stack); thread = args->thread; thread->set_context(this); } diff --git a/compiler-rt/lib/lsan/lsan_allocator.cc b/compiler-rt/lib/lsan/lsan_allocator.cc index 32d76d1..8be2a2a 100644 --- a/compiler-rt/lib/lsan/lsan_allocator.cc +++ b/compiler-rt/lib/lsan/lsan_allocator.cc @@ -63,7 +63,7 @@ static void RegisterAllocation(const StackTrace &stack, void *p, uptr size) { ChunkMetadata *m = Metadata(p); CHECK(m); m->tag = DisabledInThisThread() ? kIgnored : kDirectlyLeaked; - m->stack_trace_id = StackDepotPut(stack.trace, stack.size); + m->stack_trace_id = StackDepotPut(stack); m->requested_size = size; atomic_store(reinterpret_cast(m), 1, memory_order_relaxed); } diff --git a/compiler-rt/lib/lsan/lsan_common.cc b/compiler-rt/lib/lsan/lsan_common.cc index ec518d7..746244c 100644 --- a/compiler-rt/lib/lsan/lsan_common.cc +++ b/compiler-rt/lib/lsan/lsan_common.cc @@ -371,8 +371,8 @@ static void CollectLeaksCb(uptr chunk, void *arg) { u32 stack_trace_id = 0; if (resolution > 0) { StackTrace stack = StackDepotGet(m.stack_trace_id()); - uptr size = Min(stack.size, resolution); - stack_trace_id = StackDepotPut(stack.trace, size); + stack.size = Min(stack.size, resolution); + stack_trace_id = StackDepotPut(stack); } else { stack_trace_id = m.stack_trace_id(); } diff --git a/compiler-rt/lib/lsan/lsan_common_linux.cc b/compiler-rt/lib/lsan/lsan_common_linux.cc index faa24d7..ba51868 100644 --- a/compiler-rt/lib/lsan/lsan_common_linux.cc +++ b/compiler-rt/lib/lsan/lsan_common_linux.cc @@ -94,11 +94,10 @@ void ProcessGlobalRegions(Frontier *frontier) { static uptr GetCallerPC(u32 stack_id, StackDepotReverseMap *map) { CHECK(stack_id); - uptr size = 0; - const uptr *trace = map->Get(stack_id, &size); + StackTrace stack = map->Get(stack_id); // The top frame is our malloc/calloc/etc. The next frame is the caller. - if (size >= 2) - return trace[1]; + if (stack.size >= 2) + return stack.trace[1]; return 0; } diff --git a/compiler-rt/lib/msan/msan.cc b/compiler-rt/lib/msan/msan.cc index ffd4a75..f58ac44 100644 --- a/compiler-rt/lib/msan/msan.cc +++ b/compiler-rt/lib/msan/msan.cc @@ -280,7 +280,7 @@ u32 ChainOrigin(u32 id, StackTrace *stack) { } } - StackDepotHandle h = StackDepotPut_WithHandle(stack->trace, stack->size); + StackDepotHandle h = StackDepotPut_WithHandle(*stack); if (!h.valid()) return id; if (flags()->origin_history_per_stack_limit > 0) { diff --git a/compiler-rt/lib/msan/msan_allocator.cc b/compiler-rt/lib/msan/msan_allocator.cc index 6949297..cabc94c 100644 --- a/compiler-rt/lib/msan/msan_allocator.cc +++ b/compiler-rt/lib/msan/msan_allocator.cc @@ -102,7 +102,7 @@ static void *MsanAllocate(StackTrace *stack, uptr size, uptr alignment, } else if (flags()->poison_in_malloc) { __msan_poison(allocated, size); if (__msan_get_track_origins()) { - u32 stack_id = StackDepotPut(stack->trace, stack->size); + u32 stack_id = StackDepotPut(*stack); CHECK(stack_id); u32 id; ChainedOriginDepotPut(stack_id, Origin::kHeapRoot, &id); @@ -125,7 +125,7 @@ void MsanDeallocate(StackTrace *stack, void *p) { if (flags()->poison_in_free) { __msan_poison(p, size); if (__msan_get_track_origins()) { - u32 stack_id = StackDepotPut(stack->trace, stack->size); + u32 stack_id = StackDepotPut(*stack); CHECK(stack_id); u32 id; ChainedOriginDepotPut(stack_id, Origin::kHeapRoot, &id); diff --git a/compiler-rt/lib/msan/msan_interceptors.cc b/compiler-rt/lib/msan/msan_interceptors.cc index 18d8336..86c12a4 100644 --- a/compiler-rt/lib/msan/msan_interceptors.cc +++ b/compiler-rt/lib/msan/msan_interceptors.cc @@ -842,7 +842,7 @@ void __msan_allocated_memory(const void* data, uptr size) { if (flags()->poison_in_malloc) __msan_poison(data, size); if (__msan_get_track_origins()) { - u32 stack_id = StackDepotPut(stack.trace, stack.size); + u32 stack_id = StackDepotPut(stack); u32 id; ChainedOriginDepotPut(stack_id, Origin::kHeapRoot, &id); __msan_set_origin(data, size, Origin(id, 1).raw_id()); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cc b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cc index 8652d51..afd45dd 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cc @@ -18,32 +18,6 @@ namespace __sanitizer { -// FIXME: Get rid of this class in favor of StackTrace. -struct StackDepotDesc { - const uptr *stack; - uptr size; - u32 hash() const { - // murmur2 - const u32 m = 0x5bd1e995; - const u32 seed = 0x9747b28c; - const u32 r = 24; - u32 h = seed ^ (size * sizeof(uptr)); - for (uptr i = 0; i < size; i++) { - u32 k = stack[i]; - k *= m; - k ^= k >> r; - k *= m; - h *= m; - h ^= k; - } - h ^= h >> 13; - h *= m; - h ^= h >> 15; - return h; - } - bool is_valid() { return size > 0 && stack; } -}; - struct StackDepotNode { StackDepotNode *link; u32 id; @@ -59,14 +33,14 @@ struct StackDepotNode { static const u32 kUseCountMask = (1 << kUseCountBits) - 1; static const u32 kHashMask = ~kUseCountMask; - typedef StackDepotDesc args_type; + typedef StackTrace args_type; bool eq(u32 hash, const args_type &args) const { u32 hash_bits = atomic_load(&hash_and_use_count, memory_order_relaxed) & kHashMask; if ((hash & kHashMask) != hash_bits || args.size != size) return false; uptr i = 0; for (; i < size; i++) { - if (stack[i] != args.stack[i]) return false; + if (stack[i] != args.trace[i]) return false; } return true; } @@ -76,11 +50,10 @@ struct StackDepotNode { void store(const args_type &args, u32 hash) { atomic_store(&hash_and_use_count, hash & kHashMask, memory_order_relaxed); size = args.size; - internal_memcpy(stack, args.stack, size * sizeof(uptr)); + internal_memcpy(stack, args.trace, size * sizeof(uptr)); } args_type load() const { - args_type ret = {&stack[0], size}; - return ret; + return args_type(&stack[0], size); } StackDepotHandle get_handle() { return StackDepotHandle(this); } @@ -100,8 +73,6 @@ void StackDepotHandle::inc_use_count_unsafe() { StackDepotNode::kUseCountMask; CHECK_LT(prev + 1, StackDepotNode::kMaxUseCount); } -uptr StackDepotHandle::size() { return node_->size; } -uptr *StackDepotHandle::stack() { return &node_->stack[0]; } // FIXME(dvyukov): this single reserved bit is used in TSan. typedef StackDepotBase @@ -112,20 +83,17 @@ StackDepotStats *StackDepotGetStats() { return theDepot.GetStats(); } -u32 StackDepotPut(const uptr *stack, uptr size) { - StackDepotDesc desc = {stack, size}; - StackDepotHandle h = theDepot.Put(desc); +u32 StackDepotPut(StackTrace stack) { + StackDepotHandle h = theDepot.Put(stack); return h.valid() ? h.id() : 0; } -StackDepotHandle StackDepotPut_WithHandle(const uptr *stack, uptr size) { - StackDepotDesc desc = {stack, size}; - return theDepot.Put(desc); +StackDepotHandle StackDepotPut_WithHandle(StackTrace stack) { + return theDepot.Put(stack); } StackTrace StackDepotGet(u32 id) { - StackDepotDesc desc = theDepot.Get(id); - return StackTrace(desc.stack, desc.size); + return theDepot.Get(id); } void StackDepotLockAll() { @@ -156,18 +124,15 @@ StackDepotReverseMap::StackDepotReverseMap() InternalSort(&map_, map_.size(), IdDescPair::IdComparator); } -const uptr *StackDepotReverseMap::Get(u32 id, uptr *size) { - if (!map_.size()) return 0; +StackTrace StackDepotReverseMap::Get(u32 id) { + if (!map_.size()) + return StackTrace(); IdDescPair pair = {id, 0}; uptr idx = InternalBinarySearch(map_, 0, map_.size(), pair, IdDescPair::IdComparator); - if (idx > map_.size()) { - *size = 0; - return 0; - } - StackDepotNode *desc = map_[idx].desc; - *size = desc->size; - return desc->stack; + if (idx > map_.size()) + return StackTrace(); + return map_[idx].desc->load(); } } // namespace __sanitizer diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h index c76c075..5e3a8b7 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h @@ -29,16 +29,13 @@ struct StackDepotHandle { u32 id(); int use_count(); void inc_use_count_unsafe(); - uptr size(); - uptr *stack(); }; const int kStackDepotMaxUseCount = 1U << 20; StackDepotStats *StackDepotGetStats(); -// FIXME: Pass StackTrace as an input argument here. -u32 StackDepotPut(const uptr *stack, uptr size); -StackDepotHandle StackDepotPut_WithHandle(const uptr *stack, uptr size); +u32 StackDepotPut(StackTrace stack); +StackDepotHandle StackDepotPut_WithHandle(StackTrace stack); // Retrieves a stored stack trace by the id. StackTrace StackDepotGet(u32 id); @@ -52,7 +49,7 @@ void StackDepotUnlockAll(); class StackDepotReverseMap { public: StackDepotReverseMap(); - const uptr *Get(u32 id, uptr *size); + StackTrace Get(u32 id); private: struct IdDescPair { diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h index 4ce53e4..a3d5488 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h @@ -33,11 +33,33 @@ struct StackTrace { const uptr *trace; uptr size; + StackTrace() : trace(nullptr), size(0) {} StackTrace(const uptr *trace, uptr size) : trace(trace), size(size) {} // Prints a symbolized stacktrace, followed by an empty line. void Print() const; + u32 hash() const { + // murmur2 + const u32 m = 0x5bd1e995; + const u32 seed = 0x9747b28c; + const u32 r = 24; + u32 h = seed ^ (size * sizeof(uptr)); + for (uptr i = 0; i < size; i++) { + u32 k = trace[i]; + k *= m; + k ^= k >> r; + k *= m; + h *= m; + h ^= k; + } + h ^= h >> 13; + h *= m; + h ^= h >> 15; + return h; + } + bool is_valid() const { return size > 0 && trace; } + static bool WillUseFastUnwind(bool request_fast_unwind) { // Check if fast unwind is available. Fast unwind is the only option on Mac. // It is also the only option on FreeBSD as the slow unwinding that diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_stackdepot_test.cc b/compiler-rt/lib/sanitizer_common/tests/sanitizer_stackdepot_test.cc index 5a36d15..513432f 100644 --- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_stackdepot_test.cc +++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_stackdepot_test.cc @@ -18,12 +18,13 @@ namespace __sanitizer { TEST(SanitizerCommon, StackDepotBasic) { - uptr s1[] = {1, 2, 3, 4, 5}; - u32 i1 = StackDepotPut(s1, ARRAY_SIZE(s1)); + uptr array[] = {1, 2, 3, 4, 5}; + StackTrace s1(array, ARRAY_SIZE(array)); + u32 i1 = StackDepotPut(s1); StackTrace stack = StackDepotGet(i1); EXPECT_NE(stack.trace, (uptr*)0); - EXPECT_EQ(ARRAY_SIZE(s1), stack.size); - EXPECT_EQ(0, internal_memcmp(stack.trace, s1, sizeof(s1))); + EXPECT_EQ(ARRAY_SIZE(array), stack.size); + EXPECT_EQ(0, internal_memcmp(stack.trace, array, sizeof(array))); } TEST(SanitizerCommon, StackDepotAbsent) { @@ -32,7 +33,7 @@ TEST(SanitizerCommon, StackDepotAbsent) { } TEST(SanitizerCommon, StackDepotEmptyStack) { - u32 i1 = StackDepotPut(0, 0); + u32 i1 = StackDepotPut(StackTrace()); StackTrace stack = StackDepotGet(i1); EXPECT_EQ((uptr*)0, stack.trace); } @@ -43,44 +44,49 @@ TEST(SanitizerCommon, StackDepotZeroId) { } TEST(SanitizerCommon, StackDepotSame) { - uptr s1[] = {1, 2, 3, 4, 6}; - u32 i1 = StackDepotPut(s1, ARRAY_SIZE(s1)); - u32 i2 = StackDepotPut(s1, ARRAY_SIZE(s1)); + uptr array[] = {1, 2, 3, 4, 6}; + StackTrace s1(array, ARRAY_SIZE(array)); + u32 i1 = StackDepotPut(s1); + u32 i2 = StackDepotPut(s1); EXPECT_EQ(i1, i2); StackTrace stack = StackDepotGet(i1); EXPECT_NE(stack.trace, (uptr*)0); - EXPECT_EQ(ARRAY_SIZE(s1), stack.size); - EXPECT_EQ(0, internal_memcmp(stack.trace, s1, sizeof(s1))); + EXPECT_EQ(ARRAY_SIZE(array), stack.size); + EXPECT_EQ(0, internal_memcmp(stack.trace, array, sizeof(array))); } TEST(SanitizerCommon, StackDepotSeveral) { - uptr s1[] = {1, 2, 3, 4, 7}; - u32 i1 = StackDepotPut(s1, ARRAY_SIZE(s1)); - uptr s2[] = {1, 2, 3, 4, 8, 9}; - u32 i2 = StackDepotPut(s2, ARRAY_SIZE(s2)); + uptr array1[] = {1, 2, 3, 4, 7}; + StackTrace s1(array1, ARRAY_SIZE(array1)); + u32 i1 = StackDepotPut(s1); + uptr array2[] = {1, 2, 3, 4, 8, 9}; + StackTrace s2(array2, ARRAY_SIZE(array2)); + u32 i2 = StackDepotPut(s2); EXPECT_NE(i1, i2); } TEST(SanitizerCommon, StackDepotReverseMap) { - uptr s1[] = {1, 2, 3, 4, 5}; - uptr s2[] = {7, 1, 3, 0}; - uptr s3[] = {10, 2, 5, 3}; - uptr s4[] = {1, 3, 2, 5}; + uptr array1[] = {1, 2, 3, 4, 5}; + uptr array2[] = {7, 1, 3, 0}; + uptr array3[] = {10, 2, 5, 3}; + uptr array4[] = {1, 3, 2, 5}; u32 ids[4] = {0}; - ids[0] = StackDepotPut(s1, ARRAY_SIZE(s1)); - ids[1] = StackDepotPut(s2, ARRAY_SIZE(s2)); - ids[2] = StackDepotPut(s3, ARRAY_SIZE(s3)); - ids[3] = StackDepotPut(s4, ARRAY_SIZE(s4)); + StackTrace s1(array1, ARRAY_SIZE(array1)); + StackTrace s2(array2, ARRAY_SIZE(array2)); + StackTrace s3(array3, ARRAY_SIZE(array3)); + StackTrace s4(array4, ARRAY_SIZE(array4)); + ids[0] = StackDepotPut(s1); + ids[1] = StackDepotPut(s2); + ids[2] = StackDepotPut(s3); + ids[3] = StackDepotPut(s4); StackDepotReverseMap map; for (uptr i = 0; i < 4; i++) { - uptr sz_map; - const uptr *sp_map; StackTrace stack = StackDepotGet(ids[i]); - sp_map = map.Get(ids[i], &sz_map); - EXPECT_EQ(stack.size, sz_map); - EXPECT_EQ(stack.trace, sp_map); + StackTrace from_map = map.Get(ids[i]); + EXPECT_EQ(stack.size, from_map.size); + EXPECT_EQ(stack.trace, from_map.trace); } } diff --git a/compiler-rt/lib/tsan/dd/dd_rtl.cc b/compiler-rt/lib/tsan/dd/dd_rtl.cc index de9a58f..44de617 100644 --- a/compiler-rt/lib/tsan/dd/dd_rtl.cc +++ b/compiler-rt/lib/tsan/dd/dd_rtl.cc @@ -19,13 +19,13 @@ namespace __dsan { static Context *ctx; static u32 CurrentStackTrace(Thread *thr, uptr skip) { - BufferedStackTrace trace; + BufferedStackTrace stack; thr->ignore_interceptors = true; - trace.Unwind(1000, 0, 0, 0, 0, 0, false); + stack.Unwind(1000, 0, 0, 0, 0, 0, false); thr->ignore_interceptors = false; - if (trace.size <= skip) + if (stack.size <= skip) return 0; - return StackDepotPut(trace.trace + skip, trace.size - skip); + return StackDepotPut(StackTrace(stack.trace + skip, stack.size - skip)); } static void PrintStackTrace(Thread *thr, u32 stk) { diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl.cc index 4d81463..d4d11c1 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cc @@ -462,8 +462,8 @@ u32 CurrentStackId(ThreadState *thr, uptr pc) { thr->shadow_stack_pos[0] = pc; thr->shadow_stack_pos++; } - u32 id = StackDepotPut(thr->shadow_stack, - thr->shadow_stack_pos - thr->shadow_stack); + u32 id = StackDepotPut(__sanitizer::StackTrace( + thr->shadow_stack, thr->shadow_stack_pos - thr->shadow_stack)); if (pc != 0) thr->shadow_stack_pos--; return id; -- 2.7.4