[sancov] generating html report on coverage dump

[aizatsky]

Summary

If html_cov_report is set and sancov binary is found, the html report would be generated automatically on coverage dump.

[aizatsky]

cleanup round.

[aizatsky]

nits

[kcc]

lib/sanitizer_common/sanitizer_common.cc

426

remove {}

lib/sanitizer_common/sanitizer_common_nolibc.cc

27 ↗

(On Diff #45602)

this is gross :)
Put the stubs into windows and mac files instead

lib/sanitizer_common/sanitizer_coverage_libcdep.cc

793

you need InternalScopedString

830

put this into a separate function

[kcc]

also, StartSubprocess deserves a separate test

[aizatsky]

I've merged in subprocess changes. Please take another look.

[aizatsky]

extracted function.

[samsonov]

lib/sanitizer_common/sanitizer_coverage_libcdep.cc

787

Consider adding an early return here instead

if (!common_flags()->html_cov_report || sancov_args[0] == nullptr)

800

Don't you want to log unsuccessful attempt to generate html?

lib/sanitizer_common/sanitizer_flags.inc

206

Is sancov_path more appropriate flag name?

test/asan/lit.cfg

144

I think you should use llvm_tools_dir instead. See the way it's handled in test/lit.common.cfg: probably you want to add this functionality there. Also, as LLVM tools directory is in PATH, auto-discovery of "sancov" binary must work in the lit test (in the same way we auto-discover llvm-symbolizer).

147

You have to explicitly add dependency on "sancov" binary from sanitizer test suites: see test/CMakeLists.txt

[aizatsky]

review

[aizatsky]

all done.

lib/sanitizer_common/sanitizer_coverage_libcdep.cc

800

StartSubprocess & WaitForPid have logging inside. I was relying on those.

[aizatsky]

clang-format

[samsonov]

Looks reasonsable to me. I'll let Kostya sign this off.

test/asan/TestCases/Posix/coverage_html_report.cc

7

Will this test work even if you don't provide sancov=%sancovcc explicitly?

25

Should this also be [[PID]].html ?

[aizatsky]

test/asan/TestCases/Posix/coverage_html_report.cc

7

it does

25

It's a different invocation of FileCheck. Unfortunately PID is not available here.

[aizatsky]

removed explicit tool path

[samsonov]

LGTM

test/asan/TestCases/Posix/coverage_html_report.cc

25

Ack.

[aizatsky]

Thank you for review.

lib/sanitizer_common/sanitizer_common.h

 void DumpProcessMap();
 bool FileExists(const char *filename);
 const char *GetEnv(const char *name);
 bool SetEnv(const char *name, const char *value);
 const char *GetPwd();
 char *FindPathToBinary(const char *name);
 bool IsPathSeparator(const char c);
 bool IsAbsolutePath(const char *path);
+// Starts a subprocess and returs its pid.
+// If *_fd parameters are >=0 their corresponding input/output
+// streams would be redirect to the file. The files would always be closed
+// in parent process even in case of an error.
+int StartSubprocess(const char *program, char *const argv[],
+                    fd_t std_in_fd = kInvalidFd, fd_t std_out_fd = kInvalidFd,
+                    fd_t std_err_fd = kInvalidFd);
+// Wait for process to terminate and returns its exit code.
+int WaitForProcess(int pid);
 
 u32 GetUid();
 void ReExec();
 char **GetArgv();
 void PrintCmdline();
 bool StackSizeIsUnlimited();
 void SetStackSizeLimitInBytes(uptr limit);
 bool AddressSpaceIsUnlimited();

lib/sanitizer_common/sanitizer_common.cc

