Ensure proper alignment of tuples in HashMemoryChunkData buffers.

The previous coding relied (without any documentation) on the data[]
member of HashMemoryChunkData being at a MAXALIGN'ed offset.  If it
was not, the tuples would not be maxaligned either, leading to failures
on alignment-picky machines.  While there seems to be no live bug on any
platform we support, this is clearly pretty fragile: any addition to or
rearrangement of the fields in HashMemoryChunkData could break it.
Let's remove the hazard by getting rid of the data[] member and instead
using pointer arithmetic with an explicitly maxalign'ed offset.

Discussion: https://postgr.es/m/14483.1514938129@sss.pgh.pa.us

1 files changed, 15 insertions, 19 deletions

diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index 4e1a2806b55..38a84cc14cf 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -979,7 +979,7 @@ ExecHashIncreaseNumBatches(HashJoinTable hashtable)
 		/* process all tuples stored in this chunk (and then free it) */
 		while (idx < oldchunks->used)
 		{
-			HashJoinTuple hashTuple = (HashJoinTuple) (oldchunks->data + idx);
+			HashJoinTuple hashTuple = (HashJoinTuple) (HASH_CHUNK_DATA(oldchunks) + idx);
 			MinimalTuple tuple = HJTUPLE_MINTUPLE(hashTuple);
 			int			hashTupleSize = (HJTUPLE_OVERHEAD + tuple->t_len);
 			int			bucketno;
@@ -1285,7 +1285,7 @@ ExecParallelHashRepartitionFirst(HashJoinTable hashtable)
 		/* Repartition all tuples in this chunk. */
 		while (idx < chunk->used)
 		{
-			HashJoinTuple hashTuple = (HashJoinTuple) (chunk->data + idx);
+			HashJoinTuple hashTuple = (HashJoinTuple) (HASH_CHUNK_DATA(chunk) + idx);
 			MinimalTuple tuple = HJTUPLE_MINTUPLE(hashTuple);
 			HashJoinTuple copyTuple;
 			dsa_pointer shared;
@@ -1469,7 +1469,7 @@ ExecHashIncreaseNumBuckets(HashJoinTable hashtable)
 
 		while (idx < chunk->used)
 		{
-			HashJoinTuple hashTuple = (HashJoinTuple) (chunk->data + idx);
+			HashJoinTuple hashTuple = (HashJoinTuple) (HASH_CHUNK_DATA(chunk) + idx);
 			int			bucketno;
 			int			batchno;
 
@@ -1552,7 +1552,7 @@ ExecParallelHashIncreaseNumBuckets(HashJoinTable hashtable)
 
 				while (idx < chunk->used)
 				{
-					HashJoinTuple hashTuple = (HashJoinTuple) (chunk->data + idx);
+					HashJoinTuple hashTuple = (HashJoinTuple) (HASH_CHUNK_DATA(chunk) + idx);
 					dsa_pointer shared = chunk_s + HASH_CHUNK_HEADER_SIZE + idx;
 					int			bucketno;
 					int			batchno;
@@ -2651,17 +2651,16 @@ dense_alloc(HashJoinTable hashtable, Size size)
 	size = MAXALIGN(size);
 
 	/*
-	 * If tuple size is larger than of 1/4 of chunk size, allocate a separate
-	 * chunk.
+	 * If tuple size is larger than threshold, allocate a separate chunk.
 	 */
 	if (size > HASH_CHUNK_THRESHOLD)
 	{
 		/* allocate new chunk and put it at the beginning of the list */
 		newChunk = (HashMemoryChunk) MemoryContextAlloc(hashtable->batchCxt,
-														offsetof(HashMemoryChunkData, data) + size);
+														HASH_CHUNK_HEADER_SIZE + size);
 		newChunk->maxlen = size;
-		newChunk->used = 0;
-		newChunk->ntuples = 0;
+		newChunk->used = size;
+		newChunk->ntuples = 1;
 
 		/*
 		 * Add this chunk to the list after the first existing chunk, so that
@@ -2678,10 +2677,7 @@ dense_alloc(HashJoinTable hashtable, Size size)
 			hashtable->chunks = newChunk;
 		}
 
-		newChunk->used += size;
-		newChunk->ntuples += 1;
-
-		return newChunk->data;
+		return HASH_CHUNK_DATA(newChunk);
 	}
 
 	/*
@@ -2693,7 +2689,7 @@ dense_alloc(HashJoinTable hashtable, Size size)
 	{
 		/* allocate new chunk and put it at the beginning of the list */
 		newChunk = (HashMemoryChunk) MemoryContextAlloc(hashtable->batchCxt,
-														offsetof(HashMemoryChunkData, data) + HASH_CHUNK_SIZE);
+														HASH_CHUNK_HEADER_SIZE + HASH_CHUNK_SIZE);
 
 		newChunk->maxlen = HASH_CHUNK_SIZE;
 		newChunk->used = size;
@@ -2702,11 +2698,11 @@ dense_alloc(HashJoinTable hashtable, Size size)
 		newChunk->next.unshared = hashtable->chunks;
 		hashtable->chunks = newChunk;
 
-		return newChunk->data;
+		return HASH_CHUNK_DATA(newChunk);
 	}
 
 	/* There is enough space in the current chunk, let's add the tuple */
-	ptr = hashtable->chunks->data + hashtable->chunks->used;
+	ptr = HASH_CHUNK_DATA(hashtable->chunks) + hashtable->chunks->used;
 	hashtable->chunks->used += size;
 	hashtable->chunks->ntuples += 1;
 
@@ -2751,7 +2747,7 @@ ExecParallelHashTupleAlloc(HashJoinTable hashtable, size_t size,
 		chunk_shared = hashtable->current_chunk_shared;
 		Assert(chunk == dsa_get_address(hashtable->area, chunk_shared));
 		*shared = chunk_shared + HASH_CHUNK_HEADER_SIZE + chunk->used;
-		result = (HashJoinTuple) (chunk->data + chunk->used);
+		result = (HashJoinTuple) (HASH_CHUNK_DATA(chunk) + chunk->used);
 		chunk->used += size;
 
 		Assert(chunk->used <= chunk->maxlen);
@@ -2859,8 +2855,8 @@ ExecParallelHashTupleAlloc(HashJoinTable hashtable, size_t size,
 	}
 	LWLockRelease(&pstate->lock);
 
-	Assert(chunk->data == dsa_get_address(hashtable->area, *shared));
-	result = (HashJoinTuple) chunk->data;
+	Assert(HASH_CHUNK_DATA(chunk) == dsa_get_address(hashtable->area, *shared));
+	result = (HashJoinTuple) HASH_CHUNK_DATA(chunk);
 
 	return result;
 }