feat: tsan build

.github/workflows/ci.yml

@@ -76,25 +76,27 @@ jobs:
 
       - name: Install GCC
         run: |
-          nix profile install nixpkgs#gcc15
-          echo "$HOME/.nix-profile/bin" >> "$GITHUB_PATH"
+          nix develop .
 
       # TODO: Currently builds are not fully hermetic. We will care about the state of the machine 
       #       until we can invest in making this repo fully hermetic
       - name: Print state
         run: |
+          nix develop .
           gcc --version
           ulimit -s
           cat /etc/os-release
           which gcc
 
       - name: Build postgres extension
         run: |
+          nix develop .
           cd examples/hello
           nix run .#xpg${{ matrix.pg-version }} -- build
 
       - name: Test postgres extension
         run: |
+          nix develop .
           cd examples/hello
           nix run .#xpg${{ matrix.pg-version }} -- psql -v ON_ERROR_STOP=1 -c "SELECT true;"
           nix run .#xpg${{ matrix.pg-version }} -- psql -v ON_ERROR_STOP=1 -c "SELECT true;"
@@ -116,6 +118,7 @@ jobs:
 
       - name: Test UBSAN reporting
         run: |
+          nix develop .
           cd examples/hello
           rm -rf ./build-{{ matrix.pg-version }}
 
@@ -125,11 +128,22 @@ jobs:
 
       - name: Test ASAN reporting
         run: |
+          nix develop .
           cd examples/hello
           # run test with cassert and injected OOB_WRITE error, print all stdout but exit non zero if 
           # expected error code not found
           (HELLO_OOB_WRITE=1 nix run .#xpg${{ matrix.pg-version }} -- --cassert -v ${{ matrix.pg-version }} test 2>&1 || true) | awk '/SUMMARY: AddressSanitizer: heap-buffer-overflow/  { found=1 } { print } END { if (!found) exit 1 }'
 
+      - name: Test TSAN reporting
+        if: matrix.pg-version >= 17
+        run: |
+          nix develop .
+          cd examples/hello
+          rm -rf ./build-{{ matrix.pg-version }}
+          make clean
+          (HELLO_THREAD_DATA_RACE=1 nix run .#xpg${{ matrix.pg-version }} -- --tsan -v ${{ matrix.pg-version }} test 2>&1 || true) | awk '/WARNING: ThreadSanitizer: data race/  { found=1 } { print } END { if (!found) exit 1 }'
+
+
       - if: ${{ failure() }}
         run: |
           cd examples/hello

examples/hello/Makefile

@@ -19,6 +19,8 @@ endif
 
 UNAME_S := $(shell uname -s)
 
+SHLIB_LINK = -lpthread
+
 ifeq ($(UNAME_S),Darwin)
     SHARED_EXT  := dylib
 else

examples/hello/src/hello.c

@@ -1,4 +1,14 @@
 #include "pg_prelude.h"
+#include <pthread.h>
+
+
+static int Global;
+
+static void *Thread1(void *x) {
+  Global = 42;
+  return x;
+}
+
 
 PG_MODULE_MAGIC;
 
@@ -9,6 +19,7 @@ Datum hello(PG_FUNCTION_ARGS){
   StringInfoData buf;
   initStringInfo(&buf);
 
+
   // ASAN should flag this
   if (getenv("HELLO_OOB_WRITE")) {
     char *buf2 = (char *)malloc(4);
@@ -22,6 +33,16 @@ Datum hello(PG_FUNCTION_ARGS){
     printf("%d\n", *i);
   }
 
+  // TSAN should flag this
+  if (getenv("HELLO_THREAD_DATA_RACE")) {
+    pthread_t t;
+    pthread_create(&t, NULL, Thread1, NULL);
+    Global = 43;
+    pthread_join(t, NULL);
+    printf("%d\n", Global);
+    printf("%d\n", *((int *)Thread1(&Global)));
+  }
+
   appendStringInfoString(&buf, "hello ");
   appendBinaryStringInfo(&buf, VARDATA_ANY(input), VARSIZE_ANY_EXHDR(input));
 

flake.nix

@@ -35,6 +35,25 @@
         pkgs.nixfmt-tree
 
       );
+      devShells = forAllSystems (
+        system:
+        let
+          pkgs = import nixpkgs { inherit system; };
+          xpgPkgs = import ./nix/packages.nix { inherit pkgs; };
+        in
+        {
+          default = pkgs.mkShell {
+            buildInputs = [
+              pkgs.gcc15
+            ];
+            shellHook = ''
+              export HISTFILE=.history
+            '';
+          };
+
+        }
+      );
+
       packages = forAllSystems (
         system:
         let

nix/postgresql/default.nix

@@ -21,17 +21,29 @@ let
   };
 
   mkAttributes =
-    { jitSupport, cassertSupport }:
+    {
+      jitSupport,
+      cassertSupport,
+      tsanSupport,
+    }:
     self.lib.mapAttrs' (
       version: path:
       let
-        suffixes = self.lib.optional cassertSupport "cassert" ++ self.lib.optional jitSupport "jit";
+        suffixes =
+          self.lib.optional cassertSupport "cassert"
+          ++ self.lib.optional jitSupport "jit"
+          ++ self.lib.optional tsanSupport "tsan";
         attrName =
           version + self.lib.optionalString (suffixes != [ ]) ("_" + self.lib.concatStringsSep "_" suffixes);
       in
       self.lib.nameValuePair attrName (
         import path {
-          inherit jitSupport cassertSupport self;
+          inherit
+            jitSupport
+            cassertSupport
+            tsanSupport
+            self
+            ;
         }
       )
     ) versions;
@@ -41,16 +53,30 @@ in
 (mkAttributes {
   jitSupport = false;
   cassertSupport = false;
+  tsanSupport = false;
 })
 // (mkAttributes {
   jitSupport = true;
   cassertSupport = false;
+  tsanSupport = false;
 })
 // (mkAttributes {
   jitSupport = false;
   cassertSupport = true;
