From 4a8e6f43a6b56289cd3806b239c20ae31aa4cf2e Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 9 Dec 2025 10:39:08 +0900 Subject: [PATCH] libpq: Refactor logic checking for exit() in shared library builds This commit refactors the sanity check done by libpq to ensure that there is no exit() reference in the build, moving the check from a standalone Makefile rule to a perl script. Platform-specific checks are now part of the script, avoiding most of the duplication created by the introduction of this check for meson, but not all of them: - Solaris and Windows skipped in the script. - Whitelist of symbols is in the script. - nm availability, with its path given as an option of the script. Its execution is checked in the script. - Check is disabled if coverage reports are enabled. This part is not pushed down to the script. - Check is disabled for static builds of libpq. This part is filtered out in each build script. A trick is required for the stamp file, in the shape of an optional argument that can be given to the script. Meson expects the stamp in output and uses this argument, generating the stamp file in the script. Meson is able to handle the removal of the stamp file internally when libpq needs to be rebuilt and the check done again. This refactoring piece has come up while discussing the addition of more items in the symbols considered as acceptable. This sanity check has never been run by meson since its introduction in dc227eb82ea8, so it is possible that this fails in some of the buildfarm members. At least the CI is happy with it, but let's see how it goes. Author: Nazir Bilal Yavuz Co-authored-by: VASUKI M Reviewed-by: Daniel Gustafsson Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/19095-6d8256d0c37d4be2@postgresql.org --- configure | 42 +++++++++++++ configure.ac | 2 + meson.build | 1 + src/Makefile.global.in | 1 + src/interfaces/libpq/Makefile | 23 ++------ src/interfaces/libpq/libpq_check.pl | 91 +++++++++++++++++++++++++++++ src/interfaces/libpq/meson.build | 18 ++++++ 7 files changed, 161 insertions(+), 17 deletions(-) create mode 100755 src/interfaces/libpq/libpq_check.pl diff --git a/configure b/configure index 3a0ed11fa8e..ec35de5ba65 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 c2413720a18..7284f1ff622 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 6e7ddd74683..622598546ae 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 0aa389bc710..371cd7eba2c 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 da6650066d4..9fe321147fc 100644 --- a/src/interfaces/libpq/Makefile +++ b/src/interfaces/libpq/Makefile @@ -129,25 +129,14 @@ $(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. 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 -endif + $(PERL) $(srcdir)/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 00000000000..835638cc5a2 --- /dev/null +++ b/src/interfaces/libpq/libpq_check.pl @@ -0,0 +1,91 @@ +#!/usr/bin/perl +# +# src/interfaces/libpq/libpq_check.pl +# +# Copyright (c) 2025, PostgreSQL Global Development Group +# +# Check 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. + + # The exclusion of __cxa_atexit is necessary on OpenBSD, which seems + # to insert references to that even in pure C code. + next if /__cxa_atexit/; + + # Excluding __tsan_func_exit is necessary when using ThreadSanitizer data + # race detector which uses this function for instrumentation of function + # exit. + 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 a74e885b169..b259c998fa2 100644 --- a/src/interfaces/libpq/meson.build +++ b/src/interfaces/libpq/meson.build @@ -85,6 +85,24 @@ 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.39.5