add more checkers to the sanitizer and integration check runs

3 files changed, 131 insertions, 18 deletions

diff --git a/Makefile b/Makefile
index ae01373..b172af6 100644
--- a/Makefile
+++ b/Makefile
@@ -134,6 +134,19 @@ $(TESTBIN): $(TEST_SOURCES) $(TEST_UNITS) $(TEST_HEADERS)
 check: $(TESTBIN) $(BINARY)
         ./$(TESTBIN)
 
+# The same unit tests under the sanitizers. Use clang over $(CC) because the
+# two toolchains do not check the same things
+# Override CC_sanitize to use another compiler.
+CC_sanitize?=clang
+CFLAGS_sanitize=$(CFLAGS_test) -fsanitize=address,undefined -fno-omit-frame-pointer
+LDFLAGS_sanitize=$(LDFLAGS) -fsanitize=address,undefined
+
+$(TESTBIN).sanitize: $(TEST_SOURCES) $(TEST_UNITS) $(TEST_HEADERS)
+        $(CC_sanitize) -o $@ $(CFLAGS_sanitize) $(TEST_SOURCES) $(TEST_UNITS) $(LDFLAGS_sanitize)
+
+check-sanitize: $(TESTBIN).sanitize
+        ./$(TESTBIN).sanitize
+
 # Black box tests: these drive the real binary over the wire, so they cover
 # the event loop and the parsers working together, which the unit tests
 # cannot reach. Built with the sanitizers on, so the tracker reports its own
@@ -149,10 +162,10 @@ opentracker.integration: $(SOURCES) $(HEADERS)
 check-integration: opentracker.integration
         OPENTRACKER=./opentracker.integration $(PYTHON) -m unittest discover -s tests/integration
 
-.PHONY: all check check-integration clean install
+.PHONY: all check check-sanitize check-integration clean install
 
 clean:
-        rm -rf opentracker opentracker.debug opentracker.integration proxy proxy.debug $(TESTBIN) *.o *~
+        rm -rf opentracker opentracker.debug opentracker.integration proxy proxy.debug $(TESTBIN) $(TESTBIN).sanitize *.o *~
 
 install:
         install -m 755 opentracker $(DESTDIR)$(BINDIR)
