Run the threading rule under TSan, and both C suites under valgrind

.github/workflows/ci.yml

@@ -156,6 +156,121 @@ jobs:
           ASAN_OPTIONS: detect_leaks=1
           UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1
 
+  # Races, which are the failure the address sanitizer cannot see and
+  # the one an ABI used from four threads is most likely to have.
+  #
+  # gcc rather than clang, because the suppression file below was
+  # measured against gcc's libtsan and a suppression nobody checked is
+  # worse than no suppression at all.
+  tsan:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v5
+
+      - uses: actions/checkout@v5
+        with:
+          repository: tamnd/zu
+          path: engine
+
+      - uses: Swatinem/rust-cache@v2
+        with:
+          workspaces: engine
+
+      - uses: lukka/get-cmake@latest
+
+      - name: Build libzu
+        working-directory: engine
+        run: cargo build --release -p zu-capi
+
+      - name: Configure
+        run: >
+          cmake -B build-tsan -DCMAKE_BUILD_TYPE=RelWithDebInfo
+          -DZU_ROOT="$GITHUB_WORKSPACE/engine"
+          -DZU_CPP_SANITIZE=thread
+        env:
+          CC: gcc
+          CXX: g++
+
+      - name: Build the threading suite
+        run: cmake --build build-tsan --target threads
+
+      # Only the threading suite, because it is the only file here that
+      # starts a thread. The rest of the tree under TSan would be a
+      # slower way of compiling code that never races.
+      #
+      # The suppression file says at length why the engine's own frames
+      # are dropped: libzu takes no pthread lock, so TSan has no edge to
+      # learn from and reports two threads as racing even when one of
+      # them plainly waited. What is left is this repository's half,
+      # and it is checked rather than assumed: a race planted in the
+      # suite's own memory is still reported with these suppressions on.
+      - name: The threading rule holds when it is kept
+        run: ./build-tsan/test/threads
+        env:
+          TSAN_OPTIONS: suppressions=${{ github.workspace }}/test/tsan.supp:halt_on_error=1
+
+  # And memcheck, which sees what the sanitizers do not: an
+  # uninstrumented library's own reads and writes. libzu is compiled by
+  # cargo without instrumentation, so ASan knows nothing about what it
+  # does between the malloc and the free that it intercepts, and
+  # valgrind knows all of it.
+  valgrind:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v5
+
+      - uses: actions/checkout@v5
+        with:
+          repository: tamnd/zu
+          path: engine
+
+      - uses: Swatinem/rust-cache@v2
+        with:
+          workspaces: engine
+
+      - uses: lukka/get-cmake@latest
+
+      - name: Install valgrind
+        run: sudo apt-get update && sudo apt-get install -y valgrind
+
+      - name: Build libzu
+        working-directory: engine
+        run: cargo build --release -p zu-capi
+
+      - name: Configure
+        run: >
+          cmake -B build-vg -DCMAKE_BUILD_TYPE=RelWithDebInfo
+          -DZU_ROOT="$GITHUB_WORKSPACE/engine"
+
+      - name: Build the C suites
+        run: cmake --build build-vg --target misuse threads
+
+      # The two C files, because they are the ones that hand raw
+      # pointers across and give everything back by hand.
+      #
+      # Definitely and indirectly lost are errors. Possibly lost is not:
+      # it is one block of 180 bytes, it belongs to a thread the library
+      # started, and a pointer into the middle of a block is how a
+      # thread-local arena looks to a leak checker rather than how a
+      # leak looks.
+      #
+      # ZU_TEST_ROWS shrinks the two fixtures that exist to be slow.
+      # Memcheck already runs the machine forty times slower, which is
+      # another way of getting a statement that lasts long enough to be
+      # interrupted, so the smaller number tests the same thing in a
+      # fifth of the wall clock.
+      - name: The C suites under memcheck
+        run: |
+          set -eu
+          for t in misuse threads; do
+            valgrind --error-exitcode=99 --leak-check=full \
+              --show-leak-kinds=definite,indirect \
+              --errors-for-leak-kinds=definite,indirect \
+              --track-origins=yes "./build-vg/test/$t"
+          done
+        env:
+          ZU_TEST_ROWS: 300
+
   # The claim the packaging makes is that a project that installed this
   # can find it. Nothing in the suite can check that, because the suite
   # is inside the build tree and finds the header by being next to it.

README.md

@@ -107,7 +107,7 @@ lynn
 - `bench/`, the numbers below, with a timing harness that needs no package manager to run.
 - `cmake/`, `find_package(Zu)` to find the engine and `find_package(zu-cpp)` to find this. vcpkg, Conan and pkg-config packaging come with the first release.
 
