| From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
|---|---|
| To: | Alexander Kuzmenkov <akuzmenkov(at)tigerdata(dot)com> |
| Cc: | Andres Freund <andres(at)anarazel(dot)de>, Anthonin Bonnefoy <anthonin(dot)bonnefoy(at)datadoghq(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Fix uninitialized xl_running_xacts padding |
| Date: | 2026-03-12 12:54:52 |
| Message-ID: | f2875d6b-5ff6-4744-b151-f5ab54cbd2c5@iki.fi |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 11/03/2026 13:07, Alexander Kuzmenkov wrote:
> On Wed, Mar 11, 2026 at 11:45 AM Alexander Kuzmenkov
> <akuzmenkov(at)tigerdata(dot)com> wrote:
>>
>> On Tue, Mar 10, 2026 at 11:09 PM Heikki Linnakangas <hlinnaka(at)iki(dot)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
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-Initialize-WAL-record-structs.patch | text/x-patch | 14.4 KB |
| 0002-Avoid-padding-in-xl_multixact_create-WAL-record.patch | text/x-patch | 1.7 KB |
| 0003-XXX-a-few-more-places-that-maybe-need-clearing.patch | text/x-patch | 3.8 KB |
| 0004-Add-an-explicit-valgrind-check-in-XLogInsert-for-uni.patch | text/x-patch | 1.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Aleksander Alekseev | 2026-03-12 12:56:26 | [PATCH] Silence a new Valgrind warning |
| Previous Message | Ashutosh Sharma | 2026-03-12 12:43:33 | Re: Report bytes and transactions actually sent downtream |