From 46fb9c29e2990ba470bb741ff6dd60f2ae218e64 Mon Sep 17 00:00:00 2001
From: Bruce Momjian <bruce@momjian.us>
Date: Tue, 11 Jul 2000 14:30:37 +0000
Subject: [PATCH] Here is the patch with memory leak checker. This checker
allow detect in-chunk leaks, overwrite-next-chunk leaks and overwrite
block-freeptr leaks.
A in-chunk leak --- if something overwrite space after wanted (via palloc()
size, but it is still inside chunk. For example
x = palloc(12); /* create 16b chunk */
memset(x, '#', 13);
this leak is in the current source total invisible, because chunk is 16b and
leak is in the "align space".
For this feature I add data_size to StandardChunk, and all memory which go
from AllocSetAlloc() is marked as 0x7F.
The MemoryContextCheck() is compiled '#ifdef USE_ASSERT_CHECKING'.
I add this checking to 'tcop/postgres.c' and is active after each backend
query, but it is probably not sufficient, because some MemoryContext exist
only during memory processing --- will good if someone who known where
it is needful (Tom:-) add it for others contexts;
A problem in the current source is that we have still some malloc()
allocation that is not needful and this allocation is total invisible for
all context routines. For example Dllist in backend (pretty dirty it is in
catcache where values in Dllist are palloc-ed, but list is malloc-ed).
--- and BTW. this Dllist design stand in the way for query cache :-)
Tom, if you agree I start replace some mallocs.
BTW. --- Tom, have you idea for across transaction presistent allocation for
SQL functions? (like regex - now it is via malloc)
I almost forget. I add one if() to AllocSetAlloc(), for 'size' that are
greater than ALLOC_BIGCHUNK_LIMIT is not needful check AllocSetFreeIndex(),
because 'fidx' is always 'ALLOCSET_NUM_FREELISTS - 1'. It a little brisk up
allocation for very large chunks. Right?
Karel
---
src/backend/tcop/postgres.c | 11 +-
src/backend/utils/mmgr/aset.c | 266 ++++++++++++++++++++++++++++++----
src/backend/utils/mmgr/mcxt.c | 23 ++-
src/include/config.h.in | 8 +-
src/include/nodes/memnodes.h | 5 +-
src/include/utils/memutils.h | 6 +-
6 files changed, 284 insertions(+), 35 deletions(-)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 575e18351ad..3c672d8f4b2 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.167 2000/07/08 03:04:15 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.168 2000/07/11 14:30:27 momjian Exp $
*
* NOTES
* this is the "main" module of the postgres backend and
@@ -1415,7 +1415,7 @@ PostgresMain(int argc, char *argv[], int real_argc, char *real_argv[])
if (!IsUnderPostmaster)
{
puts("\nPOSTGRES backend interactive interface ");
- puts("$Revision: 1.167 $ $Date: 2000/07/08 03:04:15 $\n");
+ puts("$Revision: 1.168 $ $Date: 2000/07/11 14:30:27 $\n");
}
/*
@@ -1653,6 +1653,13 @@ PostgresMain(int argc, char *argv[], int real_argc, char *real_argv[])
if (IsUnderPostmaster)
NullCommand(Remote);
}
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ /*
+ * Check all memory after each backend loop
+ */
+ MemoryContextCheck(TopMemoryContext);
+#endif
} /* infinite for-loop */
proc_exit(0); /* shouldn't get here... */
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index e0244ade783..bf212ff7bb5 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -11,7 +11,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/utils/mmgr/aset.c,v 1.28 2000/06/28 03:32:50 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/utils/mmgr/aset.c,v 1.29 2000/07/11 14:30:28 momjian Exp $
*
* NOTE:
* This is a new (Feb. 05, 1999) implementation of the allocation set
@@ -38,6 +38,9 @@
#include "utils/memutils.h"
+/* Define this to detail debug alloc information
+ */
+/*#define HAVE_ALLOCINFO 1*/
/*
* AllocSetContext is defined in nodes/memnodes.h.
@@ -79,9 +82,12 @@ typedef struct AllocBlockData
typedef struct AllocChunkData
{
/* aset is the owning aset if allocated, or the freelist link if free */
- void *aset;
+ void *aset;
/* size is always the size of the usable space in the chunk */
- Size size;
+ Size size;
+#ifdef MEMORY_CONTEXT_CHECKING
+ Size data_size;
+#endif
} AllocChunkData;
/*
@@ -161,6 +167,11 @@ static void *AllocSetRealloc(MemoryContext context, void *pointer, Size size);
static void AllocSetInit(MemoryContext context);
static void AllocSetReset(MemoryContext context);
static void AllocSetDelete(MemoryContext context);
+
+#ifdef MEMORY_CONTEXT_CHECKING
+static void AllocSetCheck(MemoryContext context);
+#endif
+
static void AllocSetStats(MemoryContext context);
/*
@@ -173,10 +184,29 @@ static MemoryContextMethods AllocSetMethods = {
AllocSetInit,
AllocSetReset,
AllocSetDelete,
+#ifdef MEMORY_CONTEXT_CHECKING
+ AllocSetCheck,
+#endif
AllocSetStats
};
+/* ----------
+ * Debug macros
+ * ----------
+ */
+#ifdef HAVE_ALLOCINFO
+#define AllocFreeInfo(_cxt, _chunk) \
+ fprintf(stderr, "AllocFree: %s: %p, %d\n", \
+ (_cxt)->header.name, (_chunk), (_chunk)->size)
+#define AllocAllocInfo(_cxt, _chunk) \
+ fprintf(stderr, "AllocAlloc: %s: %p, %d\n", \
+ (_cxt)->header.name, (_chunk), (_chunk)->size)
+#else
+#define AllocFreeInfo(_cxt, _chunk)
+#define AllocAllocInfo(_cxt, _chunk)
+#endif
+
/* ----------
* AllocSetFreeIndex -
*
@@ -263,6 +293,11 @@ AllocSetContextCreate(MemoryContext parent,
context->blocks = block;
/* Mark block as not to be released at reset time */
context->keeper = block;
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ /* mark memory for memory leak searching */
+ memset(block->freeptr, 0x7F, blksize - ALLOC_BLOCKHDRSZ);
+#endif
}
return (MemoryContext) context;
@@ -386,38 +421,53 @@ AllocSetAlloc(MemoryContext context, Size size)
AllocBlock block;
AllocChunk chunk;
AllocChunk priorfree = NULL;
- int fidx;
+ int fidx;
Size chunk_size;
Size blksize;
AssertArg(AllocSetIsValid(set));
/*
- * Lookup in the corresponding free list if there is a free chunk we
- * could reuse
+ * Small size can be in free list
*/
- fidx = AllocSetFreeIndex(size);
- for (chunk = set->freelist[fidx]; chunk; chunk = (AllocChunk) chunk->aset)
+ if (size < ALLOC_BIGCHUNK_LIMIT)
{
- if (chunk->size >= size)
- break;
- priorfree = chunk;
- }
+ /*
+ * Lookup in the corresponding free list if there is a free chunk we
+ * could reuse
+ */
+ fidx = AllocSetFreeIndex(size);
+ for (chunk = set->freelist[fidx]; chunk; chunk = (AllocChunk) chunk->aset)
+ {
+ if (chunk->size >= size)
+ break;
+ priorfree = chunk;
+ }
- /*
- * If one is found, remove it from the free list, make it again a
- * member of the alloc set and return its data address.
- */
- if (chunk != NULL)
- {
- if (priorfree == NULL)
- set->freelist[fidx] = (AllocChunk) chunk->aset;
- else
- priorfree->aset = chunk->aset;
+ /*
+ * If one is found, remove it from the free list, make it again a
+ * member of the alloc set and return its data address.
+ */
+ if (chunk != NULL)
+ {
+ if (priorfree == NULL)
+ set->freelist[fidx] = (AllocChunk) chunk->aset;
+ else
+ priorfree->aset = chunk->aset;
+
+ chunk->aset = (void *) set;
- chunk->aset = (void *) set;
- return AllocChunkGetPointer(chunk);
- }
+#ifdef MEMORY_CONTEXT_CHECKING
+ chunk->data_size = size;
+#endif
+ AllocAllocInfo(set, chunk);
+ return AllocChunkGetPointer(chunk);
+ }
+ }
+ else
+ /* Big chunk
+ */
+ fidx = ALLOCSET_NUM_FREELISTS - 1;
/*
* Choose the actual chunk size to allocate.
@@ -474,7 +524,13 @@ AllocSetAlloc(MemoryContext context, Size size)
block->next = NULL;
set->blocks = block;
}
-
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ chunk->data_size = size;
+ /* mark memory for memory leak searching */
+ memset(AllocChunkGetPointer(chunk), 0x7F, chunk->size);
+#endif
+ AllocAllocInfo(set, chunk);
return AllocChunkGetPointer(chunk);
}
@@ -524,9 +580,15 @@ AllocSetAlloc(MemoryContext context, Size size)
if (block == NULL)
elog(ERROR, "Memory exhausted in AllocSetAlloc()");
+
block->aset = set;
block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
block->endptr = ((char *) block) + blksize;
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ /* mark memory for memory leak searching */
+ memset(block->freeptr, 0x7F, blksize - ALLOC_BLOCKHDRSZ);
+#endif
block->next = set->blocks;
set->blocks = block;
@@ -538,9 +600,14 @@ AllocSetAlloc(MemoryContext context, Size size)
chunk = (AllocChunk) (block->freeptr);
chunk->aset = (void *) set;
chunk->size = chunk_size;
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ chunk->data_size = size;
+#endif
block->freeptr += (chunk_size + ALLOC_CHUNKHDRSZ);
Assert(block->freeptr <= block->endptr);
+ AllocAllocInfo(set, chunk);
return AllocChunkGetPointer(chunk);
}
@@ -554,14 +621,17 @@ AllocSetFree(MemoryContext context, void *pointer)
AllocSet set = (AllocSet) context;
AllocChunk chunk = AllocPointerGetChunk(pointer);
-#ifdef CLOBBER_FREED_MEMORY
- /* Wipe freed memory for debugging purposes */
+#if defined(CLOBBER_FREED_MEMORY) || defined(MEMORY_CONTEXT_CHECKING)
+ /* Wipe freed memory for debugging purposes or for memory leak
+ * searching (in freelist[] must be mark memory
+ */
memset(pointer, 0x7F, chunk->size);
#endif
+ AllocFreeInfo(set, chunk);
+
if (chunk->size >= ALLOC_BIGCHUNK_LIMIT)
{
-
/*
* Big chunks are certain to have been allocated as single-chunk
* blocks. Find the containing block and return it to malloc().
@@ -598,6 +668,10 @@ AllocSetFree(MemoryContext context, void *pointer)
int fidx = AllocSetFreeIndex(chunk->size);
chunk->aset = (void *) set->freelist[fidx];
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ chunk->data_size = 0;
+#endif
set->freelist[fidx] = chunk;
}
}
@@ -637,6 +711,10 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
AllocBlock prevblock = NULL;
Size blksize;
+#ifdef MEMORY_CONTEXT_CHECKING
+ Size data_size = size;
+#endif
+
while (block != NULL)
{
if (chunk == (AllocChunk) (((char *) block) + ALLOC_BLOCKHDRSZ))
@@ -665,6 +743,13 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
else
prevblock->next = block;
chunk->size = size;
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ /* mark memory for memory leak searching */
+ memset(((char *) chunk) + (ALLOC_CHUNKHDRSZ + data_size),
+ 0x7F, size - data_size);
+ chunk->data_size = data_size;
+#endif
return AllocChunkGetPointer(chunk);
}
else
@@ -720,3 +805,126 @@ AllocSetStats(MemoryContext context)
set->header.name, totalspace, nblocks, freespace, nchunks,
totalspace - freespace);
}
+
+
+/*
+ * AllocSetCheck
+ * Walk on chunks and check consistence of memory.
+ */
+#ifdef MEMORY_CONTEXT_CHECKING
+
+static void
+AllocSetCheck(MemoryContext context)
+{
+ AllocSet set = (AllocSet) context;
+ AllocBlock block = NULL;
+ AllocChunk chunk = NULL;
+ char *name = set->header.name;
+
+ for (block = set->blocks; block != NULL; block = block->next)
+ {
+ char *bpoz = ((char *) block) + ALLOC_BLOCKHDRSZ;
+ /* long blk_size = block->endptr - ((char *) block);*/
+ long blk_free = block->endptr - block->freeptr;
+ long blk_used = block->freeptr - bpoz;
+ long blk_data = 0;
+ long nchunks = 0;
+
+ /*
+ * Empty block - empty can be keeper-block only
+ */
+ if (!blk_used)
+ {
+ if (set->keeper == block)
+ continue;
+ else
+ elog(ERROR, "AllocSetCheck(): %s: empty block %p",
+ name, block);
+ }
+
+ /*
+ * Chunk walker
+ */
+ do {
+ Size chsize,
+ dsize;
+ char *chdata_end,
+ *chend;
+
+ chunk = (AllocChunk) bpoz;
+
+ chsize = chunk->size; /* align chunk size */
+ dsize = chunk->data_size; /* real data */
+
+ chdata_end = ((char *) chunk) + (ALLOC_CHUNKHDRSZ + dsize);
+ chend = ((char *) chunk) + (ALLOC_CHUNKHDRSZ + chsize);
+
+ if (!dsize && chsize < dsize)
+ elog(ERROR, "AllocSetCheck(): %s: internal error for chunk %p in block %p",
+ name, chunk, block);
+ /*
+ * Check chunk size
+ */
+ if (chsize < (1 << ALLOC_MINBITS))
+ elog(ERROR, "AllocSetCheck(): %s: bad size '%d' for chunk %p in block %p",
+ name, chsize, chunk, block);
+
+ /* single-chunk block */
+ if (chsize >= ALLOC_BIGCHUNK_LIMIT &&
+ chsize + ALLOC_CHUNKHDRSZ != blk_used)
+ elog(ERROR, "AllocSetCheck(): %s: bad singel-chunk %p in block %p",
+ name, chunk, block);
+
+ /*
+ * Check in-chunk leak
+ */
+ if (dsize < chsize && *chdata_end != 0x7F)
+ {
+ fprintf(stderr, "\n--- Leak %p ---\n", chdata_end);
+ fprintf(stderr, "Chunk dump size: %ld (chunk-header %ld + chunk-size: %d), data must be: %d\n--- dump begin ---\n",
+ chsize + ALLOC_CHUNKHDRSZ,
+ ALLOC_CHUNKHDRSZ, chsize, dsize);
+
+ fwrite((void *) chunk, chsize+ALLOC_CHUNKHDRSZ, sizeof(char), stderr);
+ fputs("\n--- dump end ---\n", stderr);
+
+ elog(ERROR, "AllocSetCheck(): %s: found in-chunk memory leak (block %p; chunk %p; leak at %p",
+ name, block, chunk, chdata_end);
+ }
+
+ /*
+ * Check block-freeptr leak
+ */
+ if (chend == block->freeptr && blk_free &&
+ *chdata_end != 0x7F) {
+
+ fprintf(stderr, "\n--- Leak %p ---\n", chdata_end);
+ fprintf(stderr, "Dump size: %ld (chunk-header %ld + chunk-size: %d + block-freespace: %ld), data must be: %d\n--- dump begin ---\n",
+ chsize + ALLOC_CHUNKHDRSZ + blk_free,
+ ALLOC_CHUNKHDRSZ, chsize, blk_free, dsize);
+
+ fwrite((void *) chunk, chsize+ALLOC_CHUNKHDRSZ+blk_free, sizeof(char), stderr);
+ fputs("\n--- dump end ---\n", stderr);
+
+ elog(ERROR, "AllocSetCheck(): %s: found block-freeptr memory leak (block %p; chunk %p; leak at %p",
+ name, block, chunk, chdata_end);
+ }
+
+ blk_data += chsize;
+ nchunks++;
+
+ if (chend < block->freeptr)
+ bpoz += ALLOC_CHUNKHDRSZ + chsize;
+ else
+ break;
+
+ } while(block->freeptr > bpoz); /* chunk walker */
+
+
+ if ((blk_data + (nchunks * ALLOC_CHUNKHDRSZ)) != blk_used)
+
+ elog(ERROR, "AllocSetCheck(): %s: found non-consistent memory block %p",
+ name, block);
+ }
+}
+#endif
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 71877c4e62a..874f609671d 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -14,7 +14,7 @@
*
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/utils/mmgr/mcxt.c,v 1.22 2000/06/28 03:32:50 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/utils/mmgr/mcxt.c,v 1.23 2000/07/11 14:30:28 momjian Exp $
*
*-------------------------------------------------------------------------
*/
@@ -238,6 +238,27 @@ MemoryContextStats(MemoryContext context)
}
}
+
+/*
+ * MemoryContextCheck
+ * Check all chunks in the named context.
+ *
+ * This is just a debugging utility, so it's not fancy.
+ */
+#ifdef MEMORY_CONTEXT_CHECKING
+void
+MemoryContextCheck(MemoryContext context)
+{
+ MemoryContext child;
+
+ (*context->methods->check) (context);
+ for (child = context->firstchild; child != NULL; child = child->nextchild)
+ {
+ MemoryContextCheck(child);
+ }
+}
+#endif
+
/*
* MemoryContextContains
* Detect whether an allocated chunk of memory belongs to a given
diff --git a/src/include/config.h.in b/src/include/config.h.in
index 25e8be291ff..583c20dc545 100644
--- a/src/include/config.h.in
+++ b/src/include/config.h.in
@@ -8,7 +8,7 @@
* or in config.h afterwards. Of course, if you edit config.h, then your
* changes will be overwritten the next time you run configure.
*
- * $Id: config.h.in,v 1.124 2000/07/09 13:14:13 petere Exp $
+ * $Id: config.h.in,v 1.125 2000/07/11 14:30:30 momjian Exp $
*/
#ifndef CONFIG_H
@@ -229,6 +229,12 @@
#define CLOBBER_FREED_MEMORY
#endif
+/* Define this to check memory leaks
+ */
+#ifdef USE_ASSERT_CHECKING
+#define MEMORY_CONTEXT_CHECKING
+#endif
+
/* Define this to force all parse and plan trees to be passed through
* copyObject(), to facilitate catching errors and omissions in copyObject().
* XXX For 7.1 development, define this automatically if --enable-cassert.
diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h
index abc38e5c60d..4296795e5d1 100644
--- a/src/include/nodes/memnodes.h
+++ b/src/include/nodes/memnodes.h
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2000, PostgreSQL, Inc
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $Id: memnodes.h,v 1.17 2000/06/28 03:33:15 tgl Exp $
+ * $Id: memnodes.h,v 1.18 2000/07/11 14:30:34 momjian Exp $
*
*-------------------------------------------------------------------------
*/
@@ -42,6 +42,9 @@ typedef struct MemoryContextMethods
void (*init) (MemoryContext context);
void (*reset) (MemoryContext context);
void (*delete) (MemoryContext context);
+#ifdef MEMORY_CONTEXT_CHECKING
+ void (*check) (MemoryContext context);
+#endif
void (*stats) (MemoryContext context);
} MemoryContextMethods;
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index cbabdbf275e..be322cbccc3 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -10,7 +10,7 @@
* Portions Copyright (c) 1996-2000, PostgreSQL, Inc
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $Id: memutils.h,v 1.36 2000/06/28 03:33:33 tgl Exp $
+ * $Id: memutils.h,v 1.37 2000/07/11 14:30:37 momjian Exp $
*
*-------------------------------------------------------------------------
*/
@@ -48,6 +48,9 @@ typedef struct StandardChunkHeader
{
MemoryContext context; /* owning context */
Size size; /* size of data space allocated in chunk */
+#ifdef MEMORY_CONTEXT_CHECKING
+ Size data_size; /* real data size (without align) */
+#endif
} StandardChunkHeader;
#define STANDARDCHUNKHEADERSIZE MAXALIGN(sizeof(StandardChunkHeader))
@@ -78,6 +81,7 @@ extern void MemoryContextResetChildren(MemoryContext context);
extern void MemoryContextDeleteChildren(MemoryContext context);
extern void MemoryContextResetAndDeleteChildren(MemoryContext context);
extern void MemoryContextStats(MemoryContext context);
+extern void MemoryContextCheck(MemoryContext context);
extern bool MemoryContextContains(MemoryContext context, void *pointer);
/*
--
GitLab