Re: abi-compliance-checker

From: Andres Freund <andres(at)anarazel(dot)de>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: abi-compliance-checker
Date: 2023-06-10 19:48:46
Message-ID: 20230610194846.bpnawuofjnt2r4mf@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2023-06-06 18:30:38 +0200, Peter Eisentraut wrote:
> On 30.05.23 06:32, Peter Eisentraut wrote:
> > I think the way to use this would be to compute the ABI for the .0
> > release (or rc1 or something like that) and commit it into the tree. And
> > then compute the current ABI and compare that against the recorded base
> > ABI.
> >
> > Here is the workflow:
> >
> > # build REL_11_0
> > abidw src/backend/postgres > src/tools/postgres-abi-REL_11_0.xml
> > # build REL_11_20
> > abidw src/backend/postgres > src/tools/postgres-abi.xml
> > abidiff --no-added-syms src/tools/postgres-abi-REL_11_0.xml
> > src/tools/postgres-abi.xml
>
> Here is a demo patch that implements this.
>
> Right now, I have only added support for libpq and postgres. For
> completeness, the ecpg libraries should be covered as well.

I think plpgsql would also be good to include, due to things like plpgsql
debuggers.

> * Different Linux distributions produce slightly different ABI reports. In
> some cases, symbols like 'optarg(at)GLIBC_2(dot)17' leak out.

Hm, that's somewhat annoying.

> * PostgreSQL compilation options affect the exposed ABI. This is perhaps
> expected to some degree, but there are some curious details.
>
> * For example, --enable-cassert exposes additional symbols, and it's maybe
> not impossible for those to leak into an extension.

They *definitely* leak into extensions. A single Assert() in an extension or
use of an inline function or macro with an Assertion suffices to end up with a
reference to ExceptionalCondition.

> diff --git a/src/interfaces/libpq/libpq.base.abi.xml b/src/interfaces/libpq/libpq.base.abi.xml
> new file mode 100644
> index 0000000000..691bf192af
> --- /dev/null
> +++ b/src/interfaces/libpq/libpq.base.abi.xml
> @@ -0,0 +1,2634 @@
> +<abi-corpus path='src/interfaces/libpq/libpq.so.5.16' soname='libpq.so.5'>
> + <elf-function-symbols>
> + <elf-symbol name='PQbackendPID' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
> + <elf-symbol name='PQbinaryTuples' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
> + <elf-symbol name='PQcancel' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
> [...]
> + <elf-symbol name='termPQExpBuffer' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
> + </elf-function-symbols>

This seems somewhat painful in its verbosity. We also effectively already have
it in the tree, in src/interfaces/libpq/exports.txt. But I guess that's
somewhat inevitable :/

It sounds we are planning to mostly rely on CI for this, perhaps we should
rely on an artifact from a prior build for a major version + specific task,
instead of committing this to source? That'd automatically take care of
differences in ABI across different platforms etc.

If we want to commit something to the tree, I think we need a fairly
complicated "fingerprint" to avoid false positives. OS, OS version, configure
options, compiler, compiler version at least?

> + <abi-instr version='1.0' address-size='64' path='../src/common/encnames.c' language='LANG_C99'>
> + <array-type-def dimensions='1' type-id='c8dedbef' size-in-bits='5376' id='752c85d9'>
> + <subrange length='42' type-id='7359adad' id='cb7c937f'/>
> + </array-type-def>
> + <array-type-def dimensions='1' type-id='c8dedbef' size-in-bits='infinite' id='ac835593'>
> + <subrange length='infinite' id='031f2035'/>
> + </array-type-def>
> + <array-type-def dimensions='1' type-id='56ef96d7' size-in-bits='5376' id='728d2ee1'>
> + <subrange length='42' type-id='7359adad' id='cb7c937f'/>
> + </array-type-def>
> + <array-type-def dimensions='1' type-id='56ef96d7' size-in-bits='infinite' id='a01b33bb'>
> + <subrange length='infinite' id='031f2035'/>
> + </array-type-def>
> + <typedef-decl name='pg_enc2name' type-id='79f06fd8' id='7a4268c7'/>
> + <class-decl name='pg_enc2name' size-in-bits='128' is-struct='yes' visibility='default' id='79f06fd8'>
> + <data-member access='public' layout-offset-in-bits='0'>
> + <var-decl name='name' type-id='80f4b756' visibility='default'/>
> + </data-member>
> + <data-member access='public' layout-offset-in-bits='64'>
> + <var-decl name='encoding' type-id='66325df6' visibility='default'/>
> + </data-member>
> + </class-decl>
> + <typedef-decl name='pg_enc' type-id='ea65169a' id='66325df6'/>
> + <enum-decl name='pg_enc' id='ea65169a'>
> + <underlying-type type-id='9cac1fee'/>

Hm - why is all of this stuff even ending up in the external ABI? It should
all be internal, unless I am missing something?

I might be looking the wrong way, but to me it sure looks like none of that
ends up being externally visible?

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2023-06-10 20:12:36 Re: Do we want a hashset type?
Previous Message Tom Lane 2023-06-10 19:32:36 Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG