Feedback-ID: id4a34324:Fastmail
Date: Mon, 16 Feb 2026 15:10:40 -0500
From: Andres Freund <andres@anarazel.de>
To: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, 
	Michael Paquier <michael@paquier.xyz>, Thomas Munro <thomas.munro@gmail.com>, 
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Fix uninitialized xl_running_xacts padding
Message-ID: <aoaj45foewpjtu6r5cs67yrx4en3pkurs23e4azv6tbikpw6c3@h3pnaqaksoeg>
References: <CAO6_Xqoxp7C+y0L==xZXH5V=9PjpBx4T9vJYs87EbxFp_9nwOA@mail.gmail.com>
 <CA+hUKG++LE6P6g4n+-QPHBwAnvcVRyG1tUnzscUriWAFHc6s6Q@mail.gmail.com>
 <aZJuoggMUaXjwoDO@paquier.xyz>
 <aZLHYtCsEldmm8Eu@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAO6_XqpGYneOe357s4QhOTYpU6UEgk3JJrBiyeuk-9n=f7p=Vw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAO6_XqpGYneOe357s4QhOTYpU6UEgk3JJrBiyeuk-9n=f7p=Vw@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/aoaj45foewpjtu6r5cs67yrx4en3pkurs23e4azv6tbikpw6c3%40h3pnaqaksoeg>
Precedence: bulk

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