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
#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 */
* 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;
#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;
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;
}
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);
}
#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);
}
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;
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;
#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
* realloc() to make the containing block bigger, or smaller, with
* minimum space wastage.
*/
+ AllocBlock newblock;
Size chksize;
Size blksize;
Size oldblksize;
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;
/* 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;
#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
* 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 */
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;
{
/* 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);
}
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 */
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 */
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);
}
#define Generation_BLOCKHDRSZ MAXALIGN(sizeof(GenerationBlock))
#define Generation_CHUNKHDRSZ sizeof(MemoryChunk)
+#define FIRST_BLOCKHDRSZ (MAXALIGN(sizeof(GenerationContext)) + \
+ Generation_BLOCKHDRSZ)
#define Generation_CHUNK_FRACTION 8
* 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 */
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);
{
/* 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);
}
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 */
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 */
wipe_mem(block, block->blksize);
#endif
+ /* As in aset.c, free block-header vchunks explicitly */
+ VALGRIND_MEMPOOL_FREE(set, block);
+
free(block);
}
* 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
context->methods->reset(context);
context->isReset = true;
- VALGRIND_DESTROY_MEMPOOL(context);
- VALGRIND_CREATE_MEMPOOL(context, 0, false);
}
}
context->ident = NULL;
context->methods->delete_context(context);
-
- VALGRIND_DESTROY_MEMPOOL(context);
}
/*
node->nextchild = NULL;
node->allowInCritSection = false;
}
-
- VALGRIND_CREATE_MEMPOOL(node, 0, false);
}
/*
* 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;
#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;
}
#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);
{
/* 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);
}
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;
#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;
}
#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
projects / thirdparty / postgresql.git / commitdiff
