Fix uninitialized xl_running_xacts padding

On Fri, Feb 13, 2026 at 10:39:14AM +0100, Anthonin Bonnefoy wrote:

The 3 bytes of padding after subxid_overflow were left uninitialized,
leading to the random 'ca ce 9b' data being written in the WAL. The
attached patch fixes the issue by zeroing the xl_running_xacts
structure in LogCurrentRunningXacts using MemSet.

This uninitialized padding exists for as long as this code exists,
down to efc16ea52067. No objection here to clean up that on HEAD.
--
Michael

Hi,

On Fri, Feb 13, 2026 at 06:50:08PM +0900, Michael Paquier wrote:

On Fri, Feb 13, 2026 at 10:39:14AM +0100, Anthonin Bonnefoy wrote:

The 3 bytes of padding after subxid_overflow were left uninitialized,
leading to the random 'ca ce 9b' data being written in the WAL. The
attached patch fixes the issue by zeroing the xl_running_xacts
structure in LogCurrentRunningXacts using MemSet.

This uninitialized padding exists for as long as this code exists,
down to efc16ea52067. No objection here to clean up that on HEAD.

It's not as important as when a struct which is used as an hash key has padding
bytes uninitialized (and byte comparisons are done on the key) but I'm also
+1 to make it "cleaner".

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Feb 13, 2026, at 18:08, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:

Hi,

On Fri, Feb 13, 2026 at 06:50:08PM +0900, Michael Paquier wrote:

On Fri, Feb 13, 2026 at 10:39:14AM +0100, Anthonin Bonnefoy wrote:

The 3 bytes of padding after subxid_overflow were left uninitialized,
leading to the random 'ca ce 9b' data being written in the WAL. The
attached patch fixes the issue by zeroing the xl_running_xacts
structure in LogCurrentRunningXacts using MemSet.

This uninitialized padding exists for as long as this code exists,
down to efc16ea52067. No objection here to clean up that on HEAD.

It's not as important as when a struct which is used as an hash key has padding
bytes uninitialized (and byte comparisons are done on the key) but I'm also
+1 to make it "cleaner".

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

I have no objection on cleanup the padding bytes. As the structure is small, maybe we can use {0} initializer:
```
xl_running_xacts xlrec = {0};
```
That will allow compilers to optimize the initialization. Anyway, that’s not a big deal, no strong opinion here.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Fri, Feb 13, 2026 at 10:39 PM Anthonin Bonnefoy
<anthonin.bonnefoy@datadoghq.com> wrote:

The 3 bytes of padding after subxid_overflow were left uninitialized,
leading to the random 'ca ce 9b' data being written in the WAL. The
attached patch fixes the issue by zeroing the xl_running_xacts
structure in LogCurrentRunningXacts using MemSet.

Nitpick: the so-called universal zero initialiser syntax (var = {0})
is a nicer way to do this and generally preferred in new code AFAIK.

But in this case, it seems we don't actually worry about initialising
WAL padding bytes in general. valgrind.supp has an entry to prevent
warnings about it. Should we?

On Mon, Feb 16, 2026 at 01:17:56PM +1300, Thomas Munro wrote:

Nitpick: the so-called universal zero initialiser syntax (var = {0})
is a nicer way to do this and generally preferred in new code AFAIK.

My memory on the matter may be fuzzy, of course, but the initializer
does not guarantee that the padding bytes are initialized to zero
because the padding bytes are not associated to a member in the
structure. A memset(0), however, makes sure that the padding bytes
are full of zeros by taking into account the full size of the
structure. We could couple a {0} with some dummy fields in
xl_running_xacts, of course. But actually, there may be an even
smarter move in this case: LogCurrentRunningXacts() uses
MinSizeOfXactRunningXacts to store the data of a xl_running_xacts,
based on an offset of xl_running_xacts.xids. So we could move
subxid_overflow at the end of xl_running_xacts before xids, shaving
these padding bytes away while inserting the record's data.

But in this case, it seems we don't actually worry about initialising
WAL padding bytes in general. valgrind.supp has an entry to prevent
warnings about it. Should we?

True about the initialization part, mostly I guess, still we tend to
worry about eliminating padding because these are wasted bytes in the
WAL records. For example, xlhp_freeze_plans has two bytes of padding,
that we eliminate while inserting its record by splitting the
FLEXIBLE_ARRAY_MEMBER part.
--
Michael

Hi,