diff --git a/tests/integration/harness.py b/tests/integration/harness.py
index c8da1e2..868075d 100644
--- a/tests/integration/harness.py
+++ b/tests/integration/harness.py
@@ -7,6 +7,8 @@ reports its own memory errors, which the teardown turns into test failures.
 """
 
 import os
+import shutil
+import signal
 import socket
 import struct
 import subprocess
@@ -61,15 +63,34 @@ class Tracker:
     def __init__(self, extra_config=""):
         self.port = free_port()
         self.dir = tempfile.mkdtemp(prefix="ot-integration-")
-        self.config = os.path.join(self.dir, "opentracker.conf")
-        with open(self.config, "w") as handle:
-            handle.write("listen.tcp 127.0.0.1:%d\n" % self.port)
-            handle.write("listen.udp 127.0.0.1:%d\n" % self.port)
-            handle.write(extra_config)
-        self.log = open(os.path.join(self.dir, "tracker.log"), "w+b")
-        environment = dict(os.environ, ASAN_OPTIONS="detect_leaks=0:abort_on_error=0")
-        self.process = subprocess.Popen([TRACKER, "-f", self.config], stdout=self.log, stderr=self.log, env=environment)
-        self._wait_for_port()
+        self.log = None
+        self.process = None
+        try:
+            self.config = os.path.join(self.dir, "opentracker.conf")
+            with open(self.config, "w") as handle:
+                handle.write("listen.tcp 127.0.0.1:%d\n" % self.port)
+                handle.write("listen.udp 127.0.0.1:%d\n" % self.port)
+                handle.write(extra_config)
+            self.log = open(os.path.join(self.dir, "tracker.log"), "w+b")
+            # Leak detection is left at the sanitizer's own default, which is on
+            # where LeakSanitizer exists and off where it does not. Asking for it
+            # by name makes the tracker refuse to start on macOS and FreeBSD.
+            environment = dict(os.environ, ASAN_OPTIONS="abort_on_error=0")
+            self.process = subprocess.Popen([TRACKER, "-f", self.config], stdout=self.log, stderr=self.log, env=environment)
+            self._wait_for_port()
+        except BaseException:
+            self._cleanup()
+            raise
+
+    def _cleanup(self):
+        """Close what we opened and take the scratch directory with it."""
+        if self.process is not None and self.process.poll() is None:
+            self.process.kill()
+            self.process.wait()
+        if self.log is not None:
+            self.log.close()
+            self.log = None
+        shutil.rmtree(self.dir, ignore_errors=True)
 
     def _wait_for_port(self):
         for _ in range(200):
@@ -84,21 +105,32 @@ class Tracker:
         raise AssertionError("tracker never listened on %d:\n%s" % (self.port, self.output()))
 
     def output(self):
+        if self.log is None:
+            return ""
         self.log.flush()
         self.log.seek(0)
         return self.log.read().decode("utf-8", "replace")
 
     def stop(self):
-        """Returns anything the tracker complained about on the way out."""
-        if self.process.poll() is None:
-            self.process.kill()
-        self.process.wait()
+        """Returns anything the tracker complained about on the way out.
+
+        The interrupt rather than a kill is what makes the leak check work at
+        all: LeakSanitizer reports from an exit handler, so a killed tracker
+        reports nothing and the suite passes over leaks it never looked for.
+        """
+        if self.process is not None and self.process.poll() is None:
+            self.process.send_signal(signal.SIGINT)
+            try:
+                self.process.wait(timeout=5)
+            except subprocess.TimeoutExpired:
+                self.process.kill()
+                self.process.wait()
         complaints = [
             line
             for line in self.output().splitlines()
             if "Sanitizer" in line or "runtime error" in line or line.startswith("ERROR")
         ]
-        self.log.close()
+        self._cleanup()
         return complaints
 
     # ----------------------------------------------------------- over http
diff --git a/tests/runner.c b/tests/runner.c
index a92315e..71a5e2a 100644
--- a/tests/runner.c
+++ b/tests/runner.c
@@ -4,7 +4,11 @@
    $id$ */
 
 /* System */
+#include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
+#include <sys/wait.h>
+#include <unistd.h>
 
 /* Opentracker */
 #include "test.h"
@@ -31,15 +35,79 @@ static const struct module modules[] = {{"ot_urlencoded", test_urlencoded},
                                         {"ot_accesslist", test_accesslist},
                                         {NULL, NULL}};
 
-/* Run every module, or only those whose name contains argv[1] */
+struct tally {
+  unsigned long checks;
+  unsigned long failures;
+};
+
+/* Each module runs in a child of its own, so that a crash or a sanitizer
+   abort costs that module's results rather than every module after it. The
+   child reports its tally through a pipe; one that dies without reporting is
+   counted as a failure under its own name. */
+static void run_forked(const struct module *m) {
+  struct tally tally = {0, 0};
+  int          pipefd[2], status = 0;
+  pid_t        child;
+  ssize_t      got;
+
+  if (pipe(pipefd)) {
+    test_failf(__FILE__, __LINE__, "pipe failed, cannot isolate %s", m->name);
+    return;
+  }
+
+  fflush(stdout); /* or the child inherits a copy and prints it again */
+  child = fork();
+  if (child < 0) {
+    close(pipefd[0]);
+    close(pipefd[1]);
+    test_failf(__FILE__, __LINE__, "fork failed, cannot isolate %s", m->name);
+    return;
+  }
+
+  if (!child) {
+    close(pipefd[0]);
+    test_checks = test_failures = 0;
+    m->run();
+    tally.checks   = test_checks;
+    tally.failures = test_failures;
+    if (write(pipefd[1], &tally, sizeof(tally)) != (ssize_t)sizeof(tally))
+      _exit(1);
+    fflush(stdout);
+    _exit(0);
+  }
+
+  close(pipefd[1]);
+  got = read(pipefd[0], &tally, sizeof(tally));
+  close(pipefd[0]);
+  while (waitpid(child, &status, 0) < 0)
+    ;
+
+  if (got == (ssize_t)sizeof(tally)) {
+    test_checks   += tally.checks;
+    test_failures += tally.failures;
+  }
+  if (got != (ssize_t)sizeof(tally) || !WIFEXITED(status) || WEXITSTATUS(status))
+    test_failf(__FILE__, __LINE__, "%s did not run to the end", m->name);
+}
+
+/* Run every module, or only those whose name contains argv[1].
+   OT_TEST_FORK=0 keeps everything in one process, which is what a debugger
+   wants. */
 int main(int argc, char **argv) {
+  const char          *fork_setting = getenv("OT_TEST_FORK");
+  int                  isolate      = !fork_setting || strcmp(fork_setting, "0");
   const struct module *m;
 
   for (m = modules; m->name; ++m) {
     if (argc > 1 && !strstr(m->name, argv[1]))
       continue;
     test_module(m->name);
-    m->run();
+    if (!isolate) {
+      m->run();
+      continue;
+    }
+    test_case("the module runs to the end");
+    run_forked(m);
   }
 
   return test_summary();