other_ideas_to_fix_MemoryContextContains.patch

text/plain

Filename: other_ideas_to_fix_MemoryContextContains.patch
Type: text/plain
Part: 0
Message: Re: Reducing the chunk header sizes on all memory context types

Patch

Format: unified
File+
src/backend/executor/nodeAgg.c 23 18
src/backend/executor/nodeWindowAgg.c 24 19
src/backend/utils/adt/numeric.c 1 1
src/backend/utils/mmgr/aset.c 22 0
src/backend/utils/mmgr/generation.c 26 0
src/backend/utils/mmgr/mcxt.c 55 29
src/backend/utils/mmgr/slab.c 29 0
src/include/nodes/memnodes.h 1 0
src/include/utils/memutils.h 3 1
src/include/utils/memutils_internal.h 3 0
src/include/utils/memutils_memorychunk.h 19 0
src/include/windowapi.h 12 0
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index fe74e49814..bc7eeaf84d 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -1107,6 +1107,11 @@ finalize_aggregate(AggState *aggstate,
 		{
 			*resultVal = FunctionCallInvoke(fcinfo);
 			*resultIsNull = fcinfo->isnull;
+
+			/* ensure by-ref types are allocated in CurrentMemoryContext */
+			Assert(peragg->resulttypeByVal || *resultIsNull ||
+				   MemoryContextContains(CurrentMemoryContext, (void *) *resultVal));
+
 		}
 		aggstate->curperagg = NULL;
 	}
@@ -1115,17 +1120,13 @@ finalize_aggregate(AggState *aggstate,
 		/* Don't need MakeExpandedObjectReadOnly; datumCopy will copy it */
 		*resultVal = pergroupstate->transValue;
 		*resultIsNull = pergroupstate->transValueIsNull;
-	}
 
-	/*
-	 * If result is pass-by-ref, make sure it is in the right context.
-	 */
-	if (!peragg->resulttypeByVal && !*resultIsNull &&
-		!MemoryContextContains(CurrentMemoryContext,
-							   DatumGetPointer(*resultVal)))
-		*resultVal = datumCopy(*resultVal,
-							   peragg->resulttypeByVal,
-							   peragg->resulttypeLen);
+		/* if result is pass-by-ref, copy into the CurrentMemoryContext */
+		if (!peragg->resulttypeByVal && !*resultIsNull)
+			*resultVal = datumCopy(*resultVal,
+								   peragg->resulttypeByVal,
+								   peragg->resulttypeLen);
+	}
 
 	MemoryContextSwitchTo(oldContext);
 }
@@ -1172,6 +1173,11 @@ finalize_partialaggregate(AggState *aggstate,
 
 			*resultVal = FunctionCallInvoke(fcinfo);
 			*resultIsNull = fcinfo->isnull;
+
+			/* ensure by-ref types are allocated in CurrentMemoryContext */
+			Assert(peragg->resulttypeByVal || *resultIsNull ||
+				   MemoryContextContains(CurrentMemoryContext, (void *) *resultVal));
+
 		}
 	}
 	else
@@ -1179,15 +1185,14 @@ finalize_partialaggregate(AggState *aggstate,
 		/* Don't need MakeExpandedObjectReadOnly; datumCopy will copy it */
 		*resultVal = pergroupstate->transValue;
 		*resultIsNull = pergroupstate->transValueIsNull;
-	}
 
-	/* If result is pass-by-ref, make sure it is in the right context. */
-	if (!peragg->resulttypeByVal && !*resultIsNull &&
-		!MemoryContextContains(CurrentMemoryContext,
-							   DatumGetPointer(*resultVal)))
-		*resultVal = datumCopy(*resultVal,
-							   peragg->resulttypeByVal,
-							   peragg->resulttypeLen);
+		/* if result is pass-by-ref, copy into the CurrentMemoryContext */
+		if (!peragg->resulttypeByVal && !*resultIsNull)
+			*resultVal = datumCopy(*resultVal,
+								   peragg->resulttypeByVal,
+								   peragg->resulttypeLen);
+
+	}
 
 	MemoryContextSwitchTo(oldContext);
 }
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 8b0858e9f5..b196328060 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -626,6 +626,10 @@ finalize_windowaggregate(WindowAggState *winstate,
 			*result = FunctionCallInvoke(fcinfo);
 			winstate->curaggcontext = NULL;
 			*isnull = fcinfo->isnull;
+
+			/* ensure by-ref types are allocated in CurrentMemoryContext */
+			Assert(peraggstate->resulttypeByVal || *isnull ||
+				   MemoryContextContains(CurrentMemoryContext, (void *) *result));
 		}
 	}
 	else
