From c7e69e4bccfc4da97b0b999399732e3dc806b67e Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 17 Mar 2021 10:46:39 -0700
Subject: [PATCH] Make memory contexts themselves more visible to valgrind.

---
 src/include/utils/memdebug.h        |  1 +
 src/backend/utils/mmgr/aset.c       | 22 ++++++++++++++++++++++
 src/backend/utils/mmgr/generation.c |  8 ++++++++
 src/backend/utils/mmgr/mcxt.c       |  5 -----
 src/backend/utils/mmgr/slab.c       |  8 ++++++++
 5 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/src/include/utils/memdebug.h b/src/include/utils/memdebug.h
index e88b4c6e8ef..5988bff8839 100644
--- a/src/include/utils/memdebug.h
+++ b/src/include/utils/memdebug.h
@@ -23,6 +23,7 @@
 #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_TRIM(context, ptr, size)			do {} while (0)
 #define VALGRIND_MAKE_MEM_DEFINED(addr, size)				do {} while (0)
 #define VALGRIND_MAKE_MEM_NOACCESS(addr, size)				do {} while (0)
 #define VALGRIND_MAKE_MEM_UNDEFINED(addr, size)				do {} while (0)
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index ec6c130d0fb..9793ddf4a3d 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -433,6 +433,12 @@ AllocSetContextCreateInternal(MemoryContext parent,
 		{
 			/* Remove entry from freelist */
 			set = freelist->first_free;
+
+			VALGRIND_CREATE_MEMPOOL(set, 0, false);
+			VALGRIND_MEMPOOL_ALLOC(set, set, sizeof(AllocSetContext));
+			/* the contents are still valid, but valgrind can't know that */
+			VALGRIND_MAKE_MEM_DEFINED(set, sizeof(AllocSetContext));
+
 			freelist->first_free = (AllocSet) set->header.nextchild;
 			freelist->num_free--;
 
@@ -477,6 +483,8 @@ AllocSetContextCreateInternal(MemoryContext parent,
 						   name)));
 	}
 
+	VALGRIND_CREATE_MEMPOOL(set, 0, false);
+	VALGRIND_MEMPOOL_ALLOC(set, set, sizeof(AllocSetContext));
 	/*
 	 * Avoid writing code that can fail between here and MemoryContextCreate;
 	 * we'd leak the header/initial block if we ereport in this stretch.
@@ -611,6 +619,8 @@ AllocSetReset(MemoryContext context)
 
 	Assert(context->mem_allocated == keepersize);
 
+	VALGRIND_MEMPOOL_TRIM(set, set, sizeof(AllocSetAlloc));
+
 	/* Reset block size allocation sequence, too */
 	set->nextBlockSize = set->initBlockSize;
 }
@@ -652,6 +662,16 @@ AllocSetDelete(MemoryContext context)
 		if (!context->isReset)
 			MemoryContextResetOnly(context);
 
+		VALGRIND_DESTROY_MEMPOOL(context);
+
+		/*
+		 * Still need to access the memory marked as invalid due to the
+		 * DESTROY. We leave the memory accessible, as otherwise valgrind will
+		 * complain about having leaked the memory underlying the cached
+		 * sets...
+		 */
+		VALGRIND_MAKE_MEM_DEFINED(set, sizeof(AllocSetContext));
+
 		/*
 		 * If the freelist is full, just discard what's already in it.  See
 		 * comments with context_freelists[].
@@ -699,6 +719,8 @@ AllocSetDelete(MemoryContext context)
 
 	Assert(context->mem_allocated == keepersize);
 
+	VALGRIND_DESTROY_MEMPOOL(context);
+
 	/* Finally, free the context header, including the keeper block */
 	free(set);
 }
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index 2b900347645..55d1f6a6526 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -235,6 +235,9 @@ GenerationContextCreate(MemoryContext parent,
 						   name)));
 	}
 
+	VALGRIND_CREATE_MEMPOOL(set, 0, false);
+	VALGRIND_MEMPOOL_ALLOC(set, set, sizeof(GenerationContext));
+
 	/*
 	 * Avoid writing code that can fail between here and MemoryContextCreate;
 	 * we'd leak the header if we ereport in this stretch.
@@ -293,6 +296,8 @@ GenerationReset(MemoryContext context)
 	set->block = NULL;
 
 	Assert(dlist_is_empty(&set->blocks));
+
+	VALGRIND_MEMPOOL_TRIM(set, set, sizeof(GenerationContext));
 }
 
 /*
@@ -304,6 +309,9 @@ GenerationDelete(MemoryContext context)
 {
 	/* Reset to release all the GenerationBlocks */
 	GenerationReset(context);
+
+	VALGRIND_DESTROY_MEMPOOL(context);
+
 	/* And free the context header */
 	free(context);
 }
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 84472b9158e..27f969872bc 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -174,8 +174,6 @@ MemoryContextResetOnly(MemoryContext context)
 
 		context->methods->reset(context);
 		context->isReset = true;
-		VALGRIND_DESTROY_MEMPOOL(context);
-		VALGRIND_CREATE_MEMPOOL(context, 0, false);
 	}
 }
 
@@ -245,7 +243,6 @@ MemoryContextDelete(MemoryContext context)
 
 	context->methods->delete_context(context);
 
-	VALGRIND_DESTROY_MEMPOOL(context);
 }
 
 /*
@@ -782,8 +779,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 9213be7c956..c51ede1e906 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -237,6 +237,9 @@ SlabContextCreate(MemoryContext parent,
 						   name)));
 	}
 
+	VALGRIND_CREATE_MEMPOOL(slab, 0, false);
+	VALGRIND_MEMPOOL_ALLOC(slab, slab, sizeof(SlabContext));
+
 	/*
 	 * Avoid writing code that can fail between here and MemoryContextCreate;
 	 * we'd leak the header if we ereport in this stretch.
@@ -315,6 +318,8 @@ SlabReset(MemoryContext context)
 
 	Assert(slab->nblocks == 0);
 	Assert(context->mem_allocated == 0);
+
+	VALGRIND_MEMPOOL_TRIM(slab, slab, sizeof(SlabContext));
 }
 
 /*
@@ -326,6 +331,9 @@ SlabDelete(MemoryContext context)
 {
 	/* Reset to release all the SlabBlocks */
 	SlabReset(context);
+
+	VALGRIND_DESTROY_MEMPOOL(context);
+
 	/* And free the context header */
 	free(context);
 }
-- 
2.29.2.540.g3cf59784d4

