From 2bebebe8fcbe652e945c6d1ea3b6e0bd654d672c Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Wed, 27 May 2026 16:58:02 +0200 Subject: [PATCH] Improve our Postgres AddressSanitizer instrumentation Turns out it's broken since PG16. Now, besides the "VALGRIND" macros that are used to poison the memory, some invocations of these macros themselves are under #ifdef USE_VALGRIND. Just remove these ifdefs. --- .github/workflows/llm-fuzzer.yaml | 4 +- .../workflows/sanitizer-build-and-test.yaml | 4 +- test/postgres-asan-instrumentation-PG15.patch | 68 +++++++++++++++++ ...postgres-asan-instrumentation-PG18GE.patch | 72 +++++++++++++++++- test/postgres-asan-instrumentation.patch | 74 ++++++++++++++++++- 5 files changed, 217 insertions(+), 5 deletions(-) create mode 100644 test/postgres-asan-instrumentation-PG15.patch diff --git a/.github/workflows/llm-fuzzer.yaml b/.github/workflows/llm-fuzzer.yaml index 8a4b925cdd9..8d713a3c546 100644 --- a/.github/workflows/llm-fuzzer.yaml +++ b/.github/workflows/llm-fuzzer.yaml @@ -123,7 +123,9 @@ jobs: # Add instrumentation to the Postgres memory contexts. For more details, see # https://github.com/timescale/eng-database/wiki/Using-Address-Sanitizer#adding-more-instrumentation PG_MAJOR=$(echo "${{ needs.config.outputs.pg_latest }}" | sed -e 's![.].*!!') - if [ ${PG_MAJOR} -lt 18 ]; then + if [ ${PG_MAJOR} -lt 16 ]; then + patch -F5 -p1 -d ~/$PG_SRC_DIR < test/postgres-asan-instrumentation-PG15.patch + elif [ ${PG_MAJOR} -lt 18 ]; then patch -F5 -p1 -d ~/$PG_SRC_DIR < test/postgres-asan-instrumentation.patch else patch -F5 -p1 -d ~/$PG_SRC_DIR < test/postgres-asan-instrumentation-PG18GE.patch diff --git a/.github/workflows/sanitizer-build-and-test.yaml b/.github/workflows/sanitizer-build-and-test.yaml index cb3167e75c9..6ab564b39c7 100644 --- a/.github/workflows/sanitizer-build-and-test.yaml +++ b/.github/workflows/sanitizer-build-and-test.yaml @@ -125,7 +125,9 @@ jobs: # Add instrumentation to the Postgres memory contexts. For more details, see # https://github.com/timescale/eng-database/wiki/Using-Address-Sanitizer#adding-more-instrumentation PG_MAJOR=$(echo "${{ matrix.pg }}" | sed -e 's![.].*!!') - if [ ${PG_MAJOR} -lt 18 ]; then + if [ ${PG_MAJOR} -lt 16 ]; then + patch -F5 -p1 -d ~/$PG_SRC_DIR < test/postgres-asan-instrumentation-PG15.patch + elif [ ${PG_MAJOR} -lt 18 ]; then patch -F5 -p1 -d ~/$PG_SRC_DIR < test/postgres-asan-instrumentation.patch else patch -F5 -p1 -d ~/$PG_SRC_DIR < test/postgres-asan-instrumentation-PG18GE.patch diff --git a/test/postgres-asan-instrumentation-PG15.patch b/test/postgres-asan-instrumentation-PG15.patch new file mode 100644 index 00000000000..31d70b331e5 --- /dev/null +++ b/test/postgres-asan-instrumentation-PG15.patch @@ -0,0 +1,68 @@ +diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c +index 2c50575b37..11b6c688c7 100644 +--- a/src/backend/tcop/postgres.c ++++ b/src/backend/tcop/postgres.c +@@ -3492,6 +4476,12 @@ check_stack_depth(void) + bool + stack_is_too_deep(void) + { ++ /* ++ * Pointer arithmetics to determine stack depth doesn't work under ++ * AddressSanitizer. ++ */ ++ return false; ++ + char stack_top_loc; + long stack_depth; + +diff --git a/src/include/utils/memdebug.h b/src/include/utils/memdebug.h +index e88b4c6e8e..4ccbbf0146 100644 +--- a/src/include/utils/memdebug.h ++++ b/src/include/utils/memdebug.h +@@ -19,6 +19,31 @@ + + #ifdef USE_VALGRIND + #include ++ ++#elif __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__) ++ ++#include ++ ++#define VALGRIND_MAKE_MEM_DEFINED(addr, size) \ ++ ASAN_UNPOISON_MEMORY_REGION(addr, size) ++ ++#define VALGRIND_MAKE_MEM_NOACCESS(addr, size) \ ++ ASAN_POISON_MEMORY_REGION(addr, size) ++ ++#define VALGRIND_MAKE_MEM_UNDEFINED(addr, size) \ ++ ASAN_UNPOISON_MEMORY_REGION(addr, size) ++ ++#define VALGRIND_MEMPOOL_ALLOC(context, addr, size) \ ++ ASAN_UNPOISON_MEMORY_REGION(addr, size) ++ ++#define VALGRIND_MEMPOOL_FREE(context, addr) \ ++ ASAN_POISON_MEMORY_REGION(addr, 1 /* Length unknown, poison first byte. */) ++ ++#define VALGRIND_CHECK_MEM_IS_DEFINED(addr, size) do {} while (0) ++#define VALGRIND_CREATE_MEMPOOL(context, redzones, zeroed) do {} while (0) ++#define VALGRIND_DESTROY_MEMPOOL(context) do {} while (0) ++#define VALGRIND_MEMPOOL_CHANGE(context, optr, nptr, size) do {} while (0) ++ + #else + #define VALGRIND_CHECK_MEM_IS_DEFINED(addr, size) do {} while (0) + #define VALGRIND_CREATE_MEMPOOL(context, redzones, zeroed) do {} while (0) +diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c +--- a/src/backend/utils/mmgr/aset.c ++++ b/src/backend/utils/mmgr/aset.c +@@ -1267,11 +1267,9 @@ + * Make sure not to mark too many bytes in case chunk->requested_size + * < size < oldchksize. + */ +-#ifdef USE_VALGRIND + if (Min(size, oldchksize) > chunk->requested_size) + VALGRIND_MAKE_MEM_UNDEFINED((char *) pointer + chunk->requested_size, + Min(size, oldchksize) - chunk->requested_size); +-#endif + #endif + + chunk->requested_size = size; diff --git a/test/postgres-asan-instrumentation-PG18GE.patch b/test/postgres-asan-instrumentation-PG18GE.patch index 5dc83af3ec4..6f6cfd663f2 100644 --- a/test/postgres-asan-instrumentation-PG18GE.patch +++ b/test/postgres-asan-instrumentation-PG18GE.patch @@ -20,7 +20,7 @@ index e88b4c6e8e..4ccbbf0146 100644 --- a/src/include/utils/memdebug.h +++ b/src/include/utils/memdebug.h @@ -19,6 +19,31 @@ - + #ifdef USE_VALGRIND #include + @@ -51,3 +51,73 @@ index e88b4c6e8e..4ccbbf0146 100644 #else #define VALGRIND_CHECK_MEM_IS_DEFINED(addr, size) do {} while (0) #define VALGRIND_CREATE_MEMPOOL(context, redzones, zeroed) do {} while (0) +diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c +--- a/src/backend/utils/mmgr/mcxt.c ++++ b/src/backend/utils/mmgr/mcxt.c +@@ -1549,17 +1549,13 @@ + void + pfree(void *pointer) + { +-#ifdef USE_VALGRIND + MemoryContextMethodID method = GetMemoryChunkMethodID(pointer); + MemoryContext context = GetMemoryChunkContext(pointer); +-#endif + + MCXT_METHOD(pointer, free_p) (pointer); + +-#ifdef USE_VALGRIND + if (method != MCTX_ALIGNED_REDIRECT_ID) + VALGRIND_MEMPOOL_FREE(context, pointer); +-#endif + } + + /* +@@ -1569,12 +1565,8 @@ + void * + repalloc(void *pointer, Size size) + { +-#ifdef USE_VALGRIND + MemoryContextMethodID method = GetMemoryChunkMethodID(pointer); +-#endif +-#if defined(USE_ASSERT_CHECKING) || defined(USE_VALGRIND) + MemoryContext context = GetMemoryChunkContext(pointer); +-#endif + void *ret; + + AssertNotInCriticalSection(context); +@@ -1597,10 +1589,8 @@ + */ + ret = MCXT_METHOD(pointer, realloc) (pointer, size, 0); + +-#ifdef USE_VALGRIND + if (method != MCTX_ALIGNED_REDIRECT_ID) + VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size); +-#endif + + return ret; + } +@@ -1613,9 +1603,7 @@ + void * + repalloc_extended(void *pointer, Size size, int flags) + { +-#if defined(USE_ASSERT_CHECKING) || defined(USE_VALGRIND) + MemoryContext context = GetMemoryChunkContext(pointer); +-#endif + void *ret; + + AssertNotInCriticalSection(context); +diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c +--- a/src/backend/utils/mmgr/aset.c ++++ b/src/backend/utils/mmgr/aset.c +@@ -1267,11 +1267,9 @@ + * Make sure not to mark too many bytes in case chunk->requested_size + * < size < oldchksize. + */ +-#ifdef USE_VALGRIND + if (Min(size, oldchksize) > chunk->requested_size) + VALGRIND_MAKE_MEM_UNDEFINED((char *) pointer + chunk->requested_size, + Min(size, oldchksize) - chunk->requested_size); +-#endif + #endif + + chunk->requested_size = size; diff --git a/test/postgres-asan-instrumentation.patch b/test/postgres-asan-instrumentation.patch index bc8f4e184b5..4b0100c4e6e 100644 --- a/test/postgres-asan-instrumentation.patch +++ b/test/postgres-asan-instrumentation.patch @@ -14,13 +14,13 @@ index 2c50575b37..11b6c688c7 100644 + char stack_top_loc; long stack_depth; - + diff --git a/src/include/utils/memdebug.h b/src/include/utils/memdebug.h index e88b4c6e8e..4ccbbf0146 100644 --- a/src/include/utils/memdebug.h +++ b/src/include/utils/memdebug.h @@ -19,6 +19,31 @@ - + #ifdef USE_VALGRIND #include + @@ -51,3 +51,73 @@ index e88b4c6e8e..4ccbbf0146 100644 #else #define VALGRIND_CHECK_MEM_IS_DEFINED(addr, size) do {} while (0) #define VALGRIND_CREATE_MEMPOOL(context, redzones, zeroed) do {} while (0) +diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c +--- a/src/backend/utils/mmgr/mcxt.c ++++ b/src/backend/utils/mmgr/mcxt.c +@@ -1549,17 +1549,13 @@ + void + pfree(void *pointer) + { +-#ifdef USE_VALGRIND + MemoryContextMethodID method = GetMemoryChunkMethodID(pointer); + MemoryContext context = GetMemoryChunkContext(pointer); +-#endif + + MCXT_METHOD(pointer, free_p) (pointer); + +-#ifdef USE_VALGRIND + if (method != MCTX_ALIGNED_REDIRECT_ID) + VALGRIND_MEMPOOL_FREE(context, pointer); +-#endif + } + + /* +@@ -1569,12 +1565,8 @@ + void * + repalloc(void *pointer, Size size) + { +-#ifdef USE_VALGRIND + MemoryContextMethodID method = GetMemoryChunkMethodID(pointer); +-#endif +-#if defined(USE_ASSERT_CHECKING) || defined(USE_VALGRIND) + MemoryContext context = GetMemoryChunkContext(pointer); +-#endif + void *ret; + + AssertNotInCriticalSection(context); +@@ -1597,10 +1589,8 @@ + */ + ret = MCXT_METHOD(pointer, realloc) (pointer, size, 0); + +-#ifdef USE_VALGRIND + if (method != MCTX_ALIGNED_REDIRECT_ID) + VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size); +-#endif + + return ret; + } +@@ -1613,9 +1603,7 @@ + void * + repalloc_extended(void *pointer, Size size, int flags) + { +-#if defined(USE_ASSERT_CHECKING) || defined(USE_VALGRIND) + MemoryContext context = GetMemoryChunkContext(pointer); +-#endif + void *ret; + + AssertNotInCriticalSection(context); +diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c +--- a/src/backend/utils/mmgr/aset.c ++++ b/src/backend/utils/mmgr/aset.c +@@ -1267,11 +1267,9 @@ + * Make sure not to mark too many bytes in case chunk->requested_size + * < size < oldchksize. + */ +-#ifdef USE_VALGRIND + if (Min(size, oldchksize) > chunk->requested_size) + VALGRIND_MAKE_MEM_UNDEFINED((char *) pointer + chunk->requested_size, + Min(size, oldchksize) - chunk->requested_size); +-#endif + #endif + + chunk->requested_size = size;