Re: Use -fvisibility=hidden for shared libraries

From: Andres Freund <andres(at)anarazel(dot)de>
To: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Use -fvisibility=hidden for shared libraries
Date: 2022-07-16 20:17:50
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 2022-03-24 16:13:31 +0100, Peter Eisentraut wrote:
> On 11.01.22 03:53, Andres Freund wrote:
> > Ugh. In the case of worker_spi it's because -fvisibility=hidden isn't applied
> > to worker_spi.c at all. Which in turn is because Makefile.shlib isn't used at
> > all for MODULES= style modules just for MODULE_big= ones :(.
> >
> > Once that's "fixed" it fails as expected...
> >
> > I'm not sure what the best way to deal with this is. Just now I copied the
> > logic from Makefile.shlib to (by the existing CFLAGS_SL use), but that
> > doesn't scale - there's other direct uses of CFLAGS_SL.
> >
> > Perhaps the best way would be to add the -fvisibility=hidden to CFLAGS_SL, and
> > work around the explicit exports issue in Makefile.shlib by adding an explicit
> > -fvisibility=default? Or perhaps CFLAGS_SL
> The easiest solution would be to change worker_spi's Makefile to use
> MODULE_big.
> There are already many cases where MODULE_big is used instead of MODULES for
> seemingly random reasons, so I wouldn't worry too much about it here if it
> helps you move forward.

If it were just worker_spi that might be tenable, but there's other extension
modules - even if they don't fail to fail right now, we shouldn't change the
symbol export rules based on MODULES vs MODULE_big.

I think the idea that I rejected above, namely adding CFLAGS_SL_MOD in
is the best approach. There aren't actually that many uses of CFLAGS_SL
otherwise - not quite sure anymore why I thought that to be bad.

Is there any reason an extension module would need to directly link against
pgport/pgcommon? I don't think so, right?

What I'm not sure about is what to do about pg_config - we can't just add
-fvisibility=hidden to pg_config --cflags_sl. We could add --cflags_sl_mod -
but I'm not sure it's worth it?

In the attached version, based on Tom's version upthread, I changed the
- moved adding central declarations for _PG_init etc to a separate patch, now
also removes the now duplicated decls
- split addition of PGDLLEXPORT to symbols into a separate patch
- added a central PGDLLEXPORT'ed prototype for_PG_archive_module_init
- Removed .DEF file generation for liraries in src/tools/msvc, only the
backend now generates a DEF file for all symbols ([1])
- added the flags to MODULES
- pgindented changed files

I added CFLAGS_SL_MOD to LDFLAGS_SL, but right now that's not really
necessary, it seems all places using LDFLAGS_SL also use CFLAGS. Which is a
bit weird, but ...

There are a few symbols in plpython that don't need to be exported right now
but are. But it seems better to export a few too many there, as the
alternative is breaking out-of-core transforms. Most of the symbols are used
by the various transform extensions.


Andres Freund

[1] It's not too hard to move away from that, and I suspect it'd be worth
it. I did prototype it. But that's a relatively large change that imo
better is discussed separately.

Attachment Content-Type Size
v3-0001-Add-central-declarations-for-dlsym-ed-symbols.patch text/x-diff 3.5 KB
v3-0002-Remove-now-superfluous-declarations-of-other-dlsy.patch text/x-diff 15.9 KB
v3-0003-Mark-all-symbols-exported-from-extension-librarie.patch text/x-diff 11.0 KB
v3-0004-Use-hidden-visibility-for-shared-libraries-where-.patch text/x-diff 12.5 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Martin Kalcher 2022-07-16 20:21:54 Re: Proposal to introduce a shuffle function to intarray extension
Previous Message Ranier Vilela 2022-07-16 19:54:56 Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)