One more thing: based on what I saw in working with my pending guc.c
changes, the assertions in GetMemoryChunkMethodID are largely useless
for detecting bogus pointers. I think we should do something more
like the attached, which will result in a clean failure if the method
ID bits are invalid.
I'm a little tempted also to rearrange the MemoryContextMethodID enum
so that likely bit patterns like 000 are not valid IDs.
While I didn't change it here, I wonder why GetMemoryChunkMethodID is
publicly exposed at all. AFAICS it could be static inside mcxt.c.
regards, tom lane
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index c80236d285..e82d9b21ba 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -32,6 +32,11 @@
#include "utils/memutils_internal.h"
+static void BogusFree(void *pointer);
+static void *BogusRealloc(void *pointer, Size size);
+static MemoryContext BogusGetChunkContext(void *pointer);
+static Size BogusGetChunkSpace(void *pointer);
+
/*****************************************************************************
* GLOBAL MEMORY *
*****************************************************************************/
@@ -74,10 +79,42 @@ static const MemoryContextMethods mcxt_methods[] = {
[MCTX_SLAB_ID].get_chunk_context = SlabGetChunkContext,
[MCTX_SLAB_ID].get_chunk_space = SlabGetChunkSpace,
[MCTX_SLAB_ID].is_empty = SlabIsEmpty,
- [MCTX_SLAB_ID].stats = SlabStats
+ [MCTX_SLAB_ID].stats = SlabStats,
#ifdef MEMORY_CONTEXT_CHECKING
- ,[MCTX_SLAB_ID].check = SlabCheck
+ [MCTX_SLAB_ID].check = SlabCheck,
#endif
+
+ /*
+ * Unused (as yet) IDs should have dummy entries here. This allows us to
+ * fail cleanly if a bogus pointer is passed to pfree or the like. It
+ * seems sufficient to provide routines for the methods that might get
+ * invoked from inspection of a chunk (see MCXT_METHOD calls below).
+ */
+
+ [MCTX_UNUSED3_ID].free_p = BogusFree,
+ [MCTX_UNUSED3_ID].realloc = BogusRealloc,
+ [MCTX_UNUSED3_ID].get_chunk_context = BogusGetChunkContext,
+ [MCTX_UNUSED3_ID].get_chunk_space = BogusGetChunkSpace,
+
+ [MCTX_UNUSED4_ID].free_p = BogusFree,
+ [MCTX_UNUSED4_ID].realloc = BogusRealloc,
+ [MCTX_UNUSED4_ID].get_chunk_context = BogusGetChunkContext,
+ [MCTX_UNUSED4_ID].get_chunk_space = BogusGetChunkSpace,
+
+ [MCTX_UNUSED5_ID].free_p = BogusFree,
+ [MCTX_UNUSED5_ID].realloc = BogusRealloc,
+ [MCTX_UNUSED5_ID].get_chunk_context = BogusGetChunkContext,
+ [MCTX_UNUSED5_ID].get_chunk_space = BogusGetChunkSpace,
+
+ [MCTX_UNUSED6_ID].free_p = BogusFree,
+ [MCTX_UNUSED6_ID].realloc = BogusRealloc,
+ [MCTX_UNUSED6_ID].get_chunk_context = BogusGetChunkContext,
+ [MCTX_UNUSED6_ID].get_chunk_space = BogusGetChunkSpace,
+
+ [MCTX_UNUSED7_ID].free_p = BogusFree,
+ [MCTX_UNUSED7_ID].realloc = BogusRealloc,
+ [MCTX_UNUSED7_ID].get_chunk_context = BogusGetChunkContext,
+ [MCTX_UNUSED7_ID].get_chunk_space = BogusGetChunkSpace,
};
/*
@@ -125,6 +162,34 @@ static void MemoryContextStatsPrint(MemoryContext context, void *passthru,
#define MCXT_METHOD(pointer, method) \
mcxt_methods[GetMemoryChunkMethodID(pointer)].method
+/*
+ * Support routines to trap use of invalid memory context method IDs
+ * (from calling pfree or the like on a bogus pointer).
+ */
+static void
+BogusFree(void *pointer)
+{
+ elog(ERROR, "pfree called with invalid pointer %p", pointer);
+}
+
+static void *
+BogusRealloc(void *pointer, Size size)
+{
+ elog(ERROR, "repalloc called with invalid pointer %p", pointer);
+}
+
+static MemoryContext
+BogusGetChunkContext(void *pointer)
+{
+ elog(ERROR, "GetMemoryChunkContext called with invalid pointer %p", pointer);
+}
+
+static Size
+BogusGetChunkSpace(void *pointer)
+{
+ elog(ERROR, "GetMemoryChunkSpace called with invalid pointer %p", pointer);
+}
+
/*****************************************************************************
* EXPORTED ROUTINES *
diff --git a/src/include/utils/memutils_internal.h b/src/include/utils/memutils_internal.h
index 377cee7a84..797409ee94 100644
--- a/src/include/utils/memutils_internal.h
+++ b/src/include/utils/memutils_internal.h
@@ -77,12 +77,20 @@ extern void SlabCheck(MemoryContext context);
* The maximum value for this enum is constrained by
* MEMORY_CONTEXT_METHODID_MASK. If an enum value higher than that is
* required then MEMORY_CONTEXT_METHODID_BITS will need to be increased.
+ * For robustness, ensure that MemoryContextMethodID has a value for
+ * each possible bit-pattern of MEMORY_CONTEXT_METHODID_MASK, and make
+ * dummy entries for unused IDs in the mcxt_methods[] array.
*/
typedef enum MemoryContextMethodID
{
MCTX_ASET_ID,
MCTX_GENERATION_ID,
MCTX_SLAB_ID,
+ MCTX_UNUSED3_ID,
+ MCTX_UNUSED4_ID,
+ MCTX_UNUSED5_ID,
+ MCTX_UNUSED6_ID,
+ MCTX_UNUSED7_ID
} MemoryContextMethodID;
/*
@@ -110,20 +118,11 @@ extern void MemoryContextCreate(MemoryContext node,
* directly precedes 'pointer'.
*/
static inline MemoryContextMethodID
-GetMemoryChunkMethodID(void *pointer)
+GetMemoryChunkMethodID(const void *pointer)
{
uint64 header;
- /*
- * Try to detect bogus pointers handed to us, poorly though we can.
- * Presumably, a pointer that isn't MAXALIGNED isn't pointing at an
- * allocated chunk.
- */
- Assert(pointer != NULL);
- Assert(pointer == (void *) MAXALIGN(pointer));
-
- header = *((uint64 *) ((char *) pointer - sizeof(uint64)));
-
+ header = *((const uint64 *) ((const char *) pointer - sizeof(uint64)));
return (MemoryContextMethodID) (header & MEMORY_CONTEXT_METHODID_MASK);
}
