From 0979e0cc68b74335c3b40f689f6006e56e1c5f47 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 14:46:03 -0400
Subject: [PATCH 01/18] Improve our support for Valgrind's leak tracking.

When determining whether an allocated chunk is still reachable,
Valgrind will consider only pointers within what it believes to be
allocated chunks.  Normally, all of a block obtained from malloc()
would be considered "allocated" --- but it turns out that if we use
VALGRIND_MEMPOOL_ALLOC to designate sub-section(s) of a malloc'ed
block as allocated, all the rest of that malloc'ed block is ignored.
This leads to lots of false positives of course.  In particular,
in any multi-malloc-block context, all but the primary block were
reported as leaked.  We also had a problem with context "ident"
strings, which were reported as leaked unless there was some other
pointer to them besides the one in the context header.

To fix, we need to use VALGRIND_MEMPOOL_ALLOC to designate
a context's management structs (the context struct itself and
any per-block headers) as allocated chunks.  That forces moving
the VALGRIND_CREATE_MEMPOOL/VALGRIND_DESTROY_MEMPOOL calls into
the per-context-type code, so that the pool identifier can be
made as soon as we've allocated the initial block, but otherwise
it's fairly straightforward.  Note that in Valgrind's eyes there
is no distinction between these allocations and the allocations
that the mmgr modules hand out to user code.  That's fine for
now, but perhaps someday we'll want to do better yet.

When reading this patch, it's helpful to start with the comments
added at the head of mcxt.c.

Author: Andres Freund <andres@anarazel.de>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
Discussion: https://postgr.es/m/20210317181531.7oggpqevzz6bka3g@alap3.anarazel.de
---
 src/backend/utils/mmgr/aset.c       | 71 +++++++++++++++++++++++++++--
 src/backend/utils/mmgr/bump.c       | 31 ++++++++++++-
 src/backend/utils/mmgr/generation.c | 29 ++++++++++++
 src/backend/utils/mmgr/mcxt.c       | 23 +++++++---
 src/backend/utils/mmgr/slab.c       | 32 +++++++++++++
 src/include/utils/memdebug.h        |  1 +
 6 files changed, 177 insertions(+), 10 deletions(-)

diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 666ecd8f78d0..9ef109ca586b 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -103,6 +103,8 @@
 
 #define ALLOC_BLOCKHDRSZ	MAXALIGN(sizeof(AllocBlockData))
 #define ALLOC_CHUNKHDRSZ	sizeof(MemoryChunk)
+#define FIRST_BLOCKHDRSZ	(MAXALIGN(sizeof(AllocSetContext)) + \
+							 ALLOC_BLOCKHDRSZ)
 
 typedef struct AllocBlockData *AllocBlock;	/* forward reference */
 
@@ -458,6 +460,21 @@ AllocSetContextCreateInternal(MemoryContext parent,
 	 * we'd leak the header/initial block if we ereport in this stretch.
 	 */
 
+	/* Create a vpool associated with the context */
+	VALGRIND_CREATE_MEMPOOL(set, 0, false);
+
+	/*
+	 * Create a vchunk covering both the AllocSetContext struct and the keeper
+	 * block's header.  (Perhaps it would be more sensible for these to be two
+	 * separate vchunks, but doing that seems to tickle bugs in some versions
+	 * of Valgrind.)  We must have these vchunks, and also a vchunk for each
+	 * subsequently-added block header, so that Valgrind considers the
+	 * pointers within them while checking for leaked memory.  Note that
+	 * Valgrind doesn't distinguish between these vchunks and those created by
+	 * mcxt.c for the user-accessible-data chunks we allocate.
+	 */
+	VALGRIND_MEMPOOL_ALLOC(set, set, FIRST_BLOCKHDRSZ);
+
 	/* Fill in the initial block's block header */
 	block = KeeperBlock(set);
 	block->aset = set;
@@ -585,6 +602,14 @@ AllocSetReset(MemoryContext context)
 #ifdef CLOBBER_FREED_MEMORY
 			wipe_mem(block, block->freeptr - ((char *) block));
 #endif
+
+			/*
+			 * We need to free the block header's vchunk explicitly, although
+			 * the user-data vchunks within will go away in the TRIM below.
+			 * Otherwise Valgrind complains about leaked allocations.
+			 */
+			VALGRIND_MEMPOOL_FREE(set, block);
+
 			free(block);
 		}
 		block = next;
@@ -592,6 +617,14 @@ AllocSetReset(MemoryContext context)
 
 	Assert(context->mem_allocated == keepersize);
 
+	/*
+	 * Instruct Valgrind to throw away all the vchunks associated with this
+	 * context, except for the one covering the AllocSetContext and
+	 * keeper-block header.  This gets rid of the vchunks for whatever user
+	 * data is getting discarded by the context reset.
+	 */
+	VALGRIND_MEMPOOL_TRIM(set, set, FIRST_BLOCKHDRSZ);
+
 	/* Reset block size allocation sequence, too */
 	set->nextBlockSize = set->initBlockSize;
 }
@@ -648,6 +681,9 @@ AllocSetDelete(MemoryContext context)
 				freelist->first_free = (AllocSetContext *) oldset->header.nextchild;
 				freelist->num_free--;
 
+				/* Destroy the context's vpool --- see notes below */
+				VALGRIND_DESTROY_MEMPOOL(oldset);
+
 				/* All that remains is to free the header/initial block */
 				free(oldset);
 			}
@@ -675,13 +711,24 @@ AllocSetDelete(MemoryContext context)
 #endif
 
 		if (!IsKeeperBlock(set, block))
+		{
+			/* As in AllocSetReset, free block-header vchunks explicitly */
+			VALGRIND_MEMPOOL_FREE(set, block);
 			free(block);
+		}
 
 		block = next;
 	}
 
 	Assert(context->mem_allocated == keepersize);
 
+	/*
+	 * Destroy the vpool.  We don't seem to need to explicitly free the
+	 * initial block's header vchunk, nor any user-data vchunks that Valgrind
+	 * still knows about; they'll all go away automatically.
+	 */
+	VALGRIND_DESTROY_MEMPOOL(set);
+
 	/* Finally, free the context header, including the keeper block */
 	free(set);
 }
@@ -716,6 +763,9 @@ AllocSetAllocLarge(MemoryContext context, Size size, int flags)
 	if (block == NULL)
 		return MemoryContextAllocationFailure(context, size, flags);
 
+	/* Make a vchunk covering the new block's header */
+	VALGRIND_MEMPOOL_ALLOC(set, block, ALLOC_BLOCKHDRSZ);
+
 	context->mem_allocated += blksize;
 
 	block->aset = set;
@@ -922,6 +972,9 @@ AllocSetAllocFromNewBlock(MemoryContext context, Size size, int flags,
 	if (block == NULL)
 		return MemoryContextAllocationFailure(context, size, flags);
 
+	/* Make a vchunk covering the new block's header */
+	VALGRIND_MEMPOOL_ALLOC(set, block, ALLOC_BLOCKHDRSZ);
+
 	context->mem_allocated += blksize;
 
 	block->aset = set;
@@ -1104,6 +1157,10 @@ AllocSetFree(void *pointer)
 #ifdef CLOBBER_FREED_MEMORY
 		wipe_mem(block, block->freeptr - ((char *) block));
 #endif
+
+		/* As in AllocSetReset, free block-header vchunks explicitly */
+		VALGRIND_MEMPOOL_FREE(set, block);
+
 		free(block);
 	}
 	else
@@ -1184,6 +1241,7 @@ AllocSetRealloc(void *pointer, Size size, int flags)
 		 * realloc() to make the containing block bigger, or smaller, with
 		 * minimum space wastage.
 		 */
+		AllocBlock	newblock;
 		Size		chksize;
 		Size		blksize;
 		Size		oldblksize;
@@ -1223,14 +1281,21 @@ AllocSetRealloc(void *pointer, Size size, int flags)
 		blksize = chksize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
 		oldblksize = block->endptr - ((char *) block);
 
-		block = (AllocBlock) realloc(block, blksize);
-		if (block == NULL)
+		newblock = (AllocBlock) realloc(block, blksize);
+		if (newblock == NULL)
 		{
 			/* Disallow access to the chunk header. */
 			VALGRIND_MAKE_MEM_NOACCESS(chunk, ALLOC_CHUNKHDRSZ);
 			return MemoryContextAllocationFailure(&set->header, size, flags);
 		}
 
+		/*
+		 * Move the block-header vchunk explicitly.  (mcxt.c will take care of
+		 * moving the vchunk for the user data.)
+		 */
+		VALGRIND_MEMPOOL_CHANGE(set, block, newblock, ALLOC_BLOCKHDRSZ);
+		block = newblock;
+
 		/* updated separately, not to underflow when (oldblksize > blksize) */
 		set->header.mem_allocated -= oldblksize;
 		set->header.mem_allocated += blksize;
@@ -1294,7 +1359,7 @@ AllocSetRealloc(void *pointer, Size size, int flags)
 		/* Ensure any padding bytes are marked NOACCESS. */
 		VALGRIND_MAKE_MEM_NOACCESS((char *) pointer + size, chksize - size);
 
-		/* Disallow access to the chunk header . */
+		/* Disallow access to the chunk header. */
 		VALGRIND_MAKE_MEM_NOACCESS(chunk, ALLOC_CHUNKHDRSZ);
 
 		return pointer;
diff --git a/src/backend/utils/mmgr/bump.c b/src/backend/utils/mmgr/bump.c
index f7a37d1b3e86..2805d55a2eca 100644
--- a/src/backend/utils/mmgr/bump.c
+++ b/src/backend/utils/mmgr/bump.c
@@ -45,7 +45,9 @@
 #include "utils/memutils_memorychunk.h"
 #include "utils/memutils_internal.h"
 
-#define Bump_BLOCKHDRSZ	MAXALIGN(sizeof(BumpBlock))
+#define Bump_BLOCKHDRSZ		MAXALIGN(sizeof(BumpBlock))
+#define FIRST_BLOCKHDRSZ	(MAXALIGN(sizeof(BumpContext)) + \
+							 Bump_BLOCKHDRSZ)
 
 /* No chunk header unless built with MEMORY_CONTEXT_CHECKING */
 #ifdef MEMORY_CONTEXT_CHECKING
@@ -189,6 +191,12 @@ BumpContextCreate(MemoryContext parent, const char *name, Size minContextSize,
 	 * Avoid writing code that can fail between here and MemoryContextCreate;
 	 * we'd leak the header and initial block if we ereport in this stretch.
 	 */
+
+	/* See comments about Valgrind interactions in aset.c */
+	VALGRIND_CREATE_MEMPOOL(set, 0, false);
+	/* This vchunk covers the BumpContext and the keeper block header */
+	VALGRIND_MEMPOOL_ALLOC(set, set, FIRST_BLOCKHDRSZ);
+
 	dlist_init(&set->blocks);
 
 	/* Fill in the initial block's block header */
@@ -262,6 +270,14 @@ BumpReset(MemoryContext context)
 			BumpBlockFree(set, block);
 	}
 
+	/*
+	 * Instruct Valgrind to throw away all the vchunks associated with this
+	 * context, except for the one covering the BumpContext and keeper-block
+	 * header.  This gets rid of the vchunks for whatever user data is getting
+	 * discarded by the context reset.
+	 */
+	VALGRIND_MEMPOOL_TRIM(set, set, FIRST_BLOCKHDRSZ);
+
 	/* Reset block size allocation sequence, too */
 	set->nextBlockSize = set->initBlockSize;
 
@@ -279,6 +295,10 @@ BumpDelete(MemoryContext context)
 {
 	/* Reset to release all releasable BumpBlocks */
 	BumpReset(context);
+
+	/* Destroy the vpool -- see notes in aset.c */
+	VALGRIND_DESTROY_MEMPOOL(context);
+
 	/* And free the context header and keeper block */
 	free(context);
 }
@@ -318,6 +338,9 @@ BumpAllocLarge(MemoryContext context, Size size, int flags)
 	if (block == NULL)
 		return MemoryContextAllocationFailure(context, size, flags);
 
+	/* Make a vchunk covering the new block's header */
+	VALGRIND_MEMPOOL_ALLOC(set, block, Bump_BLOCKHDRSZ);
+
 	context->mem_allocated += blksize;
 
 	/* the block is completely full */
@@ -455,6 +478,9 @@ BumpAllocFromNewBlock(MemoryContext context, Size size, int flags,
 	if (block == NULL)
 		return MemoryContextAllocationFailure(context, size, flags);
 
+	/* Make a vchunk covering the new block's header */
+	VALGRIND_MEMPOOL_ALLOC(set, block, Bump_BLOCKHDRSZ);
+
 	context->mem_allocated += blksize;
 
 	/* initialize the new block */
@@ -606,6 +632,9 @@ BumpBlockFree(BumpContext *set, BumpBlock *block)
 	wipe_mem(block, ((char *) block->endptr - (char *) block));
 #endif
 
+	/* As in aset.c, free block-header vchunks explicitly */
+	VALGRIND_MEMPOOL_FREE(set, block);
+
 	free(block);
 }
 
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index 18679ad4f1e4..cfafc9bf0829 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -45,6 +45,8 @@
 
 #define Generation_BLOCKHDRSZ	MAXALIGN(sizeof(GenerationBlock))
 #define Generation_CHUNKHDRSZ	sizeof(MemoryChunk)
+#define FIRST_BLOCKHDRSZ		(MAXALIGN(sizeof(GenerationContext)) + \
+								 Generation_BLOCKHDRSZ)
 
 #define Generation_CHUNK_FRACTION	8
 
@@ -221,6 +223,12 @@ GenerationContextCreate(MemoryContext parent,
 	 * Avoid writing code that can fail between here and MemoryContextCreate;
 	 * we'd leak the header if we ereport in this stretch.
 	 */
+
+	/* See comments about Valgrind interactions in aset.c */
+	VALGRIND_CREATE_MEMPOOL(set, 0, false);
+	/* This vchunk covers the GenerationContext and the keeper block header */
+	VALGRIND_MEMPOOL_ALLOC(set, set, FIRST_BLOCKHDRSZ);
+
 	dlist_init(&set->blocks);
 
 	/* Fill in the initial block's block header */
@@ -309,6 +317,14 @@ GenerationReset(MemoryContext context)
 			GenerationBlockFree(set, block);
 	}
 
+	/*
+	 * Instruct Valgrind to throw away all the vchunks associated with this
+	 * context, except for the one covering the GenerationContext and
+	 * keeper-block header.  This gets rid of the vchunks for whatever user
+	 * data is getting discarded by the context reset.
+	 */
+	VALGRIND_MEMPOOL_TRIM(set, set, FIRST_BLOCKHDRSZ);
+
 	/* set it so new allocations to make use of the keeper block */
 	set->block = KeeperBlock(set);
 
@@ -329,6 +345,10 @@ GenerationDelete(MemoryContext context)
 {
 	/* Reset to release all releasable GenerationBlocks */
 	GenerationReset(context);
+
+	/* Destroy the vpool -- see notes in aset.c */
+	VALGRIND_DESTROY_MEMPOOL(context);
+
 	/* And free the context header and keeper block */
 	free(context);
 }
@@ -365,6 +385,9 @@ GenerationAllocLarge(MemoryContext context, Size size, int flags)
 	if (block == NULL)
 		return MemoryContextAllocationFailure(context, size, flags);
 
+	/* Make a vchunk covering the new block's header */
+	VALGRIND_MEMPOOL_ALLOC(set, block, Generation_BLOCKHDRSZ);
+
 	context->mem_allocated += blksize;
 
 	/* block with a single (used) chunk */
@@ -487,6 +510,9 @@ GenerationAllocFromNewBlock(MemoryContext context, Size size, int flags,
 	if (block == NULL)
 		return MemoryContextAllocationFailure(context, size, flags);
 
+	/* Make a vchunk covering the new block's header */
+	VALGRIND_MEMPOOL_ALLOC(set, block, Generation_BLOCKHDRSZ);
+
 	context->mem_allocated += blksize;
 
 	/* initialize the new block */
@@ -677,6 +703,9 @@ GenerationBlockFree(GenerationContext *set, GenerationBlock *block)
 	wipe_mem(block, block->blksize);
 #endif
 
+	/* As in aset.c, free block-header vchunks explicitly */
+	VALGRIND_MEMPOOL_FREE(set, block);
+
 	free(block);
 }
 
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 15fa4d0a55ee..7097cd8a25e0 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -8,6 +8,23 @@
  * context-type-specific operations via the function pointers in a
  * context's MemoryContextMethods struct.
  *
+ * A note about Valgrind support: when USE_VALGRIND is defined, we provide
+ * support for memory leak tracking at the allocation-unit level.  Valgrind
+ * does leak detection by tracking allocated "chunks", which can be grouped
+ * into "pools".  The "chunk" terminology is overloaded, since we use that
+ * word for our allocation units, and it's sometimes important to distinguish
+ * those from the Valgrind objects that describe them.  To reduce confusion,
+ * let's use the terms "vchunk" and "vpool" for the Valgrind objects.
+ *
+ * We use a separate vpool for each memory context.  The context-type-specific
+ * code is responsible for creating and deleting the vpools, and also for
+ * creating vchunks to cover its management data structures such as block
+ * headers.  (There must be a vchunk that includes every pointer we want
+ * Valgrind to consider for leak-tracking purposes.)  This module creates
+ * and deletes the vchunks that cover the caller-visible allocated chunks.
+ * However, the context-type-specific code must handle cleaning up those
+ * vchunks too during memory context reset operations.
+ *
  *
  * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
@@ -418,8 +435,6 @@ MemoryContextResetOnly(MemoryContext context)
 
 		context->methods->reset(context);
 		context->isReset = true;
-		VALGRIND_DESTROY_MEMPOOL(context);
-		VALGRIND_CREATE_MEMPOOL(context, 0, false);
 	}
 }
 
@@ -526,8 +541,6 @@ MemoryContextDeleteOnly(MemoryContext context)
 	context->ident = NULL;
 
 	context->methods->delete_context(context);
-
-	VALGRIND_DESTROY_MEMPOOL(context);
 }
 
 /*
@@ -1137,8 +1150,6 @@ MemoryContextCreate(MemoryContext node,
 		node->nextchild = NULL;
 		node->allowInCritSection = false;
 	}
-
-	VALGRIND_CREATE_MEMPOOL(node, 0, false);
 }
 
 /*
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index d32c0d318fbf..0e35abcf5a05 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -377,6 +377,11 @@ SlabContextCreate(MemoryContext parent,
 	 * we'd leak the header if we ereport in this stretch.
 	 */
 
+	/* See comments about Valgrind interactions in aset.c */
+	VALGRIND_CREATE_MEMPOOL(slab, 0, false);
+	/* This vchunk covers the SlabContext only */
+	VALGRIND_MEMPOOL_ALLOC(slab, slab, sizeof(SlabContext));
+
 	/* Fill in SlabContext-specific header fields */
 	slab->chunkSize = (uint32) chunkSize;
 	slab->fullChunkSize = (uint32) fullChunkSize;
@@ -451,6 +456,10 @@ SlabReset(MemoryContext context)
 #ifdef CLOBBER_FREED_MEMORY
 		wipe_mem(block, slab->blockSize);
 #endif
+
+		/* As in aset.c, free block-header vchunks explicitly */
+		VALGRIND_MEMPOOL_FREE(slab, block);
+
 		free(block);
 		context->mem_allocated -= slab->blockSize;
 	}
@@ -467,11 +476,23 @@ SlabReset(MemoryContext context)
 #ifdef CLOBBER_FREED_MEMORY
 			wipe_mem(block, slab->blockSize);
 #endif
+
+			/* As in aset.c, free block-header vchunks explicitly */
+			VALGRIND_MEMPOOL_FREE(slab, block);
+
 			free(block);
 			context->mem_allocated -= slab->blockSize;
 		}
 	}
 
+	/*
+	 * Instruct Valgrind to throw away all the vchunks associated with this
+	 * context, except for the one covering the SlabContext.  This gets rid of
+	 * the vchunks for whatever user data is getting discarded by the context
+	 * reset.
+	 */
+	VALGRIND_MEMPOOL_TRIM(slab, slab, sizeof(SlabContext));
+
 	slab->curBlocklistIndex = 0;
 
 	Assert(context->mem_allocated == 0);