On Mon, Feb 16, 2026 at 10:10:58AM +0900, Michael Paquier wrote:

On Mon, Feb 16, 2026 at 01:17:56PM +1300, Thomas Munro wrote:

Nitpick: the so-called universal zero initialiser syntax (var = {0})
is a nicer way to do this and generally preferred in new code AFAIK.

My memory on the matter may be fuzzy, of course, but the initializer
does not guarantee that the padding bytes are initialized to zero
because the padding bytes are not associated to a member in the
structure. A memset(0), however, makes sure that the padding bytes
are full of zeros by taking into account the full size of the
structure.

That's also what I recall, and what we followed in [1]/messages/by-id/CAGECzQS37h0twutb=kkS6v0rSnQ0vWxhVncqVNYoOTsv6gOmcw@mail.gmail.com.

But in this case, it seems we don't actually worry about initialising
WAL padding bytes in general. valgrind.supp has an entry to prevent
warnings about it. Should we?

True about the initialization part, mostly I guess, still we tend to
worry about eliminating padding because these are wasted bytes in the
WAL records. For example, xlhp_freeze_plans has two bytes of padding,
that we eliminate while inserting its record by splitting the
FLEXIBLE_ARRAY_MEMBER part.

But in the case of this thread it's in the middle of the struct, so I'm not
sure the "wasted" bytes would be elminated, would it?

Regards,

[1]: /messages/by-id/CAGECzQS37h0twutb=kkS6v0rSnQ0vWxhVncqVNYoOTsv6gOmcw@mail.gmail.com

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Fri, Feb 13, 2026 at 11:08 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

It's not as important as when a struct which is used as an hash key has padding
bytes uninitialized (and byte comparisons are done on the key) but I'm also
+1 to make it "cleaner".

Yeah, there's no direct issue of having those uninitialized. The only
impact I can think of is reducing compression efficiency of the WAL
due to the random padding bytes.

On Mon, Feb 16, 2026 at 8:29 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

My memory on the matter may be fuzzy, of course, but the initializer
does not guarantee that the padding bytes are initialized to zero
because the padding bytes are not associated to a member in the
structure. A memset(0), however, makes sure that the padding bytes
are full of zeros by taking into account the full size of the
structure.

That's also what I recall, and what we followed in [1].

I think that depends on the C standard used. With C99, there's no rule
for the padding bytes initialization.
With C11, in 6.7.9 Initialization of the standard: "the remainder of
the aggregate shall be initialized implicitly the same as objects that
have static storage duration", and with static storage will "every
member is initialized (recursively) according to these rules, and any
padding is initialized to zero bits".

So if I read this correctly, '{0}' will set padding bytes to 0 when
using C11. But given Postgres is using C99, that's not something we
can rely on?

True about the initialization part, mostly I guess, still we tend to
worry about eliminating padding because these are wasted bytes in the
WAL records. For example, xlhp_freeze_plans has two bytes of padding,
that we eliminate while inserting its record by splitting the
FLEXIBLE_ARRAY_MEMBER part.

But in the case of this thread it's in the middle of the struct, so I'm not
sure the "wasted" bytes would be elminated, would it?

Moving subxid_overflow before xids, wouldn't you have 3 bytes of
padding at the end of the struct for the whole struct alignment?

Hi,

On Mon, Feb 16, 2026 at 12:02:33PM +0100, Anthonin Bonnefoy wrote:

On Fri, Feb 13, 2026 at 11:08 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

It's not as important as when a struct which is used as an hash key has padding
bytes uninitialized (and byte comparisons are done on the key) but I'm also
+1 to make it "cleaner".

Yeah, there's no direct issue of having those uninitialized. The only
impact I can think of is reducing compression efficiency of the WAL
due to the random padding bytes.

Yeah, good point about the compression.

That's also what I recall, and what we followed in [1].

I think that depends on the C standard used. With C99, there's no rule
for the padding bytes initialization.
With C11, in 6.7.9 Initialization of the standard: "the remainder of
the aggregate shall be initialized implicitly the same as objects that
have static storage duration", and with static storage will "every
member is initialized (recursively) according to these rules, and any
padding is initialized to zero bits".

Thanks for the research!

So if I read this correctly, '{0}' will set padding bytes to 0 when
using C11. But given Postgres is using C99, that's not something we
can rely on?

C11 is required as of f5e0186f865c so it looks like we could make use of
{0} instead.

