On 16.09.26 11:16, Peter Eisentraut wrote:
On 08.09.26 12:08, Peter Eisentraut wrote:
There are a number of places where palloc()/malloc()/etc. was used
solely to obtain an aligned buffer. We can do these much simpler by
using alignas with a local variable instead. See attached patch.
Here is a new patch set that aims to address all the comments.
Here is a rebased patch set. No functional changes over v3.
First of all, while changing this to make use of the existing
"AlignedBlock" types, I noticed that PGAlignedXLogBlock is misnamed: It
should be PGIOAlignedXLogBlock, to maintain the similarity with
PGAlignedBlock and PGIOAlignedBlock, respectively. So I'm proposing to
rename it in patch 0001.
We could then re-introduce the "correct" PGAlignedXLogBlock and make use
of it, which is patch 0003. But I'm hesitant to change the meaning of
PGAlignedXLogBlock without some gap in between, so I'm not sure about
this patch.
Patch 0002 is as before, but with the "AlignedBlock" types used, and the
copy_file() change backed out and a comment added.
Patch 0004 adds some comments and an assertion for HEADER_SCRATCH_SIZE,
and patch 0005 refactors things to convert the workspace from static
variable to a normal (non-static) local variable. (This could be
squashed into 0002, but it seems cleaner to review this way at least.)
(The pgindent changes were already committed separately.)
From 053fef110990c704eb19373f502c47ebd9a803d6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 16 Sep 2026 09:21:56 +0200
Subject: [PATCH v4 1/5] Rename PGAlignedXLogBlock to PGIOAlignedXLogBlock
Before commit faeedbcefd4, PGAlignedXLogBlock was MAXALIGNed and a
sibling of PGAlignedBlock. That commit introduced PGIOAlignedBlock
that was aligned as PG_IO_ALIGN_SIZE and *changed* PGAlignedXLogBlock
to also be aligned as PG_IO_ALIGN_SIZE. This creates an inconsistent
naming, as there would now be no room for a MAXALIGNed block of size
XLOG_BLCKSZ. To improve this, rename PGAlignedXLogBlock to
PGIOAlignedXLogBlock, which now correctly reflects its purpose. A
MAXALIGNed PGAlignedXLogBlock could be re-introduced later.
---
src/backend/access/transam/xlog.c | 4 ++--
src/bin/pg_basebackup/walmethods.c | 2 +-
src/bin/pg_resetwal/pg_resetwal.c | 2 +-
src/bin/pg_test_fsync/pg_test_fsync.c | 2 +-
src/bin/pg_waldump/pg_waldump.c | 2 +-
src/include/c.h | 6 +++---
src/tools/pgindent/typedefs.list | 2 +-
7 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 8636ad9ab2c..0ebf0810ece 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3539,7 +3539,7 @@ XLogFileCopy(TimeLineID destTLI, XLogSegNo destsegno,
{
char path[MAXPGPATH];
char tmppath[MAXPGPATH];
- PGAlignedXLogBlock buffer;
+ PGIOAlignedXLogBlock buffer;
int srcfd;
int fd;
int nbytes;
@@ -5691,7 +5691,7 @@ void
BootStrapXLOG(uint32 data_checksum_version)
{
CheckPoint checkPoint;
- PGAlignedXLogBlock buffer;
+ PGIOAlignedXLogBlock buffer;
XLogPageHeader page;
XLogLongPageHeader longpage;
XLogRecord *record;
diff --git a/src/bin/pg_basebackup/walmethods.c
b/src/bin/pg_basebackup/walmethods.c
index 3a6b3b5f45b..44cacaf97ea 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -804,7 +804,7 @@ tar_write(Walfile *f, const void *buf, size_t count)
static bool
tar_write_padding_data(TarMethodFile *f, size_t bytes)
{
- PGAlignedXLogBlock zerobuf;
+ PGIOAlignedXLogBlock zerobuf;
size_t bytesleft = bytes;
memset(zerobuf.data, 0, XLOG_BLCKSZ);
diff --git a/src/bin/pg_resetwal/pg_resetwal.c
b/src/bin/pg_resetwal/pg_resetwal.c
index 634d966da9e..67e4154b168 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -1139,7 +1139,7 @@ KillExistingWALSummaries(void)
static void
WriteEmptyXLOG(void)
{
- PGAlignedXLogBlock buffer;
+ PGIOAlignedXLogBlock buffer;
XLogPageHeader page;
XLogLongPageHeader longpage;
XLogRecord *record;
diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c
b/src/bin/pg_test_fsync/pg_test_fsync.c
index 5732d4c0de3..945e4bfc322 100644
--- a/src/bin/pg_test_fsync/pg_test_fsync.c
+++ b/src/bin/pg_test_fsync/pg_test_fsync.c
@@ -71,7 +71,7 @@ static const char *progname;
static unsigned int secs_per_test = 5;
static int needs_unlink = 0;
-alignas(PGAlignedXLogBlock) static char buf[DEFAULT_XLOG_SEG_SIZE];
+alignas(PGIOAlignedXLogBlock) static char buf[DEFAULT_XLOG_SEG_SIZE];
static char *filename = FSYNC_FILENAME;
static struct timeval start_t,
stop_t;
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 6f0574a8764..908a2080411 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -236,7 +236,7 @@ search_directory(const char *directory, const char *fname,
int *WalSegSz)
/* set WalSegSz if file is successfully opened */
if (fd >= 0)
{
- PGAlignedXLogBlock buf;
+ PGIOAlignedXLogBlock buf;
ssize_t r;
r = read(fd, buf.data, XLOG_BLCKSZ);
diff --git a/src/include/c.h b/src/include/c.h
index 367b32c5b6f..926824c6dbe 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -1318,16 +1318,16 @@ typedef struct PGIOAlignedBlock
} PGIOAlignedBlock;
/* Same, but for an XLOG_BLCKSZ-sized buffer */
-typedef struct PGAlignedXLogBlock
+typedef struct PGIOAlignedXLogBlock
{
alignas(PG_IO_ALIGN_SIZE) char data[XLOG_BLCKSZ];
-} PGAlignedXLogBlock;
+} PGIOAlignedXLogBlock;
#else /* (g++ < 9) */
/* Allow these types to be used as abstract types when using old g++ */
typedef struct PGIOAlignedBlock PGIOAlignedBlock;
-typedef struct PGAlignedXLogBlock PGAlignedXLogBlock;
+typedef struct PGIOAlignedXLogBlock PGIOAlignedXLogBlock;
#endif /* !(g++ < 9) */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index a7a8dbdd23f..741ccc4bd91 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1912,7 +1912,6 @@ PERL_CONTEXT
PERL_SI
PFN
PGAlignedBlock
-PGAlignedXLogBlock
PGAsyncStatusType
PGCALL2
PGCRYPTO_SHA_t
@@ -1931,6 +1930,7 @@ PGFInfoFunction
PGFileType
PGFunction
PGIOAlignedBlock
+PGIOAlignedXLogBlock
PGLZ_HistEntry
PGLZ_Strategy
PGLoadBalanceType
base-commit: e355089a57d6f480cffc2820c46fcf0b5e371cf6
--
2.56.0
From 08cfc8fa31b2ab634ff5aaec1c820dfcfaa7961c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 16 Sep 2026 09:38:13 +0200
Subject: [PATCH v4 2/5] Use C11 alignas instead of palloc/malloc for alignment
Replace several cases where palloc()/malloc()/etc. was used solely to
obtain an aligned buffer. Use alignas with a local variable instead,
or one of the existing "AlignedBlock" types, where appropriate.
The previous alignment guarantees are carried over. palloc-based
allocations are replaced by alignas(MAXIMUM_ALIGNOF). Theoretically,
malloc-based allocations should be replaced by alignas(max_align_t),
but MSVC doesn't provide max_align_t, and so we use MAXIMUM_ALIGNOF
here as well. They should be the same in practice.
The allocations in InitWalRecovery() are not converted, because the
comment says it is also this way to avoid wasting storage. The
comment in XLogReaderAllocate(), on the other hand, was probably
copied from InitWalRecovery(), but the part of the comment about
wasting storage does not make sense in that context, so it is
converted.
The allocation in copy_file() is kept because we want to avoid putting
large buffers on the stack and we might want that buffer to be even
bigger sometime.
Discussion:
https://www.postgresql.org/message-id/flat/94a128da-bf3e-46bd-9e2d-609573c484da%40eisentraut.org
---
src/backend/access/transam/xloginsert.c | 28 ++++++++---------------
src/backend/access/transam/xlogreader.c | 17 --------------
src/backend/access/transam/xlogrecovery.c | 8 +++----
src/backend/commands/sequence_xlog.c | 9 +++-----
src/backend/storage/file/copydir.c | 6 ++++-
src/backend/storage/ipc/dsm_impl.c | 10 ++++----
src/backend/storage/smgr/md.c | 6 ++---
src/bin/pg_resetwal/pg_resetwal.c | 7 ++----
src/include/access/xlogreader.h | 5 ++--
9 files changed, 32 insertions(+), 64 deletions(-)
diff --git a/src/backend/access/transam/xloginsert.c
b/src/backend/access/transam/xloginsert.c
index 59b0dfe436d..1a3a89665bb 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -105,17 +105,6 @@ static uint64 mainrdata_len; /* total # of bytes in
chain */
/* flags for the in-progress insertion */
static uint8 curinsert_flags = 0;
-/*
- * These are used to hold the record header while constructing a record.
- * 'hdr_scratch' is not a plain variable, but is palloc'd at initialization,
- * because we want it to be MAXALIGNed and padding bytes zeroed.
- *
- * For simplicity, it's allocated large enough to hold the headers for any
- * WAL record.
- */
-static XLogRecData hdr_rdt;
-static char *hdr_scratch = NULL;
-
#ifdef USE_ZSTD
/* Compression context reused for zstd-compressed full-page images. */
static ZSTD_CCtx *zstd_cctx = NULL;
@@ -627,6 +616,16 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
XLogRecPtr *fpw_lsn, int *num_fpi, uint64
*fpi_bytes,
bool *topxid_included)
{
+ /*
+ * These are used to hold the record header while constructing a record.
+ * 'hdr_scratch' must be MAXALIGNed and padding bytes zeroed.
+ *
+ * For simplicity, it's allocated large enough to hold the headers for
any
+ * WAL record.
+ */
+ static XLogRecData hdr_rdt;
+ static alignas(MAXIMUM_ALIGNOF) char hdr_scratch[HEADER_SCRATCH_SIZE];
+
XLogRecData *rdt;
uint64 total_len = 0;
int block_id;
@@ -1449,13 +1448,6 @@ InitXLogInsert(void)
max_rdatas = XLR_NORMAL_RDATAS;
}
- /*
- * Allocate a buffer to hold the header information for a WAL record.
- */
- if (hdr_scratch == NULL)
- hdr_scratch = MemoryContextAllocZero(xloginsert_cxt,
-
HEADER_SCRATCH_SIZE);
-
/* Extra initialization for wal_debug */
#ifdef WAL_DEBUG
InitWalDebug();
diff --git a/src/backend/access/transam/xlogreader.c
b/src/backend/access/transam/xlogreader.c
index e05a831b6e6..7f4a3f03d94 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -126,21 +126,6 @@ XLogReaderAllocate(int wal_segment_size, const char
*waldir,
/* initialize caller-provided support functions */
state->routine = *routine;
- /*
- * Permanently allocate readBuf. We do it this way, rather than just
- * making a static array, for two reasons: (1) no need to waste the
- * storage in most instantiations of the backend; (2) a static char
array
- * isn't guaranteed to have any particular alignment, whereas
- * palloc_extended() will provide MAXALIGN'd storage.
- */
- state->readBuf = (char *) palloc_extended(XLOG_BLCKSZ,
-
MCXT_ALLOC_NO_OOM);
- if (!state->readBuf)
- {
- pfree(state);
- return NULL;
- }
-
/* Initialize segment info. */
WALOpenSegmentInit(&state->seg, &state->segcxt, wal_segment_size,
waldir);
@@ -152,7 +137,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir,
MCXT_ALLOC_NO_OOM);
if (!state->errormsg_buf)
{
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -194,7 +178,6 @@ XLogReaderFree(XLogReaderState *state)
ZSTD_freeDCtx(state->zstd_dctx);
}
#endif
- pfree(state->readBuf);
pfree(state);
}
diff --git a/src/backend/access/transam/xlogrecovery.c
b/src/backend/access/transam/xlogrecovery.c
index 20be788da78..fa1a1ef8a88 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -532,10 +532,10 @@ InitWalRecovery(ControlFileData *ControlFile, bool
*wasShutdown_ptr,
/*
* Allocate two page buffers dedicated to WAL consistency checks. We do
- * it this way, rather than just making static arrays, for two reasons:
- * (1) no need to waste the storage in most instantiations of the
backend;
- * (2) a static char array isn't guaranteed to have any particular
- * alignment, whereas palloc() will provide MAXALIGN'd storage.
+ * it this way, rather than just making static arrays, because there is
no
+ * need to waste the storage in most instantiations of the backend.
+ * (palloc() also gives us MAXALIGN'd storage, but nowadays that part
+ * could be handled with alignas.)
*/
replay_image_masked = (char *) palloc(BLCKSZ);
primary_image_masked = (char *) palloc(BLCKSZ);
diff --git a/src/backend/commands/sequence_xlog.c
b/src/backend/commands/sequence_xlog.c
index fcb3230cf3b..27e5e9b30db 100644
--- a/src/backend/commands/sequence_xlog.c
+++ b/src/backend/commands/sequence_xlog.c
@@ -26,7 +26,8 @@ seq_redo(XLogReaderState *record)
uint8 info = XLogRecGetInfo(record) & ~XLR_INFO_MASK;
Buffer buffer;
Page page;
- Page localpage;
+ PGAlignedBlock localpage_buf;
+ Page localpage = (Page) &localpage_buf;
char *item;
Size itemsz;
xl_seq_rec *xlrec = (xl_seq_rec *) XLogRecGetData(record);
@@ -44,10 +45,8 @@ seq_redo(XLogReaderState *record)
* is examining the page concurrently; so we mustn't transiently trash
the
* buffer. The solution is to build the correct new page contents in
* local workspace and then memcpy into the buffer. Then only bytes
that
- * are supposed to change will change, even transiently. We must palloc
- * the local page for alignment reasons.
+ * are supposed to change will change, even transiently.
*/
- localpage = (Page) palloc(BufferGetPageSize(buffer));
PageInit(localpage, BufferGetPageSize(buffer), sizeof(sequence_magic));
sm = (sequence_magic *) PageGetSpecialPointer(localpage);
@@ -65,8 +64,6 @@ seq_redo(XLogReaderState *record)
MarkBufferDirty(buffer);
XLogFlushBufferForRedoIfInit(record, 0, buffer);
UnlockReleaseBuffer(buffer);
-
- pfree(localpage);
}
/*
diff --git a/src/backend/storage/file/copydir.c
b/src/backend/storage/file/copydir.c
index ee42c796f77..fc0fe629855 100644
--- a/src/backend/storage/file/copydir.c
+++ b/src/backend/storage/file/copydir.c
@@ -155,7 +155,11 @@ copy_file(const char *fromfile, const char *tofile)
#define FLUSH_DISTANCE (1024 * 1024)
#endif
- /* Use palloc to ensure we get a maxaligned buffer */
+ /*
+ * Allocate the buffer dynamically, both because it is too large to put
on
+ * the stack and because that gets us a maxaligned buffer. (For
+ * performance, we might want an even bigger buffer here.)
+ */
buffer = palloc(COPY_BUF_SIZE);
/*
diff --git a/src/backend/storage/ipc/dsm_impl.c
b/src/backend/storage/ipc/dsm_impl.c
index e8c07805f59..23869c1e7a3 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -867,13 +867,11 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size
request_size,
else
{
/*
- * Allocate a buffer full of zeros.
- *
- * Note: palloc zbuffer, instead of just using a local char
array, to
- * ensure it is reasonably well-aligned; this may save a few
cycles
- * transferring data to the kernel.
+ * A buffer full of zeros. alignas ensures it is reasonably
+ * well-aligned, which may save a few cycles transferring it to
the
+ * kernel.
*/
- char *zbuffer = (char *) palloc0(ZBUFFER_SIZE);
+ alignas(MAXIMUM_ALIGNOF) const char zbuffer[ZBUFFER_SIZE] = {0};
Size remaining = request_size;
bool success = true;
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 4c777757757..58326ebcfaa 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -1823,13 +1823,11 @@ _mdfd_getseg(SMgrRelation reln, ForkNumber forknum,
BlockNumber blkno,
*/
if (nblocks < ((BlockNumber) RELSEG_SIZE))
{
- char *zerobuf = palloc_aligned(BLCKSZ,
PG_IO_ALIGN_SIZE,
-
MCXT_ALLOC_ZERO);
+ const PGIOAlignedBlock zerobuf = {0};
mdextend(reln, forknum,
nextsegno * ((BlockNumber)
RELSEG_SIZE) - 1,
- zerobuf, skipFsync);
- pfree(zerobuf);
+ &zerobuf, skipFsync);
}
flags = O_CREAT;
}
diff --git a/src/bin/pg_resetwal/pg_resetwal.c
b/src/bin/pg_resetwal/pg_resetwal.c
index 67e4154b168..c1b42addd5c 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -603,7 +603,7 @@ read_controlfile(void)
{
int fd;
ssize_t len;
- char *buffer;
+ alignas(MAXIMUM_ALIGNOF) char buffer[PG_CONTROL_FILE_SIZE];
pg_crc32c crc;
if ((fd = open(XLOG_CONTROL_FILE, O_RDONLY | PG_BINARY, 0)) < 0)
@@ -623,10 +623,7 @@ read_controlfile(void)
exit(1);
}
- /* Use malloc to ensure we have a maxaligned buffer */
- buffer = (char *) pg_malloc(PG_CONTROL_FILE_SIZE);
-
- len = read(fd, buffer, PG_CONTROL_FILE_SIZE);
+ len = read(fd, buffer, sizeof buffer);
if (len < 0)
pg_fatal("could not read file \"%s\": %m", XLOG_CONTROL_FILE);
close(fd);
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e48a0db9ce9..c70fff7e3f1 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -271,10 +271,9 @@ struct XLogReaderState
DecodedXLogRecord *decode_queue_tail; /* newest decoded record */
/*
- * Buffer for currently read page (XLOG_BLCKSZ bytes, valid up to at
least
- * readLen bytes)
+ * Buffer for currently read page (valid up to at least readLen bytes)
*/
- char *readBuf;
+ alignas(MAXIMUM_ALIGNOF) char readBuf[XLOG_BLCKSZ];
uint32 readLen;
/* last read XLOG position for data currently in readBuf */
--
2.56.0
From db625ed5e2d738c4e472205b345412eec2e76ee0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 12 Sep 2026 13:04:52 +0200
Subject: [PATCH v4 3/5] XXX Reintroduce PGAlignedXLogBlock and use it
---
src/backend/access/transam/xlogreader.c | 26 +++++++++++------------
src/backend/access/transam/xlogrecovery.c | 2 +-
src/include/access/xlogreader.h | 2 +-
src/include/c.h | 6 ++++++
src/tools/pgindent/typedefs.list | 1 +
5 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/src/backend/access/transam/xlogreader.c
b/src/backend/access/transam/xlogreader.c
index 7f4a3f03d94..60575e6cb41 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -665,7 +665,7 @@ XLogDecodeNextRecord(XLogReaderState *state, bool
nonblocking)
* ReadPageInternal always returns at least the page header, so we can
* examine it now.
*/
- pageHeaderSize = XLogPageHeaderSize((XLogPageHeader) state->readBuf);
+ pageHeaderSize = XLogPageHeaderSize((XLogPageHeader)
state->readBuf.data);
if (targetRecOff == 0)
{
/*
@@ -682,7 +682,7 @@ XLogDecodeNextRecord(XLogReaderState *state, bool
nonblocking)
goto err;
}
- if ((((XLogPageHeader) state->readBuf)->xlp_info &
XLP_FIRST_IS_CONTRECORD) &&
+ if ((((XLogPageHeader) state->readBuf.data)->xlp_info &
XLP_FIRST_IS_CONTRECORD) &&
targetRecOff == pageHeaderSize)
{
report_invalid_record(state, "contrecord is requested by
%X/%08X",
@@ -702,7 +702,7 @@ XLogDecodeNextRecord(XLogReaderState *state, bool
nonblocking)
* cannot access any other fields until we've verified that we got the
* whole header.
*/
- record = (XLogRecord *) (state->readBuf + RecPtr % XLOG_BLCKSZ);
+ record = (XLogRecord *) (state->readBuf.data + RecPtr % XLOG_BLCKSZ);
total_len = record->xl_tot_len;
/*
@@ -788,7 +788,7 @@ XLogDecodeNextRecord(XLogReaderState *state, bool
nonblocking)
/* Copy the first fragment of the record from the first page. */
memcpy(state->readRecordBuf,
- state->readBuf + RecPtr % XLOG_BLCKSZ, len);
+ state->readBuf.data + RecPtr % XLOG_BLCKSZ, len);
buffer = state->readRecordBuf + len;
gotlen = len;
@@ -810,7 +810,7 @@ XLogDecodeNextRecord(XLogReaderState *state, bool
nonblocking)
Assert(SizeOfXLogShortPHD <= readOff);
- pageHeader = (XLogPageHeader) state->readBuf;
+ pageHeader = (XLogPageHeader) state->readBuf.data;
/*
* If we were expecting a continuation record and got an
@@ -869,7 +869,7 @@ XLogDecodeNextRecord(XLogReaderState *state, bool
nonblocking)
Assert(pageHeaderSize <= readOff);
- contdata = (char *) state->readBuf + pageHeaderSize;
+ contdata = (char *) state->readBuf.data +
pageHeaderSize;
len = XLOG_BLCKSZ - pageHeaderSize;
if (pageHeader->xlp_rem_len < len)
len = pageHeader->xlp_rem_len;
@@ -920,7 +920,7 @@ XLogDecodeNextRecord(XLogReaderState *state, bool
nonblocking)
if (!ValidXLogRecord(state, record, RecPtr))
goto err;
- pageHeaderSize = XLogPageHeaderSize((XLogPageHeader)
state->readBuf);
+ pageHeaderSize = XLogPageHeaderSize((XLogPageHeader)
state->readBuf.data);
state->DecodeRecPtr = RecPtr;
state->NextRecPtr = targetPagePtr + pageHeaderSize
+ MAXALIGN(pageHeader->xlp_rem_len);
@@ -1124,7 +1124,7 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr
pageptr, int reqLen)
readLen = state->routine.page_read(state, targetSegmentPtr,
XLOG_BLCKSZ,
state->currRecPtr,
-
state->readBuf);
+
state->readBuf.data);
if (readLen == XLREAD_WOULDBLOCK)
return XLREAD_WOULDBLOCK;
else if (readLen < 0)
@@ -1134,7 +1134,7 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr
pageptr, int reqLen)
Assert(readLen == XLOG_BLCKSZ);
if (!XLogReaderValidatePageHeader(state, targetSegmentPtr,
-
state->readBuf))
+
state->readBuf.data))
goto err;
}
@@ -1144,7 +1144,7 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr
pageptr, int reqLen)
*/
readLen = state->routine.page_read(state, pageptr, Max(reqLen,
SizeOfXLogShortPHD),
state->currRecPtr,
-
state->readBuf);
+
state->readBuf.data);
if (readLen == XLREAD_WOULDBLOCK)
return XLREAD_WOULDBLOCK;
else if (readLen < 0)
@@ -1158,14 +1158,14 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr
pageptr, int reqLen)
Assert(readLen >= reqLen);
- hdr = (XLogPageHeader) state->readBuf;
+ hdr = (XLogPageHeader) state->readBuf.data;
/* still not enough */
if (readLen < XLogPageHeaderSize(hdr))
{
readLen = state->routine.page_read(state, pageptr,
XLogPageHeaderSize(hdr),
state->currRecPtr,
-
state->readBuf);
+
state->readBuf.data);
if (readLen == XLREAD_WOULDBLOCK)
return XLREAD_WOULDBLOCK;
else if (readLen < 0)
@@ -1524,7 +1524,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr
RecPtr, char **errormsg)
if (readLen < 0)
goto err;
- header = (XLogPageHeader) state->readBuf;
+ header = (XLogPageHeader) state->readBuf.data;
pageHeaderSize = XLogPageHeaderSize(header);
diff --git a/src/backend/access/transam/xlogrecovery.c
b/src/backend/access/transam/xlogrecovery.c
index fa1a1ef8a88..728916cddc5 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -1543,7 +1543,7 @@ FinishWalRecovery(void)
/* Copy the valid part of the last block */
len = endOfLog % XLOG_BLCKSZ;
page = palloc(len);
- memcpy(page, xlogreader->readBuf, len);
+ memcpy(page, xlogreader->readBuf.data, len);
result->lastPageBeginPtr = pageBeginPtr;
result->lastPage = page;
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index c70fff7e3f1..c9709c3a0d9 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -273,7 +273,7 @@ struct XLogReaderState
/*
* Buffer for currently read page (valid up to at least readLen bytes)
*/
- alignas(MAXIMUM_ALIGNOF) char readBuf[XLOG_BLCKSZ];
+ PGAlignedXLogBlock readBuf;
uint32 readLen;
/* last read XLOG position for data currently in readBuf */
diff --git a/src/include/c.h b/src/include/c.h
index 926824c6dbe..04de34c3c92 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -1296,6 +1296,12 @@ typedef struct PGAlignedBlock
alignas(MAXIMUM_ALIGNOF) char data[BLCKSZ];
} PGAlignedBlock;
+/* Same, but for an XLOG_BLCKSZ-sized buffer */
+typedef struct PGAlignedXLogBlock
+{
+ alignas(MAXIMUM_ALIGNOF) char data[XLOG_BLCKSZ];
+} PGAlignedXLogBlock;
+
/*
* alignas with extended alignments is buggy in g++ < 9. As a simple
* workaround, we disable these definitions in that case.
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 741ccc4bd91..912dc062d35 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1912,6 +1912,7 @@ PERL_CONTEXT
PERL_SI
PFN
PGAlignedBlock
+PGAlignedXLogBlock
PGAsyncStatusType
PGCALL2
PGCRYPTO_SHA_t
--
2.56.0
From f4d7ec11ff9e9cb9e1f7d0df1cb74ae4329a86ff Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 16 Sep 2026 09:49:28 +0200
Subject: [PATCH v4 4/5] Add an assertion and comments for HEADER_SCRATCH_SIZE
HEADER_SCRATCH_SIZE made a complicated calculation without any
comments, and there was no check that the calculation was correct.
Add some comments and assertion.
Suggested-by: Heikki Linnakangas <[email protected]>
Discussion:
https://www.postgresql.org/message-id/flat/94a128da-bf3e-46bd-9e2d-609573c484da%40eisentraut.org
---
src/backend/access/transam/xloginsert.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/backend/access/transam/xloginsert.c
b/src/backend/access/transam/xloginsert.c
index 1a3a89665bb..8e6fe04b368 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -113,6 +113,18 @@ static ZSTD_CCtx *zstd_cctx = NULL;
#define SizeOfXlogOrigin (sizeof(ReplOriginId) + sizeof(char))
#define SizeOfXLogTransactionId (sizeof(TransactionId) + sizeof(char))
+/*
+ * Size of the workspace used to hold the record header while constructing a
+ * record. It is large enough for the header of any WAL record: the
+ * fixed-size XLogRecord, a block header for every possible block ID, the main
+ * data header, and one term for each of the "special" block IDs.
+ *
+ * Every "special" block ID needs its own term here. If you add one alongside
+ * XLR_BLOCK_ID_ORIGIN (SizeOfXlogOrigin) and XLR_BLOCK_ID_TOPLEVEL_XID
+ * (SizeOfXLogTransactionId), you must add a corresponding term below as well.
+ *
+ * XLogRecordAssemble() asserts that the assembled header actually fits.
+ */
#define HEADER_SCRATCH_SIZE \
(SizeOfXLogRecord + \
MaxSizeOfXLogRecordBlockHeader * (XLR_MAX_BLOCK_ID + 1) + \
@@ -970,6 +982,7 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
rdt_datas_last->next = NULL;
hdr_rdt.len = (scratch - hdr_scratch);
+ Assert(hdr_rdt.len <= HEADER_SCRATCH_SIZE);
total_len += hdr_rdt.len;
/*
--
2.56.0
From afc7622d334f36dad8ddb1f7fb350b2106c1dc67 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 16 Sep 2026 10:17:13 +0200
Subject: [PATCH v4 5/5] Avoid static variables in XLogRecordAssemble()
XLogRecordAssemble() built the WAL record header in a pair of static
variables: the head of the record's XLogRecData chain and a MAXALIGNed
scratch buffer sized to hold the header of any WAL record. Have
XLogInsert() declare that workspace as a local variable instead and pass
it down.
The static variable was previously automatically zero-initialized,
including padding. With the new local variable, we need to do the
zeroing ourselves.
Suggested-by: Heikki Linnakangas <[email protected]>
Discussion:
https://www.postgresql.org/message-id/flat/94a128da-bf3e-46bd-9e2d-609573c484da%40eisentraut.org
---
src/backend/access/transam/xloginsert.c | 59 +++++++++++++++----------
1 file changed, 35 insertions(+), 24 deletions(-)
diff --git a/src/backend/access/transam/xloginsert.c
b/src/backend/access/transam/xloginsert.c
index 8e6fe04b368..7a50da44ae8 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -131,6 +131,19 @@ static ZSTD_CCtx *zstd_cctx = NULL;
SizeOfXLogRecordDataHeaderLong + SizeOfXlogOrigin + \
SizeOfXLogTransactionId)
+/*
+ * This is used to hold the record header while constructing a record.
+ * 'scratch' must be MAXALIGNed and padding bytes zeroed.
+ *
+ * For simplicity, it's allocated large enough to hold the headers for any WAL
+ * record.
+ */
+struct XLogRecordHeaderScratch
+{
+ XLogRecData rdt;
+ alignas(MAXIMUM_ALIGNOF) char scratch[HEADER_SCRATCH_SIZE];
+};
+
/*
* An array of XLogRecData structs, to hold registered data.
*/
@@ -147,7 +160,8 @@ static XLogRecData *XLogRecordAssemble(RmgrId rmid, uint8
info,
XLogRecPtr RedoRecPtr, bool doPageWrites,
XLogRecPtr *fpw_lsn, int *num_fpi,
uint64 *fpi_bytes,
- bool
*topxid_included);
+ bool
*topxid_included,
+
struct XLogRecordHeaderScratch *hdr);
static bool XLogCompressBackupBlock(const PageData *page, uint16 hole_offset,
uint16
hole_length, void *dest, uint16 *dlen);
@@ -516,6 +530,7 @@ XLogInsert(RmgrId rmid, uint8 info)
do
{
+ struct XLogRecordHeaderScratch hdr;
XLogRecPtr RedoRecPtr;
bool doPageWrites;
bool topxid_included = false;
@@ -533,7 +548,7 @@ XLogInsert(RmgrId rmid, uint8 info)
rdt = XLogRecordAssemble(rmid, info, RedoRecPtr, doPageWrites,
&fpw_lsn,
&num_fpi, &fpi_bytes,
-
&topxid_included);
+
&topxid_included, &hdr);
EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags,
num_fpi,
fpi_bytes,
topxid_included);
@@ -621,23 +636,15 @@ XLogGetFakeLSN(Relation rel)
*
* *topxid_included is set if the topmost transaction ID is logged with the
* current subtransaction.
+ *
+ * 'hdr' is caller-provided workspace for the record header.
*/
static XLogRecData *
XLogRecordAssemble(RmgrId rmid, uint8 info,
XLogRecPtr RedoRecPtr, bool doPageWrites,
XLogRecPtr *fpw_lsn, int *num_fpi, uint64
*fpi_bytes,
- bool *topxid_included)
+ bool *topxid_included, struct
XLogRecordHeaderScratch *hdr)
{
- /*
- * These are used to hold the record header while constructing a record.
- * 'hdr_scratch' must be MAXALIGNed and padding bytes zeroed.
- *
- * For simplicity, it's allocated large enough to hold the headers for
any
- * WAL record.
- */
- static XLogRecData hdr_rdt;
- static alignas(MAXIMUM_ALIGNOF) char hdr_scratch[HEADER_SCRATCH_SIZE];
-
XLogRecData *rdt;
uint64 total_len = 0;
int block_id;
@@ -645,20 +652,24 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
registered_buffer *prev_regbuf = NULL;
XLogRecData *rdt_datas_last;
XLogRecord *rechdr;
- char *scratch = hdr_scratch;
+ char *scratch = hdr->scratch;
/*
* Note: this function can be called multiple times for the same record.
* All the modifications we do to the rdata chains below must handle
that.
*/
- /* The record begins with the fixed-size header */
+ /*
+ * The record begins with the fixed-size header. Zero it out first so
+ * that there is no garbage in the padding bytes.
+ */
rechdr = (XLogRecord *) scratch;
+ memset(rechdr, 0, SizeOfXLogRecord);
scratch += SizeOfXLogRecord;
- hdr_rdt.next = NULL;
- rdt_datas_last = &hdr_rdt;
- hdr_rdt.data = hdr_scratch;
+ hdr->rdt.next = NULL;
+ rdt_datas_last = &hdr->rdt;
+ hdr->rdt.data = hdr->scratch;
/*
* Enforce consistency checks for this record if user is looking for it.
@@ -981,9 +992,9 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
}
rdt_datas_last->next = NULL;
- hdr_rdt.len = (scratch - hdr_scratch);
- Assert(hdr_rdt.len <= HEADER_SCRATCH_SIZE);
- total_len += hdr_rdt.len;
+ hdr->rdt.len = (scratch - hdr->scratch);
+ Assert(hdr->rdt.len <= HEADER_SCRATCH_SIZE);
+ total_len += hdr->rdt.len;
/*
* Calculate CRC of the data
@@ -994,8 +1005,8 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
* header.
*/
INIT_CRC32C(rdata_crc);
- COMP_CRC32C(rdata_crc, hdr_scratch + SizeOfXLogRecord, hdr_rdt.len -
SizeOfXLogRecord);
- for (rdt = hdr_rdt.next; rdt != NULL; rdt = rdt->next)
+ COMP_CRC32C(rdata_crc, hdr->scratch + SizeOfXLogRecord, hdr->rdt.len -
SizeOfXLogRecord);
+ for (rdt = hdr->rdt.next; rdt != NULL; rdt = rdt->next)
COMP_CRC32C(rdata_crc, rdt->data, rdt->len);
/*
@@ -1023,7 +1034,7 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
rechdr->xl_prev = InvalidXLogRecPtr;
rechdr->xl_crc = rdata_crc;
- return &hdr_rdt;
+ return &hdr->rdt;
}
/*
--
2.56.0
