[PATCH] [asancov] Write coverage directly to a memory-mapped file

Evgeniy Stepanov eugenis at google.com
Fri May 23 07:55:56 PDT 2014


================
Comment at: lib/sanitizer_common/sanitizer_coverage_direct.cc:17
@@ +16,3 @@
+// format:
+// <pointer size in bits>  // 1 line
+// <mapping start> <mapping end> <base address> <dso name> // repeated
----------------
Kostya Serebryany wrote:
> You mean, 32 or 64?
sure, I expanded the comment
we need this to parse the .sancov.raw file from a 32-bit process on a 64-bit machine.

================
Comment at: lib/sanitizer_common/sanitizer_coverage_direct.cc:73
@@ +72,3 @@
+bool CoverageMap::TryAdd(uptr pc) {
+  for (;;) {
+    uptr cmp = atomic_load(&pc_next, memory_order_acquire);
----------------
Kostya Serebryany wrote:
> Just in case, I would replace this with 
>    for (int i = 0; i < (1<<30); i++) {
>          ....
>    }
>    CHECK(0 && "CoverageMap::TryAdd wanted to loop forever")
This is gone in the new patch.

================
Comment at: lib/sanitizer_common/sanitizer_coverage_direct.cc:87
@@ +86,3 @@
+  if (TryAdd(pc)) return;
+  SpinMutexLock l(&mu);
+  for (;;) {
----------------
Kostya Serebryany wrote:
> I wonder if this thing is AS-safe.
> This at least deserves a comment. 
It was not. Good point. The new implementation is.


================
Comment at: test/asan/TestCases/Linux/coverage-direct-large.cc:19
@@ +18,3 @@
+
+// RUN: diff -u coverage-direct-large/normal/out.txt coverage-direct-large/direct/out.txt
+//
----------------
Kostya Serebryany wrote:
> don't you need to cleanup after the test, as other coverage tests do?
I think it's better to cleanup before the test, and leave test artifacts (under the Output dir) for inspection.

This way an interrupted test won't mess up the next run.

http://reviews.llvm.org/D3874






More information about the llvm-commits mailing list