@@ -633,17 +637,14 @@ finalize_windowaggregate(WindowAggState *winstate,
 		/* Don't need MakeExpandedObjectReadOnly; datumCopy will copy it */
 		*result = peraggstate->transValue;
 		*isnull = peraggstate->transValueIsNull;
+
+		/* if result is pass-by-ref, copy into the CurrentMemoryContext */
+		if (!peraggstate->resulttypeByVal && !*isnull)
+			*result = datumCopy(*result,
+								peraggstate->resulttypeByVal,
+								peraggstate->resulttypeLen);
 	}
 
-	/*
-	 * If result is pass-by-ref, make sure it is in the right context.
-	 */
-	if (!peraggstate->resulttypeByVal && !*isnull &&
-		!MemoryContextContains(CurrentMemoryContext,
-							   DatumGetPointer(*result)))
-		*result = datumCopy(*result,
-							peraggstate->resulttypeByVal,
-							peraggstate->resulttypeLen);
 	MemoryContextSwitchTo(oldContext);
 }
 
@@ -1034,9 +1035,13 @@ eval_windowfunction(WindowAggState *winstate, WindowStatePerFunc perfuncstate,
 {
 	LOCAL_FCINFO(fcinfo, FUNC_MAX_ARGS);
 	MemoryContext oldContext;
+	WindowResultInfo resultinfo;
 
 	oldContext = MemoryContextSwitchTo(winstate->ss.ps.ps_ExprContext->ecxt_per_tuple_memory);
 
+	resultinfo.resulttypeLen = perfuncstate->resulttypeLen;
+	resultinfo.resulttypeByVal = perfuncstate->resulttypeByVal;
+
 	/*
 	 * We don't pass any normal arguments to a window function, but we do pass
 	 * it the number of arguments, in order to permit window function
@@ -1046,7 +1051,8 @@ eval_windowfunction(WindowAggState *winstate, WindowStatePerFunc perfuncstate,
 	InitFunctionCallInfoData(*fcinfo, &(perfuncstate->flinfo),
 							 perfuncstate->numArguments,
 							 perfuncstate->winCollation,
-							 (void *) perfuncstate->winobj, NULL);
+							 (void *) perfuncstate->winobj,
+							 (void *) &resultinfo);
 	/* Just in case, make all the regular argument slots be null */
 	for (int argno = 0; argno < perfuncstate->numArguments; argno++)
 		fcinfo->args[argno].isnull = true;
@@ -1057,16 +1063,15 @@ eval_windowfunction(WindowAggState *winstate, WindowStatePerFunc perfuncstate,
 	*isnull = fcinfo->isnull;
 
 	/*
-	 * Make sure pass-by-ref data is allocated in the appropriate context. (We
-	 * need this in case the function returns a pointer into some short-lived
-	 * tuple, as is entirely possible.)
+	 * Ensure byref Datums are allocated in the tuple or query context.  The
+	 * result may also be a Const which is stored in the MessageContext or
+	 * cached by some cached plan.
 	 */
-	if (!perfuncstate->resulttypeByVal && !fcinfo->isnull &&
-		!MemoryContextContains(CurrentMemoryContext,
-							   DatumGetPointer(*result)))
-		*result = datumCopy(*result,
-							perfuncstate->resulttypeByVal,
-							perfuncstate->resulttypeLen);
+	Assert(perfuncstate->resulttypeByVal || fcinfo->isnull ||
+		   MemoryContextContains(winstate->ss.ps.ps_ExprContext->ecxt_per_tuple_memory, (void *) *result) ||
+		   MemoryContextContains(winstate->ss.ps.ps_ExprContext->ecxt_per_query_memory, (void *) *result) ||
+		   MemoryContextContains(MessageContext, (void *) *result) ||
+		   MemoryContextOrChildOfContains(CacheMemoryContext, (void *) *result));
 
 	MemoryContextSwitchTo(oldContext);
 }
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 85ee9ec426..f9eaa413e9 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -6643,7 +6643,7 @@ int2int4_sum(PG_FUNCTION_ARGS)
 	if (transdata->count == 0)
 		PG_RETURN_NULL();
 
-	PG_RETURN_DATUM(Int64GetDatumFast(transdata->sum));
+	PG_RETURN_DATUM(Int64GetDatum(transdata->sum));
 }
 
 
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index ec423375ae..a67d72016e 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -1333,6 +1333,28 @@ AllocSetGetChunkContext(void *pointer)
 	return &set->header;
 }
 
