Andres Freund:
Gah. Apples tendency to just break stuff that has worked across *nix-y
platforms for decades is pretty annoying. They could just have made
--export-dynamic an alias for --export_dynamic, but no, everyone needs a
special macos thingy in their build scripts.

Interesting enough my Linux ld does support -export_dynamic, too.. but it doesn't say anywhere in the man pages or so.


Also, passing the LTO flag on Linux "just works" (clang, not GCC
necessarily).

It should just work on gcc, or at least has in the recent past.

Well it "works" in a sense that the build succeeds and check-world as well. But there are some symbols in all the client binaries that I know are unused (paths to .../include etc.), and which LLVM's LTO strips out happily - that are still in there after GCC's LTO.

GCC can remove them with -fdata-sections -ffunction-sections -fmerge-constants and -Wl,--gc-sections. But not with -flto. At least I didn't manage to.


ISTM if we want to test for -export_dynamic like what you proposed, we should
do so only if --export-dynamic wasn't found. No need to incur the overhead on
!macos.

Makes sense! v2 attached.

I also attached a .backpatch to show what that would look like for v15 and down.


Peter Eisentraut:
> With the native compiler tooling on macOS, it is not safe to assume
> anything, including that the man pages are accurate or that the
> documented options actually work correctly and don't break anything
> else.  Unless we have actual testing on all the supported macOS
> versions, I don't believe it.

Which macOS versions are "supported"?

I just set up a VM with macOS Mojave (2018) and tested both the .patch on HEAD as well as the .backpatch on REL_12_STABLE with -flto. Build passed, make check-world as well.

clang --version for Mojave:
Apple LLVM version 10.0.1 (clang-1001.0.46.4)
Target: x86_64-apple-darwin18.5.0

clang --version for Sonoma (where I tested before):
Apple clang version 15.0.0 (clang-1500.3.9.4)
Target: x86_64-apple-darwin@23.5.0

Since PostgreSQL 12 is from 2019 and Mojave from 2018, I think that's far enough back?


> Given that LTO apparently never worked on macOS, this is not a
> regression, so I wouldn't backpatch it.  I'm not objecting, but I don't
> want to touch it.

Fair enough! Hopefully my testing convinces more than the man pages ;)

Best,

Wolfgang
From 3ca5357bbdb9aae29a1785d5ca2179d6cca15cdd Mon Sep 17 00:00:00 2001
From: Wolfgang Walther <walt...@technowledgy.de>
Date: Sun, 2 Jun 2024 10:46:56 +0200
Subject: [PATCH v2] Make building with clang's LTO work on macOS

When building with -flto the backend binary must keep many otherwise
unused symbols to make them available to dynamically loaded modules /
extensions.  This has been done via -Wl,--export-dynamic on many
platforms for years.  This flag is not supported by Apple's clang,
though.  Here it's called -Wl,-export_dynamic instead.

Thus, make configure pick up on this variant of the flag as well.  Meson
has the logic to detect this flag built-in, but doesn't support this
variant either.  This needs to be raised upstream.

Without this fix, building with -flto fails with errors similar to [1]
and [2].  This happens for all currently live versions, including 17 and
HEAD.

[1]: https://postgr.es/m/1581936537572-0.post%40n3.nabble.com
[2]: https://postgr.es/m/21800.1499270547%40sss.pgh.pa.us
---
 configure    | 41 +++++++++++++++++++++++++++++++++++++++++
 configure.ac |  4 ++++
 2 files changed, 45 insertions(+)

diff --git a/configure b/configure
index 7b03db56a67..771dcfbdef9 100755
--- a/configure
+++ b/configure
@@ -19135,6 +19135,7 @@ fi
 # For linkers that understand --export-dynamic, add that to the LDFLAGS_EX_BE
 # (backend specific ldflags). One some platforms this will always fail (e.g.,
 # windows), but on others it depends on the choice of linker (e.g., solaris).