True about the initialization part, mostly I guess, still we tend to
worry about eliminating padding because these are wasted bytes in the
WAL records. For example, xlhp_freeze_plans has two bytes of padding,
that we eliminate while inserting its record by splitting the
FLEXIBLE_ARRAY_MEMBER part.

But in the case of this thread it's in the middle of the struct, so I'm not
sure the "wasted" bytes would be elminated, would it?

Moving subxid_overflow before xids, wouldn't you have 3 bytes of
padding at the end of the struct for the whole struct alignment?

Yeah, we'd go from:

/* offset | size */ type = struct xl_running_xacts {
/* 0 | 4 */ int xcnt;
/* 4 | 4 */ int subxcnt;
/* 8 | 1 */ _Bool subxid_overflow;
/* XXX 3-byte hole */
/* 12 | 4 */ TransactionId nextXid;
/* 16 | 4 */ TransactionId oldestRunningXid;
/* 20 | 4 */ TransactionId latestCompletedXid;
/* 24 | 0 */ TransactionId xids[];

/* total size (bytes): 24 */
}

to

/* offset | size */ type = struct xl_running_xacts {
/* 0 | 4 */ int xcnt;
/* 4 | 4 */ int subxcnt;
/* 8 | 4 */ TransactionId nextXid;
/* 12 | 4 */ TransactionId oldestRunningXid;
/* 16 | 4 */ TransactionId latestCompletedXid;
/* 20 | 1 */ _Bool subxid_overflow;
/* XXX 3-byte hole */
/* 24 | 0 */ TransactionId xids[];

/* total size (bytes): 24 */
}

By moving subxid_overflow before the flexible array.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Mon, Feb 16, 2026 at 1:18 AM Thomas Munro <thomas.munro@gmail.com> wrote:

But in this case, it seems we don't actually worry about initialising
WAL padding bytes in general. valgrind.supp has an entry to prevent
warnings about it. Should we?

Most of the xlog records are zeroed, so having different behaviour for
some records feels inconsistent. For context, I've been trying to
write an ImHex's pattern for WAL files[0]https://github.com/bonnefoa/ImHex-Patterns/blob/pg_xlog/patterns/postgres_xlog.hexpat, and stumbling upon random
values was definitely confusing.

On Mon, Feb 16, 2026 at 12:17 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

So if I read this correctly, '{0}' will set padding bytes to 0 when
using C11. But given Postgres is using C99, that's not something we
can rely on?

C11 is required as of f5e0186f865c so it looks like we could make use of
{0} instead.