@@ -486,6 +507,10 @@ SlabDelete(MemoryContext context)
 {
 	/* Reset to release all the SlabBlocks */
 	SlabReset(context);
+
+	/* Destroy the vpool -- see notes in aset.c */
+	VALGRIND_DESTROY_MEMPOOL(context);
+
 	/* And free the context header */
 	free(context);
 }
@@ -567,6 +592,9 @@ SlabAllocFromNewBlock(MemoryContext context, Size size, int flags)
 		if (unlikely(block == NULL))
 			return MemoryContextAllocationFailure(context, size, flags);
 
+		/* Make a vchunk covering the new block's header */
+		VALGRIND_MEMPOOL_ALLOC(slab, block, Slab_BLOCKHDRSZ);
+
 		block->slab = slab;
 		context->mem_allocated += slab->blockSize;
 
@@ -795,6 +823,10 @@ SlabFree(void *pointer)
 #ifdef CLOBBER_FREED_MEMORY
 			wipe_mem(block, slab->blockSize);
 #endif
+
+			/* As in aset.c, free block-header vchunks explicitly */
+			VALGRIND_MEMPOOL_FREE(slab, block);
+
 			free(block);
 			slab->header.mem_allocated -= slab->blockSize;
 		}
diff --git a/src/include/utils/memdebug.h b/src/include/utils/memdebug.h
index 7309271834b9..80692dcef938 100644
--- a/src/include/utils/memdebug.h
+++ b/src/include/utils/memdebug.h
@@ -29,6 +29,7 @@
 #define VALGRIND_MEMPOOL_ALLOC(context, addr, size)			do {} while (0)
 #define VALGRIND_MEMPOOL_FREE(context, addr)				do {} while (0)
 #define VALGRIND_MEMPOOL_CHANGE(context, optr, nptr, size)	do {} while (0)
+#define VALGRIND_MEMPOOL_TRIM(context, addr, size)			do {} while (0)
 #endif
 
 

From 436483e428ee2c29871d4f1370d483f1aa894d3a Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 14:46:32 -0400
Subject: [PATCH 02/18] Fix MemoryContextAllocAligned's interaction with
 Valgrind.

Arrange that only the "aligned chunk" part of the allocated space is
included in a Valgrind vchunk.  This suppresses complaints about that
vchunk being possibly lost because PG is retaining only pointers to
the aligned chunk.  Also make sure that trailing wasted space is
marked NOACCESS.

As a tiny performance improvement, arrange that MCXT_ALLOC_ZERO zeroes
only the returned "aligned chunk", not the wasted padding space.

In passing, fix GetLocalBufferStorage to use MemoryContextAllocAligned
instead of rolling its own implementation, which was equally broken
according to Valgrind.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/storage/buffer/localbuf.c |  9 +++--
 src/backend/utils/mmgr/alignedalloc.c | 18 +++++++++
 src/backend/utils/mmgr/mcxt.c         | 54 ++++++++++++++++-----------
 3 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index ba26627f7b00..0eb5b5bf3004 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -925,10 +925,11 @@ GetLocalBufferStorage(void)
 		num_bufs = Min(num_bufs, MaxAllocSize / BLCKSZ);
 
 		/* Buffers should be I/O aligned. */
-		cur_block = (char *)
-			TYPEALIGN(PG_IO_ALIGN_SIZE,
-					  MemoryContextAlloc(LocalBufferContext,
-										 num_bufs * BLCKSZ + PG_IO_ALIGN_SIZE));
+		cur_block = MemoryContextAllocAligned(LocalBufferContext,
+											  num_bufs * BLCKSZ,
+											  PG_IO_ALIGN_SIZE,
+											  0);
+
 		next_buf_in_block = 0;
 		num_bufs_in_block = num_bufs;
 	}
diff --git a/src/backend/utils/mmgr/alignedalloc.c b/src/backend/utils/mmgr/alignedalloc.c
index 7eea695de62c..b1be74269149 100644
--- a/src/backend/utils/mmgr/alignedalloc.c
+++ b/src/backend/utils/mmgr/alignedalloc.c
@@ -45,6 +45,15 @@ AlignedAllocFree(void *pointer)
 			 GetMemoryChunkContext(unaligned)->name, chunk);
 #endif
 
+	/*
+	 * Create a dummy vchunk covering the start of the unaligned chunk, but
+	 * not overlapping the aligned chunk.  This will be freed while pfree'ing
+	 * the unaligned chunk, keeping Valgrind happy.  Then when we return to
+	 * the outer pfree, that will clean up the vchunk for the aligned chunk.
+	 */
+	VALGRIND_MEMPOOL_ALLOC(GetMemoryChunkContext(unaligned), unaligned,
+						   (char *) pointer - (char *) unaligned);
+
 	/* Recursively pfree the unaligned chunk */
 	pfree(unaligned);
 }
@@ -123,6 +132,15 @@ AlignedAllocRealloc(void *pointer, Size size, int flags)
 	VALGRIND_MAKE_MEM_DEFINED(pointer, old_size);
 	memcpy(newptr, pointer, Min(size, old_size));
 
+	/*
+	 * Create a dummy vchunk covering the start of the old unaligned chunk,
+	 * but not overlapping the aligned chunk.  This will be freed while
+	 * pfree'ing the old unaligned chunk, keeping Valgrind happy.  Then when
+	 * we return to repalloc, it will move the vchunk for the aligned chunk.
+	 */
+	VALGRIND_MEMPOOL_ALLOC(ctx, unaligned,
+						   (char *) pointer - (char *) unaligned);
+
 	pfree(unaligned);
 
 	return newptr;
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 7097cd8a25e0..4096331978b9 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1432,7 +1432,13 @@ MemoryContextAllocAligned(MemoryContext context,
 	void	   *unaligned;
 	void	   *aligned;
 
-	/* wouldn't make much sense to waste that much space */
+	/*
+	 * Restrict alignto to ensure that it can fit into the "value" field of
+	 * the redirection MemoryChunk, and that the distance back to the start of
+	 * the unaligned chunk will fit into the space available for that.  This
+	 * isn't a limitation in practice, since it wouldn't make much sense to
+	 * waste that much space.
+	 */
 	Assert(alignto < (128 * 1024 * 1024));
 
 	/* ensure alignto is a power of 2 */
@@ -1469,10 +1475,15 @@ MemoryContextAllocAligned(MemoryContext context,
 	alloc_size += 1;
 #endif
 
-	/* perform the actual allocation */
-	unaligned = MemoryContextAllocExtended(context, alloc_size, flags);
+	/*
+	 * Perform the actual allocation, but do not pass down MCXT_ALLOC_ZERO.
+	 * This ensures that wasted bytes beyond the aligned chunk do not become
+	 * DEFINED.
+	 */
+	unaligned = MemoryContextAllocExtended(context, alloc_size,
+										   flags & ~MCXT_ALLOC_ZERO);
 
-	/* set the aligned pointer */
+	/* compute the aligned pointer */
 	aligned = (void *) TYPEALIGN(alignto, (char *) unaligned +
 								 sizeof(MemoryChunk));
 
@@ -1500,12 +1511,23 @@ MemoryContextAllocAligned(MemoryContext context,
 	set_sentinel(aligned, size);
 #endif
 
-	/* Mark the bytes before the redirection header as noaccess */
-	VALGRIND_MAKE_MEM_NOACCESS(unaligned,
-							   (char *) alignedchunk - (char *) unaligned);
+	/*
+	 * MemoryContextAllocExtended marked the whole unaligned chunk as a
+	 * vchunk.  Undo that, instead making just the aligned chunk be a vchunk.
+	 * This prevents Valgrind from complaining that the vchunk is possibly
+	 * leaked, since only pointers to the aligned chunk will exist.
+	 *
+	 * After these calls, the aligned chunk will be marked UNDEFINED, and all
+	 * the rest of the unaligned chunk (the redirection chunk header, the
+	 * padding bytes before it, and any wasted trailing bytes) will be marked
+	 * NOACCESS, which is what we want.
+	 */
+	VALGRIND_MEMPOOL_FREE(context, unaligned);
+	VALGRIND_MEMPOOL_ALLOC(context, aligned, size);
 
-	/* Disallow access to the redirection chunk header. */
-	VALGRIND_MAKE_MEM_NOACCESS(alignedchunk, sizeof(MemoryChunk));
+	/* Now zero (and make DEFINED) just the aligned chunk, if requested */
+	if ((flags & MCXT_ALLOC_ZERO) != 0)
+		MemSetAligned(aligned, 0, size);
 
 	return aligned;
 }
@@ -1539,16 +1561,12 @@ 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
+	VALGRIND_MEMPOOL_FREE(context, pointer);
 }
 
 /*
@@ -1558,9 +1576,6 @@ pfree(void *pointer)
 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
@@ -1583,10 +1598,7 @@ repalloc(void *pointer, Size size)
 	 */
 	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
+	VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
 
 	return ret;
 }

From c490b240f62232e2692254e4745caeeb9696cfb6 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 14:47:28 -0400
Subject: [PATCH 03/18] Silence complaints about leaked dynahash storage.

