Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

meson: check for availability of dladdr #1286

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

eli-schwartz
Copy link
Contributor

Delete erroneous comment claiming that compiler.has_function is broken. This is based on a misunderstanding of the original logfile. See: #1252 (comment)

Checking for dladdr works fine. Checking for dependency('dl') on versions of meson that are too old to support it does not work, and this is what happened on RedHat ubi-8.

/cc @dgreatwood

@eli-schwartz eli-schwartz force-pushed the dladdr-meson branch 5 times, most recently from da93507 to 7858c46 Compare January 22, 2025 02:39
@kiplingw kiplingw requested a review from dgreatwood January 22, 2025 02:57
Delete erroneous comment claiming that compiler.has_function is broken.
This is based on a misunderstanding of the original logfile.
See: pistacheio#1252 (comment)

Checking for dladdr works fine. Checking for `dependency('dl')` on
versions of meson that are too old to support it does not work, and this
is what happened on RedHat ubi-8.
@dgreatwood
Copy link
Collaborator

@eli-schwartz - thanks for cogent analysis and update.

I had just one remaining question. Would it be OK to guard this new meson.build code with an initial test of whether dladdr is already available? In particular, in the meson.version() < 0.62 case, if dladdr is already available, we are sending meson to look for library 'dl', and then, if library dl is found, we test it for dl_addr and add it as a dependency, none of which (presumably?) we'd actually need to do in that scenario. In other words it would look like:

if host_machine.system() != 'windows'
    if not compiler.has_function('dladdr')
        # libdl may be required for function dladdr, used in logStackTrace,
        # which is called by PS_LogWoBreak, used in turn by the stack-trace
        # logging macro PS_LOG_WO_BREAK_LIMITED and its derivatives. Issue
        # #1230.
        if meson.version().version_compare('>=0.62.0')
          dl_dep = dependency('dl')
        else
          dl_dep = compiler.find_library('dl', required: false)
        endif
        # Note: the Single Unix Specification version 2, issue 5 (1997) defines dlopen
        # but not dladdr. POSIX 2024 issue 8 requires dladdr. Older systems might
        # therefore lack what we need, even if libdl exists.
        #
        # Meson's "dl" check looks for dlopen. We assume that, if dladdr exists
        # at all, it is in the same DSO (whether it is libc or -ldl) as dlopen.
        # But it may not exist at all, so check to be sure.
        if not compiler.has_function('dladdr', prefix: '#include <dlfcn.h>', dependencies: dl_dep)
          error('dladdr could not be detected')
        endif
        deps_libpistache += dl_dep
    endif
endif

WDYT?

@eli-schwartz
Copy link
Contributor Author

I had just one remaining question. Would it be OK to guard this new meson.build code with an initial test of whether dladdr is already available? In particular, in the meson.version() < 0.62 case, if dladdr is already available, we are sending meson to look for library 'dl', and then, if library dl is found, we test it for dl_addr and add it as a dependency, none of which (presumably?) we'd actually need to do in that scenario.

On the flip side, in the >0.62 case we are now running 3 compile checks instead of 2.

Keep in mind: if dladdr is already available, then there will not be a separate "dl" library at all. On some systems, where there used to be one but isn't any longer (such as very recent Linux glibc systems), -ldl will add 4 bytes to the linker command line but resolves to /usr/lib64/libdl.a which is an empty archive containing no objects.

$ cat /usr/lib64/libdl.a 
!<arch>
$ ls /usr/lib64/libdl.*
/usr/lib64/libdl.a  /usr/lib64/libdl.so.2

You cannot link directly to the shared library since -ldl doesn't search for .so.2, past versions of glibc provided both a libdl.a that contained statically linkable object files and a libdl.so that was a symlink to libdl.so.2 (which contained a variety of symbols, but now is just a compatibility placeholder to avoid breaking existing compiled binaries with a DT_NEEDED entry for libdl.so.2)

There are no consequences for the resulting installable pistache and it's just there as a compatibility shim for users of old meson versions, so I figured it's easier to change less. That way once you bump the minimum supported meson version you can just delete the version check without rewriting the surrounding code.

@dgreatwood
Copy link
Collaborator

That way once you bump the minimum supported meson version you can just delete the version check without rewriting the surrounding code.

OK

@dgreatwood dgreatwood merged commit c1e0d76 into pistacheio:master Jan 22, 2025
134 of 135 checks passed
@Tachi107
Copy link
Member

Thanks for the fix, Eli!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants