Temporarily make MemoryContextContains return false

david-rowley · david-rowley · commit b76fb6c2a99e · 2022-09-09T00:28:38.000+12:00
5265e91 changed MemoryContextContains to update it so that it works correctly with the new MemoryChunk code added in c6e0fe1. However, 5265e91 was done with the assumption that MemoryContextContains would only ever be given pointers to memory that had been returned by one of our MemoryContext allocators. It seems that's not true and many of our 32-bit buildfarm animals are clearly showing that. There are some code paths that call MemoryContextContains with a pointer pointing part way into an allocated chunk. The example of this found by the 32-bit buildfarm animals is the int2int4_sum() function. This function returns transdata->sum, which is not a pointer to memory that was allocated directly. This return value is then subsequently passed to MemoryContextContains which causes it to crash due to it thinking the memory directly prior to that pointer is a MemoryChunk. What's actually in that memory is the field in the struct that comes prior to the "sum" field. This problem didn't occur in 64-bit world because BIGINT is a byval type and the code which was calling MemoryContextContains with the bad pointer only does so with non-byval types. Here, instead of reverting 5265e91 and making MemoryContextContains completely broken again, let's just make it always return false for now. Effectively prior to 5265e91 it was doing that anyway, this at least makes that more explicit. The only repercussions of this with the current MemoryContextContains calls are that we perform a datumCopy() when we might not need to. This should make the 32-bit buildfarm animals happy again and give us more time to consider a long-term fix. Discussion: https://postgr.es/m/20220907130552.sfjri7jublfxyyi4%40jrouhaud
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
@@ -826,6 +826,14 @@ MemoryContextCheck(MemoryContext context)
 bool
 MemoryContextContains(MemoryContext context, void *pointer)
 {
+	/*
+	 * Temporarily make this always return false as we don't yet have a fully
+	 * baked idea on how to make it work correctly with the new MemoryChunk
+	 * code.
+	 */
+	return false;
+
+#ifdef NOT_USED
 	MemoryContext ptr_context;
 
 	/*
@@ -845,6 +853,7 @@ MemoryContextContains(MemoryContext context, void *pointer)
 	ptr_context = GetMemoryChunkContext(pointer);
 
 	return ptr_context == context;
+#endif
 }
 
 /*
@@ -960,8 +969,6 @@ MemoryContextAlloc(MemoryContext context, Size size)
 
 	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
-	Assert(MemoryContextContains(context, ret));
-
 	return ret;
 }
 
@@ -1000,8 +1007,6 @@ MemoryContextAllocZero(MemoryContext context, Size size)
 
 	MemSetAligned(ret, 0, size);
 
-	Assert(MemoryContextContains(context, ret));
-
 	return ret;
 }
 
@@ -1040,8 +1045,6 @@ MemoryContextAllocZeroAligned(MemoryContext context, Size size)
 
 	MemSetLoop(ret, 0, size);
 
-	Assert(MemoryContextContains(context, ret));
-
 	return ret;
 }
 
@@ -1083,8 +1086,6 @@ MemoryContextAllocExtended(MemoryContext context, Size size, int flags)
 	if ((flags & MCXT_ALLOC_ZERO) != 0)
 		MemSetAligned(ret, 0, size);
 
-	Assert(MemoryContextContains(context, ret));
-
 	return ret;
 }
 
@@ -1168,8 +1169,6 @@ palloc(Size size)
 
 	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
-	Assert(MemoryContextContains(context, ret));
-
 	return ret;
 }
 
@@ -1203,8 +1202,6 @@ palloc0(Size size)
 
 	MemSetAligned(ret, 0, size);
 
-	Assert(MemoryContextContains(context, ret));
-
 	return ret;
 }
 
@@ -1244,8 +1241,6 @@ palloc_extended(Size size, int flags)
 	if ((flags & MCXT_ALLOC_ZERO) != 0)
 		MemSetAligned(ret, 0, size);
 
-	Assert(MemoryContextContains(context, ret));
-
 	return ret;
 }
 
@@ -1299,8 +1294,6 @@ repalloc(void *pointer, Size size)
 
 	VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
 
-	Assert(MemoryContextContains(context, ret));
-
 	return ret;
 }
 
@@ -1336,8 +1329,6 @@ MemoryContextAllocHuge(MemoryContext context, Size size)
 
 	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
-	Assert(MemoryContextContains(context, ret));
-
 	return ret;
 }
 
@@ -1377,8 +1368,6 @@ repalloc_huge(void *pointer, Size size)
 
 	VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
 
-	Assert(MemoryContextContains(context, ret));
-
 	return ret;
 }
 
