Avoid using potentially-under-aligned page buffers.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 1 Sep 2018 19:27:13 +0000 (15:27 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 1 Sep 2018 19:27:13 +0000 (15:27 -0400)
There's a project policy against using plain "char buf[BLCKSZ]" local
or static variables as page buffers; preferred style is to palloc or
malloc each buffer to ensure it is MAXALIGN'd.  However, that policy's
been ignored in an increasing number of places.  We've apparently got
away with it so far, probably because (a) relatively few people use
platforms on which misalignment causes core dumps and/or (b) the
variables chance to be sufficiently aligned anyway.  But this is not
something to rely on.  Moreover, even if we don't get a core dump,
we might be paying a lot of cycles for misaligned accesses.

To fix, invent new union types PGAlignedBlock and PGAlignedXLogBlock
that the compiler must allocate with sufficient alignment, and use
those in place of plain char arrays.

I used these types even for variables where there's no risk of a
misaligned access, since ensuring proper alignment should make
kernel data transfers faster.  I also changed some places where
we had been palloc'ing short-lived buffers, for coding style
uniformity and to save palloc/pfree overhead.

Since this seems to be a live portability hazard (despite the lack
of field reports), back-patch to all supported versions.

Patch by me; thanks to Michael Paquier for review.

Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de

20 files changed:
contrib/bloom/blinsert.c
contrib/pg_prewarm/pg_prewarm.c
src/backend/access/gin/ginentrypage.c
src/backend/access/gin/ginfast.c
src/backend/access/hash/hashpage.c
src/backend/access/heap/heapam.c
src/backend/access/heap/visibilitymap.c
src/backend/access/transam/generic_xlog.c
src/backend/access/transam/xlog.c
src/backend/access/transam/xloginsert.c
src/backend/access/transam/xlogreader.c
src/backend/commands/tablecmds.c
src/backend/replication/walsender.c
src/backend/storage/file/buffile.c
src/backend/storage/freespace/freespace.c
src/bin/pg_basebackup/receivelog.c
src/bin/pg_resetxlog/pg_resetxlog.c
src/bin/pg_rewind/copy_fetch.c
src/bin/pg_upgrade/file.c
src/include/c.h

index 946902a61d90a82a49509c74a3b34e74c4172b55..3a6e99466a15fdaa3d94cd8f76f3dadff3dcf4fc 100644 (file)
@@ -36,7 +36,7 @@ typedef struct
    int64       indtuples;      /* total number of tuples indexed */
    MemoryContext tmpCtx;       /* temporary memory context reset after each
                                 * tuple */
-   char        data[BLCKSZ];   /* cached page */
+   PGAlignedBlock data;        /* cached page */
    int         count;          /* number of tuples in cached page */
 } BloomBuildState;
 
@@ -52,7 +52,7 @@ flushCachedPage(Relation index, BloomBuildState *buildstate)
 
    state = GenericXLogStart(index);
    page = GenericXLogRegisterBuffer(state, buffer, GENERIC_XLOG_FULL_IMAGE);
-   memcpy(page, buildstate->data, BLCKSZ);
+   memcpy(page, buildstate->data.data, BLCKSZ);
    GenericXLogFinish(state);
    UnlockReleaseBuffer(buffer);
 }
@@ -63,8 +63,8 @@ flushCachedPage(Relation index, BloomBuildState *buildstate)
 static void
 initCachedPage(BloomBuildState *buildstate)
 {
-   memset(buildstate->data, 0, BLCKSZ);
-   BloomInitPage(buildstate->data, 0);
+   memset(buildstate->data.data, 0, BLCKSZ);
+   BloomInitPage(buildstate->data.data, 0);
    buildstate->count = 0;
 }
 
@@ -84,7 +84,7 @@ bloomBuildCallback(Relation index, HeapTuple htup, Datum *values,
    itup = BloomFormTuple(&buildstate->blstate, &htup->t_self, values, isnull);
 
    /* Try to add next item to cached page */
-   if (BloomPageAddItem(&buildstate->blstate, buildstate->data, itup))
+   if (BloomPageAddItem(&buildstate->blstate, buildstate->data.data, itup))
    {
        /* Next item was added successfully */
        buildstate->count++;
@@ -98,7 +98,7 @@ bloomBuildCallback(Relation index, HeapTuple htup, Datum *values,
 
        initCachedPage(buildstate);
 
-       if (!BloomPageAddItem(&buildstate->blstate, buildstate->data, itup))
+       if (!BloomPageAddItem(&buildstate->blstate, buildstate->data.data, itup))
        {
            /* We shouldn't be here since we're inserting to the empty page */
            elog(ERROR, "could not add new bloom tuple to empty page");
index c3a518cfc290632fd5c11944d166444695076eb6..979e0b0d1075f33df64f1b12de433951dcb57c0b 100644 (file)
@@ -37,7 +37,7 @@ typedef enum
    PREWARM_BUFFER
 } PrewarmType;
 
-static char blockbuffer[BLCKSZ];
+static PGAlignedBlock blockbuffer;
 
 /*
  * pg_prewarm(regclass, mode text, fork text,
@@ -179,7 +179,7 @@ pg_prewarm(PG_FUNCTION_ARGS)
        for (block = first_block; block <= last_block; ++block)
        {
            CHECK_FOR_INTERRUPTS();
-           smgrread(rel->rd_smgr, forkNumber, block, blockbuffer);
+           smgrread(rel->rd_smgr, forkNumber, block, blockbuffer.data);
            ++blocks_done;
        }
    }
index 8c0bfe9fdebe265a3bb3e84028a158ccc738e183..906a3fe761d250c10ca532f5b2b2e35ada036d69 100644 (file)
@@ -615,7 +615,7 @@ entrySplitPage(GinBtree btree, Buffer origbuf,
    Page        lpage = PageGetTempPageCopy(BufferGetPage(origbuf));
    Page        rpage = PageGetTempPageCopy(BufferGetPage(origbuf));
    Size        pageSize = PageGetPageSize(lpage);
-   char        tupstore[2 * BLCKSZ];
+   PGAlignedBlock tupstore[2]; /* could need 2 pages' worth of tuples */
 
    entryPreparePage(btree, lpage, off, insertData, updateblkno);
 
@@ -624,7 +624,7 @@ entrySplitPage(GinBtree btree, Buffer origbuf,
     * one after another in a temporary workspace.
     */
    maxoff = PageGetMaxOffsetNumber(lpage);
-   ptr = tupstore;
+   ptr = tupstore[0].data;
    for (i = FirstOffsetNumber; i <= maxoff; i++)
    {
        if (i == off)
@@ -657,7 +657,7 @@ entrySplitPage(GinBtree btree, Buffer origbuf,
    GinInitPage(rpage, GinPageGetOpaque(lpage)->flags, pageSize);
    GinInitPage(lpage, GinPageGetOpaque(rpage)->flags, pageSize);
 
-   ptr = tupstore;
+   ptr = tupstore[0].data;
    maxoff++;
    lsize = 0;
 
index 56a3ff590a45f00088ff9f0fd9fece0809bbe2fe..ec708136c0eaa577da1355d457527435db18f8c4 100644 (file)
@@ -61,18 +61,15 @@ writeListPage(Relation index, Buffer buffer,
                size = 0;
    OffsetNumber l,
                off;
-   char       *workspace;
+   PGAlignedBlock workspace;
    char       *ptr;
 
-   /* workspace could be a local array; we use palloc for alignment */
-   workspace = palloc(BLCKSZ);
-
    START_CRIT_SECTION();
 
    GinInitBuffer(buffer, GIN_LIST);
 
    off = FirstOffsetNumber;
-   ptr = workspace;
+   ptr = workspace.data;
 
    for (i = 0; i < ntuples; i++)
    {
@@ -124,7 +121,7 @@ writeListPage(Relation index, Buffer buffer,
        XLogRegisterData((char *) &data, sizeof(ginxlogInsertListPage));
 
        XLogRegisterBuffer(0, buffer, REGBUF_WILL_INIT);
-       XLogRegisterBufData(0, workspace, size);
+       XLogRegisterBufData(0, workspace.data, size);
 
        recptr = XLogInsert(RM_GIN_ID, XLOG_GIN_INSERT_LISTPAGE);
        PageSetLSN(page, recptr);
@@ -137,8 +134,6 @@ writeListPage(Relation index, Buffer buffer,
 
    END_CRIT_SECTION();
 
-   pfree(workspace);
-
    return freesize;
 }
 
index 178463fcb65bbded579271a0c0360d6804833803..2d20ddc0d13e03ddd33555fccd56ac0ef7f674ce 100644 (file)
@@ -710,7 +710,7 @@ static bool
 _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
 {
    BlockNumber lastblock;
-   char        zerobuf[BLCKSZ];
+   PGAlignedBlock zerobuf;
 
    lastblock = firstblock + nblocks - 1;
 
@@ -721,10 +721,10 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
    if (lastblock < firstblock || lastblock == InvalidBlockNumber)
        return false;
 
-   MemSet(zerobuf, 0, sizeof(zerobuf));
+   MemSet(zerobuf.data, 0, sizeof(zerobuf));
 
    RelationOpenSmgr(rel);
-   smgrextend(rel->rd_smgr, MAIN_FORKNUM, lastblock, zerobuf, false);
+   smgrextend(rel->rd_smgr, MAIN_FORKNUM, lastblock, zerobuf.data, false);
 
    return true;
 }
index 9fb68cd7b3a271adeb39fe378d8c4b5bc4572ef9..4109ce4e4fa6fe23442c1e72633e5671d3ec74a2 100644 (file)
@@ -2637,7 +2637,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
    HeapTuple  *heaptuples;
    int         i;
    int         ndone;
-   char       *scratch = NULL;
+   PGAlignedBlock scratch;
    Page        page;
    bool        needwal;
    Size        saveFreeSpace;
@@ -2654,14 +2654,6 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
        heaptuples[i] = heap_prepare_insert(relation, tuples[i],
                                            xid, cid, options);
 
-   /*
-    * Allocate some memory to use for constructing the WAL record. Using
-    * palloc() within a critical section is not safe, so we allocate this
-    * beforehand.
-    */
-   if (needwal)
-       scratch = palloc(BLCKSZ);
-
    /*
     * We're about to do the actual inserts -- but check for conflict first,
     * to minimize the possibility of having to roll back work we've just
@@ -2754,7 +2746,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
            uint8       info = XLOG_HEAP2_MULTI_INSERT;
            char       *tupledata;
            int         totaldatalen;
-           char       *scratchptr = scratch;
+           char       *scratchptr = scratch.data;
            bool        init;
            int         bufflags = 0;
 
@@ -2813,7 +2805,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
                scratchptr += datalen;
            }
            totaldatalen = scratchptr - tupledata;
-           Assert((scratchptr - scratch) < BLCKSZ);
+           Assert((scratchptr - scratch.data) < BLCKSZ);
 
            if (need_tuple_data)
                xlrec->flags |= XLH_INSERT_CONTAINS_NEW_TUPLE;
@@ -2840,7 +2832,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
                bufflags |= REGBUF_KEEP_DATA;
 
            XLogBeginInsert();
-           XLogRegisterData((char *) xlrec, tupledata - scratch);
+           XLogRegisterData((char *) xlrec, tupledata - scratch.data);
            XLogRegisterBuffer(0, buffer, REGBUF_STANDARD | bufflags);
 
            XLogRegisterBufData(0, tupledata, totaldatalen);
index cd5c6d0e9afc9da5d2cbab78681ef114496c862e..7d7585850bbaf91f7a47cc524713c002fe250bcb 100644 (file)
@@ -645,10 +645,9 @@ static void
 vm_extend(Relation rel, BlockNumber vm_nblocks)
 {
    BlockNumber vm_nblocks_now;
-   Page        pg;
+   PGAlignedBlock pg;
 
-   pg = (Page) palloc(BLCKSZ);
-   PageInit(pg, BLCKSZ, 0);
+   PageInit((Page) pg.data, BLCKSZ, 0);
 
    /*
     * We use the relation extension lock to lock out other backends trying to
@@ -679,10 +678,10 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
    /* Now extend the file */
    while (vm_nblocks_now < vm_nblocks)
    {
-       PageSetChecksumInplace(pg, vm_nblocks_now);
+       PageSetChecksumInplace((Page) pg.data, vm_nblocks_now);
 
        smgrextend(rel->rd_smgr, VISIBILITYMAP_FORKNUM, vm_nblocks_now,
-                  (char *) pg, false);
+                  pg.data, false);
        vm_nblocks_now++;
    }
 
@@ -699,6 +698,4 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
    rel->rd_smgr->smgr_vm_nblocks = vm_nblocks_now;
 
    UnlockRelationForExtension(rel, ExclusiveLock);
-
-   pfree(pg);
 }
index 1926d98de00515e87952c2e57586603de9da400c..9cb9fb48848eca9653a33cc55122f6b1f9376113 100644 (file)
@@ -60,14 +60,11 @@ typedef struct
 /* State of generic xlog record construction */
 struct GenericXLogState
 {
-   /*
-    * page's images. Should be first in this struct to have MAXALIGN'ed
-    * images addresses, because some code working with pages directly aligns
-    * addresses, not offsets from beginning of page
-    */
-   char        images[MAX_GENERIC_XLOG_PAGES * BLCKSZ];
+   /* Info about each page, see above */
    PageData    pages[MAX_GENERIC_XLOG_PAGES];
    bool        isLogged;
+   /* Page images (properly aligned) */
+   PGAlignedBlock images[MAX_GENERIC_XLOG_PAGES];
 };
 
 static void writeFragment(PageData *pageData, OffsetNumber offset,
@@ -250,12 +247,12 @@ computeDelta(PageData *pageData, Page curpage, Page targetpage)
 #ifdef WAL_DEBUG
    if (XLOG_DEBUG)
    {
-       char        tmp[BLCKSZ];
+       PGAlignedBlock tmp;
 
-       memcpy(tmp, curpage, BLCKSZ);
-       applyPageRedo(tmp, pageData->delta, pageData->deltaLen);
-       if (memcmp(tmp, targetpage, targetLower) != 0 ||
-           memcmp(tmp + targetUpper, targetpage + targetUpper,
+       memcpy(tmp.data, curpage, BLCKSZ);
+       applyPageRedo(tmp.data, pageData->delta, pageData->deltaLen);
+       if (memcmp(tmp.data, targetpage, targetLower) != 0 ||
+           memcmp(tmp.data + targetUpper, targetpage + targetUpper,
                   BLCKSZ - targetUpper) != 0)
            elog(ERROR, "result of generic xlog apply does not match");
    }
@@ -276,7 +273,7 @@ GenericXLogStart(Relation relation)
 
    for (i = 0; i < MAX_GENERIC_XLOG_PAGES; i++)
    {
-       state->pages[i].image = state->images + BLCKSZ * i;
+       state->pages[i].image = state->images[i].data;
        state->pages[i].buffer = InvalidBuffer;
    }
 
index 47fe5a9290173cb2eb82ec2d0bc1d5e72259d8b4..724ff17a3ff0c19afa33e30c8ed3d7eb2948ee26 100644 (file)
@@ -3029,8 +3029,7 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
 {
    char        path[MAXPGPATH];
    char        tmppath[MAXPGPATH];
-   char        zbuffer_raw[XLOG_BLCKSZ + MAXIMUM_ALIGNOF];
-   char       *zbuffer;
+   PGAlignedXLogBlock zbuffer;
    XLogSegNo   installed_segno;
    XLogSegNo   max_segno;
    int         fd;
@@ -3084,16 +3083,12 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
     * fsync below) that all the indirect blocks are down on disk.  Therefore,
     * fdatasync(2) or O_DSYNC will be sufficient to sync future writes to the
     * log file.
-    *
-    * Note: ensure the buffer is reasonably well-aligned; this may save a few
-    * cycles transferring data to the kernel.
     */
-   zbuffer = (char *) MAXALIGN(zbuffer_raw);
-   memset(zbuffer, 0, XLOG_BLCKSZ);
+   memset(zbuffer.data, 0, XLOG_BLCKSZ);
    for (nbytes = 0; nbytes < XLogSegSize; nbytes += XLOG_BLCKSZ)
    {
        errno = 0;
-       if ((int) write(fd, zbuffer, XLOG_BLCKSZ) != (int) XLOG_BLCKSZ)
+       if ((int) write(fd, zbuffer.data, XLOG_BLCKSZ) != (int) XLOG_BLCKSZ)
        {
            int         save_errno = errno;
 
@@ -3198,7 +3193,7 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
 {
    char        path[MAXPGPATH];
    char        tmppath[MAXPGPATH];
-   char        buffer[XLOG_BLCKSZ];
+   PGAlignedXLogBlock buffer;
    int         srcfd;
    int         fd;
    int         nbytes;
@@ -3242,14 +3237,14 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
         * zeros.
         */
        if (nread < sizeof(buffer))
-           memset(buffer, 0, sizeof(buffer));
+           memset(buffer.data, 0, sizeof(buffer));
 
        if (nread > 0)
        {
            if (nread > sizeof(buffer))
                nread = sizeof(buffer);
            errno = 0;
-           if (read(srcfd, buffer, nread) != nread)
+           if (read(srcfd, buffer.data, nread) != nread)
            {
                if (errno != 0)
                    ereport(ERROR,
@@ -3263,7 +3258,7 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
            }
        }
        errno = 0;
-       if ((int) write(fd, buffer, sizeof(buffer)) != (int) sizeof(buffer))
+       if ((int) write(fd, buffer.data, sizeof(buffer)) != (int) sizeof(buffer))
        {
            int         save_errno = errno;
 
index a92534e2256f2fb7051f048fe4673858155a89dd..d141944133460028a9ebac7433b3835d8505e077 100644 (file)
@@ -774,12 +774,12 @@ XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length,
    int32       len;
    int32       extra_bytes = 0;
    char       *source;
-   char        tmp[BLCKSZ];
+   PGAlignedBlock tmp;
 
    if (hole_length != 0)
    {
        /* must skip the hole */
-       source = tmp;
+       source = tmp.data;
        memcpy(source, page, hole_offset);
        memcpy(source + hole_offset,
               page + (hole_offset + hole_length),
@@ -882,7 +882,7 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std)
    if (lsn <= RedoRecPtr)
    {
        int         flags;
-       char        copied_buffer[BLCKSZ];
+       PGAlignedBlock copied_buffer;
        char       *origdata = (char *) BufferGetBlock(buffer);
        RelFileNode rnode;
        ForkNumber  forkno;
@@ -900,11 +900,11 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std)
            uint16      lower = ((PageHeader) page)->pd_lower;
            uint16      upper = ((PageHeader) page)->pd_upper;
 
-           memcpy(copied_buffer, origdata, lower);
-           memcpy(copied_buffer + upper, origdata + upper, BLCKSZ - upper);
+           memcpy(copied_buffer.data, origdata, lower);
+           memcpy(copied_buffer.data + upper, origdata + upper, BLCKSZ - upper);
        }
        else
-           memcpy(copied_buffer, origdata, BLCKSZ);
+           memcpy(copied_buffer.data, origdata, BLCKSZ);
 
        XLogBeginInsert();
 
@@ -913,7 +913,7 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std)
            flags |= REGBUF_STANDARD;
 
        BufferGetTag(buffer, &rnode, &forkno, &blkno);
-       XLogRegisterBlock(0, &rnode, forkno, blkno, copied_buffer, flags);
+       XLogRegisterBlock(0, &rnode, forkno, blkno, copied_buffer.data, flags);
 
        recptr = XLogInsert(RM_XLOG_ID, XLOG_FPI_FOR_HINT);
    }
index 8e0d17d0f515dd430ac23e123f8ba9276156267f..522f7585814fc06eb19895b57dcc490193772ace 100644 (file)
@@ -1395,7 +1395,7 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page)
 {
    DecodedBkpBlock *bkpb;
    char       *ptr;
-   char        tmp[BLCKSZ];
+   PGAlignedBlock tmp;
 
    if (!record->blocks[block_id].in_use)
        return false;
@@ -1408,7 +1408,7 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page)
    if (bkpb->bimg_info & BKPIMAGE_IS_COMPRESSED)
    {
        /* If a backup block image is compressed, decompress it */
-       if (pglz_decompress(ptr, bkpb->bimg_len, tmp,
+       if (pglz_decompress(ptr, bkpb->bimg_len, tmp.data,
                            BLCKSZ - bkpb->hole_length) < 0)
        {
            report_invalid_record(record, "invalid compressed image at %X/%X, block %d",
@@ -1417,7 +1417,7 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page)
                                  block_id);
            return false;
        }
-       ptr = tmp;
+       ptr = tmp.data;
    }
 
    /* generate page, taking into account hole if necessary */
index 9a82313e2f4a6fa5641fdb8640f7e4bd0c577569..29e648bdae4a0371a472ca1a3a074a7bc1cd7bc3 100644 (file)
@@ -10001,21 +10001,14 @@ static void
 copy_relation_data(SMgrRelation src, SMgrRelation dst,
                   ForkNumber forkNum, char relpersistence)
 {
-   char       *buf;
+   PGAlignedBlock buf;
    Page        page;
    bool        use_wal;
    bool        copying_initfork;
    BlockNumber nblocks;
    BlockNumber blkno;
 
-   /*
-    * palloc the buffer so that it's MAXALIGN'd.  If it were just a local
-    * char[] array, the compiler might align it on any byte boundary, which
-    * can seriously hurt transfer speed to and from the kernel; not to
-    * mention possibly making log_newpage's accesses to the page header fail.
-    */
-   buf = (char *) palloc(BLCKSZ);
-   page = (Page) buf;
+   page = (Page) buf.data;
 
    /*
     * The init fork for an unlogged relation in many respects has to be
@@ -10039,7 +10032,7 @@ copy_relation_data(SMgrRelation src, SMgrRelation dst,
        /* If we got a cancel signal during the copy of the data, quit */
        CHECK_FOR_INTERRUPTS();
 
-       smgrread(src, forkNum, blkno, buf);
+       smgrread(src, forkNum, blkno, buf.data);
 
        if (!PageIsVerified(page, blkno))
            ereport(ERROR,
@@ -10065,11 +10058,9 @@ copy_relation_data(SMgrRelation src, SMgrRelation dst,
         * rel, because there's no need for smgr to schedule an fsync for this
         * write; we'll do it ourselves below.
         */
-       smgrextend(dst, forkNum, blkno, buf, true);
+       smgrextend(dst, forkNum, blkno, buf.data, true);
    }
 
-   pfree(buf);
-
    /*
     * If the rel is WAL-logged, must fsync before commit.  We use heap_sync
     * to ensure that the toast table gets fsync'd too.  (For a temp or
index 6645ed860815c5f632078e3e3ab3c53029c94dc1..0447136cc55a28122097c5d693f148e7164d0414 100644 (file)
@@ -497,16 +497,16 @@ SendTimeLineHistory(TimeLineHistoryCmd *cmd)
    bytesleft = histfilelen;
    while (bytesleft > 0)
    {
-       char        rbuf[BLCKSZ];
+       PGAlignedBlock rbuf;
        int         nread;
 
-       nread = read(fd, rbuf, sizeof(rbuf));
+       nread = read(fd, rbuf.data, sizeof(rbuf));
        if (nread <= 0)
            ereport(ERROR,
                    (errcode_for_file_access(),
                     errmsg("could not read file \"%s\": %m",
                            path)));
-       pq_sendbytes(&buf, rbuf, nread);
+       pq_sendbytes(&buf, rbuf.data, nread);
        bytesleft -= nread;
    }
    CloseTransientFile(fd);
index 042be79440cd02345c56e93329532164774d229a..5d95d5c45d7ee61d1e75d7df48f507211956074d 100644 (file)
@@ -86,7 +86,7 @@ struct BufFile
    off_t       curOffset;      /* offset part of current pos */
    int         pos;            /* next read/write position in buffer */
    int         nbytes;         /* total # of valid bytes in buffer */
-   char        buffer[BLCKSZ];
+   PGAlignedBlock buffer;
 };
 
 static BufFile *makeBufFile(File firstfile);
@@ -254,7 +254,7 @@ BufFileLoadBuffer(BufFile *file)
    /*
     * Read whatever we can get, up to a full bufferload.
     */
-   file->nbytes = FileRead(thisfile, file->buffer, sizeof(file->buffer));
+   file->nbytes = FileRead(thisfile, file->buffer.data, sizeof(file->buffer));
    if (file->nbytes < 0)
        file->nbytes = 0;
    file->offsets[file->curFile] += file->nbytes;
@@ -317,7 +317,7 @@ BufFileDumpBuffer(BufFile *file)
                return;         /* seek failed, give up */
            file->offsets[file->curFile] = file->curOffset;
        }
-       bytestowrite = FileWrite(thisfile, file->buffer + wpos, bytestowrite);
+       bytestowrite = FileWrite(thisfile, file->buffer.data + wpos, bytestowrite);
        if (bytestowrite <= 0)
            return;             /* failed to write */
        file->offsets[file->curFile] += bytestowrite;
@@ -385,7 +385,7 @@ BufFileRead(BufFile *file, void *ptr, size_t size)
            nthistime = size;
        Assert(nthistime > 0);
 
-       memcpy(ptr, file->buffer + file->pos, nthistime);
+       memcpy(ptr, file->buffer.data + file->pos, nthistime);
 
        file->pos += nthistime;
        ptr = (void *) ((char *) ptr + nthistime);
@@ -432,7 +432,7 @@ BufFileWrite(BufFile *file, void *ptr, size_t size)
            nthistime = size;
        Assert(nthistime > 0);
 
-       memcpy(file->buffer + file->pos, ptr, nthistime);
+       memcpy(file->buffer.data + file->pos, ptr, nthistime);
 
        file->dirty = true;
        file->pos += nthistime;
index 9c7f325ab3ed3717a377f9d80742babba158d440..45d1d6333f00f9576fb57aecf22f004ff4954f4f 100644 (file)
@@ -628,10 +628,9 @@ static void
 fsm_extend(Relation rel, BlockNumber fsm_nblocks)
 {
    BlockNumber fsm_nblocks_now;
-   Page        pg;
+   PGAlignedBlock pg;
 
-   pg = (Page) palloc(BLCKSZ);
-   PageInit(pg, BLCKSZ, 0);
+   PageInit((Page) pg.data, BLCKSZ, 0);
 
    /*
     * We use the relation extension lock to lock out other backends trying to
@@ -661,10 +660,10 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks)
 
    while (fsm_nblocks_now < fsm_nblocks)
    {
-       PageSetChecksumInplace(pg, fsm_nblocks_now);
+       PageSetChecksumInplace((Page) pg.data, fsm_nblocks_now);
 
        smgrextend(rel->rd_smgr, FSM_FORKNUM, fsm_nblocks_now,
-                  (char *) pg, false);
+                  pg.data, false);
        fsm_nblocks_now++;
    }
 
@@ -672,8 +671,6 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks)
    rel->rd_smgr->smgr_fsm_nblocks = fsm_nblocks_now;
 
    UnlockRelationForExtension(rel, ExclusiveLock);
-
-   pfree(pg);
 }
 
 /*
index 79072a7939a6c4b91c77e7683d04364650fc8c0f..d127355c31f0d846f3236916f56af20e9b466147 100644 (file)
@@ -98,7 +98,7 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
    int         f;
    char        fn[MAXPGPATH];
    struct stat statbuf;
-   char       *zerobuf;
+   PGAlignedXLogBlock zerobuf;
    int         bytes;
    XLogSegNo   segno;
 
@@ -144,11 +144,11 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
    }
 
    /* New, empty, file. So pad it to 16Mb with zeroes */
-   zerobuf = pg_malloc0(XLOG_BLCKSZ);
+   memset(zerobuf.data, 0, XLOG_BLCKSZ);
    for (bytes = 0; bytes < XLogSegSize; bytes += XLOG_BLCKSZ)
    {
        errno = 0;
-       if (write(f, zerobuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
+       if (write(f, zerobuf.data, XLOG_BLCKSZ) != XLOG_BLCKSZ)
        {
            /* if write didn't set errno, assume problem is no disk space */
            if (errno == 0)
@@ -156,13 +156,11 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
            fprintf(stderr,
                    _("%s: could not pad transaction log file \"%s\": %s\n"),
                    progname, fn, strerror(errno));
-           free(zerobuf);
            close(f);
            unlink(fn);
            return false;
        }
    }
-   free(zerobuf);
 
    if (lseek(f, SEEK_SET, 0) != 0)
    {
index fdd75044de8b42f09ec1a4ac61649b8851ac0992..3e79482ca22dfc43cca0c5714f8731f3c3562657 100644 (file)
@@ -1133,7 +1133,7 @@ KillExistingArchiveStatus(void)
 static void
 WriteEmptyXLOG(void)
 {
-   char       *buffer;
+   PGAlignedXLogBlock buffer;
    XLogPageHeader page;
    XLogLongPageHeader longpage;
    XLogRecord *record;
@@ -1143,12 +1143,10 @@ WriteEmptyXLOG(void)
    int         nbytes;
    char       *recptr;
 
-   /* Use malloc() to ensure buffer is MAXALIGNED */
-   buffer = (char *) pg_malloc(XLOG_BLCKSZ);
-   page = (XLogPageHeader) buffer;
-   memset(buffer, 0, XLOG_BLCKSZ);
+   memset(buffer.data, 0, XLOG_BLCKSZ);
 
    /* Set up the XLOG page header */
+   page = (XLogPageHeader) buffer.data;
    page->xlp_magic = XLOG_PAGE_MAGIC;
    page->xlp_info = XLP_LONG_HEADER;
    page->xlp_tli = ControlFile.checkPointCopy.ThisTimeLineID;
@@ -1194,7 +1192,7 @@ WriteEmptyXLOG(void)
    }
 
    errno = 0;
-   if (write(fd, buffer, XLOG_BLCKSZ) != XLOG_BLCKSZ)
+   if (write(fd, buffer.data, XLOG_BLCKSZ) != XLOG_BLCKSZ)
    {
        /* if write didn't set errno, assume problem is no disk space */
        if (errno == 0)
@@ -1205,11 +1203,11 @@ WriteEmptyXLOG(void)
    }
 
    /* Fill the rest of the file with zeroes */
-   memset(buffer, 0, XLOG_BLCKSZ);
+   memset(buffer.data, 0, XLOG_BLCKSZ);
    for (nbytes = XLOG_BLCKSZ; nbytes < XLogSegSize; nbytes += XLOG_BLCKSZ)
    {
        errno = 0;
-       if (write(fd, buffer, XLOG_BLCKSZ) != XLOG_BLCKSZ)
+       if (write(fd, buffer.data, XLOG_BLCKSZ) != XLOG_BLCKSZ)
        {
            if (errno == 0)
                errno = ENOSPC;
index d7a31c16f4e0a6f97daf2a8301eeeadb4a548977..47f4f4195205e425b64e432bedbac9ac300c6a5b 100644 (file)
@@ -160,7 +160,7 @@ recurse_dir(const char *datadir, const char *parentpath,
 static void
 rewind_copy_file_range(const char *path, off_t begin, off_t end, bool trunc)
 {
-   char        buf[BLCKSZ];
+   PGAlignedBlock buf;
    char        srcpath[MAXPGPATH];
    int         srcfd;
 
@@ -186,7 +186,7 @@ rewind_copy_file_range(const char *path, off_t begin, off_t end, bool trunc)
        else
            len = end - begin;
 
-       readlen = read(srcfd, buf, len);
+       readlen = read(srcfd, buf.data, len);
 
        if (readlen < 0)
            pg_fatal("could not read file \"%s\": %s\n",
@@ -194,7 +194,7 @@ rewind_copy_file_range(const char *path, off_t begin, off_t end, bool trunc)
        else if (readlen == 0)
            pg_fatal("unexpected EOF while reading file \"%s\"\n", srcpath);
 
-       write_target_range(buf, begin, readlen);
+       write_target_range(buf.data, begin, readlen);
        begin += readlen;
    }
 
index 6bb811be69362d3b82619c31b649c9f8ba470430..628784e32b0a8af4f720bee7af332b56647c539b 100644 (file)
@@ -131,8 +131,8 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
 {
    int         src_fd;
    int         dst_fd;
-   char       *buffer;
-   char       *new_vmbuf;
+   PGAlignedBlock buffer;
+   PGAlignedBlock new_vmbuf;
    ssize_t     totalBytesRead = 0;
    ssize_t     src_filesize;
    int         rewriteVmBytesPerPage;
@@ -158,13 +158,6 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
    /* Save old file size */
    src_filesize = statbuf.st_size;
 
-   /*
-    * Malloc the work buffers, rather than making them local arrays, to
-    * ensure adequate alignment.
-    */
-   buffer = (char *) pg_malloc(BLCKSZ);
-   new_vmbuf = (char *) pg_malloc(BLCKSZ);
-
    /*
     * Turn each visibility map page into 2 pages one by one. Each new page
     * has the same page header as the old one.  If the last section of the
@@ -180,7 +173,7 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
        PageHeaderData pageheader;
        bool        old_lastblk;
 
-       if ((bytesRead = read(src_fd, buffer, BLCKSZ)) != BLCKSZ)
+       if ((bytesRead = read(src_fd, buffer.data, BLCKSZ)) != BLCKSZ)
        {
            if (bytesRead < 0)
                pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %s\n",
@@ -194,7 +187,7 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
        old_lastblk = (totalBytesRead == src_filesize);
 
        /* Save the page header data */
-       memcpy(&pageheader, buffer, SizeOfPageHeaderData);
+       memcpy(&pageheader, buffer.data, SizeOfPageHeaderData);
 
        /*
         * These old_* variables point to old visibility map page. old_cur
@@ -202,8 +195,8 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
         * old block.  old_break is the end+1 position on the old page for the
         * data that will be transferred to the current new page.
         */
-       old_cur = buffer + SizeOfPageHeaderData;
-       old_blkend = buffer + bytesRead;
+       old_cur = buffer.data + SizeOfPageHeaderData;
+       old_blkend = buffer.data + bytesRead;
        old_break = old_cur + rewriteVmBytesPerPage;
 
        while (old_break <= old_blkend)
@@ -213,12 +206,12 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
            bool        old_lastpart;
 
            /* First, copy old page header to new page */
-           memcpy(new_vmbuf, &pageheader, SizeOfPageHeaderData);
+           memcpy(new_vmbuf.data, &pageheader, SizeOfPageHeaderData);
 
            /* Rewriting the last part of the last old page? */
            old_lastpart = old_lastblk && (old_break == old_blkend);
 
-           new_cur = new_vmbuf + SizeOfPageHeaderData;
+           new_cur = new_vmbuf.data + SizeOfPageHeaderData;
 
            /* Process old page bytes one by one, and turn it into new page. */
            while (old_cur < old_break)
@@ -252,11 +245,11 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
 
            /* Set new checksum for visibility map page, if enabled */
            if (new_cluster.controldata.data_checksum_version != 0)
-               ((PageHeader) new_vmbuf)->pd_checksum =
-                   pg_checksum_page(new_vmbuf, new_blkno);
+               ((PageHeader) new_vmbuf.data)->pd_checksum =
+                   pg_checksum_page(new_vmbuf.data, new_blkno);
 
            errno = 0;
-           if (write(dst_fd, new_vmbuf, BLCKSZ) != BLCKSZ)
+           if (write(dst_fd, new_vmbuf.data, BLCKSZ) != BLCKSZ)
            {
                /* if write didn't set errno, assume problem is no disk space */
                if (errno == 0)
@@ -272,8 +265,6 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
    }
 
    /* Clean up */
-   pg_free(buffer);
-   pg_free(new_vmbuf);
    close(dst_fd);
    close(src_fd);
 }
index 8633657aebd76b093c1e4f8c70b1ac7d6ded0420..81d8e642da2119eab49306eabc82447e9b7da74c 100644 (file)
@@ -949,6 +949,32 @@ typedef NameData *Name;
  * ----------------------------------------------------------------
  */
 
+/*
+ * Use this, not "char buf[BLCKSZ]", to declare a field or local variable
+ * holding a page buffer, if that page might be accessed as a page and not
+ * just a string of bytes.  Otherwise the variable might be under-aligned,
+ * causing problems on alignment-picky hardware.  (In some places, we use
+ * this to declare buffers even though we only pass them to read() and
+ * write(), because copying to/from aligned buffers is usually faster than
+ * using unaligned buffers.)  We include both "double" and "int64" in the
+ * union to ensure that the compiler knows the value must be MAXALIGN'ed
+ * (cf. configure's computation of MAXIMUM_ALIGNOF).
+ */
+typedef union PGAlignedBlock
+{
+   char        data[BLCKSZ];
+   double      force_align_d;
+   int64       force_align_i64;
+} PGAlignedBlock;
+
+/* Same, but for an XLOG_BLCKSZ-sized buffer */
+typedef union PGAlignedXLogBlock
+{
+   char        data[XLOG_BLCKSZ];
+   double      force_align_d;
+   int64       force_align_i64;
+} PGAlignedXLogBlock;
+
 /* msb for char */
 #define HIGHBIT                    (0x80)
 #define IS_HIGHBIT_SET(ch)     ((unsigned char)(ch) & HIGHBIT)