public inbox for [email protected]  
help / color / mirror / Atom feed
From: 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: Wed, 3 Dec 2025 17:29:33 +0530
Message-ID: <CACTYHzjjssid_jtZiLh8BTYHAvnxbSLXceyGtxM3NB6Kfv510g@mail.gmail.com> (raw)
In-Reply-To: <CACTYHzjbVeKV289+TB1mg+tVmpnOJWrtezTvRjUwYoc875p1XA@mail.gmail.com>
References: <CAAh00ETwx8_AEM0wgoi-v875uC-FLuGGOMTaP9fCNdVw4Zq=Vg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[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>

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



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: <CACTYHzjjssid_jtZiLh8BTYHAvnxbSLXceyGtxM3NB6Kfv510g@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