+#ifdef MEMORY_CONTEXT_CHECKING
+/*
+ * AllocSetContains
+ *		Attempt to determine if 'pointer' is memory which was allocated by
+ *		'context'.  Return true if it is, otherwise false.
+ */
+bool
+AllocSetContains(MemoryContext context, void *pointer)
+{
+	AllocSet	set = (AllocSet) context;
+
+	for (AllocBlock blk = set->blocks; blk != NULL; blk = blk->next)
+	{
+		/* see if pointer is in range of this block */
+		if ((char *) pointer >= (char *) blk + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ &&
+			(char *) pointer < blk->endptr)
+			return true;
+	}
+	return false;
+}
+#endif							/* MEMORY_CONTEXT_CHECKING */
+
 /*
  * AllocSetGetChunkSpace
  *		Given a currently-allocated chunk, determine the total space
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index c743b24fa7..e44f765f35 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -848,6 +848,32 @@ GenerationGetChunkContext(void *pointer)
 	return &block->context->header;
 }
 
+
+#ifdef MEMORY_CONTEXT_CHECKING
+/*
+ * GenerationContains
+ *		Attempt to determine if 'pointer' is memory which was allocated by
+ *		'context'.  Return true if it is, otherwise false.
+ */
+bool
+GenerationContains(MemoryContext context, void *pointer)
+{
+	GenerationContext *set = (GenerationContext *) context;
+	dlist_iter	iter;
+
+	dlist_foreach(iter, &set->blocks)
+	{
+		GenerationBlock *blk = dlist_container(GenerationBlock, node, iter.cur);
+
+		/* see if pointer is in range of this block */
+		if ((char *) pointer >= (char *) blk + Generation_BLOCKHDRSZ + Generation_CHUNKHDRSZ &&
+			(char *) pointer < blk->endptr)
+			return true;
+	}
+	return false;
+}
+#endif							/* MEMORY_CONTEXT_CHECKING */
+
 /*
  * GenerationGetChunkSpace
  *		Given a currently-allocated chunk, determine the total space
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 115a64cfe4..637cb1a569 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -49,6 +49,7 @@ static const MemoryContextMethods mcxt_methods[] = {
 	[MCTX_ASET_ID].stats = AllocSetStats,
 #ifdef MEMORY_CONTEXT_CHECKING
 	[MCTX_ASET_ID].check = AllocSetCheck,
+	[MCTX_ASET_ID].contains = AllocSetContains,
 #endif
 
 	/* generation.c */
@@ -63,6 +64,7 @@ static const MemoryContextMethods mcxt_methods[] = {
 	[MCTX_GENERATION_ID].stats = GenerationStats,
 #ifdef MEMORY_CONTEXT_CHECKING
 	[MCTX_GENERATION_ID].check = GenerationCheck,
+	[MCTX_GENERATION_ID].contains = GenerationContains,
 #endif
 
 	/* slab.c */
@@ -76,7 +78,8 @@ static const MemoryContextMethods mcxt_methods[] = {
 	[MCTX_SLAB_ID].is_empty = SlabIsEmpty,
 	[MCTX_SLAB_ID].stats = SlabStats
 #ifdef MEMORY_CONTEXT_CHECKING
-	,[MCTX_SLAB_ID].check = SlabCheck
+	,[MCTX_SLAB_ID].check = SlabCheck,
+	[MCTX_SLAB_ID].contains = SlabContains
 #endif
 };
 
@@ -813,48 +816,51 @@ MemoryContextCheck(MemoryContext context)
 }
 #endif
 
+#ifdef MEMORY_CONTEXT_CHECKING
 /*
  * MemoryContextContains
  *		Detect whether an allocated chunk of memory belongs to a given
  *		context or not.
  *
- * Caution: 'pointer' must point to a pointer which was allocated by a
- * MemoryContext.  It's not safe or valid to use this function on arbitrary
- * pointers as obtaining the MemoryContext which 'pointer' belongs to requires
- * possibly several pointer dereferences.
+ * Caution: this test is reliable as long as 'pointer' does point to
+ * a chunk of memory allocated from *some* context.  If 'pointer' points
+ * at memory obtained in some other way, there is a chance of a segmentation
+ * violation due to accessing the chunk header, which look for in the 8 bytes
+ * prior to the pointer.
  */
 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;
+	if (pointer == NULL)
+		return false;
 
-#ifdef NOT_USED
-	MemoryContext ptr_context;
+	return context->methods->contains(context, pointer);
+}
 
-	/*
-	 * NB: We must perform run-time checks here which GetMemoryChunkContext()
-	 * does as assertions before calling GetMemoryChunkContext().
-	 *
-	 * 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.
-	 */
-	if (pointer == NULL || pointer != (void *) MAXALIGN(pointer))
-		return false;
+/*
+ * MemoryContextOrChildOfContains
+ *		Recursive version of MemoryContextContains which checks if any of
+ *		context's child contexts contains the pointer.
+ */
+bool
+MemoryContextOrChildOfContains(MemoryContext context, void *pointer)
+{
+	if (MemoryContextContains(context, pointer))
+		return true;
+	else
+	{
+		MemoryContext child;
 
-	/*
-	 * OK, it's probably safe to look at the context.
-	 */
-	ptr_context = GetMemoryChunkContext(pointer);
+		for (child = context->firstchild; child != NULL; child = child->nextchild)
+		{
+			if (MemoryContextOrChildOfContains(child, pointer))
+				return true;
+		}
+	}
 
-	return ptr_context == context;
-#endif
+	return false;
 }
+#endif							/* MEMORY_CONTEXT_CHECKING */
 
 /*
  * MemoryContextCreate
@@ -969,6 +975,8 @@ MemoryContextAlloc(MemoryContext context, Size size)
 
 	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
+	Assert(MemoryContextContains(context, ret));
+
 	return ret;
 }
 
@@ -1007,6 +1015,8 @@ MemoryContextAllocZero(MemoryContext context, Size size)
 
 	MemSetAligned(ret, 0, size);
 
+	Assert(MemoryContextContains(context, ret));
+
 	return ret;
 }
 
@@ -1045,6 +1055,8 @@ MemoryContextAllocZeroAligned(MemoryContext context, Size size)
 
 	MemSetLoop(ret, 0, size);
 
+	Assert(MemoryContextContains(context, ret));
+
 	return ret;
 }
 
@@ -1086,6 +1098,8 @@ MemoryContextAllocExtended(MemoryContext context, Size size, int flags)
 	if ((flags & MCXT_ALLOC_ZERO) != 0)
 		MemSetAligned(ret, 0, size);
 
+	Assert(MemoryContextContains(context, ret));
+
 	return ret;
 }
 
@@ -1169,6 +1183,8 @@ palloc(Size size)
 
 	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
+	Assert(MemoryContextContains(context, ret));
+
 	return ret;
 }
 
@@ -1202,6 +1218,8 @@ palloc0(Size size)
 
 	MemSetAligned(ret, 0, size);
 
+	Assert(MemoryContextContains(context, ret));
+
 	return ret;
 }
 
@@ -1241,6 +1259,8 @@ palloc_extended(Size size, int flags)
 	if ((flags & MCXT_ALLOC_ZERO) != 0)
 		MemSetAligned(ret, 0, size);
 
+	Assert(MemoryContextContains(context, ret));
+
 	return ret;
 }
 
@@ -1294,6 +1314,8 @@ repalloc(void *pointer, Size size)
 
 	VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
 
+	Assert(MemoryContextContains(context, ret));
+
 	return ret;
 }
 
@@ -1329,6 +1351,8 @@ MemoryContextAllocHuge(MemoryContext context, Size size)
 
 	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
+	Assert(MemoryContextContains(context, ret));
+
 	return ret;
 }
 
@@ -1368,6 +1392,8 @@ repalloc_huge(void *pointer, Size size)
 
 	VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
 
+	Assert(MemoryContextContains(context, ret));
+
 	return ret;
 }
 
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index 9149aaafcb..f5ad699760 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -567,6 +567,35 @@ SlabGetChunkContext(void *pointer)
 	return &slab->header;
 }
 
+#ifdef MEMORY_CONTEXT_CHECKING
+/*
+ * SlabContains
+ *		Attempt to determine if 'pointer' is memory which was allocated by
+ *		'context'.  Return true if it is, otherwise false.
+ */
+bool
+SlabContains(MemoryContext context, void *pointer)
+{
+	SlabContext *set = (SlabContext *) context;
+
+	for (int i = 0; i <= set->chunksPerBlock; i++)
+	{
+		dlist_iter	iter;
+
+		dlist_foreach(iter, &set->freelist[i])
+		{
+			SlabBlock  *blk = dlist_container(SlabBlock, node, iter.cur);
+
+			/* see if pointer is in range of this block */
+			if ((char *) pointer >= (char *) blk + Slab_BLOCKHDRSZ + Slab_CHUNKHDRSZ &&
+				(char *) pointer < (char *) blk + set->blockSize)
+				return true;
+		}
+	}
+	return false;
+}
+#endif							/* MEMORY_CONTEXT_CHECKING */
+
 /*
  * SlabGetChunkSpace
  *		Given a currently-allocated chunk, determine the total space
diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h
index 63d07358cd..c1d6fd5391 100644
--- a/src/include/nodes/memnodes.h
+++ b/src/include/nodes/memnodes.h
@@ -72,6 +72,7 @@ typedef struct MemoryContextMethods
 						  bool print_to_stderr);
 #ifdef MEMORY_CONTEXT_CHECKING
 	void		(*check) (MemoryContext context);
+	bool		(*contains) (MemoryContext context, void *pointer);
 #endif
 } MemoryContextMethods;
 
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index 52bc41ec53..3bfadbd465 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -95,8 +95,10 @@ extern void MemoryContextAllowInCriticalSection(MemoryContext context,
 
 #ifdef MEMORY_CONTEXT_CHECKING
 extern void MemoryContextCheck(MemoryContext context);
-#endif
 extern bool MemoryContextContains(MemoryContext context, void *pointer);
+extern bool MemoryContextOrChildOfContains(MemoryContext context,
+										   void *pointer);
+#endif
 
 /* Handy macro for copying and assigning context ID ... but note double eval */
 #define MemoryContextCopyAndSetIdentifier(cxt, id) \
diff --git a/src/include/utils/memutils_internal.h b/src/include/utils/memutils_internal.h
index 377cee7a84..b767982896 100644
--- a/src/include/utils/memutils_internal.h
+++ b/src/include/utils/memutils_internal.h
@@ -33,6 +33,7 @@ extern void AllocSetStats(MemoryContext context,
 						  bool print_to_stderr);
 #ifdef MEMORY_CONTEXT_CHECKING
 extern void AllocSetCheck(MemoryContext context);
+extern bool AllocSetContains(MemoryContext context, void *pointer);
 #endif
 
 /* These functions implement the MemoryContext API for Generation context. */
@@ -50,6 +51,7 @@ extern void GenerationStats(MemoryContext context,
 							bool print_to_stderr);
 #ifdef MEMORY_CONTEXT_CHECKING
 extern void GenerationCheck(MemoryContext context);
+extern bool GenerationContains(MemoryContext context, void *pointer);
 #endif
 
 
@@ -68,6 +70,7 @@ extern void SlabStats(MemoryContext context,
 					  bool print_to_stderr);
 #ifdef MEMORY_CONTEXT_CHECKING
 extern void SlabCheck(MemoryContext context);
+extern bool SlabContains(MemoryContext context, void *pointer);
 #endif
 
 /*
diff --git a/src/include/utils/memutils_memorychunk.h b/src/include/utils/memutils_memorychunk.h
index 2eefc138e3..a6a0137e5d 100644
--- a/src/include/utils/memutils_memorychunk.h
+++ b/src/include/utils/memutils_memorychunk.h
@@ -107,9 +107,18 @@
 								 MEMORYCHUNK_VALUE_BASEBIT << \
 								 MEMORYCHUNK_VALUE_BASEBIT)
 
+#ifdef MEMORY_CONTEXT_CHECKING
+#if SIZEOF_SIZE_T >= 8
+#define MEMORYCHUNK_MAGIC2		UINT64CONST(0xB1A8DB858EB6EFBA)
+#else
+#define MEMORYCHUNK_MAGIC2		0x8EB6EFBA
+#endif
+#endif
+
 typedef struct MemoryChunk
 {
 #ifdef MEMORY_CONTEXT_CHECKING
+	Size		chunk_magic;	/* magic number */
 	Size		requested_size;
 #endif
 
@@ -161,6 +170,9 @@ MemoryChunkSetHdrMask(MemoryChunk *chunk, void *block,
 	Assert(value <= MEMORYCHUNK_MAX_VALUE);
 	Assert((int) methodid <= MEMORY_CONTEXT_METHODID_MASK);
 
+#ifdef MEMORY_CONTEXT_CHECKING
+	chunk->chunk_magic = MEMORYCHUNK_MAGIC2;
+#endif
 	chunk->hdrmask = (((uint64) blockoffset) << MEMORYCHUNK_BLOCKOFFSET_BASEBIT) |
 		(((uint64) value) << MEMORYCHUNK_VALUE_BASEBIT) |
 		methodid;
@@ -177,6 +189,9 @@ MemoryChunkSetHdrMaskExternal(MemoryChunk *chunk,
 {
 	Assert((int) methodid <= MEMORY_CONTEXT_METHODID_MASK);
 
+#ifdef MEMORY_CONTEXT_CHECKING
+	chunk->chunk_magic = MEMORYCHUNK_MAGIC2;
+#endif
 	chunk->hdrmask = MEMORYCHUNK_MAGIC | (((uint64) 1) << MEMORYCHUNK_EXTERNAL_BASEBIT) |
 		methodid;
 }
@@ -194,6 +209,7 @@ MemoryChunkIsExternal(MemoryChunk *chunk)
 	 */
 	Assert(!HdrMaskIsExternal(chunk->hdrmask) ||
 		   HdrMaskCheckMagic(chunk->hdrmask));
+	Assert(chunk->chunk_magic == MEMORYCHUNK_MAGIC2);
 
 	return HdrMaskIsExternal(chunk->hdrmask);
 }
@@ -207,6 +223,7 @@ static inline Size
 MemoryChunkGetValue(MemoryChunk *chunk)
 {
 	Assert(!HdrMaskIsExternal(chunk->hdrmask));
+	Assert(chunk->chunk_magic == MEMORYCHUNK_MAGIC2);
 
 	return HdrMaskGetValue(chunk->hdrmask);
 }
@@ -220,6 +237,7 @@ static inline void *
 MemoryChunkGetBlock(MemoryChunk *chunk)
 {
 	Assert(!HdrMaskIsExternal(chunk->hdrmask));
+	Assert(chunk->chunk_magic == MEMORYCHUNK_MAGIC2);
 
 	return (void *) ((char *) chunk - HdrMaskBlockOffset(chunk->hdrmask));
 }
@@ -229,6 +247,7 @@ MemoryChunkGetBlock(MemoryChunk *chunk)
 #undef MEMORYCHUNK_VALUE_BASEBIT
 #undef MEMORYCHUNK_BLOCKOFFSET_BASEBIT
 #undef MEMORYCHUNK_MAGIC
+#undef MEMORYCHUNK_MAGIC2
 #undef HdrMaskIsExternal
 #undef HdrMaskGetValue
 #undef HdrMaskBlockOffset
diff --git a/src/include/windowapi.h b/src/include/windowapi.h
index 5a620a2e42..c3d65b161c 100644
--- a/src/include/windowapi.h
+++ b/src/include/windowapi.h
@@ -36,7 +36,19 @@
 /* this struct is private in nodeWindowAgg.c */
 typedef struct WindowObjectData *WindowObject;
 
+/*
+ * Evaluation of window functions are passed this object and it's available
+ * via fcinfo->resultinfo and can be accessed in the window function via the
+ * PG_WINDOW_RESULTINFO macro.
+ */
+typedef struct WindowResultInfo
+{
+	int16		resulttypeLen;	/* type length of wfunc's result type */
+	bool		resulttypeByVal;	/* true if wfunc's result is byval */
+} WindowResultInfo;
+
 #define PG_WINDOW_OBJECT() ((WindowObject) fcinfo->context)
+#define PG_WINDOW_RESULTINFO() ((WindowResultInfo *) fcinfo->resultinfo)
 
 #define WindowObjectIsValid(winobj) \
 	((winobj) != NULL && IsA(winobj, WindowObjectData))