From c7da25f47d9df9f5bc55bafef00f240dbf58e23b Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 16 Sep 2026 10:17:13 +0200 Subject: [PATCH v3 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 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 73ea8117c47..72f12fd2a93 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -126,6 +126,19 @@ static uint8 curinsert_flags = 0; 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. */ @@ -142,7 +155,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); @@ -511,6 +525,7 @@ XLogInsert(RmgrId rmid, uint8 info) do { + struct XLogRecordHeaderScratch hdr; XLogRecPtr RedoRecPtr; bool doPageWrites; bool topxid_included = false; @@ -528,7 +543,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); @@ -616,23 +631,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; @@ -640,20 +647,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. @@ -976,9 +987,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 @@ -989,8 +1000,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); /* @@ -1018,7 +1029,7 @@ XLogRecordAssemble(RmgrId rmid, uint8 info, rechdr->xl_prev = InvalidXLogRecPtr; rechdr->xl_crc = rdata_crc; - return &hdr_rdt; + return &hdr->rdt; } /* -- 2.55.0