Ha good point, I've missed the switch to C11. So using '{0}' seems
like an option. I've run some additional tests to check what was the
generated machine code[1]https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1AB9U8lIAHQ5gByrdqtrFMaYulJL6yAngGVG6ADCqLQAriwMIABMpAEAMngMtuEARpjEIJJclqgKhL4MwWER0Tl5PgIJSTap6ZnZni4VDEIETMQEReGRMY3eBa3tBFXJLGkZWR5tHV0lvdPDiaPj9QCUHqihxMjsHACkGgCCe1EAzInIYVgA1HungQoE%2BIIAdAh32AfHZxdXmLf3R7oFKoEJvD5fE7nBiXUI3O4PAjERLAcGnT5HKG/OH/BFA3xojHHI4EACeFkwWCo10exFC3mujmMdIYDBRxlUTG8CkhAHYAEJfa7XRIEYXi1TIQR3QVHYWi8XChShFKS6WnWWHYUgkKK5WqvDoYyoABu6SotFQAHcZULrqFRacosYxUlVAQABqG21y%2B2O51ikJYR4AJVCrJRXvQPq1fsETpd1wMBEwj2CLAs9BT6CjMbtDvjAcZhp5AFZBaWACJ53mVxm0ZnhtlGDlcgg8jWQo4m1DekmpgjGRKECA9w2rPmag4ATiZLObwFb3PrzmQALrewFGk31c7mNrXcOY%2Bj/ceQ4Y%2BBNhtCYnPI%2BPE/3msVc6b7M5y8cq/XtwFLzVYp3HWGikLcvqKsKLz6gBP4gWBsYQdcUEqqohrGmaxAWtaP5UGISikHaiEvG6nqGrBBHgRBLxBgOYYRkYUbkYRVHJgO6aZpg2aMUB1zbgee7HAemLdr2J6HCmZ5sCwSgEKOomPoJz4Sg287vm2Cgri4MaKlJMkQCcABsX4uKBcF5AAXpgqBUBAxnIKsE4CTuHDrLQnClrwkQcFopCoCAdkkO4BCaC56wANYgKcBkvFFsUGXFsW8tIbkcJIvAsCAvIaC807xVwvJRAAHBokilk6paFaQXk%2BX5HC8AoIAgcF3kuaQcCwEgaAZnQ6TkJQXUWD1GTIJchjAFEXBcCBNC0CmxANRAKQhaQKSJO0pKcDwK1rcQpIAPIpNoTSbbwXVsIIe0MLQG0taQWApKEwCBGItANdwvBYCwY3iLd%2BCrj4ZpvT5mCqC4oQpid5CCJgKU%2BbQeApMQ63BFgy1IngGXvaQGEgkolaYF9Rjw0YIXrBaTDAAoABqeCYFae0Ul5W38IIIhiOwUgyIIigqOot26MlY0gKYxjmPDKQNZA6yoBYzRvQAtECQHIJNvCmukyLBvA6x9M0/gXrMPTSPESw1GMdTSBYuT5AIhvRJb1vNCMZsrNIusDAsdtRG7MNNB7QzO7UExTEMXtuwsgfmxMOubNsHOue5nnLbVjKFQZ8sGZI1yjUY1wTS8XAvBo1wQLghAkLcZxcKsvDNVo6wQJ1qDdfQZAUBAA1DSgwBcKWMQzXNC1Lbdq3MLtkOj%2BtB1Hd4kNnYwBCXddy33Y9z20K9kOfd9uw%2BX9fuA8tINgxDWOijDy3i0ju0o7vtfIpjW047kmD44TwDE6ALVkwYlM03TDNGCQxZsIUQ4hOYgJ5moZaugYgGBJiLMWCNJYQGlrLAoCslanErCrLa6tiCa1TNrDwvt%2Bh%2BAgAEO22QTbVCDnoK25QChULKDbBgkcVgNFIc0QYMwQjdD0O7AQPDFi0KjgIz2fCSgNAjqbOh1cNhbB2BIBOHAPJVWTpwVO6dM7XGACNa4vcXhRBLmXIgxBK6nGrrXUmbUm4t16u3TurcQDtBYCaQq8sRrwPGpNaadBB6UGHj5Se48sYhP2odY6WN54XSujdPemAHpPRem9La28iZ3zungf6eBD63WPsgcGuwtrn1hrwK%2ByMMCZPRo/Xgz88YE2%2BiiGx5M/603pozYBsg2bgOkJApQ0D%2BZ6G8cLMw%2BhkHEJlnLTgisnjK1Vr5DChCUE6y4QUfWQRJE9EqjQ5YdQposOaF7SqDDWHsP2SBQRLQJHFG2SQrw3CZGiI4Zcm5/Ciohw6OcjIU0Y6KPjvoRO6jbop1ce4%2BWTBs7eLzoXQuxdS74DMRYqxpA66tUbigZug1W59Q7liruohiAsEzj3PufjZrpCHstcJE8doRJns1LaMTF5xJXokteKSt6NIySvbJB8iH5NBoU0%2BJToZlPGYjSpqNbo1MhvU1%2B3KP7NO/nwX%2B1N2mAKZrwEBPSOZ9NkFAvmPldCnH0ELRBEqVm%2BXQQITBczsG4LVksw0RCpb3L9uQyhWyJAxF2S7Oo3tDlMO9VwGIpynayLEYGq5wiqG9DWUIp5eyMjRreVI%2BYAdI0rG9n8uOyjAWqKTiCzRhwQwAFkdEAHFAiBAMaWIxRcTGIorlCFFaKHKkAivFF48Ve0JV7aWZKnA0rApqpweqjVUWkxUVEItY66pTu/usDCeQ/CSCAA%3D%3D%3D (sorry for the long godbolt link...):
- x86 clang: Calls memset
- x86 gcc: Sets 128 bits + 64 bits to 0
- arm clang: Sets 64 bits * 3 to 0
- arm gcc: Sets 128 bits + 64 bits to 0

So it looks like '{0}' does zero the padding on this example. For
clang, the generated machine code is the exact same as calling memset.

I've found 2 other places where padding bytes aren't set: heap inplace
and invalidation messages. I've updated the patch:
- Initialize xl_running_xacts, xl_invalidations and xl_heap_inplace using '{0}'.
- Use MemoryContextAllocZero instead of MemoryContextAlloc for shared
invalidation messages