Because dynahash.c never frees hashtable storage except by deleting
the whole hashtable context, it doesn't bother to track the individual
blocks of elements allocated by element_alloc().  This results in
"possibly lost" complaints from Valgrind except when the first element
of each block is actively in use.  (Otherwise it'll be on a freelist,
but very likely only reachable via "interior pointers" within element
blocks, which doesn't satisfy Valgrind.)

To fix, if we're building with USE_VALGRIND, expend an extra pointer's
worth of space in each element block so that we can chain them all
together from the HTAB header.  Skip this in shared hashtables though:
Valgrind doesn't track those, and we'd need additional locking to make
it safe to manipulate a shared chain.

While here, update a comment obsoleted by 9c911ec06.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/utils/hash/dynahash.c | 52 +++++++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 1ad155d446e5..f81f9c199e45 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -22,10 +22,11 @@
  * lookup key's hash value as a partition number --- this will work because
  * of the way calc_bucket() maps hash values to bucket numbers.
  *
- * For hash tables in shared memory, the memory allocator function should
- * match malloc's semantics of returning NULL on failure.  For hash tables
- * in local memory, we typically use palloc() which will throw error on
- * failure.  The code in this file has to cope with both cases.
+ * The memory allocator function should match malloc's semantics of returning
+ * NULL on failure.  (This is essential for hash tables in shared memory.
+ * For hash tables in local memory, we used to use palloc() which will throw
+ * error on failure; but we no longer do, so it's untested whether this
+ * module will still cope with that behavior.)
  *
  * dynahash.c provides support for these types of lookup keys:
  *
@@ -98,6 +99,7 @@
 
 #include "access/xact.h"
 #include "common/hashfn.h"
+#include "lib/ilist.h"
 #include "port/pg_bitutils.h"
 #include "storage/shmem.h"
 #include "storage/spin.h"
@@ -236,6 +238,16 @@ struct HTAB
 	Size		keysize;		/* hash key length in bytes */
 	long		ssize;			/* segment size --- must be power of 2 */
 	int			sshift;			/* segment shift = log2(ssize) */
+
+	/*
+	 * In a USE_VALGRIND build, non-shared hashtables keep an slist chain of
+	 * all the element blocks they have allocated.  This pacifies Valgrind,
+	 * which would otherwise often claim that the element blocks are "possibly
+	 * lost" for lack of any non-interior pointers to their starts.
+	 */
+#ifdef USE_VALGRIND
+	slist_head	element_blocks;
+#endif
 };
 
 /*
@@ -1708,6 +1720,8 @@ element_alloc(HTAB *hashp, int nelem, int freelist_idx)
 {
 	HASHHDR    *hctl = hashp->hctl;
 	Size		elementSize;
+	Size		requestSize;
+	char	   *allocedBlock;
 	HASHELEMENT *firstElement;
 	HASHELEMENT *tmpElement;
 	HASHELEMENT *prevElement;
@@ -1719,12 +1733,38 @@ element_alloc(HTAB *hashp, int nelem, int freelist_idx)
 	/* Each element has a HASHELEMENT header plus user data. */
 	elementSize = MAXALIGN(sizeof(HASHELEMENT)) + MAXALIGN(hctl->entrysize);
 
+	requestSize = nelem * elementSize;
+
+	/* Add space for slist_node list link if we need one. */
+#ifdef USE_VALGRIND
+	if (!hashp->isshared)
+		requestSize += MAXALIGN(sizeof(slist_node));
+#endif
+
+	/* Allocate the memory. */
 	CurrentDynaHashCxt = hashp->hcxt;
-	firstElement = (HASHELEMENT *) hashp->alloc(nelem * elementSize);
+	allocedBlock = hashp->alloc(requestSize);
 
-	if (!firstElement)
+	if (!allocedBlock)
 		return false;
 
+	/*
+	 * If USE_VALGRIND, each allocated block of elements of a non-shared
+	 * hashtable is chained into a list, so that Valgrind won't think it's
+	 * been leaked.
+	 */
+#ifdef USE_VALGRIND
+	if (hashp->isshared)
+		firstElement = (HASHELEMENT *) allocedBlock;
+	else
+	{
+		slist_push_head(&hashp->element_blocks, (slist_node *) allocedBlock);
+		firstElement = (HASHELEMENT *) (allocedBlock + MAXALIGN(sizeof(slist_node)));
+	}
+#else
+	firstElement = (HASHELEMENT *) allocedBlock;
+#endif
+
 	/* prepare to link all the new entries into the freelist */
 	prevElement = NULL;
 	tmpElement = firstElement;

From c3fd16aa5062129a7c6897ef195f93a2ba322847 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 14:49:31 -0400
Subject: [PATCH 04/18] Silence complaints involving dlist_node lists.

Put the dlist_node fields of catctup and catclist structs first.
This ensures that the dlist pointers point to the starts of these
palloc blocks, and thus that Valgrind won't consider them
"possibly lost".

The postmaster's PMChild structs and the autovac launcher's avl_dbase
structs also have the dlist_node-is-not-first problem, but putting it
first still wouldn't silence the warning because we bulk-allocate
those structs in an array, so that Valgrind sees a single allocation.
Commonly the first array element will be pointed to only from some
later element, so that the reference would be an interior pointer even
if it pointed to the array start.  (This is the same issue fixed in
the previous patch for dynahash elements.)  Since these are pretty
simple data structures, I don't feel too bad about faking out Valgrind
by just keeping a static pointer to the array start.

This is all quite hacky, and it's not hard to imagine usages where
we'd need some other idea in order to have reasonable leak tracking of
structures that are only accessible via dlist_node lists.  But this
patch seems to be enough to silence this class of leakage complaints
for the moment.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/postmaster/autovacuum.c | 14 ++++++++++++++
 src/backend/postmaster/pmchild.c    | 18 +++++++++++++++++-
 src/include/utils/catcache.h        | 23 ++++++++++++++---------
 3 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 451fb90a610a..3d76d74d1aa6 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -310,6 +310,16 @@ static AutoVacuumShmemStruct *AutoVacuumShmem;
 static dlist_head DatabaseList = DLIST_STATIC_INIT(DatabaseList);
 static MemoryContext DatabaseListCxt = NULL;
 
+/*
+ * Dummy pointer to persuade Valgrind that we've not leaked the array of
+ * avl_dbase structs.  Make it global to ensure the compiler doesn't
+ * optimize it away.
+ */
+#ifdef USE_VALGRIND
+extern avl_dbase *avl_dbase_array;
+avl_dbase  *avl_dbase_array;
+#endif
+
 /* Pointer to my own WorkerInfo, valid on each worker */
 static WorkerInfo MyWorkerInfo = NULL;
 
@@ -1020,6 +1030,10 @@ rebuild_database_list(Oid newdb)
 
 		/* put all the hash elements into an array */
 		dbary = palloc(nelems * sizeof(avl_dbase));
+		/* keep Valgrind quiet */
+#ifdef USE_VALGRIND
+		avl_dbase_array = dbary;
+#endif
 
 		i = 0;
 		hash_seq_init(&seq, dbhash);
diff --git a/src/backend/postmaster/pmchild.c b/src/backend/postmaster/pmchild.c
index cde1d23a4ca8..584bb58c8aba 100644
--- a/src/backend/postmaster/pmchild.c
+++ b/src/backend/postmaster/pmchild.c
@@ -59,6 +59,17 @@ NON_EXEC_STATIC int num_pmchild_slots = 0;
  */
 dlist_head	ActiveChildList;
 
+/*
+ * Dummy pointer to persuade Valgrind that we've not leaked the array of
+ * PMChild structs.  Make it global to ensure the compiler doesn't
+ * optimize it away.
+ */
+#ifdef USE_VALGRIND
+extern PMChild *pmchild_array;
+PMChild    *pmchild_array;
+#endif
+
+
 /*
  * MaxLivePostmasterChildren
  *
@@ -125,8 +136,13 @@ InitPostmasterChildSlots(void)
 	for (int i = 0; i < BACKEND_NUM_TYPES; i++)
 		num_pmchild_slots += pmchild_pools[i].size;
 
-	/* Initialize them */
+	/* Allocate enough slots, and make sure Valgrind doesn't complain */
 	slots = palloc(num_pmchild_slots * sizeof(PMChild));
+#ifdef USE_VALGRIND
+	pmchild_array = slots;
+#endif
+
+	/* Initialize them */
 	slotno = 0;
 	for (int btype = 0; btype < BACKEND_NUM_TYPES; btype++)
 	{
diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h
index 277ec33c00ba..00808e23f49b 100644
--- a/src/include/utils/catcache.h
+++ b/src/include/utils/catcache.h
@@ -87,6 +87,14 @@ typedef struct catcache
 
 typedef struct catctup
 {
+	/*
+	 * Each tuple in a cache is a member of a dlist that stores the elements
+	 * of its hash bucket.  We keep each dlist in LRU order to speed repeated
+	 * lookups.  Keep the dlist_node field first so that Valgrind understands
+	 * the struct is reachable.
+	 */
+	dlist_node	cache_elem;		/* list member of per-bucket list */
+
 	int			ct_magic;		/* for identifying CatCTup entries */
 #define CT_MAGIC   0x57261502
 
@@ -98,13 +106,6 @@ typedef struct catctup
 	 */
 	Datum		keys[CATCACHE_MAXKEYS];
 
-	/*
-	 * Each tuple in a cache is a member of a dlist that stores the elements
-	 * of its hash bucket.  We keep each dlist in LRU order to speed repeated
-	 * lookups.
-	 */
-	dlist_node	cache_elem;		/* list member of per-bucket list */
-
 	/*
 	 * A tuple marked "dead" must not be returned by subsequent searches.
 	 * However, it won't be physically deleted from the cache until its
@@ -158,13 +159,17 @@ typedef struct catctup
  */
 typedef struct catclist
 {
+	/*
+	 * Keep the dlist_node field first so that Valgrind understands the struct
+	 * is reachable.
+	 */
+	dlist_node	cache_elem;		/* list member of per-catcache list */
+
 	int			cl_magic;		/* for identifying CatCList entries */
 #define CL_MAGIC   0x52765103
 
 	uint32		hash_value;		/* hash value for lookup keys */
 
-	dlist_node	cache_elem;		/* list member of per-catcache list */
-
 	/*
 	 * Lookup keys for the entry, with the first nkeys elements being valid.
 	 * All by-reference are separately allocated.

From 1883352a4e5a9e2ff396065e022fc1cc3626963b Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 14:50:33 -0400
Subject: [PATCH 05/18] Silence complaints about save_ps_display_args.

Valgrind seems not to consider the global "environ" variable as a
valid root pointer; so when we allocate a new environment array,
it claims that data is leaked.  To fix that, keep our own
statically-allocated copy of the pointer, similarly to the
previous patch.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/utils/misc/ps_status.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/backend/utils/misc/ps_status.c b/src/backend/utils/misc/ps_status.c
index e08b26e8c14f..4df25944deb3 100644
--- a/src/backend/utils/misc/ps_status.c
+++ b/src/backend/utils/misc/ps_status.c
@@ -100,6 +100,17 @@ static void flush_ps_display(void);
 static int	save_argc;
 static char **save_argv;
 
+/*
+ * Valgrind seems not to consider the global "environ" variable as a valid
+ * root pointer; so when we allocate a new environment array, it claims that
+ * data is leaked.  To fix that, keep our own statically-allocated copy of the
+ * pointer.  (Oddly, this doesn't seem to be a problem for "argv".)
+ */
+#if defined(PS_USE_CLOBBER_ARGV) && defined(USE_VALGRIND)
+extern char **ps_status_new_environ;
+char	  **ps_status_new_environ;
+#endif
+
 
 /*
  * Call this early in startup to save the original argc/argv values.
@@ -206,6 +217,11 @@ save_ps_display_args(int argc, char **argv)
 		}
 		new_environ[i] = NULL;
 		environ = new_environ;
+
+		/* See notes about Valgrind above. */
+#ifdef USE_VALGRIND
+		ps_status_new_environ = new_environ;
+#endif
 	}
 
 	/*

From 7fa4df2933e1fafb34ef57348d5769b14933df7f Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 14:51:14 -0400
Subject: [PATCH 06/18] Don't leak the startup-packet buffer in
 ProcessStartupPacket.

This is the first actual leakage bug fix in this patch series.

The amount of memory regained is quite negligible of course.
But we don't want Valgrind whining about this in every session.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/tcop/backend_startup.c | 33 +++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c
index a7d1fec981f8..595fe08337b1 100644
--- a/src/backend/tcop/backend_startup.c
+++ b/src/backend/tcop/backend_startup.c
@@ -492,7 +492,7 @@ static int
 ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 {
 	int32		len;
-	char	   *buf;
+	char	   *buf = NULL;
 	ProtocolVersion proto;
 	MemoryContext oldcontext;
 
@@ -516,7 +516,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 		 * scanners, which may be less benign, but it's not really our job to
 		 * notice those.)
 		 */
-		return STATUS_ERROR;
+		goto fail;
 	}
 
 	if (pq_getbytes(((char *) &len) + 1, 3) == EOF)
@@ -526,7 +526,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 			ereport(COMMERROR,
 					(errcode(ERRCODE_PROTOCOL_VIOLATION),
 					 errmsg("incomplete startup packet")));
-		return STATUS_ERROR;
+		goto fail;
 	}
 
 	len = pg_ntoh32(len);
@@ -538,7 +538,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 		ereport(COMMERROR,
 				(errcode(ERRCODE_PROTOCOL_VIOLATION),
 				 errmsg("invalid length of startup packet")));
-		return STATUS_ERROR;
+		goto fail;
 	}
 
 	/*
@@ -554,7 +554,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 		ereport(COMMERROR,
 				(errcode(ERRCODE_PROTOCOL_VIOLATION),
 				 errmsg("incomplete startup packet")));
-		return STATUS_ERROR;
+		goto fail;
 	}
 	pq_endmsgread();
 
@@ -568,7 +568,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 	{
 		ProcessCancelRequestPacket(port, buf, len);
 		/* Not really an error, but we don't want to proceed further */
-		return STATUS_ERROR;
+		goto fail;
 	}
 
 	if (proto == NEGOTIATE_SSL_CODE && !ssl_done)
@@ -607,14 +607,16 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 			ereport(COMMERROR,
 					(errcode_for_socket_access(),
 					 errmsg("failed to send SSL negotiation response: %m")));
-			return STATUS_ERROR;	/* close the connection */
+			goto fail;			/* close the connection */
 		}
 
 #ifdef USE_SSL
 		if (SSLok == 'S' && secure_open_server(port) == -1)
-			return STATUS_ERROR;
+			goto fail;
 #endif
 
+		pfree(buf);
+
 		/*
 		 * At this point we should have no data already buffered.  If we do,
 		 * it was received before we performed the SSL handshake, so it wasn't
@@ -661,14 +663,16 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 			ereport(COMMERROR,
 					(errcode_for_socket_access(),
 					 errmsg("failed to send GSSAPI negotiation response: %m")));
-			return STATUS_ERROR;	/* close the connection */
+			goto fail;			/* close the connection */
 		}
 
 #ifdef ENABLE_GSS
 		if (GSSok == 'G' && secure_open_gssapi(port) == -1)
-			return STATUS_ERROR;
+			goto fail;
 #endif
 
+		pfree(buf);
+
 		/*
 		 * At this point we should have no data already buffered.  If we do,
 		 * it was received before we performed the GSS handshake, so it wasn't
@@ -863,7 +867,16 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 	 */
 	MemoryContextSwitchTo(oldcontext);
 
+	pfree(buf);
+
 	return STATUS_OK;
+
+fail:
+	/* be tidy, just to avoid Valgrind complaints */
+	if (buf)
+		pfree(buf);
+
+	return STATUS_ERROR;
 }
 
 /*

From 1761f5bcb04d6ce5f92c26c0490a04f644105d79 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 14:58:48 -0400
Subject: [PATCH 07/18] Silence complaints about autovacuum workers.

Free a couple of data structures manually near the end of the run
when USE_VALGRIND, and ensure that the final vac_update_datfrozenxid()
call is done in a non-permanent context.  This doesn't have any real
effect on the process's total memory consumption, since we're going to
exit as soon as that last transaction is done.  But it does pacify
Valgrind.

In combination with commit 02502c1bc, these fixes reduce reported
leakage in autovacuum workers to zero in my tests.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/postmaster/autovacuum.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 3d76d74d1aa6..5baadad222c3 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2579,8 +2579,18 @@ do_autovacuum(void)
 
 	/*
 	 * We leak table_toast_map here (among other things), but since we're
-	 * going away soon, it's not a problem.
+	 * going away soon, it's not a problem normally.  But when using Valgrind,
+	 * release some stuff to reduce complaints about leaked storage.
 	 */
+#ifdef USE_VALGRIND
+	hash_destroy(table_toast_map);
+	FreeTupleDesc(pg_class_desc);
+	if (bstrategy)
+		pfree(bstrategy);
+#endif
+
+	/* Run the rest in xact context, mainly to avoid Valgrind leak warnings */
+	MemoryContextSwitchTo(TopTransactionContext);
 
 	/*
 	 * Update pg_database.datfrozenxid, and truncate pg_xact if possible. We

From 103b51bfc0bf897d178688c0ddb212251a48507b Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:00:59 -0400
Subject: [PATCH 08/18] Fix leakage of pq_mq_handle in a parallel worker.

The amount of storage involved here is quite negligible,
but without this change Valgrind will complain about every
parallel worker process.

While at it, move mq_putmessage's "Assert(pq_mq_handle != NULL)"
to someplace where it's not trivially useless.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/libpq/pqmq.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/backend/libpq/pqmq.c b/src/backend/libpq/pqmq.c
index f1a08bc32ca1..763ef1909b30 100644
--- a/src/backend/libpq/pqmq.c
+++ b/src/backend/libpq/pqmq.c
@@ -23,7 +23,7 @@
 #include "tcop/tcopprot.h"
 #include "utils/builtins.h"
 
-static shm_mq_handle *pq_mq_handle;
+static shm_mq_handle *pq_mq_handle = NULL;
 static bool pq_mq_busy = false;
 static pid_t pq_mq_parallel_leader_pid = 0;
 static ProcNumber pq_mq_parallel_leader_proc_number = INVALID_PROC_NUMBER;
@@ -66,6 +66,8 @@ pq_redirect_to_shm_mq(dsm_segment *seg, shm_mq_handle *mqh)
 static void
 pq_cleanup_redirect_to_shm_mq(dsm_segment *seg, Datum arg)
 {
+	if (pq_mq_handle != NULL)
+		pfree(pq_mq_handle);
 	pq_mq_handle = NULL;
 	whereToSendOutput = DestNone;
 }
@@ -131,7 +133,10 @@ mq_putmessage(char msgtype, const char *s, size_t len)
 	if (pq_mq_busy)
 	{
 		if (pq_mq_handle != NULL)
+		{
 			shm_mq_detach(pq_mq_handle);
+			pfree(pq_mq_handle);
+		}
 		pq_mq_handle = NULL;
 		return EOF;
 	}
@@ -152,8 +157,6 @@ mq_putmessage(char msgtype, const char *s, size_t len)
 	iov[1].data = s;
 	iov[1].len = len;
 
-	Assert(pq_mq_handle != NULL);
-
 	for (;;)
 	{
 		/*
@@ -161,6 +164,7 @@ mq_putmessage(char msgtype, const char *s, size_t len)
 		 * that the shared memory value is updated before we send the parallel
 		 * message signal right after this.
 		 */
+		Assert(pq_mq_handle != NULL);
 		result = shm_mq_sendv(pq_mq_handle, iov, 2, true, true);
 
 		if (pq_mq_parallel_leader_pid != 0)

From c34240c1faf437667adea4f4866b6a4963259b9a Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:01:31 -0400
Subject: [PATCH 09/18] Reduce leakage during PL/pgSQL function compilation.

format_procedure leaks memory, so run it in a short-lived context
not the session-lifespan cache context for the PL/pgSQL function.

parse_datatype called the core parser in the function's cache context,
thus leaking potentially a lot of storage into that context.  We were
also being a bit careless with the TypeName structures made in that
code path and others.  Most of the time we don't need to retain the
TypeName, so make sure it is made in the short-lived temp context,
and copy it only if we do need to retain it.