+  tsanSupport = false;
 })
 // (mkAttributes {
   jitSupport = true;
   cassertSupport = true;
+  tsanSupport = false;
+})
+// (mkAttributes {
+  jitSupport = true;
+  cassertSupport = false;
+  tsanSupport = true;
+})
+// (mkAttributes {
+  jitSupport = false;
+  cassertSupport = false;
+  tsanSupport = true;
 })

nix/postgresql/generic.nix

@@ -54,6 +54,7 @@ let
       # JIT / assertions
       jitSupport,
       cassertSupport ? false,
+      tsanSupport ? false,
       nukeReferences,
       patchelf,
       llvmPackages,
@@ -84,7 +85,11 @@ let
     in
     stdenv'.mkDerivation (finalAttrs: {
       inherit version;
-      pname = pname + lib.optionalString jitSupport "-jit" + lib.optionalString cassertSupport "-cassert";
+      pname =
+        pname
+        + lib.optionalString jitSupport "-jit"
+        + lib.optionalString cassertSupport "-cassert"
+        + lib.optionalString tsanSupport "-tsan";
 
       src = fetchurl {
         url = "mirror://postgresql/source/v${version}/${pname}-${version}.tar.bz2";
@@ -146,6 +151,9 @@ let
         ""
         + lib.optionalString cassertSupport "-fsanitize=undefined -fno-sanitize-recover=all -fno-omit-frame-pointer "
         + lib.optionalString (cassertSupport && stdenv.isDarwin) "-fno-sanitize=function"
+        + lib.optionalString (
+          tsanSupport && atLeast "16" && stdenv'.isLinux
+        ) "-fsanitize=thread -fno-sanitize-recover=all"
         # TODO: compiling with clang on darwin with -fsanitize=address results in a binary
         #       that never returns and pins at 100% CPU saturation of a single core. Possibly
         #       revisit the below if this ever gets fixed

nix/xpg.nix

@@ -7,6 +7,9 @@
   findutils,
   entr,
   lcov,
+  gcc,
+  gnumake,
+  llvm,
   gnused,
   gdb,
   writeText,
@@ -34,6 +37,7 @@
   # Disabling this halves the PostgreSQL closure for consumers (e.g. CI) that
   # never pass `--cassert`.
   cassert ? true,
+  tsan ? false,
 }:
 let
   isLinux = stdenv.isLinux;
