From 089480c077056fc20fa8d8f5a3032a9dcf5ed812 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Sun, 17 Jul 2022 17:49:51 -0700 Subject: [PATCH] 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 Author: Tom Lane Discussion: https://postgr.es/m/20211101020311.av6hphdl6xbjbuif@alap3.anarazel.de --- 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(-) mode change 100755 => 100644 configure diff --git a/configure b/configure old mode 100755 new mode 100644 index 1e63c6862b..a4f4d321fb --- a/configure +++ b/configure @@ -741,6 +741,8 @@ CPP CFLAGS_SL BITCODE_CXXFLAGS BITCODE_CFLAGS +CXXFLAGS_SL_MODULE +CFLAGS_SL_MODULE CFLAGS_VECTORIZE CFLAGS_UNROLL_LOOPS PERMIT_DECLARATION_AFTER_STATEMENT @@ -6302,6 +6304,154 @@ if test x"$pgac_cv_prog_CC_cflags__ftree_vectorize" = x"yes"; then fi + # + # If the compiler knows how to hide symbols add the switch needed for that + # to CFLAGS_SL_MODULE and define HAVE_VISIBILITY_ATTRIBUTE. + { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -fvisibility=hidden, for CFLAGS_SL_MODULE" >&5 +$as_echo_n "checking whether ${CC} supports -fvisibility=hidden, for CFLAGS_SL_MODULE... " >&6; } +if ${pgac_cv_prog_CC_cflags__fvisibility_hidden+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_save_CFLAGS=$CFLAGS +pgac_save_CC=$CC +CC=${CC} +CFLAGS="${CFLAGS_SL_MODULE} -fvisibility=hidden" +ac_save_c_werror_flag=$ac_c_werror_flag +ac_c_werror_flag=yes +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + pgac_cv_prog_CC_cflags__fvisibility_hidden=yes +else + pgac_cv_prog_CC_cflags__fvisibility_hidden=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_c_werror_flag=$ac_save_c_werror_flag +CFLAGS="$pgac_save_CFLAGS" +CC="$pgac_save_CC" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__fvisibility_hidden" >&5 +$as_echo "$pgac_cv_prog_CC_cflags__fvisibility_hidden" >&6; } +if test x"$pgac_cv_prog_CC_cflags__fvisibility_hidden" = x"yes"; then + CFLAGS_SL_MODULE="${CFLAGS_SL_MODULE} -fvisibility=hidden" +fi + + + if test "$pgac_cv_prog_CC_cflags__fvisibility_hidden" = yes; then + +$as_echo "#define HAVE_VISIBILITY_ATTRIBUTE 1" >>confdefs.h + + fi + # For C++ we additionally want -fvisibility-inlines-hidden + { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -fvisibility=hidden, for CXXFLAGS_SL_MODULE" >&5 +$as_echo_n "checking whether ${CXX} supports -fvisibility=hidden, for CXXFLAGS_SL_MODULE... " >&6; } +if ${pgac_cv_prog_CXX_cxxflags__fvisibility_hidden+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_save_CXXFLAGS=$CXXFLAGS +pgac_save_CXX=$CXX +CXX=${CXX} +CXXFLAGS="${CXXFLAGS_SL_MODULE} -fvisibility=hidden" +ac_save_cxx_werror_flag=$ac_cxx_werror_flag +ac_cxx_werror_flag=yes +ac_ext=cpp +ac_cpp='$CXXCPP $CPPFLAGS' +ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_cxx_compiler_gnu + +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_compile "$LINENO"; then : + pgac_cv_prog_CXX_cxxflags__fvisibility_hidden=yes +else + pgac_cv_prog_CXX_cxxflags__fvisibility_hidden=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_ext=c +ac_cpp='$CPP $CPPFLAGS' +ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_c_compiler_gnu + +ac_cxx_werror_flag=$ac_save_cxx_werror_flag +CXXFLAGS="$pgac_save_CXXFLAGS" +CXX="$pgac_save_CXX" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CXX_cxxflags__fvisibility_hidden" >&5 +$as_echo "$pgac_cv_prog_CXX_cxxflags__fvisibility_hidden" >&6; } +if test x"$pgac_cv_prog_CXX_cxxflags__fvisibility_hidden" = x"yes"; then + CXXFLAGS_SL_MODULE="${CXXFLAGS_SL_MODULE} -fvisibility=hidden" +fi + + { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -fvisibility-inlines-hidden, for CXXFLAGS_SL_MODULE" >&5 +$as_echo_n "checking whether ${CXX} supports -fvisibility-inlines-hidden, for CXXFLAGS_SL_MODULE... " >&6; } +if ${pgac_cv_prog_CXX_cxxflags__fvisibility_inlines_hidden+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_save_CXXFLAGS=$CXXFLAGS +pgac_save_CXX=$CXX +CXX=${CXX} +CXXFLAGS="${CXXFLAGS_SL_MODULE} -fvisibility-inlines-hidden" +ac_save_cxx_werror_flag=$ac_cxx_werror_flag +ac_cxx_werror_flag=yes +ac_ext=cpp +ac_cpp='$CXXCPP $CPPFLAGS' +ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_cxx_compiler_gnu + +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_compile "$LINENO"; then : + pgac_cv_prog_CXX_cxxflags__fvisibility_inlines_hidden=yes +else + pgac_cv_prog_CXX_cxxflags__fvisibility_inlines_hidden=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_ext=c +ac_cpp='$CPP $CPPFLAGS' +ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_c_compiler_gnu + +ac_cxx_werror_flag=$ac_save_cxx_werror_flag +CXXFLAGS="$pgac_save_CXXFLAGS" +CXX="$pgac_save_CXX" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CXX_cxxflags__fvisibility_inlines_hidden" >&5 +$as_echo "$pgac_cv_prog_CXX_cxxflags__fvisibility_inlines_hidden" >&6; } +if test x"$pgac_cv_prog_CXX_cxxflags__fvisibility_inlines_hidden" = x"yes"; then + CXXFLAGS_SL_MODULE="${CXXFLAGS_SL_MODULE} -fvisibility-inlines-hidden" +fi + # # The following tests want to suppress various unhelpful warnings by adding # -Wno-foo switches. But gcc won't complain about unrecognized -Wno-foo @@ -6860,6 +7010,8 @@ fi + + # Determine flags used to emit bitcode for JIT inlining. # 1. We must duplicate any behaviour-changing compiler flags used above, # to keep compatibility with the compiler used for normal Postgres code. diff --git a/configure.ac b/configure.ac index 71191f14ad..5bd29a4d2f 100644 --- a/configure.ac +++ b/configure.ac @@ -525,6 +525,17 @@ if test "$GCC" = yes -a "$ICC" = no; then # Optimization flags for specific files that benefit from vectorization PGAC_PROG_CC_VAR_OPT(CFLAGS_VECTORIZE, [-ftree-vectorize]) # + # If the compiler knows how to hide symbols add the switch needed for that + # to CFLAGS_SL_MODULE and define HAVE_VISIBILITY_ATTRIBUTE. + PGAC_PROG_CC_VAR_OPT(CFLAGS_SL_MODULE, [-fvisibility=hidden]) + if test "$pgac_cv_prog_CC_cflags__fvisibility_hidden" = yes; then + AC_DEFINE([HAVE_VISIBILITY_ATTRIBUTE], 1, + [Define to 1 if your compiler knows the visibility("hidden") attribute.]) + fi + # For C++ we additionally want -fvisibility-inlines-hidden + PGAC_PROG_VARCXX_VARFLAGS_OPT(CXX, CXXFLAGS_SL_MODULE, [-fvisibility=hidden]) + PGAC_PROG_VARCXX_VARFLAGS_OPT(CXX, CXXFLAGS_SL_MODULE, [-fvisibility-inlines-hidden]) + # # The following tests want to suppress various unhelpful warnings by adding # -Wno-foo switches. But gcc won't complain about unrecognized -Wno-foo # switches, so we have to test for the positive form and if that works, @@ -573,6 +584,8 @@ fi AC_SUBST(CFLAGS_UNROLL_LOOPS) AC_SUBST(CFLAGS_VECTORIZE) +AC_SUBST(CFLAGS_SL_MODULE) +AC_SUBST(CXXFLAGS_SL_MODULE) # Determine flags used to emit bitcode for JIT inlining. # 1. We must duplicate any behaviour-changing compiler flags used above, diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 138d66ac00..bb177a8162 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -258,6 +258,9 @@ SUN_STUDIO_CC = @SUN_STUDIO_CC@ CXX = @CXX@ CFLAGS = @CFLAGS@ CFLAGS_SL = @CFLAGS_SL@ +# *_MODULE are for flags applied to extension libraries +CFLAGS_SL_MODULE = @CFLAGS_SL_MODULE@ +CXXFLAGS_SL_MODULE = @CXXFLAGS_SL_MODULE@ CFLAGS_UNROLL_LOOPS = @CFLAGS_UNROLL_LOOPS@ CFLAGS_VECTORIZE = @CFLAGS_VECTORIZE@ CFLAGS_SSE42 = @CFLAGS_SSE42@ diff --git a/src/Makefile.shlib b/src/Makefile.shlib index 6df96c634b..2396bc247e 100644 --- a/src/Makefile.shlib +++ b/src/Makefile.shlib @@ -218,6 +218,19 @@ ifeq ($(PORTNAME), win32) endif +# If the shared library doesn't have an export file, mark all symbols not +# explicitly exported using PGDLLEXPORT as hidden. We can't pass these flags +# when building a library with explicit exports, as the symbols would be +# hidden before the linker script / exported symbol list takes effect. +# +# This is duplicated in pgxs.mk for MODULES style libraries. +ifeq ($(SHLIB_EXPORTS),) + # LDFLAGS_SL addition not strictly needed, CFLAGS used everywhere, but ... + override LDFLAGS_SL += $(CFLAGS_SL_MODULE) + override CFLAGS += $(CFLAGS_SL_MODULE) + override CXXFLAGS += $(CXXFLAGS_SL_MODULE) +endif + ## ## BUILD diff --git a/src/include/c.h b/src/include/c.h index 863a16c6a6..2cc2784750 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -1347,14 +1347,19 @@ extern unsigned long long strtoull(const char *str, char **endptr, int base); /* * Use "extern PGDLLEXPORT ..." to declare functions that are defined in - * loadable modules and need to be callable by the core backend. (Usually, - * this is not necessary because our build process automatically exports - * such symbols, but sometimes manual marking is required.) - * No special marking is required on most ports. + * loadable modules and need to be callable by the core backend or other + * loadable modules. + * If the compiler knows __attribute__((visibility("*"))), we use that, + * unless we already have a platform-specific definition. Otherwise, + * no special marking is required. */ #ifndef PGDLLEXPORT +#ifdef HAVE_VISIBILITY_ATTRIBUTE +#define PGDLLEXPORT __attribute__((visibility("default"))) +#else #define PGDLLEXPORT #endif +#endif /* * The following is used as the arg list for signal handlers. Any ports diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 7133c3dc66..529fb84a86 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -700,6 +700,9 @@ /* Define to 1 if you have the header file. */ #undef HAVE_UUID_UUID_H +/* Define to 1 if your compiler knows the visibility("hidden") attribute. */ +#undef HAVE_VISIBILITY_ATTRIBUTE + /* Define to 1 if you have the `wcstombs_l' function. */ #undef HAVE_WCSTOMBS_L diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index 0f71fa293d..7ba8d5bc98 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -101,8 +101,11 @@ endif # PGXS override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS) +# See equivalent block in Makefile.shlib ifdef MODULES -override CFLAGS += $(CFLAGS_SL) +override LDFLAGS_SL += $(CFLAGS_SL_MODULE) +override CFLAGS += $(CFLAGS_SL) $(CFLAGS_SL_MODULE) +override CXXFLAGS += $(CFLAGS_SL) $(CXXFLAGS_SL_MODULE) endif ifdef MODULEDIR diff --git a/src/tools/msvc/Project.pm b/src/tools/msvc/Project.pm index 570bab563a..b24a2a9815 100644 --- a/src/tools/msvc/Project.pm +++ b/src/tools/msvc/Project.pm @@ -419,13 +419,6 @@ sub Save { my ($self) = @_; - # If doing DLL and haven't specified a DEF file, do a full export of all symbols - # in the project. - if ($self->{type} eq "dll" && !$self->{def}) - { - $self->FullExportDLL($self->{name} . ".lib"); - } - # Warning 4197 is about double exporting, disable this per # http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=99193 $self->DisableLinkerWarnings('4197') if ($self->{platform} eq 'x64'); diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm index fa32dc371d..f2427008df 100644 --- a/src/tools/msvc/Solution.pm +++ b/src/tools/msvc/Solution.pm @@ -429,6 +429,7 @@ sub GenerateFiles HAVE_WINLDAP_H => undef, HAVE_WCSTOMBS_L => 1, HAVE_WCTYPE_H => 1, + HAVE_VISIBILITY_ATTRIBUTE => undef, HAVE_WRITEV => undef, HAVE_X509_GET_SIGNATURE_NID => 1, HAVE_X86_64_POPCNTQ => undef,