public inbox for [email protected]  
help / color / mirror / Atom feed
From: BharatDB <[email protected]>
To: Andres Freund <[email protected]>
Cc: 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 14:38:37 +0530
Message-ID: <CAAh00ETETkxbHmFff+2j4Lj6bbr5fWzMy=rhTg-yg4CzD=HiNA@mail.gmail.com> (raw)
In-Reply-To: <2wdijz5mucqx33vqzyep44uyucauw33egvwa7cbociesqlks5x@yj63saikpzwu>
References: <[email protected]>
	<CAAh00ETwx8_AEM0wgoi-v875uC-FLuGGOMTaP9fCNdVw4Zq=Vg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<2wdijz5mucqx33vqzyep44uyucauw33egvwa7cbociesqlks5x@yj63saikpzwu>

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-exit-check-function-for-meson-and-Makefile.patch (5.9K, 3-0001-libpq-exit-check-function-for-meson-and-Makefile.patch)
  download | inline diff:
From f711555fd2b69e74a33312f5ba8750b8fec97f1f Mon Sep 17 00:00:00 2001
From: BharatDBPG <[email protected]>
Date: Wed, 19 Nov 2025 12:17:43 +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.

Signed-off-by: Vasuki[BharatDBPG] <[email protected]>
---
 meson.build                      |  1 +
 src/interfaces/libpq/Makefile    |  7 ++--
 src/interfaces/libpq/meson.build | 70 +++++++++++++++-----------------
 3 files changed, 37 insertions(+), 41 deletions(-)

diff --git a/meson.build b/meson.build
index 24aeffe..e20bfe6 100644
--- a/meson.build
+++ b/meson.build
@@ -3812,6 +3812,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 3bd0e4f..1ad3c60 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -134,9 +134,10 @@ $(stlib): $(OBJS_STATIC)
 # 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.
+# which seems to insert references to that even in pure C code.  Excluding
+# __tsan_func_exit is necessary when using ThreadSanitizer, which emits this
+# symbol as part of its instrumentation of function exits.  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
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index 4b1e8a2..4822814 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -1,5 +1,4 @@
 # Copyright (c) 2022-2025, PostgreSQL Global Development Group
-
 libpq_sources = files(
   'fe-auth-oauth.c',
   'fe-auth-scram.c',
@@ -85,6 +84,38 @@ 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,
@@ -147,41 +178,4 @@ tests += {
   },
 }
 
-# Verify that libpq does not reference functions that may invoke exit().
-#
-# This check parallels the Makefile logic used in the autoconf build system.
-# 
-# The following symbols are considered safe and therefore ignored:
-#   - __cxa_atexit      : used for C++ static destructors
-#   - __tsan_func_exit  : thread sanitizer instrumentation
-#   - pthread_exit      : used by thread runtimes, harmless here
-#
-# The test runs only for native builds (not cross-builds) and when code
-# coverage is disable
-
-if not meson.is_cross_build() and not get_option('b_coverage') and host_system != 'sunos'
-  check_exit_target = custom_target(
-    'check-libpq-no-exit',
-    output: 'libpq-refs-stamp',
-    depends: libpq_so,
-    build_by_default: true,
-    command: [
-      'bash', '-c',
-      '''
-      echo "Running exit() reference check for libpq..."
-      if nm -A -u @0@ 2>/dev/null | \
-         grep -v -e __cxa_atexit -e __tsan_func_exit -e pthread_exit | \
-         grep exit; then
-         echo "ERROR: libpq must not be calling any function which invokes exit()"
-         exit 1
-      else
-         echo "SUCCESS: No exit() references found in libpq"
-      fi
-      '''.format(libpq_so.full_path())
-    ],
-    build_always_stale: true,
-    capture: false
-  )
-endif
-
 subdir('po', if_found: libintl)
-- 
2.43.0



  [image/png] Screenshot from 2025-11-18 17-57-16.png (83.0K, 4-Screenshot%20from%202025-11-18%2017-57-16.png)
  download | view image

  [image/png] Screenshot from 2025-11-18 17-59-06.png (21.8K, 5-Screenshot%20from%202025-11-18%2017-59-06.png)
  download | view image

view thread (13+ messages)  latest in thread

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: <CAAh00ETETkxbHmFff+2j4Lj6bbr5fWzMy=rhTg-yg4CzD=HiNA@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