These are far from the only leaks in PL/pgSQL compilation, but
they're the biggest as far as I've seen, and further improvement
looks like it'd require delicate and bug-prone surgery.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/pl/plpgsql/src/pl_comp.c | 28 ++++++++++++++++++++++------
 src/pl/plpgsql/src/pl_gram.y |  8 +++++++-
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index b80c59447fb5..22af973aae27 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -177,6 +177,7 @@ plpgsql_compile_callback(FunctionCallInfo fcinfo,
 	yyscan_t	scanner;
 	Datum		prosrcdatum;
 	char	   *proc_source;
+	char	   *proc_signature;
 	HeapTuple	typeTup;
 	Form_pg_type typeStruct;
 	PLpgSQL_variable *var;
@@ -223,6 +224,9 @@ plpgsql_compile_callback(FunctionCallInfo fcinfo,
 	plpgsql_check_syntax = forValidator;
 	plpgsql_curr_compile = function;
 
+	/* format_procedure leaks memory, so run it in temp context */
+	proc_signature = format_procedure(fcinfo->flinfo->fn_oid);
+
 	/*
 	 * All the permanent output of compilation (e.g. parse tree) is kept in a
 	 * per-function memory context, so it can be reclaimed easily.
@@ -237,7 +241,7 @@ plpgsql_compile_callback(FunctionCallInfo fcinfo,
 									 ALLOCSET_DEFAULT_SIZES);
 	plpgsql_compile_tmp_cxt = MemoryContextSwitchTo(func_cxt);
 
-	function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
+	function->fn_signature = pstrdup(proc_signature);
 	MemoryContextSetIdentifier(func_cxt, function->fn_signature);
 	function->fn_oid = fcinfo->flinfo->fn_oid;
 	function->fn_input_collation = fcinfo->fncollation;
@@ -1668,6 +1672,11 @@ plpgsql_parse_wordrowtype(char *ident)
 {
 	Oid			classOid;
 	Oid			typOid;
+	TypeName   *typName;
+	MemoryContext oldCxt;
+
+	/* Avoid memory leaks in long-term function context */
+	oldCxt = MemoryContextSwitchTo(plpgsql_compile_tmp_cxt);
 
 	/*
 	 * Look up the relation.  Note that because relation rowtypes have the
@@ -1690,9 +1699,12 @@ plpgsql_parse_wordrowtype(char *ident)
 				 errmsg("relation \"%s\" does not have a composite type",
 						ident)));
 
+	typName = makeTypeName(ident);
+
+	MemoryContextSwitchTo(oldCxt);
+
 	/* Build and return the row type struct */
-	return plpgsql_build_datatype(typOid, -1, InvalidOid,
-								  makeTypeName(ident));
+	return plpgsql_build_datatype(typOid, -1, InvalidOid, typName);
 }
 
 /* ----------
@@ -1706,6 +1718,7 @@ plpgsql_parse_cwordrowtype(List *idents)
 	Oid			classOid;
 	Oid			typOid;
 	RangeVar   *relvar;
+	TypeName   *typName;
 	MemoryContext oldCxt;
 
 	/*
@@ -1728,11 +1741,12 @@ plpgsql_parse_cwordrowtype(List *idents)
 				 errmsg("relation \"%s\" does not have a composite type",
 						relvar->relname)));
 
+	typName = makeTypeNameFromNameList(idents);
+
 	MemoryContextSwitchTo(oldCxt);
 
 	/* Build and return the row type struct */
-	return plpgsql_build_datatype(typOid, -1, InvalidOid,
-								  makeTypeNameFromNameList(idents));
+	return plpgsql_build_datatype(typOid, -1, InvalidOid, typName);
 }
 
 /*
@@ -1947,6 +1961,8 @@ plpgsql_build_recfield(PLpgSQL_rec *rec, const char *fldname)
  * origtypname is the parsed form of what the user wrote as the type name.
  * It can be NULL if the type could not be a composite type, or if it was
  * identified by OID to begin with (e.g., it's a function argument type).
+ * origtypname is in short-lived storage and must be copied if we choose
+ * to incorporate it into the function's parse tree.
  */
 PLpgSQL_type *
 plpgsql_build_datatype(Oid typeOid, int32 typmod,
@@ -2065,7 +2081,7 @@ build_datatype(HeapTuple typeTup, int32 typmod,
 					 errmsg("type %s is not composite",
 							format_type_be(typ->typoid))));
 
-		typ->origtypname = origtypname;
+		typ->origtypname = copyObject(origtypname);
 		typ->tcache = typentry;
 		typ->tupdesc_id = typentry->tupDesc_identifier;
 	}
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 5612e66d0239..6d53da4e79a1 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -3848,6 +3848,7 @@ parse_datatype(const char *string, int location, yyscan_t yyscanner)
 	int32		typmod;
 	sql_error_callback_arg cbarg;
 	ErrorContextCallback syntax_errcontext;
+	MemoryContext oldCxt;
 
 	cbarg.location = location;
 	cbarg.yyscanner = yyscanner;
@@ -3857,9 +3858,14 @@ parse_datatype(const char *string, int location, yyscan_t yyscanner)
 	syntax_errcontext.previous = error_context_stack;
 	error_context_stack = &syntax_errcontext;
 
-	/* Let the main parser try to parse it under standard SQL rules */
+	/*
+	 * Let the main parser try to parse it under standard SQL rules.  The
+	 * parser leaks memory, so run it in temp context.
+	 */
+	oldCxt = MemoryContextSwitchTo(plpgsql_compile_tmp_cxt);
 	typeName = typeStringToTypeName(string, NULL);
 	typenameTypeIdAndMod(NULL, typeName, &type_id, &typmod);
+	MemoryContextSwitchTo(oldCxt);
 
 	/* Restore former ereport callback */
 	error_context_stack = syntax_errcontext.previous;

From 45e13385f5090e97ea8332b09b351cf3442f8038 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:11:29 -0400
Subject: [PATCH 10/18] Suppress complaints about leaks in function cache
 loading.

PL/pgSQL and SQL-function parsing leak some stuff into the long-lived
function cache context.  This isn't really a huge practical problem,
since it's not a large amount of data and the cruft will be recovered
if we have to re-parse the function.  It's not clear that it's worth
working any harder than the previous patch did to eliminate these
leak complaints, so instead silence them with a suppression rule.

This suppression rule also hides the fact that CachedFunction structs
are intentionally leaked in some cases because we're unsure if any
fn_extra pointers remain.  That might be nice to do something about
eventually, but it's not clear how.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/tools/valgrind.supp | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/tools/valgrind.supp b/src/tools/valgrind.supp
index 7ea464c80941..87cf3866de00 100644
--- a/src/tools/valgrind.supp
+++ b/src/tools/valgrind.supp
@@ -180,3 +180,24 @@
    Memcheck:Cond
    fun:PyObject_Realloc
 }
+
+
+# Memory-leak suppressions
+# Note that a suppression rule will silence complaints about memory blocks
+# allocated in matching places, but it won't prevent "indirectly lost"
+# complaints about blocks that are only reachable via the suppressed blocks.
+
+# Suppress complaints about stuff leaked during function cache loading.
+# Both the PL/pgSQL and SQL-function parsing processes generate some cruft
+# within the function's cache context, which doesn't seem worth the trouble
+# to get rid of.  Moreover, there are cases where CachedFunction structs
+# are intentionally leaked because we're unsure if any fn_extra pointers
+# remain.
+{
+   hide_function_cache_leaks
+   Memcheck:Leak
+   match-leak-kinds: definite,possible,indirect
+
+   ...
+   fun:cached_function_compile
+}

From 5507e4663ca1338896b06cefa912728c100de618 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:12:40 -0400
Subject: [PATCH 11/18] Suppress complaints about leaks in TS dictionary
 loading.

Like the situation with function cache loading, text search
dictionary loading functions tend to leak some cruft into the
dictionary's long-lived cache context.  To judge by the examples in
the core regression tests, not very many bytes are at stake.
Moreover, I don't see a way to prevent such leaks without changing the
API for TS template initialization functions: right now they do not
have to worry about making sure that their results are long-lived.

Hence, I think we should install a suppression rule rather than trying
to fix this completely.  However, I did grab some low-hanging fruit:
several places were leaking the result of get_tsearch_config_filename.
This seems worth doing mostly because they are inconsistent with other
dictionaries that were freeing it already.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/tsearch/dict_ispell.c    | 18 ++++++++++++------
 src/backend/tsearch/dict_synonym.c   |  1 +
 src/backend/tsearch/dict_thesaurus.c |  7 ++++---
 src/backend/utils/cache/ts_cache.c   |  4 +++-
 src/tools/valgrind.supp              | 12 ++++++++++++
 5 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/src/backend/tsearch/dict_ispell.c b/src/backend/tsearch/dict_ispell.c
index 63bd193a78a8..debfbf956cc1 100644
--- a/src/backend/tsearch/dict_ispell.c
+++ b/src/backend/tsearch/dict_ispell.c
@@ -47,24 +47,30 @@ dispell_init(PG_FUNCTION_ARGS)
 
 		if (strcmp(defel->defname, "dictfile") == 0)
 		{
+			char	   *filename;
+
 			if (dictloaded)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 						 errmsg("multiple DictFile parameters")));
-			NIImportDictionary(&(d->obj),
-							   get_tsearch_config_filename(defGetString(defel),
-														   "dict"));
+			filename = get_tsearch_config_filename(defGetString(defel),
+												   "dict");
+			NIImportDictionary(&(d->obj), filename);
+			pfree(filename);
 			dictloaded = true;
 		}
 		else if (strcmp(defel->defname, "afffile") == 0)
 		{
+			char	   *filename;
+
 			if (affloaded)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 						 errmsg("multiple AffFile parameters")));
-			NIImportAffixes(&(d->obj),
-							get_tsearch_config_filename(defGetString(defel),
-														"affix"));
+			filename = get_tsearch_config_filename(defGetString(defel),
+												   "affix");
+			NIImportAffixes(&(d->obj), filename);
+			pfree(filename);
 			affloaded = true;
 		}
 		else if (strcmp(defel->defname, "stopwords") == 0)
diff --git a/src/backend/tsearch/dict_synonym.c b/src/backend/tsearch/dict_synonym.c
index 0da5a9d68680..c2773eb01ade 100644
--- a/src/backend/tsearch/dict_synonym.c
+++ b/src/backend/tsearch/dict_synonym.c
@@ -199,6 +199,7 @@ dsynonym_init(PG_FUNCTION_ARGS)
 	}
 
 	tsearch_readline_end(&trst);
+	pfree(filename);
 
 	d->len = cur;
 	qsort(d->syn, d->len, sizeof(Syn), compareSyn);
