[PATCH] [sanitizer] Support sandboxing in sanitizer coverage.

[Sergey Matveev]

================
Comment at: include/sanitizer/common_interface_defs.h:29
@@ +28,3 @@
+    int coverage_sandboxed;
+    int coverage_fd;
+    size_t coverage_max_block_size;
----------------
Kostya Serebryany wrote:
> I wonder how this is going to work on Windows
It's hard to speculate in the absense of any users of __sanitizer_sandbox_on_notify() on Windows.

================
Comment at: include/sanitizer/common_interface_defs.h:30
@@ +29,3 @@
+    int coverage_fd;
+    size_t coverage_max_block_size;
+  };
----------------
Kostya Serebryany wrote:
> Alexander Potapenko wrote:
> > Should this really be passed through the sandbox arguments? I don't think the clients need to care much.
> comments?
We must not send a longer message than the other side is prepared to receive, otherwise the message gets truncated.

================
Comment at: lib/sanitizer_common/sanitizer_coverage.cc:80
@@ +79,3 @@
+  int pid;
+  unsigned int module_length;
+  uptr data_size;
----------------
Alexander Potapenko wrote:
> This should be u32, like the offsets.
> I'd even suggest to define something like module_off_t in this file.
This is string length, not module data length. I agree the name is confusing, but it follows pre-existing naming in this file. I should probably add "_name" everywhere.

================
Comment at: lib/sanitizer_common/sanitizer_coverage.cc:96
@@ +95,3 @@
+    internal_write(cov_fd, &header, sizeof(header));
+    internal_write(cov_fd, module, module_length);
+    internal_write(cov_fd, blob, blob_size);
----------------
Alexander Potapenko wrote:
> I think it's more common to keep the trailing zeroes, YMMV.
I agree. However, dropping the zero makes the code here and in the parser script a bit more straightforward, and I see no other major reasons to pick one over the other.

================
Comment at: lib/sanitizer_common/sanitizer_coverage.cc:159
@@ -111,1 +158,3 @@
+                       offsets.size() * sizeof(u32));
+        VReport(1, " CovDump: %zd PCs written to packed file\n", vb - old_vb);
       } else {
----------------
Alexander Potapenko wrote:
> Are you adding the leading whitespaces on purpose (here and below)? Why?
Following the style set by the two pre-existing Report() calls in this file.

================
Comment at: lib/sanitizer_common/sanitizer_internal_defs.h:98
@@ +97,3 @@
+      __sanitizer::uptr coverage_max_block_size;
+  };
+
----------------
Alexander Potapenko wrote:
> You shouldn't need to redefine this struct.
Suggestions? We don't include the public header into private headers, or vice versa.

http://reviews.llvm.org/D3726