On 26/08/2024 03:58, Michael Paquier wrote:
An interesting thing is that I have seen ubsan complain about this
patch, due to the way WAL records xl_xact_commit are built with
XACT_XINFO_HAS_DROPPED_STATS and parsed as xl_xact_stats_item requires
an 8-byte alignment now (see pg_waldump TAP reports when using the
attached), but we don't enforce anything as the data of such WAL
records is added with a simple XLogRegisterData(), like:
# xactdesc.c:91:28: runtime error: member access within misaligned
address 0x5651e996b86c for type 'struct xl_xact_stats_items', which
requires 8 byte alignment # 0x5651e996b86c: note: pointer points here
TBH, I've looked at that for quite a bit, thinking about the addition
of some "dummy" member to some of the parsed structures to force some
padding, or play with the alignment macros, or for some alignment when
inserting the record, or looked at pg_attribute_aligned().
First I'm surprised that it did not show up as an issue yet in this
area. Second, I could not get down to something "nice", but perhaps
there are preferred approaches when it comes to that and somebody has
a fancier idea? Or perhaps the problem is bigger than that due to
the way the record is designed and built? It also feels that I'm
missing something obvious, not sure what TBH. Still I'm OK to paint
some more MAXALIGN()s to make sure that all these deparsing pointers
have a correct alignment with some more TYPEALIGN()s or similar,
because this deparsing stuff is about that, but I'm also wondering if
there is an argument for forcing that for the record itself? I'll
think more about that next week or so.
Currently, we rely on the fact that all the xl_xact_* structs require
sizeof(int) alignment. See comment above struct xl_xact_xinfo.
One idea is to store the uint64 as two uint32's.
--
Heikki Linnakangas
Neon (https://neon.tech)
