Re: Use -fvisibility=hidden for shared libraries

Hi,

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 pgxs.mk (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 pgxs.mk
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
following:
- 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 pgxs.mk 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.

Greetings,

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.