Make dl library dependency

[dgreatwood]

Fix for issue #1230

libdl may be required for function dladdr, used in logStackTrace.
Note: If 'dl' is not available, per Meson the dladdr functionality is provided by libc. So if 'dl' is not available, we assume 'dl' is not required.

Added in meson.build:
deps_libpistache += dependency('dl', required: false)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.24%. Comparing base (a450d5c) to head (eb3a556).
Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1245      +/-   ##
==========================================
- Coverage   77.97%   76.24%   -1.73%     
==========================================
  Files          54       57       +3     
  Lines        6883     9801    +2918     
==========================================
+ Hits         5367     7473    +2106     
- Misses       1516     2328     +812     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Tachi107]

Note that this will only work when users use Meson 0.62.0 or newer, while we only require Meson 0.53

[Tachi107]

This is because dependency() in Meson can do different things when looking for a dependency "foo":

• search for a "foo" dependency overridden with meson.override_dependency('foo', foo_dep)
• search for a foo.pc file and use it
• search for FooConfig.cmake files and use them
• use a custom dependency resolution method if defined for the "foo" dependency

In our case:

• nobody calls meson.override_dependency('dl')
• no dl.pc file exists
• of course no DlConfig.cmake files exist either
• Meson defines a custom dependency resolution method for "dl", but this was added only in Meson 0.62.0

Until we'll require 0.62.0, we should do this instead:

deps_libpistache += compiler.find_library('dl', required: false)

And when we can require 0.62.0,

deps_libpistache += dependency('dl')

(note how required: false is absent, since we always require libdl's functionality. Meson will take care of figuring out when dl is provided by libc and won't fail when libdl doesn't exist)

[Tachi107]

See the Meson docs:

• https://mesonbuild.com/Reference-manual_functions.html#dependency
• https://mesonbuild.com/Dependencies.html#dependency-detection-method

[dgreatwood]

Hi -

Although I agree there is a counter argument, I was reluctant to dispense with the "required: false" for
deps_libpistache += dependency('dl')
for fear meson would demand the presence of libdl in cases where we don't need it for real - noting that i) we almost never need it actually, and ii) we use only one potential-libdl function, and meson has no way of knowing which function we need (AFAIK) when determining if we need libdl. In other words, having "required: false" seems the more cautious path vs. breaking a build that might otherwise work.

Regarding meson version, could we / should we condition on the meson version like:

if meson.version().version_compare('>=0.62.0')
        deps_libpistache += dependency('dl', required: false)
else
    	deps_libpistache += compiler.find_library('dl', required: false)
endif

WDYT?

[Tachi107]

Although I agree there is a counter argument, I was reluctant to dispense with the "required: false" for deps_libpistache += dependency('dl') for fear meson would demand the presence of libdl in cases where we don't need it for real - noting that i) we almost never need it actually

We can find a solution which always works, see below :)

, and ii) we use only one potential-libdl function, and meson has no way of knowing which function we need (AFAIK) when determining if we need libdl.

Of course it has! You can check if you have the potential-libdl function (which is it?) without linking to libdl, and then add libdl if needed:

has_func = compiler.has_function('potential_libdl')
if not has_func
  libdl_dep += dependency('dl')
  has_func = compiler.has_function('potential_libdl', dependencies: libdl_dep)
  if not has_func
    error('Unable to find potential_libdl(), even when linking to libdl')
  endif
  libpistache_deps += libdl_dep
endif

In other words, having "required: false" seems the more cautious path vs. breaking a build that might otherwise work.

Regarding meson version, could we / should we condition on the meson version like:

if meson.version().version_compare('>=0.62.0')
        deps_libpistache += dependency('dl', required: false)
else
    	deps_libpistache += compiler.find_library('dl', required: false)
endif

WDYT?

The only reason why dependency('dl') exists is to be able to always require that dl functionality is available even when it is part of libc and no separate libdl exists. If we don't always require dl functionality, we might as well always use compiler.find_library('dl', required: false) instead.

In other words:

code	meaning
dependency('dl')	always require dl functionality, whether it's part of libc or in a separate libdl object
compiler.find_library('dl')	always look for a separate libdl object
compiler.find_library('dl', required: false)	link to libdl, only if found
dependency('dl', required: false)	same as above, but requires meson 0.62.0

[eli-schwartz]

for fear meson would demand the presence of libdl in cases where we don't need it for real - noting that i) we almost never need it actually, and ii) we use only one potential-libdl function, and meson has no way of knowing which function we need (AFAIK) when determining if we need libdl. In other words, having "required: false" seems the more cautious path vs. breaking a build that might otherwise work.

As a matter of curiosity -- meson uses the presence or absence of dlopen (after #include <dlfcn.h>) as a sentinel for this use case. Both dlopen and dladdr are defined by POSIX, in Issue 8 (same header for both). Issue 7 only defined dlopen and only included it in XSI.

The meson check can only be incorrect for your use case if dlopen requires -ldl, but dladdr does not. Is this a realistic concern? Are there any systems out there that implement such?

[dgreatwood]

Hi -

As per PR #1252 (and per your comment there), unfortunately the ubi-8 case (and associated meson behaviour) prevents us, as a practical matter, from using has_function for determining if we need to pull in the dl library.

As to whether there are any systems where meson would wrongly determine that the dl library was required, that's kind of impossible to determine for certain in advance. The approach taken here instead seems the lowest risk; it would only fail if dladdr was not supplied by libc and either libdl was not available or libdl did not supply dladdr. Which is a very unlikely combination.

Thanks.
P.S. I have a dim recollection that there was a version of BSD that exposed this issue in some form, but I can't swear to it.

[dgreatwood]

Ah. Under meson, compiler object - "has_function(funcname): returns true if the given function is provided by the standard library..." So yes I agree this looks even better. I'll make a PR (and double check). Thanks!

…

On Sun, Sep 22, 2024 at 4:06 PM Andrea Pappacoda ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In meson.build <#1245 (comment)>: > @@ -148,6 +148,14 @@ if host_machine.system().endswith('bsd') deps_libpistache += libexecinfo_dep endif +# 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. +# Note: If 'dl' is not available, per Meson it suggests that the +# functionality is provided by libc +deps_libpistache += dependency('dl', required: false) Although I agree there is a counter argument, I was reluctant to dispense with the "required: false" for deps_libpistache += dependency('dl') for fear meson would demand the presence of libdl in cases where we don't need it for real - noting that i) we almost never need it actually We can find a solution which always works, see below :) , and ii) we use only one potential-libdl function, and meson has no way of knowing which function we need (AFAIK) when determining if we need libdl. Of course it has! You can check if you have the potential-libdl function (which is it?) without linking to libdl, and then add libdl if needed: has_func = compiler.has_function('potential_libdl')if not has_func libdl_dep += dependency('dl') has_func = compiler.has_function('potential_libdl', dependencies: libdl_dep) if not has_func error('Unable to find potential_libdl(), even when linking to libdl') endif libpistache_deps += libdl_dependif In other words, having "required: false" seems the more cautious path vs. breaking a build that might otherwise work. Regarding meson version, could we / should we condition on the meson version like: if meson.version().version_compare('>=0.62.0') deps_libpistache += dependency('dl', required: false) else deps_libpistache += compiler.find_library('dl', required: false) endif WDYT? The only reason why dependency('dl') exists is to be able to always require that dl functionality is available even when it is part of libc and no separate libdl exists. If we don't always require dl functionality, we might as well always use compiler.find_library('dl', required: false) instead. In other words: code meaning dependency('dl') always require dl functionality, whether it's part of libc or in a separate libdl object compiler.find_library('dl') always look for a separate libdl object compiler.find_library('dl', required: false) link to libdl, only if found dependency('dl', required: false) same as above, but requires meson 0.62.0 — Reply to this email directly, view it on GitHub <#1245 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFMA22G6ZO6JZ4V3WPHG4DZX5EP3AVCNFSM6AAAAABORGMCASVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMRQHEZTSOJTGA> . You are receiving this because you were assigned.Message ID: ***@***.***>

-- NOTICE: This email and its attachments may contain privileged and confidential information, only for the viewing and use of the intended recipient. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, acting upon, or use of the information contained in this email and its attachments is strictly prohibited and that this email and its attachments must be immediately returned to the sender and deleted from your system. If you received this email erroneously, please notify the sender immediately.  Xage Security, Inc. and its affiliates will never request personal information (e.g., passwords, Social Security numbers) via email.  Report suspicious emails to ***@***.*** ***@***.***>