Adjust Valgrind macro usage to protect chunk headers

Prior to this commit we only ever protected MemoryChunk's requested_size
field with Valgrind NOACCESS.  This means that if the hdrmask field is
ever accessed accidentally then we're not going to get any warnings from
Valgrind about it.  Valgrind would have warned us about the problem fixed
in 92957ed98 had we already been doing this.

Per suggestion from Tom Lane

Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/[email protected]
Discussion: https://postgr.es/m/CAApHDvr=FZNGbj252Z6M9BSFKoq6BMxgkQ2yEAGUYoo7RquqZg@mail.gmail.com

1 files changed, 46 insertions, 5 deletions

diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index 33dca0f37c6..718dd2ba03c 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -630,6 +630,9 @@ SlabAlloc(MemoryContext context, Size size)
 	randomize_mem((char *) MemoryChunkGetPointer(chunk), size);
 #endif
 
+	/* Disallow access to the chunk header. */
+	VALGRIND_MAKE_MEM_NOACCESS(chunk, Slab_CHUNKHDRSZ);
+
 	return MemoryChunkGetPointer(chunk);
 }
 
@@ -641,11 +644,16 @@ void
 SlabFree(void *pointer)
 {
 	MemoryChunk *chunk = PointerGetMemoryChunk(pointer);
-	SlabBlock  *block = MemoryChunkGetBlock(chunk);
+	SlabBlock  *block;
 	SlabContext *slab;
 	int			curBlocklistIdx;
 	int			newBlocklistIdx;
 
+	/* Allow access to the chunk header. */
+	VALGRIND_MAKE_MEM_DEFINED(chunk, Slab_CHUNKHDRSZ);
+
+	block = MemoryChunkGetBlock(chunk);
+
 	/*
 	 * For speed reasons we just Assert that the referenced block is good.
 	 * Future field experience may show that this Assert had better become a
@@ -761,9 +769,17 @@ void *
 SlabRealloc(void *pointer, Size size)
 {
 	MemoryChunk *chunk = PointerGetMemoryChunk(pointer);
-	SlabBlock  *block = MemoryChunkGetBlock(chunk);
+	SlabBlock  *block;
 	SlabContext *slab;
 
+	/* Allow access to the chunk header. */
+	VALGRIND_MAKE_MEM_DEFINED(chunk, Slab_CHUNKHDRSZ);
+
+	block = MemoryChunkGetBlock(chunk);
+
+	/* Disallow access to the chunk header. */
+	VALGRIND_MAKE_MEM_NOACCESS(chunk, Slab_CHUNKHDRSZ);
+
 	/*
 	 * Try to verify that we have a sane block pointer: the block header
 	 * should reference a slab context.  (We use a test-and-elog, not just
@@ -790,9 +806,18 @@ MemoryContext
 SlabGetChunkContext(void *pointer)
 {
 	MemoryChunk *chunk = PointerGetMemoryChunk(pointer);
-	SlabBlock  *block = MemoryChunkGetBlock(chunk);
+	SlabBlock  *block;
+
+	/* Allow access to the chunk header. */
+	VALGRIND_MAKE_MEM_DEFINED(chunk, Slab_CHUNKHDRSZ);
+
+	block = MemoryChunkGetBlock(chunk);
+
+	/* Disallow access to the chunk header. */
+	VALGRIND_MAKE_MEM_NOACCESS(chunk, Slab_CHUNKHDRSZ);
 
 	Assert(SlabBlockIsValid(block));
+
 	return &block->slab->header;
 }
 
@@ -805,9 +830,17 @@ Size
 SlabGetChunkSpace(void *pointer)
 {
 	MemoryChunk *chunk = PointerGetMemoryChunk(pointer);
-	SlabBlock  *block = MemoryChunkGetBlock(chunk);
+	SlabBlock  *block;
 	SlabContext *slab;
 
+	/* Allow access to the chunk header. */
+	VALGRIND_MAKE_MEM_DEFINED(chunk, Slab_CHUNKHDRSZ);
+
+	block = MemoryChunkGetBlock(chunk);
+
+	/* Disallow access to the chunk header. */
+	VALGRIND_MAKE_MEM_NOACCESS(chunk, Slab_CHUNKHDRSZ);
+
 	Assert(SlabBlockIsValid(block));
 	slab = block->slab;
 
@@ -1017,7 +1050,15 @@ SlabCheck(MemoryContext context)
 				if (!slab->isChunkFree[j])
 				{
 					MemoryChunk *chunk = SlabBlockGetChunk(slab, block, j);
-					SlabBlock  *chunkblock = (SlabBlock *) MemoryChunkGetBlock(chunk);
+					SlabBlock  *chunkblock;
+
+					/* Allow access to the chunk header. */
+					VALGRIND_MAKE_MEM_DEFINED(chunk, Slab_CHUNKHDRSZ);
+
+					chunkblock = (SlabBlock *) MemoryChunkGetBlock(chunk);
+
+					/* Disallow access to the chunk header. */
+					VALGRIND_MAKE_MEM_NOACCESS(chunk, Slab_CHUNKHDRSZ);
 
 					/*
 					 * check the chunk's blockoffset correctly points back to