Differential D15080
[asan] Reports suppressions for ASan recovery mode (compiler-rt part). Authored by m.ostapenko on Nov 30 2015, 7:59 AM.
Details This is the compiler-rt part of suppression functionality for ASan recovery mode (here is the LLVM core part: http://reviews.llvm.org/D15079). In current implementation, I'm trying to reuse UBSan approach with SourceLocations, so moving SourceLocation class definition from ubsan to sanitizer_common sources.
Diff Detail Event Timeline
Comment Actions Updating the patch according to nits in related discussion (http://reviews.llvm.org/D15079).
Comment Actions Updating the diff. It seems we cannot get rid of races without using mutex, because we should protect access to different parts of AsanBuggyPcPool array. However, using a mutex here is undesirable due to performance issue. So, for now, I see such alternatives how to implement suppression:
This patch implements the second approach for now, thought it's OK to have few duplications in favor of performance. Konstantin, what do you think about it? Comment Actions Oh, sure we can. :) ?> because we should protect access to different parts of AsanBuggyPcPool array. However, using a mutex here is undesirable due to performance issue. So, for now, I see such alternatives how to implement suppression:
Yep, no mutex please.
Summoning lock-free synchronization guru
Comment Actions
I mean, before we can update AsanBuggyPcPool[pc_num], we need to iterate over the whole [0, pc_num - 1] range of array. To avoid reports duplication, we should ensure, that nobody modifies the array during lookup and I'm not sure we can achieve this w/o mutex (or something that blocks the for loop). But let's wait for Dmitry for comments. Comment Actions Okay, let's do another try. New patch implements a lock-free approach (thanks Yura for idea!) based on CAS. The only drawback of the method is that we should iterate over AsanBuggyPcPool from it's beginning each time we perform lookup, that may become a performance bottleneck. However, this way we managed to avoid races on AsanBuggyPcPool without using mutex and the only race we have is on pc_num counter, used for fast path (early returning) only. So, I wonder, if something like this can resolve the issue?
Comment Actions LGTM with one small nit
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This is unnecessary complex and racy. And also produces excessive contention on AsanBuggyPcPool due to compare_exchange even when we are suppressing already reported pc. What you need is:
static bool SuppressErrorReport(uptr pc) { for (unsigned i = 0; i < kAsanBuggyPcPoolSize; i++) { u64 cmp = atomic_load_relaxed(&AsanBuggyPcPool[i]); if (cmp == 0 && atomic_compare_exchange_strong(&AsanBuggyPcPool[i], cmp, pc, memory_order_relaxed)) return false; if (cmp == pc) return true; } Die(); }