@@ -52,21 +56,26 @@ let
       ${
         if cassert then
           ''
-                      if [ "$_arg_cassert" = on ]; then
-                        export PATH=${ourPg."postgresql_${v}_cassert"}/bin:"$PATH"
-
-                        # See below links
-                        # TODO: detect_stack_use_after_return in ASAN_OPTIONS can be removed after 
-                        #       below patch lands and is backported, thought it will still need to 
-                        #       remain for EOL'd versions
-                        # https://www.postgresql.org/message-id/flat/2kk4z4odvuyrg7qlwjd7ft4eron4cle4btb33v4qatgsdkayir%40gj6e62rgsel4
-                        # https://github.com/postgres/postgres/blob/51ae21aa313bb6b79ad914920279cd0d89e92220/.github/workflows/pg-ci.yml#L464
-
-            	        export UBSAN_OPTIONS="print_stacktrace=1:disable_coredump=0:abort_on_error=1:verbosity=2"
-            	        export ASAN_OPTIONS="print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0:detect_stack_use_after_return=0"
-                      else
-                        export PATH=${ourPg."postgresql_${v}"}/bin:"$PATH"
-                      fi
+                                    export PATH=${gnumake}/bin:${gcc}/bin:${
+                                      ourPg."postgresql_${v}_cassert"
+                                    }/bin:"$PATH"
+                                  if [ "$_arg_cassert" = on ]; then
+
+                                    # See below links
+                                    # TODO: detect_stack_use_after_return in ASAN_OPTIONS can be removed after 
+                                    #       below patch lands and is backported, thought it will still need to 
+                                    #       remain for EOL'd versions
+                                    # https://www.postgresql.org/message-id/flat/2kk4z4odvuyrg7qlwjd7ft4eron4cle4btb33v4qatgsdkayir%40gj6e62rgsel4
+                                    # https://github.com/postgres/postgres/blob/51ae21aa313bb6b79ad914920279cd0d89e92220/.github/workflows/pg-ci.yml#L464
+
+                        	        export UBSAN_OPTIONS="print_stacktrace=1:disable_coredump=0:abort_on_error=1:verbosity=2"
+                        	        export ASAN_OPTIONS="print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0:detect_stack_use_after_return=0"
+            	              elif [ "$_arg_tsan" = on ]; then
+                                    export PATH=${ourPg."postgresql_${v}_tsan"}/bin:"$PATH"
+                        	        export TSAN_OPTIONS="halt_on_error=1"
+                                  else
+                                    export PATH=${ourPg."postgresql_${v}"}/bin:"$PATH"
+                                  fi
           ''
         else
           ''
@@ -97,6 +106,7 @@ let
           "ARG_OPTIONAL_SINGLE([options], [o], [Options for the database cluster],)"
           "ARG_OPTIONAL_SINGLE([init-options], [], [Options for the initialization of pgbench],)"
           "ARG_OPTIONAL_BOOLEAN([cassert], [], [Use the cassert-enabled PostgreSQL build])"
+          "ARG_OPTIONAL_BOOLEAN([tsan], [], [Use the thread sanitizer PostgreSQL build])"
           "ARG_OPTIONAL_SINGLE([commit], [], [Run the command in a new git worktree and check out <commit>])"
           "ARG_TYPE_GROUP_SET([VERSION], [VERSION], [version], [${builtins.concatStringsSep "," versions}])"
           "ARG_LEFTOVERS([psql arguments])"
@@ -158,27 +168,27 @@ let
         # commands that require the build ready
         case "$_arg_operation" in
           test)
-            make build TEST=1 1>&2
+            ${gnumake}/bin/make build TEST=1 1>&2
             ;;
 
           coverage)
             if [ ! -f "$COVERAGE_INFO" ]; then
               rm -rf "$BUILD_DIR"/*.o "$BUILD_DIR"/*.so
             fi
 
-            make build TEST=1 COVERAGE=1 1>&2
+            ${gnumake}/bin/make build TEST=1 COVERAGE=1 1>&2
             ;;
 
           test-core)
-            make build TEST_CORE=1 1>&2
+            ${gnumake}/bin/make build TEST_CORE=1 1>&2
             ;;
 
           gdb)
             # not required here, do nothing
             ;;
 
           *)
-            make build 1>&2
+            ${gnumake}/bin/make build 1>&2
             ;;
         esac
 
@@ -267,17 +277,17 @@ let
             ;;
 
           test-core)
-            make test-core
+            ${gnumake}/bin/make test-core
             ;;
 
           test)
-            make test
+            ${gnumake}/bin/make test
             ;;
 
           coverage)
             coverage_out_dir=$BUILD_DIR/coverage_html
 
-            make test
+            ${gnumake}/bin/make test
 
             ${lcov}/bin/lcov --capture --directory . --output-file "$COVERAGE_INFO"