[0]: https://github.com/bonnefoa/ImHex-Patterns/blob/pg_xlog/patterns/postgres_xlog.hexpat
[1]: https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1AB9U8lIAHQ5gByrdqtrFMaYulJL6yAngGVG6ADCqLQAriwMIABMpAEAMngMtuEARpjEIJJclqgKhL4MwWER0Tl5PgIJSTap6ZnZni4VDEIETMQEReGRMY3eBa3tBFXJLGkZWR5tHV0lvdPDiaPj9QCUHqihxMjsHACkGgCCe1EAzInIYVgA1HungQoE%2BIIAdAh32AfHZxdXmLf3R7oFKoEJvD5fE7nBiXUI3O4PAjERLAcGnT5HKG/OH/BFA3xojHHI4EACeFkwWCo10exFC3mujmMdIYDBRxlUTG8CkhAHYAEJfa7XRIEYXi1TIQR3QVHYWi8XChShFKS6WnWWHYUgkKK5WqvDoYyoABu6SotFQAHcZULrqFRacosYxUlVAQABqG21y%2B2O51ikJYR4AJVCrJRXvQPq1fsETpd1wMBEwj2CLAs9BT6CjMbtDvjAcZhp5AFZBaWACJ53mVxm0ZnhtlGDlcgg8jWQo4m1DekmpgjGRKECA9w2rPmag4ATiZLObwFb3PrzmQALrewFGk31c7mNrXcOY%2Bj/ceQ4Y%2BBNhtCYnPI%2BPE/3msVc6b7M5y8cq/XtwFLzVYp3HWGikLcvqKsKLz6gBP4gWBsYQdcUEqqohrGmaxAWtaP5UGISikHaiEvG6nqGrBBHgRBLxBgOYYRkYUbkYRVHJgO6aZpg2aMUB1zbgee7HAemLdr2J6HCmZ5sCwSgEKOomPoJz4Sg287vm2Cgri4MaKlJMkQCcABsX4uKBcF5AAXpgqBUBAxnIKsE4CTuHDrLQnClrwkQcFopCoCAdkkO4BCaC56wANYgKcBkvFFsUGXFsW8tIbkcJIvAsCAvIaC807xVwvJRAAHBokilk6paFaQXk%2BX5HC8AoIAgcF3kuaQcCwEgaAZnQ6TkJQXUWD1GTIJchjAFEXBcCBNC0CmxANRAKQhaQKSJO0pKcDwK1rcQpIAPIpNoTSbbwXVsIIe0MLQG0taQWApKEwCBGItANdwvBYCwY3iLd%2BCrj4ZpvT5mCqC4oQpid5CCJgKU%2BbQeApMQ63BFgy1IngGXvaQGEgkolaYF9Rjw0YIXrBaTDAAoABqeCYFae0Ul5W38IIIhiOwUgyIIigqOot26MlY0gKYxjmPDKQNZA6yoBYzRvQAtECQHIJNvCmukyLBvA6x9M0/gXrMPTSPESw1GMdTSBYuT5AIhvRJb1vNCMZsrNIusDAsdtRG7MNNB7QzO7UExTEMXtuwsgfmxMOubNsHOue5nnLbVjKFQZ8sGZI1yjUY1wTS8XAvBo1wQLghAkLcZxcKsvDNVo6wQJ1qDdfQZAUBAA1DSgwBcKWMQzXNC1Lbdq3MLtkOj%2BtB1Hd4kNnYwBCXddy33Y9z20K9kOfd9uw%2BX9fuA8tINgxDWOijDy3i0ju0o7vtfIpjW047kmD44TwDE6ALVkwYlM03TDNGCQxZsIUQ4hOYgJ5moZaugYgGBJiLMWCNJYQGlrLAoCslanErCrLa6tiCa1TNrDwvt%2Bh%2BAgAEO22QTbVCDnoK25QChULKDbBgkcVgNFIc0QYMwQjdD0O7AQPDFi0KjgIz2fCSgNAjqbOh1cNhbB2BIBOHAPJVWTpwVO6dM7XGACNa4vcXhRBLmXIgxBK6nGrrXUmbUm4t16u3TurcQDtBYCaQq8sRrwPGpNaadBB6UGHj5Se48sYhP2odY6WN54XSujdPemAHpPRem9La28iZ3zungf6eBD63WPsgcGuwtrn1hrwK%2ByMMCZPRo/Xgz88YE2%2BiiGx5M/603pozYBsg2bgOkJApQ0D%2BZ6G8cLMw%2BhkHEJlnLTgisnjK1Vr5DChCUE6y4QUfWQRJE9EqjQ5YdQposOaF7SqDDWHsP2SBQRLQJHFG2SQrw3CZGiI4Zcm5/Ciohw6OcjIU0Y6KPjvoRO6jbop1ce4%2BWTBs7eLzoXQuxdS74DMRYqxpA66tUbigZug1W59Q7liruohiAsEzj3PufjZrpCHstcJE8doRJns1LaMTF5xJXokteKSt6NIySvbJB8iH5NBoU0%2BJToZlPGYjSpqNbo1MhvU1%2B3KP7NO/nwX%2B1N2mAKZrwEBPSOZ9NkFAvmPldCnH0ELRBEqVm%2BXQQITBczsG4LVksw0RCpb3L9uQyhWyJAxF2S7Oo3tDlMO9VwGIpynayLEYGq5wiqG9DWUIp5eyMjRreVI%2BYAdI0rG9n8uOyjAWqKTiCzRhwQwAFkdEAHFAiBAMaWIxRcTGIorlCFFaKHKkAivFF48Ve0JV7aWZKnA0rApqpweqjVUWkxUVEItY66pTu/usDCeQ/CSCAA%3D%3D%3D

