MIME-Version: 1.0
References: <19095-6d8256d0c37d4be2@postgresql.org> <CAAh00ETwx8_AEM0wgoi-v875uC-FLuGGOMTaP9fCNdVw4Zq=Vg@mail.gmail.com>
 <2305435.1762929527@sss.pgh.pa.us> <7C7B32ED-5742-49D5-9380-5E02AA5B03C4@yesql.se>
 <aRRCE_2ydBfCnO7Q@paquier.xyz> <057A2AED-0116-446B-8986-57FDA70CA5A3@yesql.se>
 <CAN55FZ2Wqv+maXTfNO6+y=5400w7hCMo6RQhasqp=nrsODSBpg@mail.gmail.com>
 <CAN55FZ2em+Wf6kWQ27bwQrLTTWJF7TcSDDKnjxGy+fxMVaGbvg@mail.gmail.com>
 <aSTRj99MkyI5zr0y@paquier.xyz> <CACTYHzhCNsee-4xv-HDhw8zmMWByB7YHLuUHh-bsztgZ94ewpw@mail.gmail.com>
In-Reply-To: <CACTYHzhCNsee-4xv-HDhw8zmMWByB7YHLuUHh-bsztgZ94ewpw@mail.gmail.com>
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Tue, 25 Nov 2025 13:07:24 +0300
Message-ID: <CAN55FZ04W6-wpC-7nwOKSm8Q5AQ5q52+S81sukN4oaUwsyHPYA@mail.gmail.com>
Subject: Re: BUG #19095: Test if function exit() is used fail when linked static
To: VASUKI M <vasukim1992002@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Daniel Gustafsson <daniel@yesql.se>, Tom Lane <tgl@sss.pgh.pa.us>, 
	BharatDB <bharatdbpg@gmail.com>, torsten.rupp@gmx.net, 
	pgsql-bugs@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CAN55FZ04W6-wpC-7nwOKSm8Q5AQ5q52%2BS81sukN4oaUwsyHPYA%40mail.gmail.com>
Precedence: bulk

Hi,

On Tue, 25 Nov 2025 at 12:11, VASUKI M <vasukim1992002@gmail.com> wrote:
>
> On Tue, 25 Nov 2025 at 03:14, Michael Paquier <michael@paquier.xyz> 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