Thread: pgsql: Default to hidden visibility for extension libraries where possi

Default to hidden visibility for extension libraries where possible

Until now postgres built extension libraries with global visibility, i.e.
exporting all symbols.  On the one platform where that behavior is not
natively available, namely windows, we emulate it by analyzing the input files
to the shared library and exporting all the symbols therein.

Not exporting all symbols is actually desirable, as it can improve loading
speed, reduces the likelihood of symbol conflicts and can improve intra
extension library function call performance. It also makes the non-windows
builds more similar to windows builds.

Additionally, with meson implementing the export-all-symbols behavior for
windows, turns out to be more verbose than desirable.

This patch adds support for hiding symbols by default and, to counteract that,
explicit symbol visibility annotation for compilers that support
__attribute__((visibility("default"))) and -fvisibility=hidden. That is
expected to be most, if not all, compilers except msvc (for which we already
support explicit symbol export annotations).

Now that extension library symbols are explicitly exported, we don't need to
export all symbols on windows anymore, hence remove that behavior from
src/tools/msvc. The supporting code can't be removed, as we still need to
export all symbols from the main postgres binary.

Author: Andres Freund <andres@anarazel.de>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20211101020311.av6hphdl6xbjbuif@alap3.anarazel.de

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/089480c077056fc20fa8d8f5a3032a9dcf5ed812

Modified Files
--------------
configure                  | 152 +++++++++++++++++++++++++++++++++++++++++++++
configure.ac               |  13 ++++
src/Makefile.global.in     |   3 +
src/Makefile.shlib         |  13 ++++
src/include/c.h            |  13 ++--
src/include/pg_config.h.in |   3 +
src/makefiles/pgxs.mk      |   5 +-
src/tools/msvc/Project.pm  |   7 ---
src/tools/msvc/Solution.pm |   1 +
9 files changed, 198 insertions(+), 12 deletions(-)

Hi,

On 2022-07-18 01:05:56 +0000, Andres Freund wrote:
> Default to hidden visibility for extension libraries where possible

Looking at the odd failures, not sure what went wrong.

Greetings,

Andres Freund

On 2022-07-17 18:39:35 -0700, Andres Freund wrote:
> On 2022-07-18 01:05:56 +0000, Andres Freund wrote:
> > Default to hidden visibility for extension libraries where possible
>
> Looking at the odd failures, not sure what went wrong.

I don't know how the configure exec bit got removed, shell history doesn't
show anything odd in that regard. I do see a mistake in locally merging the
symbol-visibility branch (leading to a crucial commit being missed). I do see
in shell history that I ran check-world without a failure just before pushing,
which should have shown the failure, but didn't.

Definitely a brown paper bag moment.

Andres Freund <andres@anarazel.de> writes:
> I don't know how the configure exec bit got removed, shell history doesn't
> show anything odd in that regard. I do see a mistake in locally merging the
> symbol-visibility branch (leading to a crucial commit being missed). I do see
> in shell history that I ran check-world without a failure just before pushing,
> which should have shown the failure, but didn't.

check-world would not have re-run configure even if it was out of date
(only config.status), so that part of it's not so surprising.

> Definitely a brown paper bag moment.

We've all been there :-)

            regards, tom lane

On Sun, Jul 17, 2022 at 07:01:55PM -0700, Andres Freund wrote:
> On 2022-07-17 18:39:35 -0700, Andres Freund wrote:
> > On 2022-07-18 01:05:56 +0000, Andres Freund wrote:
> > > Default to hidden visibility for extension libraries where possible
> >
> > Looking at the odd failures, not sure what went wrong.
> 
> I don't know how the configure exec bit got removed, shell history doesn't

I think git reflog -p would help to answer that, but I suppose you already
looked.

-- 
Justin

On 2022-Jul-19, Pavel Stehule wrote:

> po 18. 7. 2022 v 3:06 odesílatel Andres Freund <andres@anarazel.de> napsal:
> 
> > Default to hidden visibility for extension libraries where possible

> Unfortunately, this commit definitely breaks plpgsql_check. Can the
> following routines be visible?

Do you just need to send a patch to add an exports.txt file to
src/pl/plpgsql/src/ for these functions?

plpgsql_build_datatype
plpgsql_compile
plpgsql_parser_setup
plpgsql_stmt_typename
plpgsql_exec_get_datum_type
plpgsql_recognize_err_condition
plpgsql_ns_lookup

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2022-Jul-19, Pavel Stehule wrote:
>> Unfortunately, this commit definitely breaks plpgsql_check. Can the
>> following routines be visible?

> Do you just need to send a patch to add an exports.txt file to
> src/pl/plpgsql/src/ for these functions?

The precedent of plpython says that PGDLLEXPORT markers are sufficient.
But yeah, we need a list of exactly which functions need to be
re-exposed.  I imagine pldebugger has its own needs.

            regards, tom lane