Hi,

On 2026-02-16 12:02:33 +0100, Anthonin Bonnefoy wrote:

I think that depends on the C standard used. With C99, there's no rule
for the padding bytes initialization.
With C11, in 6.7.9 Initialization of the standard: "the remainder of
the aggregate shall be initialized implicitly the same as objects that
have static storage duration", and with static storage will "every
member is initialized (recursively) according to these rules, and any
padding is initialized to zero bits".

I don't think that rule applies for things like xl_running_xacts, as it does
not have static storage duration.

So if I read this correctly, '{0}' will set padding bytes to 0 when
using C11. But given Postgres is using C99, that's not something we
can rely on?

We use C11, but the guarantee doesn't help us, due to the static storage
duration restriction. However, in C23, this has been fixed:

6.7.10 Initialization, point 11:

If an object that has automatic storage duration is initialized with an empty
initializer, its value is the same as the initialization of a static storage
duration object. Otherwise, if an object that has automatic storage duration
is not initialized explicitly, its representation is indeterminate. [...]

This notably includes being able to initialize everything to default with {}.

But C23 won't help us for a while :(

True about the initialization part, mostly I guess, still we tend to
worry about eliminating padding because these are wasted bytes in the
WAL records. For example, xlhp_freeze_plans has two bytes of padding,
that we eliminate while inserting its record by splitting the
FLEXIBLE_ARRAY_MEMBER part.

But in the case of this thread it's in the middle of the struct, so I'm not
sure the "wasted" bytes would be elminated, would it?

Moving subxid_overflow before xids, wouldn't you have 3 bytes of
padding at the end of the struct for the whole struct alignment?

Yes. I'm a bit doubtful the space wastage argument is strong for most of the
record types with padding, for a lot of them the waste through the padding is
such a small amount compared to the record type that it won't matter.

I don't think it makes a whole lot of sense to tackle this specifically for
xl_running_xacts. Until now we just accepted that WAL insertions can contain
random padding. If we don't want that, we should go around and make sure that
there is no padding (or padding is initialized) for *all* WAL records,
document that as the rule, and remove the relevant valgrind suppressions.

A lot of the WAL structs have holes. At least
- xl_brin_update
- xl_btree_mark_page_halfdead
- xl_btree_unlink_page
- xl_hash_vacuum_one_page
- xl_heap_inplace
- xl_heap_multi_insert
- xl_heap_rewrite_mapping
- xl_heap_truncate
- xl_invalidations
- xl_logical_message
- xl_multixact_create
- xl_running_xacts
- xl_xact_prepare
- xlhp_freeze_plan (not a toplevel type)
- xlhp_freeze_plans (not a toplevel type)

I didn't check how many WAL record have trailing padding that we don't avoid
with
offsetoff(structname, last_field) + sizeof(last_field_type)
style hackery.

Greetings,

Andres Freund

Until now we just accepted that WAL insertions can contain
random padding. If we don't want that, we should go around and make sure that
there is no padding (or padding is initialized) for *all* WAL records,
document that as the rule, and remove the relevant valgrind suppressions.

