public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: VASUKI M <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: BharatDB <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #19095: Test if function exit() is used fail when linked static
Date: Thu, 4 Dec 2025 09:32:03 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAN55FZ2Wqv+maXTfNO6+y=5400w7hCMo6RQhasqp=nrsODSBpg@mail.gmail.com>
<CAN55FZ2em+Wf6kWQ27bwQrLTTWJF7TcSDDKnjxGy+fxMVaGbvg@mail.gmail.com>
<[email protected]>
<CACTYHzhCNsee-4xv-HDhw8zmMWByB7YHLuUHh-bsztgZ94ewpw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CACTYHzjbVeKV289+TB1mg+tVmpnOJWrtezTvRjUwYoc875p1XA@mail.gmail.com>
<CACTYHzjjssid_jtZiLh8BTYHAvnxbSLXceyGtxM3NB6Kfv510g@mail.gmail.com>
<[email protected]>
On Thu, Dec 04, 2025 at 12:33:55AM +0100, Daniel Gustafsson wrote:
> Thanks for the update. This patch builds on previous patches posted in the
> thread which makes it more complicated to review. Can you please post a full
> squashed patch against master so we can be absolutely sure we are looking at
> the tree in the same state that you are.
Yeah, I was just playing with this patch a little bit this morning,
and reworked it as the attached, adjusting a bunch of stuff inside it.
The main complaint that I have with v4 is the fact that the new check
for pthread_exit() was not split as a patch of its own, being hidden
in the perl script introduced.
Saying that, passing the path of nm as argument to the perl script is
something that feels OK here, as does the coverage check added twice
for meson and Makefile, rather than relying on enable_coverage in an
ENV. Feel free to comment on these points, of course.
There were a few other things (comments, etc.) that I have tweaked
(commit message not edited, stolen from v4), but feel free to see the
attached for all the details. libpq fails properly after planting
some exit() calls reported by nm. I have run once autoreconf while on
it, before running a check in the CI, which was also OK.
--
Michael
Attachments:
[text/x-diff] v5-0001-libpq-centralize-exit-check-logic-and-unify-Makef.patch (8.6K, 2-v5-0001-libpq-centralize-exit-check-logic-and-unify-Makef.patch)
download | inline diff:
From 43a7099781d13b1ccd119acf955b8dce6fa0c279 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Thu, 4 Dec 2025 09:12:58 +0900
Subject: [PATCH v5 1/2] libpq: centralize exit() check logic and unify
Makefile/Meson behavior
Move the libpq exit() reference check into a shared Perl script
(libpq-exit-check) and use it for both autoconf/Makefile and Meson
builds. This avoids duplicated logic and ensures consistent
platform-specific handling of whitelisted symbols.
The script now receives the full path to `nm`, detected at the
top-level configure using AC_PATH_PROG and propagated via NM_PROG.
Both Makefile and Meson invoke the script with --nm=<path>.
Platform-specific behavior (Solaris skip, Windows skip, whitelisted
symbols, nm availability checks) is centralized inside the script.
This makes the two build systems apply the same exit() policy and keeps
all rules in one place.
---
src/interfaces/libpq/Makefile | 21 ++-----
src/interfaces/libpq/libpq-check.pl | 86 +++++++++++++++++++++++++++++
src/interfaces/libpq/meson.build | 19 +++++++
configure | 42 ++++++++++++++
configure.ac | 2 +
meson.build | 1 +
src/Makefile.global.in | 1 +
7 files changed, 155 insertions(+), 17 deletions(-)
create mode 100755 src/interfaces/libpq/libpq-check.pl
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index da6650066d46..1b7d3ecf8aac 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -129,25 +129,12 @@ $(shlib): $(OBJS_SHLIB)
$(stlib): override OBJS += $(OBJS_STATIC)
$(stlib): $(OBJS_STATIC)
-# Check for functions that libpq must not call, currently just exit().
-# (Ideally we'd reject abort() too, but there are various scenarios where
-# build toolchains insert abort() calls, e.g. to implement assert().)
-# If nm doesn't exist or doesn't work on shlibs, this test will do nothing,
-# which is fine. The exclusion of __cxa_atexit is necessary on OpenBSD,
-# which seems to insert references to that even in pure C code. Excluding
-# __tsan_func_exit is necessary when using ThreadSanitizer data race detector
-# which use this function for instrumentation of function exit.
-# Skip the test when profiling, as gcc may insert exit() calls for that.
-# Also skip the test on platforms where libpq infrastructure may be provided
-# by statically-linked libraries, as we can't expect them to honor this
-# coding rule.
+# Check for functions that libpq must not call. See libpq-check.pl for the
+# full set of platform rules. Skip the test when profiling, as gcc may
+# insert exit() calls for that.
libpq-refs-stamp: $(shlib)
ifneq ($(enable_coverage), yes)
-ifeq (,$(filter solaris,$(PORTNAME)))
- @if nm -A -u $< 2>/dev/null | grep -v -e __cxa_atexit -e __tsan_func_exit | grep exit; then \
- echo 'libpq must not be calling any function which invokes exit'; exit 1; \
- fi
-endif
+ $(PERL) libpq-check.pl --input_file $< --nm='$(NM)'
endif
touch $@
diff --git a/src/interfaces/libpq/libpq-check.pl b/src/interfaces/libpq/libpq-check.pl
new file mode 100755
index 000000000000..274e873d1fca
--- /dev/null
+++ b/src/interfaces/libpq/libpq-check.pl
@@ -0,0 +1,86 @@
+#!/usr/bin/perl
+#
+# src/interfaces/libpq/libpq-check.pl
+#
+# Copyright (c) 2025, PostgreSQL Global Development Group
+#
+# Check that the state of a libpq library. Currently, this script checks
+# that exit() is not called, because client libraries must not terminate
+# the host application.
+#
+# This script is called by both Makefile and Meson.
+
+use strict;
+use warnings FATAL => 'all';
+
+use Getopt::Long;
+use Config;
+
+my $nm_path;
+my $input_file;
+my $stamp_file;
+my @problematic_lines;
+
+Getopt::Long::GetOptions(
+ 'nm:s' => \$nm_path,
+ 'input_file:s' => \$input_file,
+ 'stamp_file:s' => \$stamp_file) or die "$0: wrong arguments\n";
+
+die "$0: --input_file must be specified\n" unless defined $input_file;
+die "$0: --nm must be specified\n" unless defined $nm_path and -x $nm_path;
+
+sub create_stamp_file
+{
+ if (!(-f $stamp_file))
+ {
+ open my $fh, '>', $stamp_file
+ or die "can't open $stamp_file: $!";
+ close $fh;
+ }
+}
+
+# ---- Skip on Windows and Solaris ----
+if ( $Config{osname} =~ /MSWin32|cygwin|msys/i
+ || $Config{osname} =~ /solaris/i)
+{
+ exit 0;
+}
+
+# Run nm to scan for symbols. If nm fails at runtime, skip the check.
+open my $fh, '-|', "$nm_path -A -u $input_file 2>/dev/null"
+ or exit 0;
+
+while (<$fh>)
+{
+ # Set of symbols allowed:
+ # __cxa_atexit - injected by some libcs (e.g., OpenBSD)
+ # __tsan_func_exit - ThreadSanitizer instrumentation
+
+ next if /__cxa_atexit/;
+ next if /__tsan_func_exit/;
+
+ # Anything containing "exit" is suspicious.
+ # (Ideally we should reject abort() too, but there are various scenarios
+ # where build toolchains insert abort() calls, e.g. to implement assert().)
+ if (/exit/)
+ {
+ push @problematic_lines, $_;
+ }
+}
+close $fh;
+
+if (@problematic_lines)
+{
+ print "libpq must not be calling any function which invokes exit\n";
+ print "Problematic symbol references:\n";
+ print @problematic_lines;
+
+ exit 1;
+}
+# Create stamp file, if required
+if (defined($stamp_file))
+{
+ create_stamp_file();
+}
+
+exit 0;
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index a74e885b169d..e71ee9058030 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -85,6 +85,25 @@ libpq = declare_dependency(
include_directories: [include_directories('.')]
)
+# Check for functions that libpq must not call. See libpq-check.pl for the
+# full set of platform rules. Skip the test when profiling, as gcc may
+# insert exit() calls for that.
+if nm.found() and not get_option('b_coverage')
+ custom_target(
+ 'libpq-check',
+ input: libpq_so,
+ output: 'libpq-refs-stamp',
+ command: [
+ perl,
+ files('libpq-check.pl'),
+ '--input_file', '@INPUT@',
+ '--stamp_file', '@OUTPUT@',
+ '--nm', nm.full_path()
+ ],
+ build_by_default: true,
+ )
+endif
+
private_deps = [
frontend_stlib_code,
libpq_deps,
diff --git a/configure b/configure
index 3a0ed11fa8e2..ec35de5ba657 100755
--- a/configure
+++ b/configure
@@ -682,6 +682,7 @@ FLEXFLAGS
FLEX
BISONFLAGS
BISON
+NM
MKDIR_P
LN_S
TAR
@@ -10162,6 +10163,47 @@ case $MKDIR_P in
*install-sh*) MKDIR_P='\${SHELL} \${top_srcdir}/config/install-sh -c -d';;
esac
+# Extract the first word of "nm", so it can be a program name with args.
+set dummy nm; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_path_NM+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ case $NM in
+ [\\/]* | ?:[\\/]*)
+ ac_cv_path_NM="$NM" # Let the user override the test with a path.
+ ;;
+ *)
+ as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_path_NM="$as_dir/$ac_word$ac_exec_ext"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+ ;;
+esac
+fi
+NM=$ac_cv_path_NM
+if test -n "$NM"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $NM" >&5
+$as_echo "$NM" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+
if test -z "$BISON"; then
for ac_prog in bison
do
diff --git a/configure.ac b/configure.ac
index c2413720a188..7284f1ff6229 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1214,6 +1214,8 @@ case $MKDIR_P in
*install-sh*) MKDIR_P='\${SHELL} \${top_srcdir}/config/install-sh -c -d';;
esac
+AC_PATH_PROG(NM, nm)
+AC_SUBST(NM)
PGAC_PATH_BISON
PGAC_PATH_FLEX
diff --git a/meson.build b/meson.build
index 6e7ddd74683c..622598546ae8 100644
--- a/meson.build
+++ b/meson.build
@@ -350,6 +350,7 @@ missing = find_program('config/missing', native: true)
cp = find_program('cp', required: false, native: true)
xmllint_bin = find_program(get_option('XMLLINT'), native: true, required: false)
xsltproc_bin = find_program(get_option('XSLTPROC'), native: true, required: false)
+nm = find_program('nm', required: false, native: true)
bison_flags = []
if bison.found()
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 0aa389bc7101..371cd7eba2c1 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -292,6 +292,7 @@ FLEX = @FLEX@
FLEXFLAGS = @FLEXFLAGS@ $(LFLAGS)
DTRACE = @DTRACE@
DTRACEFLAGS = @DTRACEFLAGS@
+NM = @NM@
ZIC = @ZIC@
# Linking
--
2.51.0
[text/x-diff] v5-0002-libpq-Authorize-pthread_exit-in-libpq-check.patch (975B, 3-v5-0002-libpq-Authorize-pthread_exit-in-libpq-check.patch)
download | inline diff:
From e8bf515b01902b6fea05cb8bd87df18d001dac27 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Thu, 4 Dec 2025 09:13:49 +0900
Subject: [PATCH v5 2/2] libpq: Authorize pthread_exit() in libpq-check
---
src/interfaces/libpq/libpq-check.pl | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/interfaces/libpq/libpq-check.pl b/src/interfaces/libpq/libpq-check.pl
index 274e873d1fca..1b610d9dc761 100755
--- a/src/interfaces/libpq/libpq-check.pl
+++ b/src/interfaces/libpq/libpq-check.pl
@@ -55,9 +55,11 @@ while (<$fh>)
# Set of symbols allowed:
# __cxa_atexit - injected by some libcs (e.g., OpenBSD)
# __tsan_func_exit - ThreadSanitizer instrumentation
+ # pthread_exit - legitimate thread cleanup
next if /__cxa_atexit/;
next if /__tsan_func_exit/;
+ next if /pthread_exit/;
# Anything containing "exit" is suspicious.
# (Ideally we should reject abort() too, but there are various scenarios
--
2.51.0
[application/pgp-signature] signature.asc (833B, 4-signature.asc)
download
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19095: Test if function exit() is used fail when linked static
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox