Thread

  1. Re: BUG #19095: Test if function exit() is used fail when linked static

    Nazir Bilal Yavuz <byavuz81@gmail.com> — 2025-11-25T10:07:24Z

    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