public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andres Freund <[email protected]>
To: Anthonin Bonnefoy <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Fix uninitialized xl_running_xacts padding
Date: Mon, 16 Feb 2026 15:10:40 -0500
Message-ID: <aoaj45foewpjtu6r5cs67yrx4en3pkurs23e4azv6tbikpw6c3@h3pnaqaksoeg> (raw)
In-Reply-To: <CAO6_XqpGYneOe357s4QhOTYpU6UEgk3JJrBiyeuk-9n=f7p=Vw@mail.gmail.com>
References: <CAO6_Xqoxp7C+y0L==xZXH5V=9PjpBx4T9vJYs87EbxFp_9nwOA@mail.gmail.com>
	<CA+hUKG++LE6P6g4n+-QPHBwAnvcVRyG1tUnzscUriWAFHc6s6Q@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAO6_XqpGYneOe357s4QhOTYpU6UEgk3JJrBiyeuk-9n=f7p=Vw@mail.gmail.com>

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

view thread (16+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Fix uninitialized xl_running_xacts padding
  In-Reply-To: <aoaj45foewpjtu6r5cs67yrx4en3pkurs23e4azv6tbikpw6c3@h3pnaqaksoeg>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox