public inbox for [email protected]  
help / color / mirror / Atom feed
From: BharatDB <[email protected]>
To: Andres Freund <[email protected]>
To: Tom Lane <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: VASUKI M <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #19095: Test if function exit() is used fail when linked static
Date: Wed, 19 Nov 2025 17:59:09 +0530
Message-ID: <CAAh00ERij397C5BXe=R6T38Ki7_G=0svdcFuTtz7eG-qs7vtuw@mail.gmail.com> (raw)
In-Reply-To: <CAAh00ETETkxbHmFff+2j4Lj6bbr5fWzMy=rhTg-yg4CzD=HiNA@mail.gmail.com>
References: <[email protected]>
	<CAAh00ETwx8_AEM0wgoi-v875uC-FLuGGOMTaP9fCNdVw4Zq=Vg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<2wdijz5mucqx33vqzyep44uyucauw33egvwa7cbociesqlks5x@yj63saikpzwu>
	<CAAh00ETETkxbHmFff+2j4Lj6bbr5fWzMy=rhTg-yg4CzD=HiNA@mail.gmail.com>

Sorry,for the wrong patch this is the correct one.kindly find the attached
patch for testing
I would love to hear the feedback from you committers.

regards,
Vasuki M
BharatDB,C-DAC Chennai.

On Wed, Nov 19, 2025 at 2:38 PM BharatDB <[email protected]> wrote:

> Hi Andres and hackers,
> I came up with the solution
> — short follow-up with what I changed and how the Meson check actually
> runs.
>
> Summary of what I did
>   - Added a Meson custom_target in src/interfaces/libpq/meson.build that
>     scans libpq's object files for direct exit() references.
>   - Added pthread_exit to the Makefile whitelist (the Makefile test
>     already existed; this just updates the whitelist).
>   -Added the custom target in the top level meson.build
>
> Where the Meson check lives and when it runs
>   - I put the custom_target in src/interfaces/libpq/meson.build,
>     immediately after the libpq shared/static library targets are
>     defined and after the declare_dependency for libpq.  That is the
>     correct location in the libpq build file.
>   - The custom_target declares `depends: [libpq_st, libpq_so]`.  This
>     is important: it tells Meson to build those targets first so the
>     .o files are present in the build directory before the check runs.
>     The check itself does not scan libpq_st or libpq_so; it scans
>     files in build/src/interfaces/libpq/*.o.
>   - The command uses `find <builddir>/src/interfaces/libpq -name '*.o'`
>     and runs `nm -u` on each .o, piping through `grep -v` for the
>     whitelisted names and finally checking for exit.
>   - The check is skipped for:
>       * cross-builds
>       * when coverage (b_coverage) is enabled
>       * on Windows (no nm/grep in the same form there) as Andrew questioned
>
> Why pthread_exit is whitelisted
>   - pthread_exit can legitimately appear in a few build/runtime
>     configurations (thread runtimes or link-time glue), and the
>     Makefile test was updated to whitelist it.  The Meson check has the
>     same whitelist so both build systems behave the same.
>   - Whitelisting pthread_exit doesn't remove the value of the test:
>     it only avoids false positives for legitimate thread shutdown code.
>     We still catch direct exit()/ _exit()/abort()-style calls as tom said.
>
> How to reproduce locally
>   - From repo root:
>       rm -rf build
>       meson setup build
>       cd build
>       ninja
>     The custom_target runs as part of the normal build and will fail
>     the build if any .o contains an un-whitelisted exit() reference.
>
> On Fri, Nov 14, 2025 at 7:31 PM Andres Freund <[email protected]> wrote:
>
> But more generally: If we allow pthread_exit(), what's the point of this
>> test?
>> That's one of the functions we better avoid calling, no?
>>
>  I agree it's worth questioning which functions we allow.  The
>  current choice (whitelist pthread_exit) mirrors the Makefile
>  behavior, avoids false positives, and keeps the test focused on
>  direct process-termination calls authored in our code.  If the
>  community prefers a stricter policy so we can adjust the whitelist.
>
>
>> ISTM that if we do want to continue having this test, the issue is that
>> we're
>> testing the shared library - which will have already linked against static
>> libraries like the sanitizer ones or in this case libcrypto. What we
>> ought to
>> do is to test the .o files constituting libpq.so, rather than the already
>> linked .so. That way we will find our own calls to exit etc, but not ones
>> in
>> static libraries.
>>
>
> TBH After so many test runs I have finalized
> Why scan .o files (not the final .so)?
>   - A shared library is usually linked with other static libraries
>     (libcrypto, sanitizer runtimes).  If we scan the final .so we'll
>     see references that originate in those static libraries and produce
>     false positives.
>   - If we scan the .o files that make up libpq, we only inspect our
>     own compilation units and will catch only exit() calls introduced
>     by our code.
>
> HTH! I attached the patch and also added the meson test output before
> /after of any file containing an 'exit' explicitly; it fails the
> build[ninja].
>
> Regards,
> Vasuki M
> BharatDB[CDAC chennai]
>


Attachments:

  [text/x-patch] 0001-libpq-Add-exit-function-check-for-Meson-build-and-wh.patch (4.4K, 3-0001-libpq-Add-exit-function-check-for-Meson-build-and-wh.patch)
  download | inline diff:
From f0679266bda765f4e1e15bfee97d2817d3bd78ea Mon Sep 17 00:00:00 2001
From: BharatDBPG <[email protected]>
Date: Wed, 19 Nov 2025 17:43:53 +0530
Subject: [PATCH] libpq: Add exit() function check for Meson build and
 whitelist pthread_exit()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The Makefile-based build already performs a safety check to ensure that
libpq does not accidentally reference exit() or related termination
functions.

Meson, however, did not run this check. As a result, Meson builds could
miss accidental references to exit()-family functions which ideally
should never be called inside libpq.

This patch adds the missing scan to the Meson build by:
  • Scanning the libpq .o files using nm and filtering through the same
    whitelist logic as the Makefile.
  • Adding pthread_exit() to the Meson whitelist, matching the behavior
    of the existing Makefile check.

With this change, both Makefile and Meson builds apply the same
validation for unwanted exit() usage.
---
 meson.build                      |  1 +
 src/interfaces/libpq/Makefile    |  5 +++--
 src/interfaces/libpq/meson.build | 30 ++++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index c1e17aa3040..47588ead9fb 100644
--- a/meson.build
+++ b/meson.build
@@ -3820,6 +3820,7 @@ alias_target('bin', bin_targets + [libpq_st])
 alias_target('pl', pl_targets)
 alias_target('contrib', contrib_targets)
 alias_target('testprep', testprep_targets)
+alias_target('run-check-libpq', [check_exit_target])
 
 alias_target('world', all_built, docs)
 alias_target('install-world', install_quiet, installdocs)
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index da6650066d4..7b20d84d51d 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -137,14 +137,15 @@ $(stlib): $(OBJS_STATIC)
 # 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.
+#Excluding pthread_exit allows legitimate thread shutdown paths used on some builds.
+#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 \
+	@if nm -A -u $< 2>/dev/null | grep -v -e __cxa_atexit -e __tsan_func_exit -e pthread_exit | grep exit; then \
 		echo 'libpq must not be calling any function which invokes exit'; exit 1; \
 	fi
 endif
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index a74e885b169..30324731e9c 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -85,6 +85,36 @@ libpq = declare_dependency(
   include_directories: [include_directories('.')]
 )
 
+# Sanity check to ensure libpq does not contain any unintended references
+# to exit() in its object files.  Client libraries must not terminate the
+# calling process, so we scan all libpq .o files with 'nm' and fail the
+# build if a direct exit() reference is found.  Certain harmless symbols
+# (__cxa_atexit, __tsan_func_exit, pthread_exit) are whitelisted.
+# Skip on cross-builds, sanitizer coverage, and Windows
+
+if not meson.is_cross_build() and not get_option('b_coverage') and host_system != 'windows'
+  check_exit_target = custom_target(
+    'check-libpq-no-exit',
+    output: 'libpq-nm-stamp',
+    depends: [libpq_st, libpq_so],
+    build_by_default: true,
+    command: [
+      'bash','-eu', '-c',
+      '''
+      echo "Checking that libpq object files do not reference exit()..."
+      obj_files=$(find src/interfaces/libpq -type f -name "*.o")
+      for f in $obj_files; do
+        if nm -u "$f" 2>/dev/null| grep -v -E '__cxa_atexit|__tsan_func_exit|pthread_exit'| grep exit; then
+          echo "ERROR: exit()-related reference found in: $f"
+          exit 1
+        fi
+      done
+      touch  @OUTPUT@
+      '''.format(meson.current_build_dir())
+    ],
+  )
+
+endif
 private_deps = [
   frontend_stlib_code,
   libpq_deps,
-- 
2.43.0



view thread (13+ messages)

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: <CAAh00ERij397C5BXe=R6T38Ki7_G=0svdcFuTtz7eG-qs7vtuw@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