While this would be a nice requirement, I don't think we can enforce
it for extensions, only for the core, as C has no capabilities to add
a rule for this.

But we could enforce it for the core code, what do you think about a
script that similarly to headercheck detects WAL record issues
automatically? That's also good for detecting the current issues, see
attached script and the results I get when executing it on the current
master branch.

Notes:
* It tries to find the related SizeOf macros, and if that exists,
accepts trailing padding if it's correctly calculated. But it
currently doesn't verify that the SizeOf macro is currently used
everywhere (that also seems doable with some greps)
* It also has a whitelist for non wal structs in these headers and 2
cases where we explicitly document that padding is ok (not sure if
this latter should be really in the whitelist or not)

On 16/02/2026 21:10, Andres Freund wrote:

I don't think it makes a whole lot of sense to tackle this specifically for
xl_running_xacts. Until now we just accepted that WAL insertions can contain
random padding. If we don't want that, we should go around and make sure that
there is no padding (or padding is initialized) for *all* WAL records,
document that as the rule, and remove the relevant valgrind suppressions.

That's not random, that's server memory, right? Probably not another
Heartbleed, but I'd rather initialize a few locals than find out.

Happy to see this being worked on, these uninitialized WAL records are a
major obstacle to enabling MemorySanitizer. I ran into this again today
and this is how I found this thread. Unfortunately the MemorySanitizer
can't even use the same suppressions as Valgrind, because the
suppression architecture is different (can only remove the checks from a
given function, not all stack traces that have this function like
Valgrind does).

Best regards
Alexander Kuzmenkov
TigerData

On 10/03/2026 23:51, Alexander Kuzmenkov wrote:

On 16/02/2026 21:10, Andres Freund wrote:

I don't think it makes a whole lot of sense to tackle this
specifically for
xl_running_xacts. Until now we just accepted that WAL insertions can
contain
random padding. If we don't want that, we should go around and make
sure that
there is no padding (or padding is initialized) for *all* WAL records,
document that as the rule, and remove the relevant valgrind suppressions.

That's not random, that's server memory, right? Probably not another
Heartbleed, but I'd rather initialize a few locals than find out.

Happy to see this being worked on, these uninitialized WAL records are a
major obstacle to enabling MemorySanitizer. I ran into this again today
and this is how I found this thread. Unfortunately the MemorySanitizer
can't even use the same suppressions as Valgrind, because the
suppression architecture is different (can only remove the checks from a
given function, not all stack traces that have this function like
Valgrind does).

+1 for initializing all padding in WAL records. In fact I thought that
we already did that. (Except in this case, apparently)

- Heikki

On Tue, Mar 10, 2026 at 11:09 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:

+1 for initializing all padding in WAL records. In fact I thought that
we already did that. (Except in this case, apparently)

I found 42 exceptions like this. See the attached patch, it
initializes some WAL records and removes the WAL-related Valgrind
suppressions. The regression tests pass under Valgrind with these
changes.

As discussed above, I used memset instead of = { 0 }. I could observe
the latter to not initialize the padding on some configurations.

On Wed, Mar 11, 2026 at 11:45 AM Alexander Kuzmenkov
<akuzmenkov@tigerdata.com> wrote:

On Tue, Mar 10, 2026 at 11:09 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:

+1 for initializing all padding in WAL records. In fact I thought that
we already did that. (Except in this case, apparently)

I found 42 exceptions like this. See the attached patch, it
initializes some WAL records and removes the WAL-related Valgrind
suppressions. The regression tests pass under Valgrind with these
changes.

I think I'm making some unneeded changes here though. For example in
ginxlogInsertListPage for a two-int struct with no padding. I'll need
to check them again one by one.

On 11/03/2026 13:07, Alexander Kuzmenkov wrote:

On Wed, Mar 11, 2026 at 11:45 AM Alexander Kuzmenkov
<akuzmenkov@tigerdata.com> wrote:

On Tue, Mar 10, 2026 at 11:09 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:

+1 for initializing all padding in WAL records. In fact I thought that
we already did that. (Except in this case, apparently)

I found 42 exceptions like this. See the attached patch, it
initializes some WAL records and removes the WAL-related Valgrind
suppressions. The regression tests pass under Valgrind with these
changes.

I think I'm making some unneeded changes here though. For example in
ginxlogInsertListPage for a two-int struct with no padding. I'll need
to check them again one by one.

