Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit b6c7cfa

Browse files
committed
Restore proper linkage of pg_char_to_encoding() and friends.
Back in the 8.3 era we discovered that it was problematic if libpq.so had encoding ID assignments different from the backend, which is possible because on some platforms libpq.so might be of a different major version from the calling programs. psql should use libpq's assignments, but initdb has to use the backend's, else it will put wrong values into pg_database. The solution devised in commit 8468146 relied on giving initdb its own copy of encnames.c rather than relying on the functions exported by libpq. Later, that metamorphosed into ensuring that libpgcommon got linked before libpq -- which made things OK for initdb but broke psql. We didn't notice for lack of any changes in enum pg_enc since then. Commit 06843df reversed that, fixing the latent bug in psql but adding one in initdb. The meson build infrastructure is also not being sufficiently careful about link order, and trying to make it so would be equally fragile. Hence, let's use a new scheme based on giving the libpq-exported symbols different real names than the same functions exported from libpgcommon.a or libpgcommon_srv.a. (We could distinguish those two cases as well, but there seems no need to.) libpq gets the official names to avoid an ABI break for libpq clients, while the other cases use #define's to make the real names "xxx_private" rather than "xxx". By controlling where the #define's are applied, we can force any particular client program to use one set or the other of the encnames.c functions. We cannot back-patch this, since it'd be an ABI break for backend loadable modules, but there seems little need to. We're just trying to ensure that the world is safe for hypothetical future additions to enum pg_enc. In passing this should fix "duplicate symbol" linker warnings that we've been seeing on AIX buildfarm members since commit 06843df. It's not very clear why that linker is complaining now, when there were strictly *more* duplicates visible before, but in any case this should remove the reason for complaint. Patch by me; thanks to Andres Freund for review. Discussion: https://postgr.es/m/2385119.1696354473@sss.pgh.pa.us
1 parent e8c334c commit b6c7cfa

File tree

5 files changed

+41
-9
lines changed

5 files changed

+41
-9
lines changed

src/bin/initdb/Makefile

+4-5
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,12 @@ subdir = src/bin/initdb
1616
top_builddir = ../../..
1717
include $(top_builddir)/src/Makefile.global
1818

19-
override CPPFLAGS := -I$(libpq_srcdir) -I$(top_srcdir)/src/timezone $(ICU_CFLAGS) $(CPPFLAGS)
20-
2119
# Note: it's important that we link to encnames.o from libpgcommon, not
2220
# from libpq, else we have risks of version skew if we run with a libpq
23-
# shared library from a different PG version. The libpq_pgport macro
24-
# should ensure that that happens.
25-
#
21+
# shared library from a different PG version. Define
22+
# USE_PRIVATE_ENCODING_FUNCS to ensure that that happens.
23+
override CPPFLAGS := -DUSE_PRIVATE_ENCODING_FUNCS -I$(libpq_srcdir) -I$(top_srcdir)/src/timezone $(ICU_CFLAGS) $(CPPFLAGS)
24+
2625
# We need libpq only because fe_utils does.
2726
LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(ICU_LIBS)
2827

src/bin/initdb/meson.build

+5-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ initdb_sources = files(
77

88
initdb_sources += timezone_localtime_source
99

10-
#fixme: reimplement libpq_pgport logic
11-
1210
if host_system == 'windows'
1311
initdb_sources += rc_bin_gen.process(win32ver_rc, extra_args: [
1412
'--NAME', 'initdb',
@@ -18,6 +16,11 @@ endif
1816
initdb = executable('initdb',
1917
initdb_sources,
2018
include_directories: [timezone_inc],
19+
# Note: it's important that we link to encnames.o from libpgcommon, not
20+
# from libpq, else we have risks of version skew if we run with a libpq
21+
# shared library from a different PG version. Define
22+
# USE_PRIVATE_ENCODING_FUNCS to ensure that that happens.
23+
c_args: ['-DUSE_PRIVATE_ENCODING_FUNCS'],
2124
dependencies: [frontend_code, libpq, icu, icu_i18n],
2225
kwargs: default_bin_args,
2326
)

src/common/Makefile

+7
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,13 @@ libpgcommon.a: $(OBJS_FRONTEND)
140140
rm -f $@
141141
$(AR) $(AROPT) $@ $^
142142

143+
#
144+
# Files in libpgcommon.a should use/export the "xxx_private" versions
145+
# of pg_char_to_encoding() and friends.
146+
#
147+
$(OBJS_FRONTEND): CPPFLAGS += -DUSE_PRIVATE_ENCODING_FUNCS
148+
149+
143150
#
144151
# Shared library versions of object files
145152
#

src/common/meson.build

+5-2
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,8 @@ common_sources_frontend_static += files(
112112
'logging.c',
113113
)
114114

115-
# Build pgport once for backend, once for use in frontend binaries, and once
116-
# for use in shared libraries
115+
# Build pgcommon once for backend, once for use in frontend binaries, and
116+
# once for use in shared libraries
117117
#
118118
# XXX: in most environments we could probably link_whole pgcommon_shlib
119119
# against pgcommon_static, instead of compiling twice.
@@ -131,6 +131,9 @@ pgcommon_variants = {
131131
'': default_lib_args + {
132132
'sources': common_sources_frontend_static,
133133
'dependencies': [frontend_common_code],
134+
# Files in libpgcommon.a should use/export the "xxx_private" versions
135+
# of pg_char_to_encoding() and friends.
136+
'c_args': ['-DUSE_PRIVATE_ENCODING_FUNCS'],
134137
},
135138
'_shlib': default_lib_args + {
136139
'pic': true,

src/include/mb/pg_wchar.h

+20
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
* included by libpq client programs. In particular, a libpq client
1414
* should not assume that the encoding IDs used by the version of libpq
1515
* it's linked to match up with the IDs declared here.
16+
* To help prevent mistakes, relevant functions that are exported by
17+
* libpq have a physically different name when being referenced
18+
* statically.
1619
*
1720
*-------------------------------------------------------------------------
1821
*/
@@ -562,6 +565,23 @@ surrogate_pair_to_codepoint(pg_wchar first, pg_wchar second)
562565
}
563566

564567

568+
/*
569+
* The functions in this list are exported by libpq, and we need to be sure
570+
* that we know which calls are satisfied by libpq and which are satisfied
571+
* by static linkage to libpgcommon. (This is because we might be using a
572+
* libpq.so that's of a different major version and has encoding IDs that
573+
* differ from the current version's.) The nominal function names are what
574+
* are actually used in and exported by libpq, while the names exported by
575+
* libpgcommon.a and libpgcommon_srv.a end in "_private".
576+
*/
577+
#if defined(USE_PRIVATE_ENCODING_FUNCS) || !defined(FRONTEND)
578+
#define pg_char_to_encoding pg_char_to_encoding_private
579+
#define pg_encoding_to_char pg_encoding_to_char_private
580+
#define pg_valid_server_encoding pg_valid_server_encoding_private
581+
#define pg_valid_server_encoding_id pg_valid_server_encoding_id_private
582+
#define pg_utf_mblen pg_utf_mblen_private
583+
#endif
584+
565585
/*
566586
* These functions are considered part of libpq's exported API and
567587
* are also declared in libpq-fe.h.

0 commit comments

Comments
 (0)