-Still to come: ASan, UBSan, TSan and Valgrind suites over the full ABI surface, including the deliberate misuse cases, because an ABI nine languages depend on should fail loudly rather than corrupt quietly.
+Four sanitizer jobs run over the suite, because an ABI nine languages depend on should fail loudly rather than corrupt quietly. The whole tree runs under ASan and UBSan; `test/misuse.c` runs again with leak detection on, which it can and the C++ files cannot, because it is the file that gives every handle back by hand; `test/threads.c` runs under TSan; and both C files run under valgrind, which sees what the sanitizers cannot, since libzu is compiled without instrumentation and memcheck does not need any. `test/tsan.supp` records what TSan is unable to be told about a library that takes no pthread lock, and why the reports from inside the engine are dropped rather than read.
 
 ## Building
 

test/CMakeLists.txt

@@ -18,6 +18,21 @@ if(UNIX)
     C_STANDARD_REQUIRED ON
     C_EXTENSIONS OFF)
   add_test(NAME misuse COMMAND misuse)
+
+  # And the rule kept rather than broken, which needs real threads and
+  # is therefore the only file here that starts any. Separate from
+  # misuse.c because the thread sanitizer and the address sanitizer
+  # cannot be linked into one binary, so CI builds this tree twice and
+  # runs a different one of the two each time.
+  find_package(Threads REQUIRED)
+  add_executable(threads threads.c)
+  target_link_libraries(threads PRIVATE zu::zu zu_cpp_build_settings Threads::Threads)
+  target_include_directories(threads PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}")
+  set_target_properties(threads PROPERTIES
+    C_STANDARD 11
+    C_STANDARD_REQUIRED ON
+    C_EXTENSIONS OFF)
+  add_test(NAME threads COMMAND threads)
 endif()
 
 # One executable per file, which is what gives ctest a name per file and

test/harness.h

@@ -41,6 +41,7 @@
 #include <dirent.h>
 #include <fcntl.h>
 #include <stdarg.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -229,6 +230,39 @@ static inline int zt_next_fd(void) {
   return fd;
 }
 
+/* How many rows a fixture that exists in order to be slow should hold.
+ *
+ * Two cases here need a statement that is still running when something
+ * else happens to it: the progress watcher has to fire while it runs,
+ * and the interrupt has to land before it ends. Both get that by
+ * counting pairs over three thousand people, which takes about a third
+ * of a second and is nothing.
+ *
+ * Under valgrind it is not nothing. Memcheck runs somewhere between
+ * twenty and fifty times slower, and a third of a second becomes ten
+ * seconds or more of a job whose other cases finish instantly. So the
+ * valgrind runs pass a smaller number, and get the same behaviour for
+ * the same reason: what those two cases need is a statement that lasts
+ * long enough to be interrupted, and slowing the machine down by forty
+ * is another way of arriving at one.
+ *
+ * The default is the whole number, so nothing changes for anyone who
+ * does not set it, and the value is clamped at the whole number so a
+ * larger one cannot overrun the array the caller sized. */
+static inline uint64_t zt_rows(uint64_t whole) {
+  const char *set = getenv("ZU_TEST_ROWS");
+  unsigned long asked;
+  char *end = NULL;
+  if (set == NULL || *set == '\0') {
+    return whole;
+  }
+  asked = strtoul(set, &end, 10);
+  if (end == set || asked == 0) {
+    return whole;
+  }
+  return (uint64_t)asked > whole ? whole : (uint64_t)asked;
+}
+
 typedef struct zt_case {
   const char *name;
   void (*fn)(void);

test/misuse.c

@@ -570,22 +570,23 @@ ZT_TEST(a_call_back_into_the_library_from_the_watcher_is_refused_rather_than_rac
    * call made from inside it is a call made from a second thread at a
    * moment when the first is certainly inside the executor. */
   static int64_t ids[3000];
+  const uint64_t rows = zt_rows(3000);
   zu_database *db = NULL;
   zu_conn *conn = NULL;
   zu_frame *frame = NULL;
   zu_result *res = NULL;
   struct reentry state;
-  int i = 0;
+  uint64_t i = 0;
 
-  for (i = 0; i < 3000; i++) {
-    ids[i] = i;
+  for (i = 0; i < rows; i++) {
+    ids[i] = (int64_t)i;
   }
   memset(&state, 0, sizeof state);
 
   ZT_CHECK_EQ(zu_database_memory(NULL, &db, NULL), ZU_OK);
   ZT_CHECK_EQ(zu_connect(db, &conn, NULL), ZU_OK);
-  ZT_CHECK_EQ(zu_frame_new_z("Person", 3000, NULL, NULL, &frame, NULL), ZU_OK);
-  ZT_CHECK_EQ(zu_frame_col_int(frame, "id", 2, ids, 3000, 64, 1, 1, ZU_FRAME_PLAIN, NULL), ZU_OK);
+  ZT_CHECK_EQ(zu_frame_new_z("Person", rows, NULL, NULL, &frame, NULL), ZU_OK);
+  ZT_CHECK_EQ(zu_frame_col_int(frame, "id", 2, ids, rows, 64, 1, 1, ZU_FRAME_PLAIN, NULL), ZU_OK);
   ZT_CHECK_EQ(zu_conn_register(conn, frame, NULL), ZU_OK);
 
   state.conn = conn;