public inbox for [email protected]
help / color / mirror / Atom feedRe: BUG #19095: Test if function exit() is used fail when linked static
17+ messages / 5 participants
[nested] [flat]
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-11-19 13:17 Nazir Bilal Yavuz <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Nazir Bilal Yavuz @ 2025-11-19 13:17 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; VASUKI M <[email protected]>
Hi,
On Fri, 14 Nov 2025 at 15:11, Daniel Gustafsson <[email protected]> wrote:
>
> > On 12 Nov 2025, at 09:15, Michael Paquier <[email protected]> wrote:
> >
> > On Wed, Nov 12, 2025 at 09:13:09AM +0100, Daniel Gustafsson wrote:
> >> On 12 Nov 2025, at 07:38, Tom Lane <[email protected]> wrote:
> >>> Personally I'm okay with whitelisting pthread_exit() as
> >>> Torsten suggested.
> >>
> >> +1, we already have a few whitelisted entries and pthread_exit seems perfectly
> >> reasonable to add to that list.
> >
> > WFM.
>
> The attached trivial diff adds this to the whitelist clause in the Makefile. I
> experimented with adding this to Meson, and while it's trivial enough to do the
> run_command with libpq_so.full_path, it's less clear to me exactly where in the
> build it should be added. I've pinged my colleague Bilal who is much better at
> Meson than me to collaborate on that as a separate fix.
Sorry for the late reply. I replaced the Makefile portion with the
Perl script, so that it can be used for both meson and autoconf build
systems. The script takes two arguments
- input_file -> path of library file.
- stamp_file -> to create a stamp file for the meson build, so that
meson does not run while the library file is not changed. Autoconf
build does not use this option.
--
Regards,
Nazir Bilal Yavuz
Microsoft
Attachments:
[text/x-patch] v1-0001-Add-exit-check-for-libpq.so-for-meson-build.patch (4.1K, 2-v1-0001-Add-exit-check-for-libpq.so-for-meson-build.patch)
download | inline diff:
From 63a503dc5a3963b0e45e3634495563e52181bf23 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <[email protected]>
Date: Wed, 19 Nov 2025 12:08:08 +0300
Subject: [PATCH v1] Add exit() check for libpq.so for meson build
---
src/interfaces/libpq/Makefile | 4 +-
src/interfaces/libpq/libpq-exit-check | 71 +++++++++++++++++++++++++++
src/interfaces/libpq/meson.build | 27 ++++++++++
3 files changed, 99 insertions(+), 3 deletions(-)
create mode 100755 src/interfaces/libpq/libpq-exit-check
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index da6650066d4..6b2d1769a07 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -144,9 +144,7 @@ $(stlib): $(OBJS_STATIC)
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
+ $(PERL) libpq-exit-check --input_file $<
endif
endif
touch $@
diff --git a/src/interfaces/libpq/libpq-exit-check b/src/interfaces/libpq/libpq-exit-check
new file mode 100755
index 00000000000..84fa2a7fbe0
--- /dev/null
+++ b/src/interfaces/libpq/libpq-exit-check
@@ -0,0 +1,71 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings FATAL => 'all';
+
+use Getopt::Long;
+
+my $input_file;
+my $stamp_file;
+my @problematic_lines;
+
+GetOptions(
+ 'input_file:s' => \$input_file,
+ 'stamp_file:s' => \$stamp_file) or die "$0: wrong arguments";
+
+die "$0: --input_file must be specified\n" unless defined $input_file;
+
+open my $fh, '-|', "nm -A -u $input_file 2>/dev/null"
+ or die "Cannot run nm: $!";
+
+while (<$fh>)
+{
+ next if /__cxa_atexit/;
+ next if /__tsan_func_exit/;
+ next if /pthread_exit/;
+
+ if (/exit/)
+ {
+ push @problematic_lines, $_;
+ }
+}
+
+if (@problematic_lines)
+{
+ remove_stamp_file();
+
+ print "libpq must not be calling any function which invokes exit\n";
+ print "Problematic symbol references:\n";
+ print @problematic_lines;
+
+ exit 1;
+}
+else
+{
+ # All checks are passed, we can create a stamp file meson build now
+ if ($stamp_file)
+ {
+ create_stamp_file();
+ }
+
+ exit 0;
+}
+
+sub create_stamp_file
+{
+ # Avoid touching existing stamp file to prevent unnecessary rebuilds
+ if (!(-f $stamp_file))
+ {
+ open my $fh, '>', $stamp_file
+ or die "can't open $stamp_file: $!";
+ close $fh;
+ }
+}
+
+sub remove_stamp_file
+{
+ if ($stamp_file)
+ {
+ unlink $stamp_file;
+ }
+}
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index a74e885b169..978fff192ea 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -80,6 +80,33 @@ libpq_so = shared_library('libpq',
kwargs: default_lib_args,
)
+# 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.
+if find_program('nm', required: false, native: true).found() and portname != 'solaris' and not get_option('b_coverage')
+ custom_target(
+ 'libpq-exit-check',
+ input: libpq_so,
+ output: 'libpq-refs-stamp',
+ command: [
+ perl,
+ files('libpq-exit-check'),
+ '--input_file', '@INPUT@',
+ '--stamp_file', '@OUTPUT@'
+ ],
+ build_by_default: true,
+ )
+endif
+
libpq = declare_dependency(
link_with: [libpq_so],
include_directories: [include_directories('.')]
--
2.51.0
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-11-24 11:04 Nazir Bilal Yavuz <[email protected]>
parent: Nazir Bilal Yavuz <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Nazir Bilal Yavuz @ 2025-11-24 11:04 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; VASUKI M <[email protected]>
Hi,
On Wed, 19 Nov 2025 at 16:17, Nazir Bilal Yavuz <[email protected]> wrote:
>
> Sorry for the late reply. I replaced the Makefile portion with the
> Perl script, so that it can be used for both meson and autoconf build
> systems.
Apparently we do not need to remove the stamp-file in the perl script,
meson already handles that internally. v2 is attached.
--
Regards,
Nazir Bilal Yavuz
Microsoft
Attachments:
[text/x-patch] v2-0001-Add-exit-check-for-libpq.so-for-meson-build.patch (4.0K, 2-v2-0001-Add-exit-check-for-libpq.so-for-meson-build.patch)
download | inline diff:
From 21ae5e54a73d4ea98c91ad4de38afe90c80042b7 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <[email protected]>
Date: Wed, 19 Nov 2025 12:08:08 +0300
Subject: [PATCH v2] Add exit() check for libpq.so for meson build
---
src/interfaces/libpq/Makefile | 4 +-
src/interfaces/libpq/libpq-exit-check | 61 +++++++++++++++++++++++++++
src/interfaces/libpq/meson.build | 27 ++++++++++++
3 files changed, 89 insertions(+), 3 deletions(-)
create mode 100755 src/interfaces/libpq/libpq-exit-check
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index da6650066d4..6b2d1769a07 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -144,9 +144,7 @@ $(stlib): $(OBJS_STATIC)
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
+ $(PERL) libpq-exit-check --input_file $<
endif
endif
touch $@
diff --git a/src/interfaces/libpq/libpq-exit-check b/src/interfaces/libpq/libpq-exit-check
new file mode 100755
index 00000000000..e3c2a6e768d
--- /dev/null
+++ b/src/interfaces/libpq/libpq-exit-check
@@ -0,0 +1,61 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings FATAL => 'all';
+
+use Getopt::Long;
+
+my $input_file;
+my $stamp_file;
+my @problematic_lines;
+
+GetOptions(
+ 'input_file:s' => \$input_file,
+ 'stamp_file:s' => \$stamp_file) or die "$0: wrong arguments";
+
+die "$0: --input_file must be specified\n" unless defined $input_file;
+
+open my $fh, '-|', "nm -A -u $input_file 2>/dev/null"
+ or die "Cannot run nm: $!";
+
+while (<$fh>)
+{
+ next if /__cxa_atexit/;
+ next if /__tsan_func_exit/;
+ next if /pthread_exit/;
+
+ if (/exit/)
+ {
+ push @problematic_lines, $_;
+ }
+}
+
+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;
+}
+else
+{
+ # All checks are passed, we can create a stamp file meson build now
+ if ($stamp_file)
+ {
+ create_stamp_file();
+ }
+
+ exit 0;
+}
+
+sub create_stamp_file
+{
+ # Avoid touching existing stamp file to prevent unnecessary rebuilds
+ if (!(-f $stamp_file))
+ {
+ open my $fh, '>', $stamp_file
+ or die "can't open $stamp_file: $!";
+ close $fh;
+ }
+}
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index a74e885b169..978fff192ea 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -80,6 +80,33 @@ libpq_so = shared_library('libpq',
kwargs: default_lib_args,
)
+# 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.
+if find_program('nm', required: false, native: true).found() and portname != 'solaris' and not get_option('b_coverage')
+ custom_target(
+ 'libpq-exit-check',
+ input: libpq_so,
+ output: 'libpq-refs-stamp',
+ command: [
+ perl,
+ files('libpq-exit-check'),
+ '--input_file', '@INPUT@',
+ '--stamp_file', '@OUTPUT@'
+ ],
+ build_by_default: true,
+ )
+endif
+
libpq = declare_dependency(
link_with: [libpq_so],
include_directories: [include_directories('.')]
--
2.51.0
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-11-24 21:43 Michael Paquier <[email protected]>
parent: Nazir Bilal Yavuz <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Michael Paquier @ 2025-11-24 21:43 UTC (permalink / raw)
To: Nazir Bilal Yavuz <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; VASUKI M <[email protected]>
On Mon, Nov 24, 2025 at 02:04:01PM +0300, Nazir Bilal Yavuz wrote:
> Apparently we do not need to remove the stamp-file in the perl script,
> meson already handles that internally. v2 is attached.
Good idea to embed that in a perl script!
> +# 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.
Including a reference to "nm" in this comment for meson is definitely
fine, because it is used as a pre-check in this code with
find_program. However, shouldn't we document the platform-specific
exclusions in the perl script itself? As of the patch, the
explanation is a copy-paste of src/interfaces/libpq/Makefile. I think
that we'd better group everything together, rather than have the same
contents explained in two places. Perhaps I would add an extra
comment in meson.build and the Makefile to document that all the
platform-relevant details are in the perl script itself.
I would be also tempted to move the solaris check inside the perl
script rather than have it duplicated across meson and make, then do
something based on $Config{osname} instead.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-11-25 09:11 VASUKI M <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 2 replies; 17+ messages in thread
From: VASUKI M @ 2025-11-25 09:11 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
On Tue, 25 Nov 2025 at 03:14, Michael Paquier <[email protected]> wrote:
> Including a reference to "nm" in this comment for meson is definitely
> fine, because it is used as a pre-check in this code with
> find_program. However, shouldn't we document the platform-specific
> exclusions in the perl script itself? As of the patch, the
> explanation is a copy-paste of src/interfaces/libpq/Makefile. I think
> that we'd better group everything together, rather than have the same
> contents explained in two places. Perhaps I would add an extra
> comment in meson.build and the Makefile to document that all the
> platform-relevant details are in the perl script itself.
>
> Thanks for this suggestion michael & Nazir for the code,i have made the
changes you said
Also added the check where it scans for nm in the environment if it is not
present then it gracefully skips the test.
V3 attached kindly check and review it.
> I would be also tempted to move the solaris check inside the perl
> script rather than have it duplicated across meson and make, then do
> something based on $Config{osname} instead.
>
Done
Testing performed:
- Built with both autoconf+Makefile and Meson builds.
- Verified the script runs inside Meson via the custom_target and
confirmed with ninja -v.
>grep -R "libpq-exit-check" build.ninja
build src/interfaces/libpq/libpq-refs-stamp: CUSTOM_COMMAND
src/interfaces/libpq/libpq.so.5.19 |
../src/interfaces/libpq/libpq-exit-check /usr/bin/perl
COMMAND = /usr/bin/perl ../src/interfaces/libpq/libpq-exit-check
--input_file src/interfaces/libpq/libpq.so.5.19 --stamp_file
src/interfaces/libpq/libpq-refs-stamp
description = Generating$ src/interfaces/libpq/libpq-exit-check$ with$ a$
custom$ command
- Injected a fake exit() reference into fe-connect.c and ensured the
build fails with the expected error message.
- Confirmed that removing nm causes the script to skip the check cleanly.
- Verified that Meson’s stamp file prevents re-running the check when
libpq.so has not changed.
Regards,
Vasuki M
CDAC-Chennai
Attachments:
[text/x-patch] v3-0001-libpq-exit-check.patch (6.9K, 3-v3-0001-libpq-exit-check.patch)
download | inline diff:
From 2a8d4a7defe43a31e32dae2d32e7db978cd11739 Mon Sep 17 00:00:00 2001
From: BharatDBPG <[email protected]>
Date: Tue, 25 Nov 2025 12:33:53 +0530
Subject: [PATCH v3] libpq: centralize exit() check logic in Perl script and
share with Meson and Makefile
The existing Makefile-based build performs a safety check to ensure that
libpq does not accidentally reference exit(), since client libraries
must not terminate the calling process. Meson builds did not run an
equivalent check, and the logic for platform-specific handling and
whitelisting was duplicated in the Makefile comments.
This patch introduces a new helper script, src/interfaces/libpq/libpq-exit-check,
and makes both the Makefile and Meson builds invoke it:
* Move the exit() checking logic into libpq-exit-check, written in Perl.
The script calls nm on the given library, filters out known harmless
symbols (__cxa_atexit, __tsan_func_exit, pthread_exit), and reports
any remaining exit()-related references.
* Centralize platform-specific behavior inside the script:
- Skip the check on Solaris, where libpq infrastructure may come
from statically-linked libraries.
- Skip the check entirely when nm is not found or is not usable.
This removes the need for separate Solaris checks and detailed
comments in the Makefile and Meson files.
* Reduce the Makefile and Meson-side logic to a simple invocation of
libpq-exit-check plus a brief comment that the platform rules live
in the script. Meson uses a custom_target with a stamp file so the
check only reruns when libpq.so is rebuilt.
With these changes, both autoconf/Makefile and Meson builds enforce the
same exit() policy for libpq while keeping the implementation and
platform rules in a single place.
---
src/interfaces/libpq/Makefile | 20 +-----
src/interfaces/libpq/libpq-exit-check | 98 +++++++++++++++++++++++++++
src/interfaces/libpq/meson.build | 17 +++++
3 files changed, 118 insertions(+), 17 deletions(-)
create mode 100755 src/interfaces/libpq/libpq-exit-check
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index da66500..305361f 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -130,26 +130,12 @@ $(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.
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
+ # See libpq-exit-check for full platform rules and whitelisting.
+ $(PERL) libpq-exit-check --input_file $<
endif
-endif
- touch $@
+ touch $@
# Make dependencies on pg_config_paths.h visible in all builds.
fe-connect.o: fe-connect.c $(top_builddir)/src/port/pg_config_paths.h
diff --git a/src/interfaces/libpq/libpq-exit-check b/src/interfaces/libpq/libpq-exit-check
new file mode 100755
index 0000000..f500cef
--- /dev/null
+++ b/src/interfaces/libpq/libpq-exit-check
@@ -0,0 +1,98 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings FATAL => 'all';
+
+use Getopt::Long;
+use Config;
+
+#
+# Platform & tool notes:
+#
+# - Purpose: ensure libpq never references exit(), because client libraries
+# must not terminate the host application.
+#
+# - Whitelisted injected symbols:
+# __cxa_atexit - injected by some libcs (e.g., OpenBSD)
+# __tsan_func_exit - ThreadSanitizer instrumentation
+# pthread_exit - legitimate thread cleanup
+#
+# - Solaris: libpq infrastructure may be statically linked -> skip check.
+#
+# - nm availability:
+# If nm does not exist or cannot be executed, skip the scan.
+# This matches PostgreSQL behavior in Makefile builds.
+#
+# - Makefile and Meson both rely on this script for full logic.
+#
+
+my $input_file;
+my $stamp_file;
+my @problematic_lines;
+
+Getopt::Long::GetOptions(
+ '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;
+
+# ---- Skip entirely on Solaris ----
+if ($Config{osname} =~ /solaris/i) {
+ exit 0;
+}
+
+# ---- Check if 'nm' exists on the system ----
+my $nm_path = `which nm 2>/dev/null`;
+chomp($nm_path);
+
+if (!$nm_path || ! -x $nm_path) {
+ # nm not available → skip check gracefully
+ exit 0;
+}
+
+# ---- Run nm to scan undefined symbols ----
+open my $fh, '-|', "$nm_path -A -u $input_file 2>/dev/null"
+ or exit 0; # If nm fails at runtime, skip also
+
+while (<$fh>)
+{
+ # Allowed symbols
+ next if /__cxa_atexit/;
+ next if /__tsan_func_exit/;
+ next if /pthread_exit/;
+
+ # Anything containing "exit" is suspicious
+ if (/exit/)
+ {
+ push @problematic_lines, $_;
+ }
+}
+
+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;
+}
+else
+{
+ # Meson optional stamp file
+ if ($stamp_file)
+ {
+ create_stamp_file();
+ }
+
+ exit 0;
+}
+
+sub create_stamp_file
+{
+ if (!(-f $stamp_file))
+ {
+ open my $fh, '>', $stamp_file
+ or die "can't open $stamp_file: $!";
+ close $fh;
+ }
+}
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index a74e885..1b32eed 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -85,6 +85,23 @@ libpq = declare_dependency(
include_directories: [include_directories('.')]
)
+# Run the unified exit() check script for libpq.
+# All platform-specific rules are implemented inside libpq-exit-check.
+if find_program('nm', required: false, native: true).found() and not get_option('b_coverage')
+ custom_target(
+ 'libpq-exit-check',
+ input: libpq_so,
+ output: 'libpq-refs-stamp',
+ command: [
+ perl,
+ files('libpq-exit-check'),
+ '--input_file', '@INPUT@',
+ '--stamp_file', '@OUTPUT@'
+ ],
+ build_by_default: true,
+ )
+endif
+
private_deps = [
frontend_stlib_code,
libpq_deps,
--
2.43.0
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-11-25 10:07 Nazir Bilal Yavuz <[email protected]>
parent: VASUKI M <[email protected]>
1 sibling, 0 replies; 17+ messages in thread
From: Nazir Bilal Yavuz @ 2025-11-25 10:07 UTC (permalink / raw)
To: VASUKI M <[email protected]>; +Cc: Michael Paquier <[email protected]>; Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]
Hi,
On Tue, 25 Nov 2025 at 12:11, VASUKI M <[email protected]> wrote:
>
> On Tue, 25 Nov 2025 at 03:14, Michael Paquier <[email protected]> wrote:
>>
>> Including a reference to "nm" in this comment for meson is definitely
>> fine, because it is used as a pre-check in this code with
>> find_program. However, shouldn't we document the platform-specific
>> exclusions in the perl script itself? As of the patch, the
>> explanation is a copy-paste of src/interfaces/libpq/Makefile. I think
>> that we'd better group everything together, rather than have the same
>> contents explained in two places. Perhaps I would add an extra
>> comment in meson.build and the Makefile to document that all the
>> platform-relevant details are in the perl script itself.
>>
> Thanks for this suggestion michael & Nazir for the code,i have made the changes you said
>
> Also added the check where it scans for nm in the environment if it is not present then it gracefully skips the test.
> V3 attached kindly check and review it.
Thank you for working on this!
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index da66500..305361f 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
-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
+ # See libpq-exit-check for full platform rules and whitelisting.
+ $(PERL) libpq-exit-check --input_file $<
endif
-endif
- touch $@
+ touch $@
There are unnecessary indentation changes.
diff --git a/src/interfaces/libpq/libpq-exit-check
b/src/interfaces/libpq/libpq-exit-check
new file mode 100755
index 0000000..f500cef
--- /dev/null
+++ b/src/interfaces/libpq/libpq-exit-check
I would prefer more in-line comments instead of the comment at the top
but I think this is a preference.
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index a74e885..1b32eed 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
+if find_program('nm', required: false, native: true).found() and not
get_option('b_coverage')
I would delete the 'nm' check there, since we have the same check in
the PERL script. This makes the meson.build and the Makefile more
similar.
Also, I would change the comment at the Makefile and the meson.build
with the comment below, otherwise we lose information:
# 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().)
# Skip the test when profiling, as gcc may insert exit() calls for that.
Nitpick: I suggest running pgperltidy [1] on the libq-exit-check PERL file.
[1] https://github.com/postgres/postgres/blob/master/src/tools/pgindent/pgperltidy
--
Regards,
Nazir Bilal Yavuz
Microsoft
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-11-25 10:51 Daniel Gustafsson <[email protected]>
parent: VASUKI M <[email protected]>
1 sibling, 1 reply; 17+ messages in thread
From: Daniel Gustafsson @ 2025-11-25 10:51 UTC (permalink / raw)
To: VASUKI M <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
> On 25 Nov 2025, at 10:11, VASUKI M <[email protected]> wrote:
> Thanks for this suggestion michael & Nazir for the code,i have made the changes you said
>
> Also added the check where it scans for nm in the environment if it is not present then it gracefully skips the test.
+if find_program('nm', required: false, native: true).found() and not get_option('b_coverage')
Sorry for being late to the party, but I wonder why we aren't adding this check
to the toplevel meson.build and configure.ac (via config/programs.m4) like how
we check for all others tools used by the build? Such checks should of course
not fail the configuration, merely record the presence or absence of the tool.
The path can then be exported to src/interfaces/libpq/{Makefile|meson.build} to
use.
+open my $fh, '-|', "$nm_path -A -u $input_file 2>/dev/null"
This filehandle is never closed.
+# ---- Skip entirely on Solaris ----
+if ($Config{osname} =~ /solaris/i) {
+ exit 0;
+}
This won't work on Windows either, which wasn't checked for in the Makefile
since make isn't used on Windows.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-11-26 03:43 Michael Paquier <[email protected]>
parent: Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Michael Paquier @ 2025-11-26 03:43 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: VASUKI M <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
On Tue, Nov 25, 2025 at 11:51:55AM +0100, Daniel Gustafsson wrote:
> +if find_program('nm', required: false, native: true).found() and not get_option('b_coverage')
> Sorry for being late to the party, but I wonder why we aren't adding this check
> to the toplevel meson.build and configure.ac (via config/programs.m4) like how
> we check for all others tools used by the build? Such checks should of course
> not fail the configuration, merely record the presence or absence of the tool.
> The path can then be exported to src/interfaces/libpq/{Makefile|meson.build} to
> use.
+1 for this find_program() call grouped at the top of meson.build,
grouped with the others.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-12-01 04:57 VASUKI M <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: VASUKI M @ 2025-12-01 04:57 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
Hi Daniel, Michael,Nazir
Thanks for the feedback.
For v4 I will implement the approach you outlined:
-Move the nm detection to the top-level build:
- Add find_program('nm') in the root meson.build
- Add PGAC_CHECK_PROG(NM_PROG, nm) in configure.ac
and export the result down into src/interfaces/libpq for use by both
Meson and Makefile builds.
-Pass the resolved nm path into libpq-exit-check via a --nm argument,
removing the internal which nm lookup from the script.
-Expand the platform skip logic in the script to include Windows in
addition to Solaris, and close the filehandle as suggested.
-Replace the comments in Makefile and meson.build with the unified
explanatory block, and keep all platform details inside the script.
I will post v4 with these changes shortly.
Regards,
Vasuki
On Wed, 26 Nov 2025 at 09:13, Michael Paquier <[email protected]> wrote:
> On Tue, Nov 25, 2025 at 11:51:55AM +0100, Daniel Gustafsson wrote:
> > +if find_program('nm', required: false, native: true).found() and not
> get_option('b_coverage')
> > Sorry for being late to the party, but I wonder why we aren't adding
> this check
> > to the toplevel meson.build and configure.ac (via config/programs.m4)
> like how
> > we check for all others tools used by the build? Such checks should of
> course
> > not fail the configuration, merely record the presence or absence of the
> tool.
> > The path can then be exported to
> src/interfaces/libpq/{Makefile|meson.build} to
> > use.
>
> +1 for this find_program() call grouped at the top of meson.build,
> grouped with the others.
> --
> Michael
>
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-12-03 11:59 VASUKI M <[email protected]>
parent: VASUKI M <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: VASUKI M @ 2025-12-03 11:59 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
Hi Hackers,
This is a refreshed version of the patch to unify the exit() reference check
performed during libpq builds. Earlier versions had duplicated logic between
the Makefile and Meson builds, while platform-specific details were also
split
between comments and build rules.
Following feedback from Michael, Daniel, Tom, and Nazir, v4 includes the
following changes:
Changes in v4
-
Added top-level AC_PATH_PROG(NM, nm) in configure.ac, and propagated
it via NM, so both build systems use the same nm path.
-
Updated Makefile and Meson rules to invoke the new Perl helper using
-
- -nm=$(NM) / - -nm=@NM@
-
Centralized all platform-specific behavior in the Perl script:
-
Skip on Solaris and Windows
-
Handle ThreadSanitizer (__tsan_func_exit)
-
Handle OpenBSD’s __cxa_atexit
-
Whitelist pthread_exit()
-
Gracefully skip if nm is not available
-
Removed redundant checks from meson.build and Makefile and replaced them
with
a short reference to the script.
-
Added missing cleanup (closing filehandle), and improved comments.
-
Ensured consistent behavior across autoconf and Meson builds.
Testing:
Both autoconf and Meson builds were tested with:
-
Valid nm present
-
Missing nm
-
Solaris/Windows-skip behavior
-
Injected exit() symbol to confirm failure mode works correctly
The behavior is now uniform across build systems.
Patch is attached.
Thanks for the reviews so far, and I welcome further comments.
Regards,
Vasuki
>
> On Wed, 26 Nov 2025 at 09:13, Michael Paquier <[email protected]> wrote:
>
>> On Tue, Nov 25, 2025 at 11:51:55AM +0100, Daniel Gustafsson wrote:
>> > +if find_program('nm', required: false, native: true).found() and not
>> get_option('b_coverage')
>> > Sorry for being late to the party, but I wonder why we aren't adding
>> this check
>> > to the toplevel meson.build and configure.ac (via config/programs.m4)
>> like how
>> > we check for all others tools used by the build? Such checks should of
>> course
>> > not fail the configuration, merely record the presence or absence of
>> the tool.
>> > The path can then be exported to
>> src/interfaces/libpq/{Makefile|meson.build} to
>> > use.
>>
>> +1 for this find_program() call grouped at the top of meson.build,
>> grouped with the others.
>>
> Fixed in the v4 please review it.
> --
>> Michael
>>
>
Attachments:
[text/x-patch] v4-0001-libpq-centralize-exit-check-logic-and-unify-Makef.patch (8.2K, 3-v4-0001-libpq-centralize-exit-check-logic-and-unify-Makef.patch)
download | inline diff:
From 89f4c76703ac1b50e8ee9bea6045757187f35e81 Mon Sep 17 00:00:00 2001
From: BharatDBPG <[email protected]>
Date: Wed, 3 Dec 2025 17:18:56 +0530
Subject: [PATCH v4] 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 | 2 +-
src/interfaces/libpq/Makefile | 9 ++-
src/interfaces/libpq/libpq-exit-check | 100 ++++++++++++--------------
src/interfaces/libpq/meson.build | 10 ++-
6 files changed, 62 insertions(+), 62 deletions(-)
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 c1e17aa..273cb1f 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..9d12cdb 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -285,7 +285,7 @@ LLVM_CFLAGS = @LLVM_CFLAGS@
LLVM_CXXFLAGS = @LLVM_CXXFLAGS@
# Kind-of compilers
-
+NM = @NM@
BISON = @BISON@
BISONFLAGS = @BISONFLAGS@ $(YFLAGS)
FLEX = @FLEX@
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 305361f..6e1fb30 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -130,12 +130,15 @@ $(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().)
+# Skip the test when profiling, as gcc may insert exit() calls for that.
libpq-refs-stamp: $(shlib)
ifneq ($(enable_coverage), yes)
- # See libpq-exit-check for full platform rules and whitelisting.
- $(PERL) libpq-exit-check --input_file $<
+# See libpq-exit-check for full platform rules and whitelisting.
+ $(PERL) libpq-exit-check --input_file $< --nm='$(NM)'
endif
- touch $@
+ touch $@
# Make dependencies on pg_config_paths.h visible in all builds.
fe-connect.o: fe-connect.c $(top_builddir)/src/port/pg_config_paths.h
diff --git a/src/interfaces/libpq/libpq-exit-check b/src/interfaces/libpq/libpq-exit-check
index f500cef..bfac4ca 100755
--- a/src/interfaces/libpq/libpq-exit-check
+++ b/src/interfaces/libpq/libpq-exit-check
@@ -6,93 +6,83 @@ use warnings FATAL => 'all';
use Getopt::Long;
use Config;
-#
-# Platform & tool notes:
-#
# - Purpose: ensure libpq never references exit(), because client libraries
# must not terminate the host application.
-#
-# - Whitelisted injected symbols:
-# __cxa_atexit - injected by some libcs (e.g., OpenBSD)
-# __tsan_func_exit - ThreadSanitizer instrumentation
-# pthread_exit - legitimate thread cleanup
-#
-# - Solaris: libpq infrastructure may be statically linked -> skip check.
-#
-# - nm availability:
-# If nm does not exist or cannot be executed, skip the scan.
-# This matches PostgreSQL behavior in Makefile builds.
-#
# - Makefile and Meson both rely on this script for full logic.
-#
+my $nm_path;
my $input_file;
my $stamp_file;
my @problematic_lines;
Getopt::Long::GetOptions(
- 'input_file:s' => \$input_file,
- 'stamp_file:s' => \$stamp_file) or die "$0: wrong arguments\n";
+ '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;
-# ---- Skip entirely on Solaris ----
-if ($Config{osname} =~ /solaris/i) {
- exit 0;
+# ---- Skip Windows ----
+if ($Config{osname} =~ /MSWin32|cygwin|msys/i)
+{
+ exit 0;
}
-# ---- Check if 'nm' exists on the system ----
-my $nm_path = `which nm 2>/dev/null`;
-chomp($nm_path);
-
-if (!$nm_path || ! -x $nm_path) {
- # nm not available → skip check gracefully
- exit 0;
+# ---- Skip entirely on Solaris ----
+if ($Config{osname} =~ /solaris/i)
+{
+ exit 0;
}
# ---- Run nm to scan undefined symbols ----
open my $fh, '-|', "$nm_path -A -u $input_file 2>/dev/null"
- or exit 0; # If nm fails at runtime, skip also
+ or exit 0; # If nm fails at runtime, skip also
while (<$fh>)
{
- # Allowed symbols
- next if /__cxa_atexit/;
- next if /__tsan_func_exit/;
- next if /pthread_exit/;
-
- # Anything containing "exit" is suspicious
- if (/exit/)
- {
- push @problematic_lines, $_;
- }
+ # Whitelisted injected symbols:
+ # __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
+ 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;
+ print "libpq must not be calling any function which invokes exit\n";
+ print "Problematic symbol references:\n";
+ print @problematic_lines;
- exit 1;
+ exit 1;
}
else
{
- # Meson optional stamp file
- if ($stamp_file)
- {
- create_stamp_file();
- }
+ # Meson optional stamp file
+ if ($stamp_file)
+ {
+ create_stamp_file();
+ }
- exit 0;
+ exit 0;
}
sub create_stamp_file
{
- if (!(-f $stamp_file))
- {
- open my $fh, '>', $stamp_file
- or die "can't open $stamp_file: $!";
- close $fh;
- }
+ if (!(-f $stamp_file))
+ {
+ open my $fh, '>', $stamp_file
+ or die "can't open $stamp_file: $!";
+ close $fh;
+ }
}
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index 1b32eed..6f97f49 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -85,9 +85,12 @@ libpq = declare_dependency(
include_directories: [include_directories('.')]
)
-# Run the unified exit() check script for libpq.
+# 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().)
+# Skip the test when profiling, as gcc may insert exit() calls for that.
# All platform-specific rules are implemented inside libpq-exit-check.
-if find_program('nm', required: false, native: true).found() and not get_option('b_coverage')
+if nm.found() and not get_option('b_coverage')
custom_target(
'libpq-exit-check',
input: libpq_so,
@@ -96,7 +99,8 @@ if find_program('nm', required: false, native: true).found() and not get_option(
perl,
files('libpq-exit-check'),
'--input_file', '@INPUT@',
- '--stamp_file', '@OUTPUT@'
+ '--stamp_file', '@OUTPUT@',
+ '--nm', nm.full_path()
],
build_by_default: true,
)
--
2.43.0
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-12-03 23:33 Daniel Gustafsson <[email protected]>
parent: VASUKI M <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Daniel Gustafsson @ 2025-12-03 23:33 UTC (permalink / raw)
To: VASUKI M <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
> On 3 Dec 2025, at 12:59, VASUKI M <[email protected]> wrote:
> This is a refreshed version of the patch to unify the exit() reference check
> performed during libpq builds. Earlier versions had duplicated logic between
> the Makefile and Meson builds, while platform-specific details were also split
> between comments and build rules.
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.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-12-04 00:32 Michael Paquier <[email protected]>
parent: Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Michael Paquier @ 2025-12-04 00:32 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: VASUKI M <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [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
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-12-04 11:08 VASUKI M <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: VASUKI M @ 2025-12-04 11:08 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [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
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-12-05 00:41 Michael Paquier <[email protected]>
parent: VASUKI M <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Michael Paquier @ 2025-12-05 00:41 UTC (permalink / raw)
To: VASUKI M <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
On Thu, Dec 04, 2025 at 04:38:40PM +0530, VASUKI M wrote:
> 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.
Including the changes in ./configure is not mandatory for patch
authors, as this is something that committers take care of. Still,
these are useful for the CI and testing the ./configure code.
> 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?
The only differences I can see between v5 and v6 is that the perl
script uses whitespaces for its code indentation rather than tabs,
which I guess is not expected. Most of the diffs between v6 and v5 go
away once a perl indentation is applied.
I am not going to take a bet on this one on a Friday, so let's give it
a few more days. Have others any comments and/or concerns?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-12-08 04:41 VASUKI M <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: VASUKI M @ 2025-12-08 04:41 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
Hi Michael,
Thanks for checking the v6 patch.
Regarding the whitespace comment - I had already ran the pgperltidy on the
perl
script libpq-check.pl before sending the patch,and I also verified the tab
indentation
with cat -T to ensure the tabs were being used.FS,The current version of
the scripts in
v6 should already follow the standard pgindent/pgperltidy formatting
If there is still any specific section that appears mis-indented on your
side,
please let me know and I will adjust it.
cdac@cdac-Aspire-Lite-AL15-41:~/pg-libpq$ src/tools/pgindent/pgperltidy
src/interfaces/libpq/libpq-check.pl
cdac@cdac-Aspire-Lite-AL15-41:~/pg-libpq$
cdac@cdac-Aspire-Lite-AL15-41:~/pg-libpq$ cat -T src/interfaces/libpq/
libpq-check.pl
#!/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(
^I'nm:s' => \$nm_path,
^I'input_file:s' => \$input_file,
^I'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
{
^Iif (!(-f $stamp_file))
^I{
^I^Iopen my $fh, '>', $stamp_file
^I^I or die "can't open $stamp_file: $!";
^I^Iclose $fh;
^I}
}
# ---- Skip on Windows and Solaris ----
if ( $Config{osname} =~ /MSWin32|cygwin|msys/i
^I|| $Config{osname} =~ /solaris/i)
{
^Iexit 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>)
{
^I# Set of symbols allowed:
^I# __cxa_atexit - injected by some libcs (e.g., OpenBSD)
^I# __tsan_func_exit - ThreadSanitizer instrumentation
^I# pthread_exit - legitimate thread cleanup
^Inext if /__cxa_atexit/;
^Inext if /__tsan_func_exit/;
^Inext if /pthread_exit/;
^I# Anything containing "exit" is suspicious.
^I# (Ideally we should reject abort() too, but there are various scenarios
^I# where build toolchains insert abort() calls, e.g. to implement
assert().)
^Iif (/exit/)
^I{
^I^Ipush @problematic_lines, $_;
^I}
}
close $fh;
if (@problematic_lines)
{
^Iprint "libpq must not be calling any function which invokes exit\n";
^Iprint "Problematic symbol references:\n";
^Iprint @problematic_lines;
^Iexit 1;
}
# Create stamp file, if required
if (defined($stamp_file))
{
^Icreate_stamp_file();
}
exit 0;
cdac@cdac-Aspire-Lite-AL15-41:~/pg-libpq$
Regards,
Vasuki
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-12-09 04:16 Michael Paquier <[email protected]>
parent: VASUKI M <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Michael Paquier @ 2025-12-09 04:16 UTC (permalink / raw)
To: VASUKI M <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
On Mon, Dec 08, 2025 at 10:11:08AM +0530, VASUKI M wrote:
> Regarding the whitespace comment - I had already ran the pgperltidy on the
> perl script libpq-check.pl before sending the patch,and I also verified the tab
> indentation with cat -T to ensure the tabs were being used.FS,The current version of
> the scripts in v6 should already follow the standard pgindent/pgperltidy formatting
>
> If there is still any specific section that appears mis-indented on your
> side, please let me know and I will adjust it.
The only documentation that I have been following on this matter
exists in src/tools/pgindent/README. That may be an issue with your
environment, I cannot say for sure. And I am pretty sure that my
environment is handling things the way the README tells.
Anyway, after more review of the refactoring patch, I have noticed
that we were losing quite a few details about the reasons why we are
doing things the way they are.
So I have added more details about to keep things a maximum consistent
with what we had documented:
- The shared library check requirement, mentioning that the check is
skipped for static libraries in the Makefile case (for meson, we use
libpq_so, so that did not seem strongly necessary to add there,
perhaps we should).
- The explanation for __cxa_atexit, related to OpenBSD, reusing the
same wording as previously, but in the script.
- The explanation for __tsan_func_exit with ThreadSanitizer, reusing
the same wording as previously, but in the script.
Then applied the result as 4a8e6f43a6b5. Running the check for meson
may bring some surprises, but we'll see. The buildfarm looks OK for
the moment. Once we are completely in the clear, let's move on with
the second patch.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-12-09 06:10 Peter Eisentraut <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Peter Eisentraut @ 2025-12-09 06:10 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; VASUKI M <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
On 09.12.25 05:16, Michael Paquier wrote:
> Then applied the result as 4a8e6f43a6b5. Running the check for meson
> may bring some surprises, but we'll see. The buildfarm looks OK for
> the moment. Once we are completely in the clear, let's move on with
> the second patch.
meson gives this warning now:
../src/meson.build:31: WARNING: The variable(s) 'NM' in the input file
'src/Makefile.global.in' are not present in the given configuration data.
Looks like a small addition on src/makefiles/meson.build would fix it.
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: BUG #19095: Test if function exit() is used fail when linked static
@ 2025-12-09 06:26 Michael Paquier <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Michael Paquier @ 2025-12-09 06:26 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: VASUKI M <[email protected]>; Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; BharatDB <[email protected]>; [email protected]; [email protected]; [email protected]
On Tue, Dec 09, 2025 at 07:10:09AM +0100, Peter Eisentraut wrote:
> meson gives this warning now:
>
> ../src/meson.build:31: WARNING: The variable(s) 'NM' in the input file
> 'src/Makefile.global.in' are not present in the given configuration data.
>
> Looks like a small addition on src/makefiles/meson.build would fix it.
Oops. Let me see.. Indeed, the warning shows during the "Configuring
Makefile.global using configuration" step. This information has been
showing up in my logs while testing meson.
The attached takes care of the issue here. Adding that to pgxs_bins
may look adapted at first glance, but I don't quite see the point in
exposing this information to PGXS, so pgxs_empty is a better fit?
What do you think?
--
Michael
Attachments:
[text/x-diff] meson-warning.patch (470B, 2-meson-warning.patch)
download | inline diff:
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index 0def244c9011..c6edf14ec449 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -154,9 +154,10 @@ pgxs_bins = {
pgxs_empty = [
'ICU_CFLAGS', # needs to be added, included by public server headers
- # hard to see why we'd need either?
+ # hard to see why we'd need these ones?
'ZIC',
'TCLSH',
+ 'NM',
# docs don't seem to be supported by pgxs
'XMLLINT',
[application/pgp-signature] signature.asc (833B, 3-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
end of thread, other threads:[~2025-12-09 06:26 UTC | newest]
Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-11-19 13:17 Re: BUG #19095: Test if function exit() is used fail when linked static Nazir Bilal Yavuz <[email protected]>
2025-11-24 11:04 ` Nazir Bilal Yavuz <[email protected]>
2025-11-24 21:43 ` Michael Paquier <[email protected]>
2025-11-25 09:11 ` VASUKI M <[email protected]>
2025-11-25 10:07 ` Nazir Bilal Yavuz <[email protected]>
2025-11-25 10:51 ` Daniel Gustafsson <[email protected]>
2025-11-26 03:43 ` Michael Paquier <[email protected]>
2025-12-01 04:57 ` VASUKI M <[email protected]>
2025-12-03 11:59 ` VASUKI M <[email protected]>
2025-12-03 23:33 ` Daniel Gustafsson <[email protected]>
2025-12-04 00:32 ` Michael Paquier <[email protected]>
2025-12-04 11:08 ` VASUKI M <[email protected]>
2025-12-05 00:41 ` Michael Paquier <[email protected]>
2025-12-08 04:41 ` VASUKI M <[email protected]>
2025-12-09 04:16 ` Michael Paquier <[email protected]>
2025-12-09 06:10 ` Peter Eisentraut <[email protected]>
2025-12-09 06:26 ` Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox