Use MAXALIGN() in calculations using sizeof(SlabBlock)

c6e0fe1f2 added a new pointer field to SlabBlock to make it 4 bytes larger
on 32-bit machines.  Prior to that commit, the size of that struct was a
multiple of 8, which meant that MAXALIGN(sizeof(SlabBlock)) was the same
as sizeof(SlabBlock), however, after c6e0fe1f2, due to the addition of the
new pointer field to store a pointer to the owning context, that was no
longer true on builds with sizeof(void *) == 4.

This problem was highlighted by an Assert failure which was checking that
the pointer given to pfree() was MAXALIGNED.  Various 32-bit ARM buildfarm
animals were failing.  These have MAXIMUM_ALIGNOF of 8.  The only 32-bit
testing I'd managed to do on c6e0fe1f2 had been on x86, which has a
MAXIMUM_ALIGNOF of 4, therefore did not exhibit this issue.

Here we define Slab_BLOCKHDRSZ and copy what is being done in aset.c and
generation.c for doing calculations based on the size of the context's
block type.  This means that SlabAlloc() will now always return a
MAXALIGNed pointer.

This also fixes an incorrect sentinel_ok() check in SlabCheck() which was
incorrectly checking the wrong sentinel byte.  This must have previously
not caused any issues due to the fullChunkSize never being large enough to
store the sentinel byte.

Diagnosed-by: Tomas Vondra, Tom Lane
Author: Tomas Vondra, David Rowley
Discussion: https://postgr.es/m/CAA4eK1%2B1JyW5TiL%3DyV-3Uq1CrfnTyn0Xrk5uArt31Z%3D8rgPhXQ%40mail.gmail.com

diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index ae1a735b8cb5511d290b86c07ef1cf0125ccf9a7..2d70adef09d804b3e945ced1255b619ec8eebc8d 100644 (file)
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -58,6 +58,8 @@
 #include "utils/memutils_memorychunk.h"
 #include "utils/memutils_internal.h"
 
+#define Slab_BLOCKHDRSZ    MAXALIGN(sizeof(SlabBlock))
+
 /*
  * SlabContext is a specialized implementation of MemoryContext.
  */
@@ -102,10 +104,10 @@ typedef struct SlabBlock
 #define SlabChunkGetPointer(chk)   \
    ((void *)(((char *)(chk)) + sizeof(MemoryChunk)))
 #define SlabBlockGetChunk(slab, block, idx) \
-   ((MemoryChunk *) ((char *) (block) + sizeof(SlabBlock)  \
+   ((MemoryChunk *) ((char *) (block) + Slab_BLOCKHDRSZ    \
                    + (idx * slab->fullChunkSize)))
 #define SlabBlockStart(block)  \
-   ((char *) block + sizeof(SlabBlock))
+   ((char *) block + Slab_BLOCKHDRSZ)
 #define SlabChunkIndex(slab, block, chunk) \
    (((char *) chunk - SlabBlockStart(block)) / slab->fullChunkSize)
 
@@ -146,12 +148,12 @@ SlabContextCreate(MemoryContext parent,
    fullChunkSize = Slab_CHUNKHDRSZ + MAXALIGN(chunkSize);
 
    /* Make sure the block can store at least one chunk. */
-   if (blockSize < fullChunkSize + sizeof(SlabBlock))
+   if (blockSize < fullChunkSize + Slab_BLOCKHDRSZ)
        elog(ERROR, "block size %zu for slab is too small for %zu chunks",
             blockSize, chunkSize);
 
    /* Compute maximum number of chunks per block */
-   chunksPerBlock = (blockSize - sizeof(SlabBlock)) / fullChunkSize;
+   chunksPerBlock = (blockSize - Slab_BLOCKHDRSZ) / fullChunkSize;
 
    /* The freelist starts with 0, ends with chunksPerBlock. */
    freelistSize = sizeof(dlist_head) * (chunksPerBlock + 1);
@@ -744,7 +746,7 @@ SlabCheck(MemoryContext context)
 
                    /* there might be sentinel (thanks to alignment) */
                    if (slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ))
-                       if (!sentinel_ok(chunk, slab->chunkSize))
+                       if (!sentinel_ok(chunk, Slab_CHUNKHDRSZ + slab->chunkSize))
                            elog(WARNING, "problem in slab %s: detected write past chunk end in block %p, chunk %p",
                                 name, block, chunk);
                }