From c7e69e4bccfc4da97b0b999399732e3dc806b67e Mon Sep 17 00:00:00 2001 From: Andres Freund 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