Re: ABI Compliance Checker GSoC Project

On 2025-Jun-03, Mankirat Singh wrote:

> 'struct ReadStream at read_stream.c:109:1' changed:
> type size hasn't changed
> 1 data member insertion:
> 'int16 io_combine_limit', at offset 2 (in bytes) at read_stream.c:112:1
> there are data member changes:
> 'int16 ios_in_progress' offset changed from 2 to 4 (in bytes) (by +2
> bytes)
> 'int16 queue_size' offset changed from 4 to 6 (in bytes) (by +2 bytes)
> 'int16 max_pinned_buffers' offset changed from 6 to 8 (in bytes) (by +2
> bytes)
> 'int16 pinned_buffers' offset changed from 8 to 10 (in bytes) (by +2
> bytes)
> 'int16 distance' offset changed from 10 to 12 (in bytes) (by +2 bytes)
> 'bool advice_enabled' offset changed from 12 to 14 (in bytes) (by +2
> bytes)

Well, this is an ABI difference all right, and I don't think it's the
job of the tool to determine that this ABI difference is okay.
Ultimately that's for a human to determine, and they must either add an
suppression to the Postgres source code somehow, or modify or revert the
commit so that the ABI change disappears.

I'm surprised to hear that libabigail has no way for us to declare that
an ABI diff should be ignored. Suppressions were part of the original
plan, and I fear that if they aren't possible, then this whole thing may
not work for us.

> 'struct WalSndCtlData at walsender_private.h:91:1' changed:
> type size hasn't changed
> there are data member changes:
> name of 'WalSndCtlData::sync_standbys_defined' changed to
> 'WalSndCtlData::sync_standbys_status' at walsender_private.h:110:1

Also a diff worth reporting, and suppressing from the report as
appropriate.

The only kinds of ABI changes that should be silenced by default are

1) addition of struct members that cause no change to the offsets of other
members in the struct (i.e. a new member that uses space that was
previously padding space)

2) addition of struct members at the end of structs, changing the struct
size, but only for structs that aren't used as array elements (the
problem being that the size of the "stride" when scanning the array
would mismatch). Not sure how easy it is to detect this case.

I'm also wondering what platforms/architectures are you thinking on
running this on. For instance, thinking about padding space in structs,
what is padding space in x86_64 may not be so in 32bit x86 ...

> I don't have much idea about the PostgreSQL internal code, but I really
> wanted to know how we can "classify" what exactly is a false positive for
> this report and should not be reported by the final ABI compliance checking
> tool?

I think in a first cut of this tooling, we should consider all ABI
changes as potentially problematic.

> I checked the suppression file for libabigail tools, but it seems not to
> work for the PostgreSQL case, as we need to specify each and every symbol
> to be ignored in the file, which is kinda not possible.

Please elaborate. Can you not write a suppression file that says
"ignore offset changes for ios_in_progress in ReadStream", for example?

> And these symbols to be suppressed don't follow any common Regex as
> well, and the suppression file doesn't support including whole files
> or folders.

Ummm, are you saying that it complains about changes to unexported
symbols also?

> Secondly, we can't use the -fvisibility=hidden flag while compiling
> either, as it results in a failed compilation with an error.

I didn't understand what you meant here.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"All rings of power are equal,
But some rings of power are more equal than others."
(George Orwell's The Lord of the Rings)