I'm splitting this out from [0] so that it can be tracked and referenced separately.

[0]: https://www.postgresql.org/message-id/170e7e57-fe43-4e7e-8566-a96fcdf23075%40eisentraut.org

Historically, bundles (ld -bundle) were the only file type that could
be dynamically loaded at run time on macOS (OS X at the time).  These
were distinct from dynamic libraries (ld -dynamiclib (although the man
page says -dylib, but that apparently doesn't work)) that you use at
build time (-lfoo).  Therefore, that platform had a strict distinction
between shared libraries and shared modules, unlike other platforms.

Over time, macOS gained the ability to dlopen dynamic libraries as
well, so this distinction is obsolete.  Also, it appears that the
bundle file type is obsolescent on other Apple platforms.  So, to
simplify this, change our shared library makefiles to build the
dynamic library file type for both uses.  (There are still some
distinctions between the two in the build system, such as where they
are installed and how the output files are named, but they are now
internally the same file type.)  Meson also has the same change
pending in its master branch, so this will eventually become
consistent.

But this change triggers a new variant of this issue:

https://www.postgresql.org/message-id/e1o4hov-001oyi...@gemulon.postgresql.org

With the changed linker options, the symbol search order appears to
be different, and so hash_search() gets found in the OS library first.

(Historical research reveals that this name clash is not accidental. dynahash.c was explicitly modeled after routines that existed in some BSD OS, and continue to exist in macOS, but they don't have compatible signatures.)

To avoid that, I suggest to use some preprocessor defines to rename the symbols known to clash, in a way that we can continue to use the existing API names.


I suggest that we consider the dynahash change for PG18. The Meson change will eventually come our way, and then everything on macOS will start crashing. So the earlier we fix this the better. Also, we can't backpatch this because of the ABI change, so once PG18 ships the window is over.

For the -bundle -> -dynamiclib change, I don't have any particular urgency right now.

From 57de7451676506f79a99681338efcb0a1f956c3c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 9 Apr 2025 16:08:21 +0200
Subject: [PATCH v1 1/2] Avoid symbol clashes of dynahash with OS

The dynahash routines were originally modeled after routines in some
BSD operating systems.  They continue to exist at least in macOS (see
strhash.h), but they are incompatible, and depending on link order or
symbol lookup order, a dynamically-loaded module might end up calling
the one in the OS and crash.

To avoid that, we rename the symbols known to clash, so that we can
continue to use the existing API names.

Discussion: 
https://www.postgresql.org/message-id/170e7e57-fe43-4e7e-8566-a96fcdf23075%40eisentraut.org
---
 src/include/utils/hsearch.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/include/utils/hsearch.h b/src/include/utils/hsearch.h
index 932cc4f34d9..4d742301ebd 100644
--- a/src/include/utils/hsearch.h
+++ b/src/include/utils/hsearch.h
@@ -126,6 +126,21 @@ typedef struct
        uint32          hashvalue;              /* hashvalue to start seqscan 
over hash */
 } HASH_SEQ_STATUS;
 
+/*
+ * Redefine symbols to avoid clashes with OS.
+ *
+ * These routines were originally modeled after routines in some BSD operating
+ * systems.  They continue to exist at least in macOS (see strhash.h), but
+ * they are incompatible, and depending on link order or symbol lookup order,
+ * a dynamically-loaded module might end up calling the one in the OS and
+ * crash.  To avoid that, we rename the symbols known to clash, so that we can
+ * continue to use the existing API names.
+ */
+#define hash_create pg_hash_create
+#define hash_destroy pg_hash_destroy
+#define hash_search pg_hash_search
+#define hash_stats pg_hash_stats
+
 /*
  * prototypes for functions in dynahash.c
  */
-- 
2.49.0

From 7e1c86817a884973b313fac162108524d3170f69 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 9 Apr 2025 15:49:13 +0200
Subject: [PATCH v1 2/2] Build macOS shared modules as dylib rather than bundle

Historically, bundles (ld -bundle) were the only file type that could
be dynamically loaded at run time on macOS (OS X at the time).  These
were distinct from dynamic libraries (ld -dynamiclib (although the man
page says -dylib, but that apparently doesn't work)) that you use at
build time (-lfoo).  Therefore, that platform had a strict distinction
between shared libraries and shared modules, unlike other platforms.
Over time, macOS gained the ability to dlopen dynamic libraries as
well, so this distinction is obsolete.  Also, it appears that the
bundle file type is obsolescent on other Apple platforms.  So, to
simplify this, change our shared library makefiles to build the
dynamic library file type for both uses.  (There are still some
distinctions between the two in the build system, such as where they
are installed and how the output files are named, but they are now
internally the same file type.)  Meson also has the same change
pending in its master branch, so this will eventually become
consistent.

Discussion: 
https://www.postgresql.org/message-id/170e7e57-fe43-4e7e-8566-a96fcdf23075%40eisentraut.org
---
 src/Makefile.shlib            | 5 +++--
 src/makefiles/Makefile.darwin | 8 +-------
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/src/Makefile.shlib b/src/Makefile.shlib
index fa81f6ffdd6..18fb4d6fa3f 100644
--- a/src/Makefile.shlib
+++ b/src/Makefile.shlib
@@ -107,17 +107,18 @@ override CPPFLAGS += 
-DSO_MAJOR_VERSION=$(SO_MAJOR_VERSION)
 endif
 
 ifeq ($(PORTNAME), darwin)
+  LINK.shared          = $(COMPILER) -dynamiclib
   ifdef soname
     # linkable library
     ifneq ($(SO_MAJOR_VERSION), 0)
       version_link     = -compatibility_version $(SO_MAJOR_VERSION) 
-current_version $(SO_MAJOR_VERSION).$(SO_MINOR_VERSION)
     endif
-    LINK.shared                = $(COMPILER) -dynamiclib -install_name 
'$(libdir)/lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)' $(version_link) 
$(exported_symbols_list)
+    LINK.shared                += -install_name 
'$(libdir)/lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)' $(version_link) 
$(exported_symbols_list)
     shlib              = lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)
     shlib_major                = lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)
   else
     # loadable module
-    LINK.shared                = $(COMPILER) -bundle
+    LINK.shared                += -Wl,-undefined,dynamic_lookup
   endif
   BUILD.exports                = $(AWK) '/^[^\#]/ {printf "_%s\n",$$1}' $< >$@
   exports_file         = $(SHLIB_EXPORTS:%.txt=%.list)
diff --git a/src/makefiles/Makefile.darwin b/src/makefiles/Makefile.darwin
index 7095f66e25c..ade07eadc02 100644
--- a/src/makefiles/Makefile.darwin
+++ b/src/makefiles/Makefile.darwin
@@ -1,12 +1,6 @@
 # env var name to use in place of LD_LIBRARY_PATH
 ld_library_path_var = DYLD_LIBRARY_PATH
 
-ifdef PGXS
-  BE_DLLLIBS = -bundle_loader $(bindir)/postgres
-else
-  BE_DLLLIBS = -bundle_loader $(top_builddir)/src/backend/postgres
-endif
-
 # Rule for building a shared library from a single .o file
 %$(DLSUFFIX): %.o
-       $(CC) $(CFLAGS) $< $(LDFLAGS) $(LDFLAGS_SL) -bundle $(BE_DLLLIBS) -o $@
+       $(CC) $(CFLAGS) $< $(LDFLAGS) $(LDFLAGS_SL) -dynamiclib 
-Wl,-undefined,dynamic_lookup -o $@
-- 
2.49.0

Reply via email to