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.

diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 6a111c78329e7df357ac6e7bb7129e04197b0115..de643779f064957f19dc492cc1c7c7faccd8b079 100644 (file)
--- 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 8dd3cf488964afaa3801b7b64c4b1ac1f9fa1157..c72e3470004bb4d51ab48a727e138da5194f771a 100644 (file)
--- 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;
 }
 
 /*