I experimented with this a little more. Valgrind complained about one
more place on 'master': the xl_multixact_create got padding, when
MultiXactOffset was widened to 64 bits. That could be fixed by swapping
the fields.

Another thing I did to find possible initializations: I ran 'pahole
bin/postgres' and search for all the "xl_*" structs with padding, and
then looked at where they're initialized. Attached patch (0003) shows a
few places that look suspicious to me. I don't think I caught all
structs used in WAL records, though, like the ginxlogInsertListPage
thing mentioned.

I wish we could just mark all WAL record structs with
pg_attribute_packed(). Unfortunately pg_attribute_packed() is not
available on all compilers we support.

- Heikki

The functions in the "0003" patch haven't surfaced in my "make
installcheck-parallel" runs with Valgrind, or the "make check" with
MemorySanitizer. However, I could hit most of them with some fuzzing. The
only exception was `xl_hash_vacuum_one_page` but that's probably also
triggerable.

I noticed that we also use `sizeof` in some WAL functions, so probably the
tail padding can also be written to WAL? For example, consider this:
(gdb) ptype/o gistxlogPageSplit
type = struct gistxlogPageSplit {
/* 0 | 4 */ BlockNumber origrlink;
/* XXX 4-byte hole */
/* 8 | 8 */ GistNSN orignsn;
/* 16 | 1 */ _Bool origleaf;
/* XXX 1-byte hole */
/* 18 | 2 */ uint16 npage;
/* 20 | 1 */ _Bool markfollowright;
/* XXX 3-byte padding */

/* total size (bytes): 24 */
}

And then we do XLogRegisterData((char *) &xlrec,
sizeof(gistxlogPageSplit));

In general, I'm wondering what our approach to this should be. Several
potential improvements were mentioned, but I think for now we could focus
on removing the Valgrind suppression. This is a meaningful improvement that
uses the existing test tools. Do we want to defensively zero-initialize
every case that seems to be potentially affected, i.e. written to WAL and
has holes/tail padding? That sounds cheap and simple and probably even
backportable. In the "0001" patch, there are several cases where no padding
goes into WAL, I can remove these. For example, the use of
xl_brin_createidx in brinbuild() does not have this problem.

On 12/03/2026 20:23, Alexander Kuzmenkov wrote:

The functions in the "0003" patch haven't surfaced in my "make
installcheck-parallel" runs with Valgrind, or the "make check" with
MemorySanitizer. However, I could hit most of them with some fuzzing.
The only exception was `xl_hash_vacuum_one_page` but that's probably
also triggerable.

Cool. It would be nice to have test coverage for every WAL record type.
Could you add tests to the test suite to hit those cases?

I noticed that we also use `sizeof` in some WAL functions, so probably
the tail padding can also be written to WAL? For example, consider this:
(gdb) ptype/o gistxlogPageSplit
type = struct gistxlogPageSplit {
/*      0      |       4 */    BlockNumber origrlink;
/* XXX  4-byte hole      */
/*      8      |       8 */    GistNSN orignsn;
/*     16      |       1 */    _Bool origleaf;
/* XXX  1-byte hole      */
/*     18      |       2 */    uint16 npage;
/*     20      |       1 */    _Bool markfollowright;
/* XXX  3-byte padding   */

                               /* total size (bytes):   24 */
                             }

And then we do  XLogRegisterData((char *) &xlrec,
sizeof(gistxlogPageSplit));

Yep.

In general, I'm wondering what our approach to this should be. Several
potential improvements were mentioned, but I think for now we could
focus on removing the Valgrind suppression. This is a meaningful
improvement that uses the existing test tools.

+1. I think it's a good goal that no uninitialized bytes reach the WAL.
It's not a security issue or anything, but just seems like good hygiene.

Do we want to defensively zero-initialize every case that seems to
be potentially affected, i.e. written to WAL and has holes/tail
padding? That sounds cheap and simple and probably even
backportable. In the "0001" patch, there are several cases where no
padding goes into WAL, I can remove these. For example, the use of
xl_brin_createidx in brinbuild() does not have this problem.

Sounds good to me.

- Heikki

I have removed the unnecessary memsets (for structs with no padding). With
these changes, and removing the two WAL-related suppressions, the make
installcheck under Valgrind passes. The second patch is a small addition to
the hash index test that exercises the "vacuum one page" path we discussed
above.