From a855148a29b786b179308b3bd5c59fe5b67110d8 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Wed, 26 Jun 2013 19:56:03 -0400
Subject: [PATCH] Refactor aset.c and mcxt.c in preparation for Valgrind
cooperation.
Move some repeated debugging code into functions and store intermediates
in variables where not presently necessary. No code-generation changes
in a production build, and no functional changes. This simplifies and
focuses the main patch.
---
src/backend/utils/mmgr/aset.c | 65 ++++++++++++++++++++++++-----------
src/backend/utils/mmgr/mcxt.c | 46 +++++++++++++++----------
2 files changed, 73 insertions(+), 38 deletions(-)
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 6a111c78329..de643779f06 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -308,6 +308,37 @@ AllocSetFreeIndex(Size size)
return idx;
}
+#ifdef CLOBBER_FREED_MEMORY
+
+/* Wipe freed memory for debugging purposes */
+static void
+wipe_mem(void *ptr, size_t size)
+{
+ memset(ptr, 0x7F, size);
+}
+#endif
+
+#ifdef MEMORY_CONTEXT_CHECKING
+static void
+set_sentinel(void *base, Size offset)
+{
+ char *ptr = (char *) base + offset;
+
+ *ptr = 0x7E;
+}
+
+static bool
+sentinel_ok(const void *base, Size offset)
+{
+ const char *ptr = (const char *) base + offset;
+ bool ret;
+
+ ret = *ptr == 0x7E;
+
+ return ret;
+}
+#endif
+
#ifdef RANDOMIZE_ALLOCATED_MEMORY
/*
@@ -492,8 +523,7 @@ AllocSetReset(MemoryContext context)
char *datastart = ((char *) block) + ALLOC_BLOCKHDRSZ;
#ifdef CLOBBER_FREED_MEMORY
- /* Wipe freed memory for debugging purposes */
- memset(datastart, 0x7F, block->freeptr - datastart);
+ wipe_mem(datastart, block->freeptr - datastart);
#endif
block->freeptr = datastart;
block->next = NULL;
@@ -502,8 +532,7 @@ AllocSetReset(MemoryContext context)
{
/* Normal case, release the block */
#ifdef CLOBBER_FREED_MEMORY
- /* Wipe freed memory for debugging purposes */
- memset(block, 0x7F, block->freeptr - ((char *) block));
+ wipe_mem(block, block->freeptr - ((char *) block));
#endif
free(block);
}
@@ -545,8 +574,7 @@ AllocSetDelete(MemoryContext context)
AllocBlock next = block->next;
#ifdef CLOBBER_FREED_MEMORY
- /* Wipe freed memory for debugging purposes */
- memset(block, 0x7F, block->freeptr - ((char *) block));
+ wipe_mem(block, block->freeptr - ((char *) block));
#endif
free(block);
block = next;
@@ -598,7 +626,7 @@ AllocSetAlloc(MemoryContext context, Size size)
chunk->requested_size = size;
/* set mark to catch clobber of "unused" space */
if (size < chunk_size)
- ((char *) AllocChunkGetPointer(chunk))[size] = 0x7E;
+ set_sentinel(AllocChunkGetPointer(chunk), size);
#endif
#ifdef RANDOMIZE_ALLOCATED_MEMORY
/* fill the allocated space with junk */
@@ -644,7 +672,7 @@ AllocSetAlloc(MemoryContext context, Size size)
chunk->requested_size = size;
/* set mark to catch clobber of "unused" space */
if (size < chunk->size)
- ((char *) AllocChunkGetPointer(chunk))[size] = 0x7E;
+ set_sentinel(AllocChunkGetPointer(chunk), size);
#endif
#ifdef RANDOMIZE_ALLOCATED_MEMORY
/* fill the allocated space with junk */
@@ -801,7 +829,7 @@ AllocSetAlloc(MemoryContext context, Size size)
chunk->requested_size = size;
/* set mark to catch clobber of "unused" space */
if (size < chunk->size)
- ((char *) AllocChunkGetPointer(chunk))[size] = 0x7E;
+ set_sentinel(AllocChunkGetPointer(chunk), size);
#endif
#ifdef RANDOMIZE_ALLOCATED_MEMORY
/* fill the allocated space with junk */
@@ -827,7 +855,7 @@ AllocSetFree(MemoryContext context, void *pointer)
#ifdef MEMORY_CONTEXT_CHECKING
/* Test for someone scribbling on unused space in chunk */
if (chunk->requested_size < chunk->size)
- if (((char *) pointer)[chunk->requested_size] != 0x7E)
+ if (!sentinel_ok(pointer, chunk->requested_size))
elog(WARNING, "detected write past chunk end in %s %p",
set->header.name, chunk);
#endif
@@ -860,8 +888,7 @@ AllocSetFree(MemoryContext context, void *pointer)
else
prevblock->next = block->next;
#ifdef CLOBBER_FREED_MEMORY
- /* Wipe freed memory for debugging purposes */
- memset(block, 0x7F, block->freeptr - ((char *) block));
+ wipe_mem(block, block->freeptr - ((char *) block));
#endif
free(block);
}
@@ -873,8 +900,7 @@ AllocSetFree(MemoryContext context, void *pointer)
chunk->aset = (void *) set->freelist[fidx];
#ifdef CLOBBER_FREED_MEMORY
- /* Wipe freed memory for debugging purposes */
- memset(pointer, 0x7F, chunk->size);
+ wipe_mem(pointer, chunk->size);
#endif
#ifdef MEMORY_CONTEXT_CHECKING
@@ -901,7 +927,7 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
#ifdef MEMORY_CONTEXT_CHECKING
/* Test for someone scribbling on unused space in chunk */
if (chunk->requested_size < oldsize)
- if (((char *) pointer)[chunk->requested_size] != 0x7E)
+ if (!sentinel_ok(pointer, chunk->requested_size))
elog(WARNING, "detected write past chunk end in %s %p",
set->header.name, chunk);
#endif
@@ -924,7 +950,7 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
chunk->requested_size = size;
/* set mark to catch clobber of "unused" space */
if (size < oldsize)
- ((char *) pointer)[size] = 0x7E;
+ set_sentinel(pointer, size);
#endif
return pointer;
}
@@ -987,7 +1013,7 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
chunk->requested_size = size;
/* set mark to catch clobber of "unused" space */
if (size < chunk->size)
- ((char *) AllocChunkGetPointer(chunk))[size] = 0x7E;
+ set_sentinel(AllocChunkGetPointer(chunk), size);
#endif
return AllocChunkGetPointer(chunk);
@@ -1136,11 +1162,9 @@ AllocSetCheck(MemoryContext context)
AllocChunk chunk = (AllocChunk) bpoz;
Size chsize,
dsize;
- char *chdata_end;
chsize = chunk->size; /* aligned chunk size */
dsize = chunk->requested_size; /* real data */
- chdata_end = ((char *) chunk) + (ALLOC_CHUNKHDRSZ + dsize);
/*
* Check chunk size
@@ -1170,7 +1194,8 @@ AllocSetCheck(MemoryContext context)
/*
* Check for overwrite of "unallocated" space in chunk
*/
- if (dsize > 0 && dsize < chsize && *chdata_end != 0x7E)
+ if (dsize > 0 && dsize < chsize &&
+ !sentinel_ok(chunk, ALLOC_CHUNKHDRSZ + dsize))
elog(WARNING, "problem in alloc set %s: detected write past chunk end in block %p, chunk %p",
name, block, chunk);
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 8dd3cf48896..c72e3470004 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -569,6 +569,8 @@ MemoryContextCreate(NodeTag tag, Size size,
void *
MemoryContextAlloc(MemoryContext context, Size size)
{
+ void *ret;
+
AssertArg(MemoryContextIsValid(context));
if (!AllocSizeIsValid(size))
@@ -577,7 +579,9 @@ MemoryContextAlloc(MemoryContext context, Size size)
context->isReset = false;
- return (*context->methods->alloc) (context, size);
+ ret = (*context->methods->alloc) (context, size);
+
+ return ret;
}
/*
@@ -638,6 +642,8 @@ void *
palloc(Size size)
{
/* duplicates MemoryContextAlloc to avoid increased overhead */
+ void *ret;
+
AssertArg(MemoryContextIsValid(CurrentMemoryContext));
if (!AllocSizeIsValid(size))
@@ -646,7 +652,9 @@ palloc(Size size)
CurrentMemoryContext->isReset = false;
- return (*CurrentMemoryContext->methods->alloc) (CurrentMemoryContext, size);
+ ret = (*CurrentMemoryContext->methods->alloc) (CurrentMemoryContext, size);
+
+ return ret;
}
void *
@@ -677,7 +685,7 @@ palloc0(Size size)
void
pfree(void *pointer)
{
- StandardChunkHeader *header;
+ MemoryContext context;
/*
* Try to detect bogus pointers handed to us, poorly though we can.
@@ -690,12 +698,12 @@ pfree(void *pointer)
/*
* OK, it's probably safe to look at the chunk header.
*/
- header = (StandardChunkHeader *)
- ((char *) pointer - STANDARDCHUNKHEADERSIZE);
+ context = ((StandardChunkHeader *)
+ ((char *) pointer - STANDARDCHUNKHEADERSIZE))->context;
- AssertArg(MemoryContextIsValid(header->context));
+ AssertArg(MemoryContextIsValid(context));
- (*header->context->methods->free_p) (header->context, pointer);
+ (*context->methods->free_p) (context, pointer);
}
/*
@@ -705,7 +713,12 @@ pfree(void *pointer)
void *
repalloc(void *pointer, Size size)
{
- StandardChunkHeader *header;
+ MemoryContext context;
+ void *ret;
+
+ if (!AllocSizeIsValid(size))
+ elog(ERROR, "invalid memory alloc request size %lu",
+ (unsigned long) size);
/*
* Try to detect bogus pointers handed to us, poorly though we can.
@@ -718,20 +731,17 @@ repalloc(void *pointer, Size size)
/*
* OK, it's probably safe to look at the chunk header.
*/
- header = (StandardChunkHeader *)
- ((char *) pointer - STANDARDCHUNKHEADERSIZE);
+ context = ((StandardChunkHeader *)
+ ((char *) pointer - STANDARDCHUNKHEADERSIZE))->context;
- AssertArg(MemoryContextIsValid(header->context));
-
- if (!AllocSizeIsValid(size))
- elog(ERROR, "invalid memory alloc request size %lu",
- (unsigned long) size);
+ AssertArg(MemoryContextIsValid(context));
/* isReset must be false already */
- Assert(!header->context->isReset);
+ Assert(!context->isReset);
+
+ ret = (*context->methods->realloc) (context, pointer, size);
- return (*header->context->methods->realloc) (header->context,
- pointer, size);
+ return ret;
}
/*
--
GitLab