+# macos uses -export_dynamic instead.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CC supports -Wl,--export-dynamic, for LDFLAGS_EX_BE" >&5
 $as_echo_n "checking whether $CC supports -Wl,--export-dynamic, for LDFLAGS_EX_BE... " >&6; }
 if ${pgac_cv_prog_cc_LDFLAGS_EX_BE__Wl___export_dynamic+:} false; then :
@@ -19173,6 +19174,46 @@ if test x"$pgac_cv_prog_cc_LDFLAGS_EX_BE__Wl___export_dynamic" = x"yes"; then
   LDFLAGS_EX_BE="${LDFLAGS_EX_BE} -Wl,--export-dynamic"
 fi
 
+if test x"$LDFLAGS_EX_BE" = x""; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CC supports -Wl,-export_dynamic, for LDFLAGS_EX_BE" >&5
+$as_echo_n "checking whether $CC supports -Wl,-export_dynamic, for LDFLAGS_EX_BE... " >&6; }
+if ${pgac_cv_prog_cc_LDFLAGS_EX_BE__Wl__export_dynamic+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  pgac_save_LDFLAGS=$LDFLAGS
+LDFLAGS="$pgac_save_LDFLAGS -Wl,-export_dynamic"
+if test "$cross_compiling" = yes; then :
+  pgac_cv_prog_cc_LDFLAGS_EX_BE__Wl__export_dynamic="assuming no"
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+extern void $link_test_func (); void (*fptr) () = $link_test_func;
+int
+main ()
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_run "$LINENO"; then :
+  pgac_cv_prog_cc_LDFLAGS_EX_BE__Wl__export_dynamic=yes
+else
+  pgac_cv_prog_cc_LDFLAGS_EX_BE__Wl__export_dynamic=no
+fi
+rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
+  conftest.$ac_objext conftest.beam conftest.$ac_ext
+fi
+
+LDFLAGS="$pgac_save_LDFLAGS"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_cc_LDFLAGS_EX_BE__Wl__export_dynamic" >&5
+$as_echo "$pgac_cv_prog_cc_LDFLAGS_EX_BE__Wl__export_dynamic" >&6; }
+if test x"$pgac_cv_prog_cc_LDFLAGS_EX_BE__Wl__export_dynamic" = x"yes"; then
+  LDFLAGS_EX_BE="${LDFLAGS_EX_BE} -Wl,-export_dynamic"
+fi
+
+fi
 
 
 # Create compiler version string
diff --git a/configure.ac b/configure.ac
index 63e7be38472..72e3c1d429b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2423,7 +2423,11 @@ fi
 # For linkers that understand --export-dynamic, add that to the LDFLAGS_EX_BE
 # (backend specific ldflags). One some platforms this will always fail (e.g.,
 # windows), but on others it depends on the choice of linker (e.g., solaris).
+# macos uses -export_dynamic instead.
 PGAC_PROG_CC_LD_VARFLAGS_OPT(LDFLAGS_EX_BE, [-Wl,--export-dynamic], $link_test_func)
+if test x"$LDFLAGS_EX_BE" = x""; then
+  PGAC_PROG_CC_LD_VARFLAGS_OPT(LDFLAGS_EX_BE, [-Wl,-export_dynamic], $link_test_func)
+fi
 AC_SUBST(LDFLAGS_EX_BE)
 
 # Create compiler version string
-- 
2.45.1

--- a/src/makefiles/Makefile.darwin
+++ b/src/makefiles/Makefile.darwin
@@ -5,6 +5,8 @@ DLSUFFIX = .so
 # env var name to use in place of LD_LIBRARY_PATH
 ld_library_path_var = DYLD_LIBRARY_PATH
 
+export_dynamic = -Wl,-export_dynamic
+
 ifdef PGXS
   BE_DLLLIBS = -bundle_loader $(bindir)/postgres
 else

Reply via email to