pgsql: Improve our support for Valgrind's leak tracking. - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Improve our support for Valgrind's leak tracking.
Date
Msg-id E1uiO1p-000So1-2f@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Improve our support for Valgrind's leak tracking.

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

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/bb049a79d3447e97c0d4fa220600c423c4474bf9

Modified Files
--------------
src/backend/utils/mmgr/aset.c       | 71 +++++++++++++++++++++++++++++++++++--
src/backend/utils/mmgr/bump.c       | 31 +++++++++++++++-
src/backend/utils/mmgr/generation.c | 29 +++++++++++++++
src/backend/utils/mmgr/mcxt.c       | 23 ++++++++----
src/backend/utils/mmgr/slab.c       | 32 +++++++++++++++++
src/include/utils/memdebug.h        |  1 +
6 files changed, 177 insertions(+), 10 deletions(-)


pgsql-committers by date:

Previous
From: Fujii Masao
Date:
Subject: pgsql: Fix assertion failure in pgbench when handling multiple pipeline
Next
From: Tom Lane
Date:
Subject: pgsql: Take a little more care in set_backtrace().