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

[Dmitry Vyukov]

================
Comment at: lib/sanitizer_common/sanitizer_coverage_libcdep.cc:94
@@ +93,3 @@
+void CoverageData::DirectInit() {
+  InternalScopedString path(1024);
+  internal_snprintf((char *)path.data(), path.size(), "%zd.sancov.raw",
----------------
same comment as for sancov.map

================
Comment at: lib/sanitizer_common/sanitizer_coverage_libcdep.cc:129
@@ +128,3 @@
+  uptr size =
+      atomic_fetch_add(&pc_array_size, delta, memory_order_relaxed) + delta;
+  uptr mapped_size = atomic_load(&pc_array_mapped_size, memory_order_relaxed);
----------------
You need to increment it after MapWritableFileToMemory. Otherwise the CHECK in Add does not work.

================
Comment at: lib/sanitizer_common/sanitizer_coverage_libcdep.cc:130
@@ +129,3 @@
+      atomic_fetch_add(&pc_array_size, delta, memory_order_relaxed) + delta;
+  uptr mapped_size = atomic_load(&pc_array_mapped_size, memory_order_relaxed);
+  if (size <= mapped_size) return;
----------------
Why do you need this to be atomic? It's accessed only in Init or under the mutex.

================
Comment at: lib/sanitizer_common/sanitizer_coverage_libcdep.cc:138
@@ +137,3 @@
+  uptr res = internal_ftruncate(pc_fd, new_mapped_size);
+  CHECK_EQ(res, 0);
+  void *p = MapWritableFileToMemory(pc_array + mapped_size,
----------------
please print a readable error message with errno and Die
this is not a programming error, it can happen due to out of disk space of networking error

================
Comment at: lib/sanitizer_common/sanitizer_coverage_libcdep.cc:153
@@ -66,1 +152,3 @@
+  CHECK_LT(idx * sizeof(uptr),
+           atomic_load(&pc_array_size, memory_order_acquire));
   pc_array[idx] = pc;
----------------
memory_order_acquire in CHECK looks suspicious
I think it needs to be just memory_order_relaxed

================
Comment at: lib/sanitizer_common/sanitizer_coverage_mapping_libcdep.cc:43
@@ +42,3 @@
+
+  InternalScopedString path(1024);
+  internal_snprintf((char *)path.data(), path.size(), "%zd.sancov.map",
----------------
It's just a "zd.sancov.map" string, right?
64 would be enough. 1024 makes me think that it's a full path, while it is not.

================
Comment at: lib/sanitizer_common/sanitizer_coverage_mapping_libcdep.cc:46
@@ +45,3 @@
+                    internal_getpid());
+  uptr map_fd = OpenFile(path.data(), true);
+  if (internal_iserror(map_fd)) {
----------------
If the process is killed in the middle of this function, then we will get broken cov profile. And working in the presence of sudden kills was the main motivation for this changes.
I would write the data to ancov.map.tmp and then rename it to ancov.map.


================
Comment at: lib/sanitizer_common/sanitizer_flags.cc:133
@@ -130,1 +132,3 @@
+            "If set, coverage information will be dumped directly to a memory "
+            "mapped file.");
   ParseFlag(str, &f->full_address_space, "full_address_space",
----------------
please explain when it can be useful (if the process is suddenly killed, it will leave some valid coverage file)

================
Comment at: lib/tsan/rtl/tsan_interceptors.cc:211
@@ +210,3 @@
+    }                                                    \
+    if (thr->ignore_interceptors) Printf("1 %s\n", #func);     \
+    if (thr->in_ignored_lib) Printf("2 %s\n", #func);                \
----------------
debug leftover?

http://reviews.llvm.org/D3874