diff --git a/src/backend/tsearch/dict_thesaurus.c b/src/backend/tsearch/dict_thesaurus.c
index 1bebe36a6910..1e6bbde1ca7d 100644
--- a/src/backend/tsearch/dict_thesaurus.c
+++ b/src/backend/tsearch/dict_thesaurus.c
@@ -167,17 +167,17 @@ addWrd(DictThesaurus *d, char *b, char *e, uint32 idsubst, uint16 nwrd, uint16 p
 static void
 thesaurusRead(const char *filename, DictThesaurus *d)
 {
+	char	   *real_filename = get_tsearch_config_filename(filename, "ths");
 	tsearch_readline_state trst;
 	uint32		idsubst = 0;
 	bool		useasis = false;
 	char	   *line;
 
-	filename = get_tsearch_config_filename(filename, "ths");
-	if (!tsearch_readline_begin(&trst, filename))
+	if (!tsearch_readline_begin(&trst, real_filename))
 		ereport(ERROR,
 				(errcode(ERRCODE_CONFIG_FILE_ERROR),
 				 errmsg("could not open thesaurus file \"%s\": %m",
-						filename)));
+						real_filename)));
 
 	while ((line = tsearch_readline(&trst)) != NULL)
 	{
@@ -297,6 +297,7 @@ thesaurusRead(const char *filename, DictThesaurus *d)
 	d->nsubst = idsubst;
 
 	tsearch_readline_end(&trst);
+	pfree(real_filename);
 }
 
 static TheLexeme *
diff --git a/src/backend/utils/cache/ts_cache.c b/src/backend/utils/cache/ts_cache.c
index 18cccd778fd8..e8ae53238d07 100644
--- a/src/backend/utils/cache/ts_cache.c
+++ b/src/backend/utils/cache/ts_cache.c
@@ -321,7 +321,9 @@ lookup_ts_dictionary_cache(Oid dictId)
 
 			/*
 			 * Init method runs in dictionary's private memory context, and we
-			 * make sure the options are stored there too
+			 * make sure the options are stored there too.  This typically
+			 * results in a small amount of memory leakage, but it's not worth
+			 * complicating the API for tmplinit functions to avoid it.
 			 */
 			oldcontext = MemoryContextSwitchTo(entry->dictCtx);
 
diff --git a/src/tools/valgrind.supp b/src/tools/valgrind.supp
index 87cf3866de00..d94aa59dcb4b 100644
--- a/src/tools/valgrind.supp
+++ b/src/tools/valgrind.supp
@@ -201,3 +201,15 @@
    ...
    fun:cached_function_compile
 }
+
+# Suppress complaints about stuff leaked during TS dictionary loading.
+# Not very much is typically lost there, and preventing it would
+# require a risky API change for TS tmplinit functions.
+{
+   hide_ts_dictionary_leaks
+   Memcheck:Leak
+   match-leak-kinds: definite,possible,indirect
+
+   ...
+   fun:lookup_ts_dictionary_cache
+}

From 07fd60f3e85b85251c546c113827bd43aec1b6cf Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:15:59 -0400
Subject: [PATCH 12/18] Fix leaks in load_domaintype_info().

load_domaintype_info() intentionally leaks some intermediate cruft
into the long-lived DomainConstraintCache's memory context, reasoning
that the amount of leakage will typically not be much so it's not
worth doing a copyObject() of the final tree to avoid that.  But
Valgrind knows nothing of engineering tradeoffs and complains anyway.

On the whole, the copyObject doesn't cost that much and this is
surely not a performance-critical code path, so let's do it the
clean way.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/utils/cache/typcache.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index f9aec38a11fb..6a347698edff 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -1171,9 +1171,6 @@ load_domaintype_info(TypeCacheEntry *typentry)
 				elog(ERROR, "domain \"%s\" constraint \"%s\" has NULL conbin",
 					 NameStr(typTup->typname), NameStr(c->conname));
 
-			/* Convert conbin to C string in caller context */
-			constring = TextDatumGetCString(val);
-
 			/* Create the DomainConstraintCache object and context if needed */
 			if (dcc == NULL)
 			{
@@ -1189,9 +1186,8 @@ load_domaintype_info(TypeCacheEntry *typentry)
 				dcc->dccRefCount = 0;
 			}
 
-			/* Create node trees in DomainConstraintCache's context */
-			oldcxt = MemoryContextSwitchTo(dcc->dccContext);
-
+			/* Convert conbin to a node tree, still in caller's context */
+			constring = TextDatumGetCString(val);
 			check_expr = (Expr *) stringToNode(constring);
 
 			/*
@@ -1206,10 +1202,13 @@ load_domaintype_info(TypeCacheEntry *typentry)
 			 */
 			check_expr = expression_planner(check_expr);
 
+			/* Create only the minimally needed stuff in dccContext */
+			oldcxt = MemoryContextSwitchTo(dcc->dccContext);
+
 			r = makeNode(DomainConstraintState);
 			r->constrainttype = DOM_CONSTRAINT_CHECK;
 			r->name = pstrdup(NameStr(c->conname));
-			r->check_expr = check_expr;
+			r->check_expr = copyObject(check_expr);
 			r->check_exprstate = NULL;
 
 			MemoryContextSwitchTo(oldcxt);

From 1f77836f9b275208032ede80f3f4edcf151ce29b Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:16:58 -0400
Subject: [PATCH 13/18] Silence complaints about leaks in
 PlanCacheComputeResultDesc.

CompleteCachedPlan intentionally doesn't worry about small
leaks from PlanCacheComputeResultDesc.  However, Valgrind
knows nothing of engineering tradeoffs and complains anyway.
Silence it by doing things the hard way if USE_VALGRIND.

I don't really love this patch, because it makes the handling
of plansource->resultDesc different from the handling of query
dependencies and search_path just above, which likewise are willing
to accept small leaks into the cached plan's context.  However,
those cases aren't provoking Valgrind complaints.  (Perhaps in a
CLOBBER_CACHE_ALWAYS build, they would?)  For the moment, this
makes the src/pl/plpgsql tests leak-free according to Valgrind.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/utils/cache/plancache.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 89a1c79e984d..7c4207e6b70c 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -463,8 +463,7 @@ CompleteCachedPlan(CachedPlanSource *plansource,
 
 	/*
 	 * Save the final parameter types (or other parameter specification data)
-	 * into the source_context, as well as our other parameters.  Also save
-	 * the result tuple descriptor.
+	 * into the source_context, as well as our other parameters.
 	 */
 	MemoryContextSwitchTo(source_context);
 
@@ -480,9 +479,25 @@ CompleteCachedPlan(CachedPlanSource *plansource,
 	plansource->parserSetupArg = parserSetupArg;
 	plansource->cursor_options = cursor_options;
 	plansource->fixed_result = fixed_result;
-	plansource->resultDesc = PlanCacheComputeResultDesc(querytree_list);
 
+	/*
+	 * Also save the result tuple descriptor.  PlanCacheComputeResultDesc may
+	 * leak some cruft; normally we just accept that to save a copy step, but
+	 * in USE_VALGRIND mode be tidy by running it in the caller's context.
+	 */
+#ifdef USE_VALGRIND
+	MemoryContextSwitchTo(oldcxt);
+	plansource->resultDesc = PlanCacheComputeResultDesc(querytree_list);
+	if (plansource->resultDesc)
+	{
+		MemoryContextSwitchTo(source_context);
+		plansource->resultDesc = CreateTupleDescCopy(plansource->resultDesc);
+		MemoryContextSwitchTo(oldcxt);
+	}
+#else
+	plansource->resultDesc = PlanCacheComputeResultDesc(querytree_list);
 	MemoryContextSwitchTo(oldcxt);
+#endif
 
 	plansource->is_complete = true;
 	plansource->is_valid = true;

From effdee770f78e0929df7d85d5484395db4f7f052 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:17:23 -0400
Subject: [PATCH 14/18] Fix leaks when replacing or deleting a GUC placeholder
 variable.

MarkGUCPrefixReserved didn't bother to clean up removed placeholder
GUCs at all, which shows up as a leak in one regression test.
It seems appropriate for it to do as much cleanup as
define_custom_variable does when replacing placeholders, so factor
that code out into a helper function.

Testing showed that define_custom_variable's logic was one brick shy
of a load too: it forgot to free the separate allocation for the
placeholder's name.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/utils/misc/guc.c | 38 ++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 667df448732f..8dda85ffdb1a 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -249,6 +249,7 @@ static void reapply_stacked_values(struct config_generic *variable,
 								   const char *curvalue,
 								   GucContext curscontext, GucSource cursource,
 								   Oid cursrole);
+static void free_placeholder(struct config_string *pHolder);
 static bool validate_option_array_item(const char *name, const char *value,
 									   bool skipIfNoPermissions);
 static void write_auto_conf_file(int fd, const char *filename, ConfigVariable *head);
@@ -5018,16 +5019,8 @@ define_custom_variable(struct config_generic *variable)
 		set_config_sourcefile(name, pHolder->gen.sourcefile,
 							  pHolder->gen.sourceline);
 
-	/*
-	 * Free up as much as we conveniently can of the placeholder structure.
-	 * (This neglects any stack items, so it's possible for some memory to be
-	 * leaked.  Since this can only happen once per session per variable, it
-	 * doesn't seem worth spending much code on.)
-	 */
-	set_string_field(pHolder, pHolder->variable, NULL);
-	set_string_field(pHolder, &pHolder->reset_val, NULL);
-
-	guc_free(pHolder);
+	/* Now we can free the no-longer-referenced placeholder variable */
+	free_placeholder(pHolder);
 }
 
 /*
@@ -5126,6 +5119,25 @@ reapply_stacked_values(struct config_generic *variable,
 	}
 }
 
+/*
+ * Free up a no-longer-referenced placeholder GUC variable.
+ *
+ * This neglects any stack items, so it's possible for some memory to be
+ * leaked.  Since this can only happen once per session per variable, it
+ * doesn't seem worth spending much code on.
+ */
+static void
+free_placeholder(struct config_string *pHolder)
+{
+	/* Placeholders are always STRING type, so free their values */
+	Assert(pHolder->gen.vartype == PGC_STRING);
+	set_string_field(pHolder, pHolder->variable, NULL);
+	set_string_field(pHolder, &pHolder->reset_val, NULL);
+
+	guc_free(unconstify(char *, pHolder->gen.name));
+	guc_free(pHolder);
+}
+
 /*
  * Functions for extensions to call to define their custom GUC variables.
  */
@@ -5286,9 +5298,7 @@ MarkGUCPrefixReserved(const char *className)
 
 	/*
 	 * Check for existing placeholders.  We must actually remove invalid
-	 * placeholders, else future parallel worker startups will fail.  (We
-	 * don't bother trying to free associated memory, since this shouldn't
-	 * happen often.)
+	 * placeholders, else future parallel worker startups will fail.
 	 */
 	hash_seq_init(&status, guc_hashtab);
 	while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
