<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 24, 2014 at 5:14 AM, Evgeniy Stepanov <span dir="ltr"><<a href="mailto:eugeni.stepanov@gmail.com" target="_blank">eugeni.stepanov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">On Wed, Dec 24, 2014 at 1:32 AM, Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br>
> Author: kcc<br>
> Date: Tue Dec 23 16:32:17 2014<br>
> New Revision: 224789<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224789&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=224789&view=rev</a><br>
> Log:<br>
> [asan] change the coverage collection scheme so that we can easily emit coverage for the entire process as a single bit set, and if coverage_bitset=1 actually emit that bitset<br>
><br>
> Modified:<br>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc<br>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.cc<br>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h<br>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h<br>
> compiler-rt/trunk/test/asan/TestCases/Linux/coverage-fork.cc<br>
> compiler-rt/trunk/test/asan/TestCases/Linux/coverage-levels.cc<br>
><br>
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc?rev=224789&r1=224788&r2=224789&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc?rev=224789&r1=224788&r2=224789&view=diff</a><br>
> ==============================================================================<br>
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc (original)<br>
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc Tue Dec 23 16:32:17 2014<br>
> @@ -12,14 +12,16 @@<br>
> //<br>
> // Compiler instrumentation:<br>
> // For every interesting basic block the compiler injects the following code:<br>
> -// if (Guard) {<br>
> +// if (Guard < 0) {<br>
> // __sanitizer_cov(&Guard);<br>
> // }<br>
> +// At the module start up time __sanitizer_cov_module_init sets the guards<br>
> +// to consecutive negative numbers (-1, -2, -3, ...).<br>
> // It's fine to call __sanitizer_cov more than once for a given block.<br>
> //<br>
> // Run-time:<br>
> // - __sanitizer_cov(): record that we've executed the PC (GET_CALLER_PC).<br>
> -// and atomically set Guard to 1.<br>
> +// and atomically set Guard to -Guard.<br>
> // - __sanitizer_cov_dump: dump the coverage data to disk.<br>
> // For every module of the current process that has coverage data<br>
> // this will create a file module_name.PID.sancov. The file format is simple:<br>
> @@ -65,14 +67,16 @@ class CoverageData {<br>
> void BeforeFork();<br>
> void AfterFork(int child_pid);<br>
> void Extend(uptr npcs);<br>
> - void Add(uptr pc, u8 *guard);<br>
> + void Add(uptr pc, u32 *guard);<br>
> void IndirCall(uptr caller, uptr callee, uptr callee_cache[],<br>
> uptr cache_size);<br>
> void DumpCallerCalleePairs();<br>
> void DumpTrace();<br>
><br>
> ALWAYS_INLINE<br>
> - void TraceBasicaBlock(uptr *cache);<br>
> + void TraceBasicBlock(uptr *cache);<br>
> +<br>
> + void InitializeGuards(s32 **guards, uptr n);<br>
><br>
> uptr *data();<br>
> uptr size();<br>
> @@ -149,12 +153,11 @@ void CoverageData::Init() {<br>
> pc_array = reinterpret_cast<uptr *>(<br>
> MmapNoReserveOrDie(sizeof(uptr) * kPcArrayMaxSize, "CovInit"));<br>
> pc_fd = kInvalidFd;<br>
> + atomic_store(&pc_array_index, 0, memory_order_relaxed);<br>
> if (common_flags()->coverage_direct) {<br>
> atomic_store(&pc_array_size, 0, memory_order_relaxed);<br>
> - atomic_store(&pc_array_index, 0, memory_order_relaxed);<br>
> } else {<br>
> atomic_store(&pc_array_size, kPcArrayMaxSize, memory_order_relaxed);<br>
> - atomic_store(&pc_array_index, 0, memory_order_relaxed);<br>
> }<br>
><br>
> cc_array = reinterpret_cast<uptr **>(MmapNoReserveOrDie(<br>
> @@ -230,15 +233,27 @@ void CoverageData::Extend(uptr npcs) {<br>
> atomic_store(&pc_array_size, size, memory_order_release);<br>
> }<br>
><br>
> +void CoverageData::InitializeGuards(s32 **guards, uptr n) {<br>
> + for (uptr i = 0; i < n; i++) {<br>
> + uptr idx = atomic_fetch_add(&pc_array_index, 1, memory_order_relaxed);<br>
> + *guards[i] = -static_cast<s32>(idx + 1);<br>
> + }<br>
> +}<br>
> +<br>
> // Atomically add the pc to the vector. The atomically set the guard to 1.<br>
> // If the function is called more than once for a given PC it will<br>
> // be inserted multiple times, which is fine.<br>
<br>
</div></div>This comment is no longer true, is it?<br></blockquote><div><br></div><div>Right, thanks!</div><div>Updated as part of r224999.</div><div><br></div><div><span style="font-size:12.8000001907349px">>> Btw, this is an ABI change, how about bumping __asan_init_v?</span><div class="" style="font-size:12.8000001907349px"></div></div><div><span style="font-size:12.8000001907349px">I don't want to treat the coverage part as a fixed API just yet. </span></div><div><span style="font-size:12.8000001907349px">The feature is still *experimental* and we don't have too many users</span></div><div><span style="font-size:12.8000001907349px">(most, of not all, existing users of -fsanitize-coverage build all code from scratch).</span></div><div><span style="font-size:12.8000001907349px">In future -- yes, such changes will need an API version bump. </span></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5"><br>
> -void CoverageData::Add(uptr pc, u8 *guard) {<br>
> - // Set the guard.<br>
> - atomic_uint8_t *atomic_guard = reinterpret_cast<atomic_uint8_t*>(guard);<br>
> - atomic_store(atomic_guard, 1, memory_order_relaxed);<br>
> +void CoverageData::Add(uptr pc, u32 *guard) {<br>
> + atomic_uint32_t *atomic_guard = reinterpret_cast<atomic_uint32_t*>(guard);<br>
> + s32 guard_value = atomic_load(atomic_guard, memory_order_relaxed);<br>
> + if (guard_value >= 0) return;<br>
> +<br>
> + atomic_store(atomic_guard, -guard_value, memory_order_relaxed);<br>
> if (!pc_array) return;<br>
> - uptr idx = atomic_fetch_add(&pc_array_index, 1, memory_order_relaxed);<br>
> +<br>
> + uptr idx = -guard_value - 1;<br>
> + if (idx >= atomic_load(&pc_array_index, memory_order_acquire))<br>
> + return; // May happen after fork when pc_array_index becomes 0.<br>
> CHECK_LT(idx * sizeof(uptr),<br>
> atomic_load(&pc_array_size, memory_order_acquire));<br>
> pc_array[idx] = pc;<br>
> @@ -338,18 +353,20 @@ static void CovWritePacked(int pid, cons<br>
> // If packed = true and name == 0: <pid>.<sancov>.<packed>.<br>
> // If packed = true and name != 0: <name>.<sancov>.<packed> (name is<br>
> // user-supplied).<br>
> -static int CovOpenFile(bool packed, const char* name) {<br>
> +static int CovOpenFile(bool packed, const char *name,<br>
> + const char *extension = "sancov") {<br>
> InternalScopedString path(kMaxPathLength);<br>
> if (!packed) {<br>
> CHECK(name);<br>
> - path.append("%s/%s.%zd.sancov", common_flags()->coverage_dir, name,<br>
> - internal_getpid());<br>
> + path.append("%s/%s.%zd.%s", common_flags()->coverage_dir, name,<br>
> + internal_getpid(), extension);<br>
> } else {<br>
> if (!name)<br>
> - path.append("%s/%zd.sancov.packed", common_flags()->coverage_dir,<br>
> - internal_getpid());<br>
> + path.append("%s/%zd.%s.packed", common_flags()->coverage_dir,<br>
> + internal_getpid(), extension);<br>
> else<br>
> - path.append("%s/%s.sancov.packed", common_flags()->coverage_dir, name);<br>
> + path.append("%s/%s.%s.packed", common_flags()->coverage_dir, name,<br>
> + extension);<br>
> }<br>
> uptr fd = OpenFile(path.data(), true);<br>
> if (internal_iserror(fd)) {<br>
> @@ -434,7 +451,7 @@ void CoverageData::DumpCallerCalleePairs<br>
> // Record the current PC into the event buffer.<br>
> // Every event is a u32 value (index in tr_pc_array_index) so we compute<br>
> // it once and then cache in the provided 'cache' storage.<br>
> -void CoverageData::TraceBasicaBlock(uptr *cache) {<br>
> +void CoverageData::TraceBasicBlock(uptr *cache) {<br>
> CHECK(common_flags()->coverage);<br>
> uptr idx = *cache;<br>
> if (!idx) {<br>
> @@ -450,12 +467,33 @@ void CoverageData::TraceBasicaBlock(uptr<br>
> tr_event_array_index++;<br>
> }<br>
><br>
> +static void CovDumpAsBitSet() {<br>
> + if (!common_flags()->coverage_bitset) return;<br>
> + if (!coverage_data.size()) return;<br>
> + int fd = CovOpenFile(/* packed */false, "combined", "bitset-sancov");<br>
> + if (fd < 0) return;<br>
> + uptr n = coverage_data.size();<br>
> + uptr n_set_bits = 0;<br>
> + InternalScopedBuffer<char> out(n);<br>
> + for (uptr i = 0; i < n; i++) {<br>
> + uptr pc = coverage_data.data()[i];<br>
> + out[i] = pc ? '1' : '0';<br>
> + if (pc)<br>
> + n_set_bits++;<br>
> + }<br>
> + internal_write(fd, out.data(), n);<br>
> + internal_close(fd);<br>
> + VReport(1, " CovDump: bitset of %zd bits written, %zd bits are set\n", n,<br>
> + n_set_bits);<br>
> +}<br>
> +<br>
> // Dump the coverage on disk.<br>
> static void CovDump() {<br>
> if (!common_flags()->coverage || common_flags()->coverage_direct) return;<br>
> #if !SANITIZER_WINDOWS<br>
> if (atomic_fetch_add(&dump_once_guard, 1, memory_order_relaxed))<br>
> return;<br>
> + CovDumpAsBitSet();<br>
> uptr size = coverage_data.size();<br>
> InternalMmapVector<u32> offsets(size);<br>
> uptr *vb = coverage_data.data();<br>
> @@ -539,7 +577,7 @@ void CovAfterFork(int child_pid) {<br>
> } // namespace __sanitizer<br>
><br>
> extern "C" {<br>
> -SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_cov(u8 *guard) {<br>
> +SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_cov(u32 *guard) {<br>
> coverage_data.Add(StackTrace::GetPreviousInstructionPc(GET_CALLER_PC()),<br>
> guard);<br>
> }<br>
> @@ -552,8 +590,11 @@ SANITIZER_INTERFACE_ATTRIBUTE void __san<br>
> SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_cov_init() {<br>
> coverage_data.Init();<br>
> }<br>
> -SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_cov_module_init(uptr npcs) {<br>
> - if (!common_flags()->coverage || !common_flags()->coverage_direct) return;<br>
> +SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_cov_module_init(s32 **guards,<br>
> + uptr npcs) {<br>
> + coverage_data.InitializeGuards(guards, npcs);<br>
> + if (!common_flags()->coverage || !common_flags()->coverage_direct)<br>
> + return;<br>
> if (SANITIZER_ANDROID) {<br>
> // dlopen/dlclose interceptors do not work on Android, so we rely on<br>
> // Extend() calls to update .sancov.map.<br>
> @@ -572,10 +613,10 @@ uptr __sanitizer_get_total_unique_covera<br>
><br>
> SANITIZER_INTERFACE_ATTRIBUTE<br>
> void __sanitizer_cov_trace_func_enter(uptr *cache) {<br>
> - coverage_data.TraceBasicaBlock(cache);<br>
> + coverage_data.TraceBasicBlock(cache);<br>
> }<br>
> SANITIZER_INTERFACE_ATTRIBUTE<br>
> void __sanitizer_cov_trace_basic_block(uptr *cache) {<br>
> - coverage_data.TraceBasicaBlock(cache);<br>
> + coverage_data.TraceBasicBlock(cache);<br>
> }<br>
> } // extern "C"<br>
><br>
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.cc<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.cc?rev=224789&r1=224788&r2=224789&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.cc?rev=224789&r1=224788&r2=224789&view=diff</a><br>
> ==============================================================================<br>
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.cc (original)<br>
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.cc Tue Dec 23 16:32:17 2014<br>
> @@ -63,6 +63,7 @@ void CommonFlags::SetDefaults() {<br>
> legacy_pthread_cond = false;<br>
> intercept_tls_get_addr = false;<br>
> coverage = false;<br>
> + coverage_bitset = false;<br>
> coverage_direct = SANITIZER_ANDROID;<br>
> coverage_dir = ".";<br>
> full_address_space = false;<br>
> @@ -150,6 +151,9 @@ void CommonFlags::ParseFromString(const<br>
> ParseFlag(str, &coverage, "coverage",<br>
> "If set, coverage information will be dumped at program shutdown (if the "<br>
> "coverage instrumentation was enabled at compile time).");<br>
> + ParseFlag(str, &coverage_bitset, "coverage_bitset",<br>
> + "If set (and if 'coverage' is set too), the coverage information "<br>
> + "will also be dumped as a bitset to a separate file.");<br>
> ParseFlag(str, &coverage_direct, "coverage_direct",<br>
> "If set, coverage information will be dumped directly to a memory "<br>
> "mapped file. This way data is not lost even if the process is "<br>
><br>
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h?rev=224789&r1=224788&r2=224789&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h?rev=224789&r1=224788&r2=224789&view=diff</a><br>
> ==============================================================================<br>
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h (original)<br>
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h Tue Dec 23 16:32:17 2014<br>
> @@ -56,6 +56,7 @@ struct CommonFlags {<br>
> uptr mmap_limit_mb;<br>
> uptr hard_rss_limit_mb;<br>
> bool coverage;<br>
> + bool coverage_bitset;<br>
> bool coverage_direct;<br>
> const char *coverage_dir;<br>
> bool full_address_space;<br>
><br>
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h?rev=224789&r1=224788&r2=224789&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h?rev=224789&r1=224788&r2=224789&view=diff</a><br>
> ==============================================================================<br>
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h (original)<br>
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h Tue Dec 23 16:32:17 2014<br>
> @@ -120,7 +120,7 @@ extern "C" {<br>
><br>
> SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_cov_dump();<br>
> SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_cov_init();<br>
> - SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_cov(__sanitizer::u8 *guard);<br>
> + SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_cov(__sanitizer::u32 *guard);<br>
> SANITIZER_INTERFACE_ATTRIBUTE<br>
> void __sanitizer_annotate_contiguous_container(const void *beg,<br>
> const void *end,<br>
><br>
> Modified: compiler-rt/trunk/test/asan/TestCases/Linux/coverage-fork.cc<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/asan/TestCases/Linux/coverage-fork.cc?rev=224789&r1=224788&r2=224789&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/asan/TestCases/Linux/coverage-fork.cc?rev=224789&r1=224788&r2=224789&view=diff</a><br>
> ==============================================================================<br>
> --- compiler-rt/trunk/test/asan/TestCases/Linux/coverage-fork.cc (original)<br>
> +++ compiler-rt/trunk/test/asan/TestCases/Linux/coverage-fork.cc Tue Dec 23 16:32:17 2014<br>
> @@ -33,6 +33,7 @@ int main(int argc, char **argv) {<br>
> }<br>
><br>
> // CHECK-DAG: Child PID: [[ChildPID:[0-9]+]]<br>
> -// CHECK-DAG: [[ChildPID]].sancov: 1 PCs written<br>
> +// Coverage in the child (after fork,before exec) is not expected to work.<br>
> +// (disabled): [[ChildPID]].sancov: 1 PCs written<br>
> // CHECK-DAG: Parent PID: [[ParentPID:[0-9]+]]<br>
> // CHECK-DAG: [[ParentPID]].sancov: 3 PCs written<br>
><br>
> Modified: compiler-rt/trunk/test/asan/TestCases/Linux/coverage-levels.cc<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/asan/TestCases/Linux/coverage-levels.cc?rev=224789&r1=224788&r2=224789&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/asan/TestCases/Linux/coverage-levels.cc?rev=224789&r1=224788&r2=224789&view=diff</a><br>
> ==============================================================================<br>
> --- compiler-rt/trunk/test/asan/TestCases/Linux/coverage-levels.cc (original)<br>
> +++ compiler-rt/trunk/test/asan/TestCases/Linux/coverage-levels.cc Tue Dec 23 16:32:17 2014<br>
> @@ -1,11 +1,14 @@<br>
> // Test various levels of coverage<br>
> //<br>
> // RUN: %clangxx_asan -O1 -fsanitize-coverage=1 %s -o %t<br>
> -// RUN: ASAN_OPTIONS=coverage=1:verbosity=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK1<br>
> +// RUN: ASAN_OPTIONS=coverage=1:coverage_bitset=1:verbosity=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK1<br>
> // RUN: %clangxx_asan -O1 -fsanitize-coverage=2 %s -o %t<br>
> -// RUN: ASAN_OPTIONS=coverage=1:verbosity=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK2<br>
> +// RUN: ASAN_OPTIONS=coverage=1:coverage_bitset=1:verbosity=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK2<br>
> // RUN: %clangxx_asan -O1 -fsanitize-coverage=3 %s -o %t<br>
> -// RUN: ASAN_OPTIONS=coverage=1:verbosity=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK3<br>
> +// RUN: ASAN_OPTIONS=coverage=1:coverage_bitset=1:verbosity=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK3<br>
> +<br>
> +// RUN: ASAN_OPTIONS=coverage=1:coverage_bitset=0:verbosity=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK3_NOBITSET<br>
> +// RUN: ASAN_OPTIONS=coverage=1:verbosity=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK3_NOBITSET<br>
> //<br>
> // REQUIRES: asan-64-bits<br>
><br>
> @@ -15,6 +18,10 @@ int main(int argc, char **argv) {<br>
> sink = 0;<br>
> }<br>
><br>
> +// CHECK1: CovDump: bitset of 1 bits written, 1 bits are set<br>
> // CHECK1: 1 PCs written<br>
> +// CHECK2: CovDump: bitset of 3 bits written, 2 bits are set<br>
> // CHECK2: 2 PCs written<br>
> +// CHECK3: CovDump: bitset of 4 bits written, 3 bits are set<br>
> // CHECK3: 3 PCs written<br>
> +// CHECK3_NOBITSET-NOT: bitset of<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>