Detect pfree or repalloc of a previously-freed memory chunk.

Before the major rewrite in commit c6e0fe1f2, AllocSetFree() would
typically crash when asked to free an already-free chunk.  That was
an ugly but serviceable way of detecting coding errors that led to
double pfrees.  But since that rewrite, double pfrees went through
just fine, because the "hdrmask" of a freed chunk isn't changed at all
when putting it on the freelist.  We'd end with a corrupt freelist
that circularly links back to the doubly-freed chunk, which would
usually result in trouble later, far removed from the actual bug.

This situation is no good at all for debugging purposes.  Fortunately,
we can fix it at low cost in MEMORY_CONTEXT_CHECKING builds by making
AllocSetFree() check for chunk->requested_size == InvalidAllocSize,
relying on the pre-existing code that sets it that way just below.

I investigated the alternative of changing a freed chunk's methodid
field, which would allow detection in non-MEMORY_CONTEXT_CHECKING
builds too.  But that adds measurable overhead.  Seeing that we didn't
notice this oversight for more than three years, it's hard to argue
that detecting this type of bug is worth any extra overhead in
production builds.

Likewise fix AllocSetRealloc() to detect repalloc() on a freed chunk,
and apply similar changes in generation.c and slab.c.  (generation.c
would hit an Assert failure anyway, but it seems best to make it act
like aset.c.)  bump.c doesn't need changes since it doesn't support
pfree in the first place.  Ideally alignedalloc.c would receive
similar changes, but in debugging builds it's impossible to reach
AlignedAllocFree() or AlignedAllocRealloc() on a pfreed chunk, because
the underlying context's pfree would have wiped the chunk header of
the aligned chunk.  But that means we should get an error of some
sort, so let's be content with that.

Per investigation of why the test case for bug #19438 didn't appear to
fail in v16 and up, even though the underlying bug was still present.
(This doesn't fix the underlying double-free bug, just cause it to
get detected.)

Bug: #19438
Reported-by: Dmitriy Kuzmin <kuzmin.db4@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/19438-9d37b179c56d43aa@postgresql.org
Backpatch-through: 16

src/backend/utils/mmgr/aset.c

@@ -1174,7 +1174,26 @@ AllocSetFree(void *pointer)
 		Assert(FreeListIdxIsValid(fidx));
 		link = GetFreeListLink(chunk);
 
+		/*
+		 * It might seem odd that we use elevel ERROR for double-pfree but
+		 * only WARNING for write-past-chunk-end.  But the two conditions are
+		 * not very comparable.  In the double-pfree case we can prevent
+		 * corruption before it happens; while if we let it go through, the
+		 * result would be a corrupted freelist that allows this chunk to get
+		 * re-allocated twice.  Thus the original bug could cascade into
+		 * hard-to-understand misbehavior that might manifest far away from
+		 * the actual source of the problem.  On the other hand, a write past
+		 * chunk end can be relatively benign if just a few bytes too many
+		 * were written: often, only padding or unused space gets affected.
+		 * Moreover, whatever damage was done is already done, and we're just
+		 * reporting after the fact with no ability to clean it up.  So just
+		 * warn, like AllocSetCheck would do if the chunk didn't get freed.
+		 */
 #ifdef MEMORY_CONTEXT_CHECKING
+		/* Test for previously-freed chunk */
+		if (unlikely(chunk->requested_size == InvalidAllocSize))
+			elog(ERROR, "detected double pfree in %s %p",
+				 set->header.name, chunk);
 		/* Test for someone scribbling on unused space in chunk */
 		if (chunk->requested_size < GetChunkSizeFromFreeListIdx(fidx))
 			if (!sentinel_ok(pointer, chunk->requested_size))
@@ -1373,6 +1392,11 @@ AllocSetRealloc(void *pointer, Size size, int flags)
 	oldchksize = GetChunkSizeFromFreeListIdx(fidx);
 
 #ifdef MEMORY_CONTEXT_CHECKING
+	/* See comments in AllocSetFree about uses of ERROR and WARNING here */
+	/* Test for previously-freed chunk */
+	if (unlikely(chunk->requested_size == InvalidAllocSize))
+		elog(ERROR, "detected realloc of freed chunk in %s %p",
+			 set->header.name, chunk);
 	/* Test for someone scribbling on unused space in chunk */
 	if (chunk->requested_size < oldchksize)
 		if (!sentinel_ok(pointer, chunk->requested_size))

src/backend/utils/mmgr/generation.c

@@ -762,6 +762,11 @@ GenerationFree(void *pointer)
 	}
 
 #ifdef MEMORY_CONTEXT_CHECKING
+	/* See comments in AllocSetFree about uses of ERROR and WARNING here */
+	/* Test for previously-freed chunk */
+	if (unlikely(chunk->requested_size == InvalidAllocSize))
+		elog(ERROR, "detected double pfree in %s %p",
+			 ((MemoryContext) block->context)->name, chunk);
 	/* Test for someone scribbling on unused space in chunk */
 	Assert(chunk->requested_size < chunksize);
 	if (!sentinel_ok(pointer, chunk->requested_size))
@@ -867,6 +872,11 @@ GenerationRealloc(void *pointer, Size size, int flags)
 	set = block->context;
 
 #ifdef MEMORY_CONTEXT_CHECKING
+	/* See comments in AllocSetFree about uses of ERROR and WARNING here */
+	/* Test for previously-freed chunk */
+	if (unlikely(chunk->requested_size == InvalidAllocSize))
+		elog(ERROR, "detected realloc of freed chunk in %s %p",
+			 ((MemoryContext) set)->name, chunk);
 	/* Test for someone scribbling on unused space in chunk */
 	Assert(chunk->requested_size < oldsize);
 	if (!sentinel_ok(pointer, chunk->requested_size))

src/backend/utils/mmgr/slab.c

@@ -539,6 +539,7 @@ SlabAllocSetupNewChunk(MemoryContext context, SlabBlock *block,
 	MemoryChunkSetHdrMask(chunk, block, MAXALIGN(slab->chunkSize), MCTX_SLAB_ID);
 
 #ifdef MEMORY_CONTEXT_CHECKING
+	chunk->requested_size = size;
 	/* slab mark to catch clobber of "unused" space */
 	Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
 	set_sentinel(MemoryChunkGetPointer(chunk), size);
@@ -748,11 +749,18 @@ SlabFree(void *pointer)
 	slab = block->slab;
 
 #ifdef MEMORY_CONTEXT_CHECKING
+	/* See comments in AllocSetFree about uses of ERROR and WARNING here */
+	/* Test for previously-freed chunk */
+	if (unlikely(chunk->requested_size == InvalidAllocSize))
+		elog(ERROR, "detected double pfree in %s %p",
+			 slab->header.name, chunk);
 	/* Test for someone scribbling on unused space in chunk */
 	Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
 	if (!sentinel_ok(pointer, slab->chunkSize))
 		elog(WARNING, "detected write past chunk end in %s %p",
 			 slab->header.name, chunk);
+	/* Reset requested_size to InvalidAllocSize in free chunks */
+	chunk->requested_size = InvalidAllocSize;
 #endif
 
 	/* push this chunk onto the head of the block's free list */