| From: | Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> |
|---|---|
| To: | VASUKI M <vasukim1992002(at)gmail(dot)com> |
| Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, BharatDB <bharatdbpg(at)gmail(dot)com>, torsten(dot)rupp(at)gmx(dot)net, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: BUG #19095: Test if function exit() is used fail when linked static |
| Date: | 2025-11-25 10:07:24 |
| Message-ID: | CAN55FZ04W6-wpC-7nwOKSm8Q5AQ5q52+S81sukN4oaUwsyHPYA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs pgsql-hackers |
Hi,
On Tue, 25 Nov 2025 at 12:11, VASUKI M <vasukim1992002(at)gmail(dot)com> wrote:
>
> On Tue, 25 Nov 2025 at 03:14, Michael Paquier <michael(at)paquier(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Daniel Gustafsson | 2025-11-25 10:51:55 | Re: BUG #19095: Test if function exit() is used fail when linked static |
| Previous Message | VASUKI M | 2025-11-25 09:11:32 | Re: BUG #19095: Test if function exit() is used fail when linked static |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Heikki Linnakangas | 2025-11-25 10:07:27 | Re: POC: make mxidoff 64 bits |
| Previous Message | Michael Paquier | 2025-11-25 10:06:09 | Re: Extended Statistics set/restore/clear functions. |