Re: ABI Compliance Checker GSoC Project - Mailing list pgsql-hackers

On Tue, 3 Jun 2025 at 20:49, Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> 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,

 Yes, but it would be better if we could automate that thing to some
extent, along with the development of the ABI compliance checker.

> 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.
That's exactly what I aim for with this project, by just automatically
run the comparing the latest commit with the previous using abidiff
and if some ABI instability found, then immediately report it to the
commiter or some mailing list so that maintainers don't have to do
that manually everytime - That's the reason for asking about false
positives and how to suppress them.


> Also a diff worth reporting, and suppressing from the report as
> appropriate.
Okay - It is a change in name, I assume that's the reason it's important.


> 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.

These sound very similar to 17.1 minor release ABI instability.
abidiff seems to do most of the work in this case:

$ abidiff ./install/with_debug/REL_17_0/bin/postgres
./install/with_debug/REL_17_1/bin/postgres --leaf-changes-only
--no-added-syms --show-bytes
Leaf changes summary: 1 artifact changed
Changed leaf types summary: 1 leaf type changed
Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added
function (11 filtered out)
Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable

'struct ResultRelInfo at execnodes.h:450:1' changed:
  type size changed from 376 to 384 (in bytes)
  1 data member insertion:
    'bool ri_needLockTagTuple', at offset 376 (in bytes) at execnodes.h:597:1


> 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 ...

The reporting system is planned to be developed as an extension to the
postgresql build farm, so the abidiff reports across architectures
from various animals could be received, processed and reported
accordingly.


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

Certainly, will do that only until I understand how to identify and
implement some techniques for the removal of false positives.


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

 I can do that, and that's what's causing the problem. According to
the documentation for these suppression files[1], we have to mention
the particular symbol name we need to suppress like "ReadStream" or
something particular like "ignore offset changes for ios_in_progress
in ReadStream between member1 and member 2" which is humanly very hard
to do as for sure there will be 100s of symbols in postgres like this
which needs to be ignored in that case.


> > 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.

This led me to another doubt, which might make things clear, that is,
any symbol which abidiff gives in output is important to report?
because my initial thought was that symbols exported in the binary
created when we build postgres also contain some symbols which need to
be suppressed because they are some postgres internal functions[2] -
is that true?
If it's not true, then things seem to be much sorted than before.

Note - The -fvisibility=hidden flag I mentioned was on a similar note
that when we compile postgres with this flag, it should generate
postgres binary by removing internal symbols, but instead it gives a
compilation error.

Regards,
Mankirat

[1] - https://sourceware.org/libabigail/manual/suppression-specifications.html#suppr-spec-label
[2] - https://www.postgresql.org/message-id/CAH2-Wzm-W6hSn71sUkz0Rem%3DqDEU7TnFmc7_jG2DjrLFef_WKQ%40mail.gmail.com