@@ -5312,6 +5322,8 @@ MarkGUCPrefixReserved(const char *className)
 						NULL);
 			/* Remove it from any lists it's in, too */
 			RemoveGUCFromLists(var);
+			/* And free it */
+			free_placeholder((struct config_string *) var);
 		}
 	}
 

From cdc3473a60b66f8258df8029608320aaf5f6ba9d Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:17:47 -0400
Subject: [PATCH 15/18] Fix leak in logicalrep_worker_detach().

This runs in a long-lived context, so Valgrind complains about it.
It's not clear to me that very much memory can actually be lost.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/replication/logical/launcher.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 10677da56b2b..557e8b42b583 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -766,6 +766,8 @@ logicalrep_worker_detach(void)
 		}
 
 		LWLockRelease(LogicalRepWorkerLock);
+
+		list_free(workers);
 	}
 
 	/* Block concurrent access. */

From d3d44c375d5f7078603f6e9f55f7936fbe2365cb Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:18:16 -0400
Subject: [PATCH 16/18] Fix leak in evtcache.c's DecodeTextArrayToBitmapset().

If the presented array is toasted, this neglected to free the
detoasted copy, which was then leaked into EventTriggerCacheContext.

I'm a bit distressed by the amount of code that BuildEventTriggerCache
is willing to run while switched into a long-lived cache context.
Although the detoasted array is the only leak that Valgrind reports,
let's tighten things up while we're here.  (DecodeTextArrayToBitmapset
is still run in the cache context, so doing that doesn't remove the
need for the detoast fix.  But it reduces the surface area for other
leaks.)

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/utils/cache/evtcache.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/backend/utils/cache/evtcache.c b/src/backend/utils/cache/evtcache.c
index ce596bf56385..b9d5a5998be5 100644
--- a/src/backend/utils/cache/evtcache.c
+++ b/src/backend/utils/cache/evtcache.c
@@ -78,7 +78,6 @@ BuildEventTriggerCache(void)
 {
 	HASHCTL		ctl;
 	HTAB	   *cache;
-	MemoryContext oldcontext;
 	Relation	rel;
 	Relation	irel;
 	SysScanDesc scan;
@@ -110,9 +109,6 @@ BuildEventTriggerCache(void)
 									  (Datum) 0);
 	}
 
-	/* Switch to correct memory context. */
-	oldcontext = MemoryContextSwitchTo(EventTriggerCacheContext);
-
 	/* Prevent the memory context from being nuked while we're rebuilding. */
 	EventTriggerCacheState = ETCS_REBUILD_STARTED;
 
@@ -145,6 +141,7 @@ BuildEventTriggerCache(void)
 		bool		evttags_isnull;
 		EventTriggerCacheEntry *entry;
 		bool		found;
+		MemoryContext oldcontext;
 
 		/* Get next tuple. */
 		tup = systable_getnext_ordered(scan, ForwardScanDirection);
@@ -171,6 +168,9 @@ BuildEventTriggerCache(void)
 		else
 			continue;
 
+		/* Switch to correct memory context. */
+		oldcontext = MemoryContextSwitchTo(EventTriggerCacheContext);
+
 		/* Allocate new cache item. */
 		item = palloc0(sizeof(EventTriggerCacheItem));
 		item->fnoid = form->evtfoid;
@@ -188,6 +188,9 @@ BuildEventTriggerCache(void)
 			entry->triggerlist = lappend(entry->triggerlist, item);
 		else
 			entry->triggerlist = list_make1(item);
+
+		/* Restore previous memory context. */
+		MemoryContextSwitchTo(oldcontext);
 	}
 
 	/* Done with pg_event_trigger scan. */
@@ -195,9 +198,6 @@ BuildEventTriggerCache(void)
 	index_close(irel, AccessShareLock);
 	relation_close(rel, AccessShareLock);
 
-	/* Restore previous memory context. */
-	MemoryContextSwitchTo(oldcontext);
-
 	/* Install new cache. */
 	EventTriggerCache = cache;
 
@@ -240,6 +240,8 @@ DecodeTextArrayToBitmapset(Datum array)
 	}
 
 	pfree(elems);
+	if ((Pointer) arr != DatumGetPointer(array))
+		pfree(arr);
 
 	return bms;
 }

From 71511b385c53c74a14c42fcb73f52a15dc7bc2ac Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:18:39 -0400
Subject: [PATCH 17/18] Silence complaints about leaks in postmaster.

Valgrind complains about the postmaster's socket-files and
lock-files lists being leaked, which we can silence by just
not nulling out the static pointers to them.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/libpq/pqcomm.c        | 1 -
 src/backend/utils/init/miscinit.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index e5171467de18..25f739a6a17d 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -858,7 +858,6 @@ RemoveSocketFiles(void)
 		(void) unlink(sock_path);
 	}
 	/* Since we're about to exit, no need to reclaim storage */
-	sock_paths = NIL;
 }
 
 
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 43b4dbccc3de..65d8cbfaed58 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -1183,7 +1183,6 @@ UnlinkLockFiles(int status, Datum arg)
 		/* Should we complain if the unlink fails? */
 	}
 	/* Since we're about to exit, no need to reclaim storage */
-	lock_files = NIL;
 
 	/*
 	 * Lock file removal should always be the last externally visible action

From 800799727d094d32be612f3caa7d96876b9a3c1b Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 23 May 2025 15:19:04 -0400
Subject: [PATCH 18/18] Fix leaks in startup process.

These leaks are of absolutely no real-world consequence,
since they are small one-time leaks in a one-time process.
But this is the last step to get to zero reported leaks
in a run of the core regression tests, so let's do it.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
 src/backend/access/transam/xlog.c         | 33 +++++++++++++++--------
 src/backend/access/transam/xlogrecovery.c |  1 +
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 1914859b2eed..867f5f6a2e41 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -703,7 +703,7 @@ static void InitControlFile(uint64 sysidentifier, uint32 data_checksum_version);
 static void WriteControlFile(void);
 static void ReadControlFile(void);
 static void UpdateControlFile(void);
-static char *str_time(pg_time_t tnow);
+static char *str_time(pg_time_t tnow, char *buf, size_t bufsize);
 
 static int	get_sync_bit(int method);
 
@@ -5381,11 +5381,9 @@ BootStrapXLOG(uint32 data_checksum_version)
 }
 
 static char *
-str_time(pg_time_t tnow)
+str_time(pg_time_t tnow, char *buf, size_t bufsize)
 {
-	char	   *buf = palloc(128);
-
-	pg_strftime(buf, 128,
+	pg_strftime(buf, bufsize,
 				"%Y-%m-%d %H:%M:%S %Z",
 				pg_localtime(&tnow, log_timezone));
 
@@ -5628,6 +5626,7 @@ StartupXLOG(void)
 	XLogRecPtr	missingContrecPtr;
 	TransactionId oldestActiveXID;
 	bool		promoted = false;
+	char		timebuf[128];
 
 	/*
 	 * We should have an aux process resource owner to use, and we should not
@@ -5656,25 +5655,29 @@ StartupXLOG(void)
 			 */
 			ereport(IsPostmasterEnvironment ? LOG : NOTICE,
 					(errmsg("database system was shut down at %s",
-							str_time(ControlFile->time))));
+							str_time(ControlFile->time,
+									 timebuf, sizeof(timebuf)))));
 			break;
 
 		case DB_SHUTDOWNED_IN_RECOVERY:
 			ereport(LOG,
 					(errmsg("database system was shut down in recovery at %s",
-							str_time(ControlFile->time))));
+							str_time(ControlFile->time,
+									 timebuf, sizeof(timebuf)))));
 			break;
 
 		case DB_SHUTDOWNING:
 			ereport(LOG,
 					(errmsg("database system shutdown was interrupted; last known up at %s",
-							str_time(ControlFile->time))));
+							str_time(ControlFile->time,
+									 timebuf, sizeof(timebuf)))));
 			break;
 
 		case DB_IN_CRASH_RECOVERY:
 			ereport(LOG,
 					(errmsg("database system was interrupted while in recovery at %s",
-							str_time(ControlFile->time)),
+							str_time(ControlFile->time,
+									 timebuf, sizeof(timebuf))),
 					 errhint("This probably means that some data is corrupted and"
 							 " you will have to use the last backup for recovery.")));
 			break;
@@ -5682,7 +5685,8 @@ StartupXLOG(void)
 		case DB_IN_ARCHIVE_RECOVERY:
 			ereport(LOG,
 					(errmsg("database system was interrupted while in recovery at log time %s",
-							str_time(ControlFile->checkPointCopy.time)),
+							str_time(ControlFile->checkPointCopy.time,
+									 timebuf, sizeof(timebuf))),
 					 errhint("If this has occurred more than once some data might be corrupted"
 							 " and you might need to choose an earlier recovery target.")));
 			break;
@@ -5690,7 +5694,8 @@ StartupXLOG(void)
 		case DB_IN_PRODUCTION:
 			ereport(LOG,
 					(errmsg("database system was interrupted; last known up at %s",
-							str_time(ControlFile->time))));
+							str_time(ControlFile->time,
+									 timebuf, sizeof(timebuf)))));
 			break;
 
 		default:
@@ -6336,6 +6341,12 @@ StartupXLOG(void)
 	 */
 	CompleteCommitTsInitialization();
 
+	/* Clean up EndOfWalRecoveryInfo data to appease Valgrind leak checking */
+	if (endOfRecoveryInfo->lastPage)
+		pfree(endOfRecoveryInfo->lastPage);
+	pfree(endOfRecoveryInfo->recoveryStopReason);
+	pfree(endOfRecoveryInfo);
+
 	/*
 	 * All done with end-of-recovery actions.
 	 *
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 6ce979f2d8bc..498fc0d763e0 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -1626,6 +1626,7 @@ ShutdownWalRecovery(void)
 		close(readFile);
 		readFile = -1;
 	}
+	pfree(xlogreader->private_data);
 	XLogReaderFree(xlogreader);
 	XLogPrefetcherFree(xlogprefetcher);