while (templ && templ[0]) {
     asterisk = false;
   }
   return true;
 }
 
 static const char kPathSeparator = SANITIZER_WINDOWS ? ';' : ':';
 
 char *FindPathToBinary(const char *name) {
+  if (FileExists(name)) {
+    return internal_strdup(name);
+  }
+
   const char *path = GetEnv("PATH");
   if (!path)
     return nullptr;
   uptr name_len = internal_strlen(name);
   InternalScopedBuffer<char> buffer(kMaxPathLength);
   const char *beg = path;
   while (true) {
     const char *end = internal_strchrnul(beg, kPathSeparator);

lib/sanitizer_common/sanitizer_coverage_libcdep.cc

void CoverageData::GetRangeOffsets(const NamedPcRange& r, Symbolizer* sym,
 
   CHECK_GE(offsets->size(), kNumWordsForMagic);
   SortArray(offsets->data(), offsets->size());
   for (uptr i = 0; i < offsets->size(); i++)
     (*offsets)[i] = UnbundlePc((*offsets)[i]);
 }
 
 void CoverageData::DumpOffsets() {
   auto sym = Symbolizer::GetOrInit();
   if (!common_flags()->coverage_pcs) return;
   CHECK_NE(sym, nullptr);
   InternalMmapVector<uptr> offsets(0);
   InternalScopedString path(kMaxPathLength);
+
+  InternalMmapVector<char *> sancov_argv(module_name_vec.size() + 2);
+  sancov_argv.push_back(FindPathToBinary(common_flags()->sancov));
+  sancov_argv.push_back(internal_strdup("-obj"));
+  sancov_argv.push_back(internal_strdup(GetArgv()[0]));
+  sancov_argv.push_back(internal_strdup("-html-report"));
+
   for (uptr m = 0; m < module_name_vec.size(); m++) {
     auto r = module_name_vec[m];
     GetRangeOffsets(r, sym, &offsets);
 
     uptr num_offsets = offsets.size() - kNumWordsForMagic;
     u64 *magic_p = reinterpret_cast<u64*>(offsets.data());
     CHECK_EQ(*magic_p, 0ULL);
     // FIXME: we may want to write 32-bit offsets even in 64-mode
     // if all the offsets are small enough.
     *magic_p = kMagic;
 
     const char *module_name = StripModuleName(r.copied_module_name);
     if (cov_sandboxed) {
       if (cov_fd != kInvalidFd) {
         CovWritePacked(internal_getpid(), module_name, offsets.data(),
                        offsets.size() * sizeof(offsets[0]));
         VReport(1, " CovDump: %zd PCs written to packed file\n", num_offsets);
       }
     } else {
       // One file per module per process.
       fd_t fd = CovOpenFile(&path, false /* packed */, module_name);
       if (fd == kInvalidFd) continue;
       WriteToFile(fd, offsets.data(), offsets.size() * sizeof(offsets[0]));
       CloseFile(fd);
+      sancov_argv.push_back(internal_strdup(path.data()));
       VReport(1, " CovDump: %s: %zd PCs written\n", path.data(), num_offsets);
     }
   }
   if (cov_fd != kInvalidFd)
     CloseFile(cov_fd);
+
+  if (common_flags()->html_cov_report) {
+    sancov_argv.push_back(nullptr);
+    if (sancov_argv[0]) {
+      InternalScopedString report_path(kMaxPathLength);
+      fd_t report_fd = CovOpenFile(&report_path, false /* packed */,
+                                   GetProcessName(), "html");
+      int pid =
+          StartSubprocess(sancov_argv[0], sancov_argv.data(),
+                          kInvalidFd /* stdin */, report_fd /* std_out */);
+      if (pid > 0) {
+        int result = WaitForProcess(pid);
+        if (result == 0) {
+          VReport(1, " CovDump: html report generated to %s (%d)\n",
+                  report_path.data(), result);
+        }
+      }
+    }
+  }
+  for (uptr i = 0; i < sancov_argv.size(); ++i) {
+    InternalFree(sancov_argv[i]);
+  }
 }
 
 void CoverageData::DumpAll() {
   if (!coverage_enabled || common_flags()->coverage_direct) return;
   if (atomic_fetch_add(&dump_once_guard, 1, memory_order_relaxed))
     return;
   DumpAsBitSet();
   DumpCounters();

lib/sanitizer_common/sanitizer_flags.inc

COMMON_FLAG(
     bool, abort_on_error, SANITIZER_MAC,
     "If set, the tool calls abort() instead of _exit() after printing the "
     "error report.")
 COMMON_FLAG(bool, suppress_equal_pcs, true,
             "Deduplicate multiple reports for single source location in "
             "halt_on_error=false mode (asan only).")
 COMMON_FLAG(bool, print_cmdline, false, "Print command line on crash "
             "(asan only).")
+COMMON_FLAG(bool, html_cov_report, false, "Generate html coverage report.")
+COMMON_FLAG(const char *, sancov, "sancov", "Sancov tool location.")

lib/sanitizer_common/sanitizer_linux.cc

 void DisableReexec() {
   // No need to re-exec on Linux.
 }
 
 void MaybeReexec() {
   // No need to re-exec on Linux.
 }
 
+int StartSubprocess(const char *program, char *const argv[], fd_t std_in_fd,
+                    fd_t std_out_fd, fd_t std_err_fd) {
+  if (!FileExists(program)) {
+    Report("WARNING: Program %s not found!\n", program);
+    return -1;
+  }
+
+  int pid = internal_fork();
+
+  int rverrno;
+  if (internal_iserror(pid, &rverrno)) {
+    Report("WARNING: failed to fork (errno %d)\n", rverrno);
+  } else if (pid == 0) {
+    // Child subprocess
+    if (std_in_fd >= 0) {
+      internal_close(STDIN_FILENO);
+      internal_dup2(std_in_fd, STDIN_FILENO);
+      internal_close(std_in_fd);
+    }
+    if (std_out_fd >= 0) {
+      internal_close(STDOUT_FILENO);
+      internal_dup2(std_out_fd, STDOUT_FILENO);
+      internal_close(std_out_fd);
+    }
+    if (std_err_fd >= 0) {
+      internal_close(STDERR_FILENO);
+      internal_dup2(std_err_fd, STDERR_FILENO);
+      internal_close(std_err_fd);
+    }
+
+    internal_execve(program, argv, nullptr);
+    internal__exit(1);
+  }
+
+  // Parent process
+  if (std_in_fd >= 0) {
+    internal_close(std_in_fd);
+  }
+  if (std_out_fd >= 0) {
+    internal_close(std_out_fd);
+  }
+  if (std_err_fd >= 0) {
+    internal_close(std_err_fd);
+  }
+
+  return pid;
+}
+
+int WaitForProcess(int pid) {
+  int process_status;
+  uptr waitpid_status = internal_waitpid(pid, &process_status, 0);
+  int local_errno;
+  if (internal_iserror(waitpid_status, &local_errno)) {
+    VReport(1, "Waiting on the process failed (errno %d).\n", local_errno);
+    return -1;
+  }
+  return process_status;
+}
+
 } // namespace __sanitizer
 
 #endif // SANITIZER_FREEBSD || SANITIZER_LINUX

test/asan/TestCases/Posix/coverage_html_report.cc

+// EQUIRES: has_sancovcc
+// RUN: %clangxx_asan -fsanitize-coverage=func -DSHARED %s -shared -o %dynamiclib -fPIC %ld_flags_rpath_so
+// RUN: %clangxx_asan -fsanitize-coverage=func %s %ld_flags_rpath_exe -o %t
+// RUN: rm -rf %T/coverage_html_report && mkdir -p %T/coverage_html_report && cd %T/coverage_html_report
+// RUN: %env_asan_opts=coverage=1:verbosity=1:html_cov_report=1:sancov=%sancovcc %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-main
+// RUN: ls *.html | FileCheck %s --check-prefix=CHECK-ls
+// RUN: rm -r %T/coverage_html_report
+//
+// https://code.google.com/p/address-sanitizer/issues/detail?id=263
+// XFAIL: android
+
+#include <assert.h>
+#include <sanitizer/coverage_interface.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+#ifdef SHARED
+void bar() { printf("bar\n"); }
+#else
+__attribute__((noinline)) void foo() { printf("foo\n"); }
+extern void bar();
+
+int G[4];
+
+int main(int argc, char **argv) {
+  fprintf(stderr, "PID: %d\n", getpid());
+  for (int i = 1; i < argc; i++) {
+    if (!strcmp(argv[i], "foo")) {
+      uintptr_t old_coverage = __sanitizer_get_total_unique_coverage();
+      foo();
+      uintptr_t new_coverage = __sanitizer_get_total_unique_coverage();
+      assert(new_coverage > old_coverage);
+    }
+    if (!strcmp(argv[i], "bar"))
+      bar();
+  }
+  if (argc == 5) {
+    static volatile char *zero = 0;
+    *zero = 0; // SEGV if argc == 5.
+  }
+  return G[argc]; // Buffer overflow if argc >= 4.
+}
+#endif
+
+// CHECK-main: PID: [[PID:[0-9]+]]
+// CHECK-main: [[PID]].sancov: 1 PCs written
+// CHECK-main: html report generated to ./coverage_html_report.cc.tmp.[[PID]].html
+// CHECK-ls: coverage_html_report.cc.tmp.{{[0-9]+}}.html

test/asan/lit.cfg

 config.substitutions.append( ("%asan_symbolize", python_exec + " " + asan_symbolize + " ") )
 # Setup path to sancov.py script.
 sanitizer_common_source_dir = os.path.join(
   get_required_attr(config, "compiler_rt_src_root"), "lib", "sanitizer_common")
 sancov = os.path.join(sanitizer_common_source_dir, "scripts", "sancov.py")
 if not os.path.exists(sancov):
   lit_config.fatal("Can't find script on path %r" % sancov)
 python_exec = get_required_attr(config, "python_executable")
-config.substitutions.append( ("%sancov", python_exec + " " + sancov + " ") )
+config.substitutions.append( ("%sancov ", python_exec + " " + sancov + " ") )
 
+sancovcc_path = os.path.join(
+  get_required_attr(config, "llvm_obj_root"), "bin/sancov") 
+print "****, sancovcc_path=", sancovcc_path
+if os.path.exists(sancovcc_path):
+  print "*** has_sancovcc"
+  config.substitutions.append( ("%sancovcc ", sancovcc_path) )
+  config.available_features.add("has_sanvovcc")
+
 # Determine kernel bitness
 if config.host_arch.find('64') != -1 and config.android != "1":
   kernel_bits = '64'
 else:
   kernel_bits = '32'
 
 config.substitutions.append( ('CHECK-%kernel_bits', ("CHECK-kernel-" + kernel_bits + "-bits")))