diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 892a3da..e967159 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,13 +76,13 @@ 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 @@ -90,11 +90,13 @@ jobs: - 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 diff --git a/examples/hello/Makefile b/examples/hello/Makefile index 17b955d..3ce234a 100644 --- a/examples/hello/Makefile +++ b/examples/hello/Makefile @@ -19,6 +19,8 @@ endif UNAME_S := $(shell uname -s) +SHLIB_LINK = -lpthread + ifeq ($(UNAME_S),Darwin) SHARED_EXT := dylib else diff --git a/examples/hello/src/hello.c b/examples/hello/src/hello.c index a7b7714..d8068ba 100644 --- a/examples/hello/src/hello.c +++ b/examples/hello/src/hello.c @@ -1,4 +1,14 @@ #include "pg_prelude.h" +#include + + +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)); diff --git a/flake.nix b/flake.nix index 3c62a21..6e04df0 100644 --- a/flake.nix +++ b/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 diff --git a/nix/postgresql/default.nix b/nix/postgresql/default.nix index 4e51c5a..d00b9d8 100644 --- a/nix/postgresql/default.nix +++ b/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; }) diff --git a/nix/postgresql/generic.nix b/nix/postgresql/generic.nix index ed64e16..8365191 100644 --- a/nix/postgresql/generic.nix +++ b/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 diff --git a/nix/xpg.nix b/nix/xpg.nix index 733a93b..8210f07 100644 --- a/nix/xpg.nix +++ b/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 ])" "ARG_TYPE_GROUP_SET([VERSION], [VERSION], [version], [${builtins.concatStringsSep "," versions}])" "ARG_LEFTOVERS([psql arguments])" @@ -158,7 +168,7 @@ 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) @@ -166,11 +176,11 @@ let 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) @@ -178,7 +188,7 @@ let ;; *) - 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"