public inbox for [email protected]
help / color / mirror / Atom feedFrom: VASUKI M <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Daniel Gustafsson <[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 16:38:40 +0530
Message-ID: <CACTYHzimf5+6gp6HzCerZpWh4o2_TW6-S3k3+2YK5mnoQa-+tg@mail.gmail.com> (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]>
<[email protected]>
Hi all,
I am back with the changes in v6,
On Thu, 4 Dec 2025 at 06:02, Michael Paquier <[email protected]> wrote:
> 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.
This v6 series is squashed and rebased on the current master. It also
includes the revisions and clarifications suggested in the previous round
of review, including Michael’s comments.
> 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.
>
First of all,thanks for testing the patch.
Sorry for the inconvenience ,here I have attached separate patches for
libpq-exit-check and whitelisting of pthread_exit().
In Michael’s v5 1/2 patch, the configure file had changes because it was
generated after running autoreconf.Is it correct to include generated files
in the patches (such as configure,configure.status or other auto-generated
files).
Or only the change in configure.ac should be part of the patch.
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.
>
I agree with the approach of keeping the coverage
condition local to each build system rather than relying on
enable_coverage as
an environment variable. In practice, autoconf (enable_coverage) and Meson
(b_coverage) expose their coverage settings independently, so duplicating
the
check avoids any accidental cross-build inconsistencies. It also prevents
the
script from having to detect build-system-specific flags internally, keeping
libpq-check.pl focused only on symbol inspection. So I think it makes sense
for Makefile and Meson to each guard their own invocation. What do you
think?
I welcome any additional suggestions regarding the comments or the code.
Regards,
Vasuki M
Attachments:
[text/x-patch] v6-0001-libpq-centralize-exit-check-logic-and-unify-Makef.patch (7.2K, 3-v6-0001-libpq-centralize-exit-check-logic-and-unify-Makef.patch)
download | inline diff:
From 66aac3d1a3c1e51517b91175088bd99a0c20681c Mon Sep 17 00:00:00 2001
From: BharatDBPG <[email protected]>
Date: Thu, 4 Dec 2025 12:02:48 +0530
Subject: [PATCH v6 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.
---
configure.ac | 2 +
meson.build | 1 +
src/Makefile.global.in | 1 +
src/interfaces/libpq/Makefile | 21 ++------
src/interfaces/libpq/libpq-check.pl | 83 +++++++++++++++++++++++++++++
src/interfaces/libpq/meson.build | 19 +++++++
6 files changed, 110 insertions(+), 17 deletions(-)
create mode 100644 src/interfaces/libpq/libpq-check.pl
diff --git a/configure.ac b/configure.ac
index c241372..7284f1f 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 6e7ddd7..6225985 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 0aa389b..371cd7e 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
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index da66500..1b7d3ec 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 100644
index 0000000..d4220ef
--- /dev/null
+++ b/src/interfaces/libpq/libpq-check.pl
@@ -0,0 +1,83 @@
+#!/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 a74e885..52da79c 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,
--
2.43.0
[text/x-patch] v6-0002-libpq-whitelisting-pthread_exit-in-libpq-check.patch (994B, 4-v6-0002-libpq-whitelisting-pthread_exit-in-libpq-check.patch)
download | inline diff:
From ae11ebe777cd228fb3ddf69cc05cbc816301c41f Mon Sep 17 00:00:00 2001
From: BharatDBPG <[email protected]>
Date: Thu, 4 Dec 2025 12:08:07 +0530
Subject: [PATCH v6 2/2] libpq: whitelisting 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 d4220ef..13e977d 100644
--- a/src/interfaces/libpq/libpq-check.pl
+++ b/src/interfaces/libpq/libpq-check.pl
@@ -54,9 +54,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.43.0
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: <CACTYHzimf5+6gp6HzCerZpWh4o2_TW6-S3k3+2YK5mnoQa-+tg@mail.gmail.com>
* 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