I wrote:
> So I'm still inclined to leave 001 and 010 both unused, but the
> reason why is different than I thought before.
Which leaves me with the attached proposed wording.
regards, tom lane
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index c80236d285..b1a3c74830 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_UNUSED1_ID].free_p = BogusFree,
+ [MCTX_UNUSED1_ID].realloc = BogusRealloc,
+ [MCTX_UNUSED1_ID].get_chunk_context = BogusGetChunkContext,
+ [MCTX_UNUSED1_ID].get_chunk_space = BogusGetChunkSpace,
+
+ [MCTX_UNUSED2_ID].free_p = BogusFree,
+ [MCTX_UNUSED2_ID].realloc = BogusRealloc,
+ [MCTX_UNUSED2_ID].get_chunk_context = BogusGetChunkContext,
+ [MCTX_UNUSED2_ID].get_chunk_space = BogusGetChunkSpace,
+
+ [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,
};
/*
@@ -125,6 +162,77 @@ static void MemoryContextStatsPrint(MemoryContext context, void *passthru,
#define MCXT_METHOD(pointer, method) \
mcxt_methods[GetMemoryChunkMethodID(pointer)].method
+/*
+ * GetMemoryChunkMethodID
+ * Return the MemoryContextMethodID from the uint64 chunk header which
+ * directly precedes 'pointer'.
+ */
+static inline MemoryContextMethodID
+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 == (const void *) MAXALIGN(pointer));
+
+ header = *((const uint64 *) ((const char *) pointer - sizeof(uint64)));
+
+ return (MemoryContextMethodID) (header & MEMORY_CONTEXT_METHODID_MASK);
+}
+
+/*
+ * GetMemoryChunkHeader
+ * Return the uint64 chunk header which directly precedes 'pointer'.
+ *
+ * This is only used after GetMemoryChunkMethodID, so no need for error checks.
+ */
+static inline uint64
+GetMemoryChunkHeader(const void *pointer)
+{
+ return *((const uint64 *) ((const char *) pointer - sizeof(uint64)));
+}
+
+/*
+ * Support routines to trap use of invalid memory context method IDs
+ * (from calling pfree or the like on a bogus pointer). As a possible
+ * aid in debugging, we report the header word along with the pointer
+ * address (if we got here, there must be an accessible header word).
+ */
+static void
+BogusFree(void *pointer)
+{
+ elog(ERROR, "pfree called with invalid pointer %p (header 0x%016llx)",
+ pointer, (long long) GetMemoryChunkHeader(pointer));
+}
+
+static void *
+BogusRealloc(void *pointer, Size size)
+{
+ elog(ERROR, "repalloc called with invalid pointer %p (header 0x%016llx)",
+ pointer, (long long) GetMemoryChunkHeader(pointer));
+ return NULL; /* keep compiler quiet */
+}
+
+static MemoryContext
+BogusGetChunkContext(void *pointer)
+{
+ elog(ERROR, "GetMemoryChunkContext called with invalid pointer %p (header 0x%016llx)",
+ pointer, (long long) GetMemoryChunkHeader(pointer));
+ return NULL; /* keep compiler quiet */
+}
+
+static Size
+BogusGetChunkSpace(void *pointer)
+{
+ elog(ERROR, "GetMemoryChunkSpace called with invalid pointer %p (header 0x%016llx)",
+ pointer, (long long) GetMemoryChunkHeader(pointer));
+ return 0; /* keep compiler quiet */
+}
+
/*****************************************************************************
* EXPORTED ROUTINES *
diff --git a/src/include/utils/memutils_internal.h b/src/include/utils/memutils_internal.h
index 377cee7a84..bc2cbdd506 100644
--- a/src/include/utils/memutils_internal.h
+++ b/src/include/utils/memutils_internal.h
@@ -74,15 +74,26 @@ extern void SlabCheck(MemoryContext context);
* MemoryContextMethodID
* A unique identifier for each MemoryContext implementation which
* indicates the index into the mcxt_methods[] array. See mcxt.c.
- * 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 robust error detection, 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. We also try
+ * to avoid using bit-patterns as valid IDs if they are likely to occur in
+ * garbage data, or if they could falsely match on chunks that are really from
+ * malloc not palloc. (We can't tell that for most malloc implementations,
+ * but it happens that glibc stores flag bits in the same place where we put
+ * the MemoryContextMethodID, so the possible values are predictable for it.)
*/
typedef enum MemoryContextMethodID
{
+ MCTX_UNUSED1_ID, /* 000 occurs in never-used memory */
+ MCTX_UNUSED2_ID, /* glibc malloc'd chunks usually match 001 */
+ MCTX_UNUSED3_ID, /* glibc malloc'd chunks > 128kB match 010 */
MCTX_ASET_ID,
MCTX_GENERATION_ID,
MCTX_SLAB_ID,
+ MCTX_UNUSED4_ID, /* available */
+ MCTX_UNUSED5_ID /* 111 occurs in wipe_mem'd memory */
} MemoryContextMethodID;
/*
@@ -104,27 +115,4 @@ extern void MemoryContextCreate(MemoryContext node,
MemoryContext parent,
const char *name);
-/*
- * GetMemoryChunkMethodID
- * Return the MemoryContextMethodID from the uint64 chunk header which
- * directly precedes 'pointer'.
- */
-static inline MemoryContextMethodID
-GetMemoryChunkMethodID(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)));
-
- return (MemoryContextMethodID) (header & MEMORY_CONTEXT_METHODID_MASK);
-}
-
#endif /* MEMUTILS_INTERNAL_H */
