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

problems with libm detection for ceil() #72

Closed
sthen opened this issue Feb 21, 2020 · 4 comments
Closed

problems with libm detection for ceil() #72

sthen opened this issue Feb 21, 2020 · 4 comments

Comments

@sthen
Copy link
Contributor

sthen commented Feb 21, 2020

7497e2e tries to detect if libm is required for ceil() (as used in snmplib/keytools.c if configure is called with --enable-blumenthal-aes), but there are some problems.

  • An earlier check for exp() - "diskIOLAx requires exp which may require -lm" in configure.d/config_os_libs1 leaves -lm in LIBS which is added to other checks including the one for ceil, so if exp requires -lm then you always get "checking for ceil without -lm... yes".

  • The ceil() check used is a link check for code calling "ceil((double)2.4)". Some compilers (at least I see it on clang 8.0.1 on OpenBSD) optimize away ceil with a fixed constant so you also always get "checking for ceil without -lm... yes" with such a compiler.

  • Even when it's detected as needed for ceil(), -lm is only added to LIBS not LNETSNMPLIBS so there's an unresolved symbol in the library. (Tools do pull in libm so work ok, but other software linking the library fails).

In my case -lm is already added to LIBS as needed from the abs() check and I can add the library linkage with AC_CHECK_LIB(m, ceil, [LNETSNMPLIBS="$LNETSNMPLIBS -lm"]) in configure.d/config_os_libs2 but it probably needs something smarter in the more general case.

bvanassche added a commit to bvanassche/net-snmp that referenced this issue Feb 22, 2020
See also net-snmp#72.

Reported-by: Stuart Henderson
Fixes: 7497e2e ("check if libm is needed for ceil function") # v5.8.pre2
bvanassche added a commit that referenced this issue Feb 22, 2020
See also #72.

Reported-by: Stuart Henderson
Fixes: 7497e2e ("check if libm is needed for ceil function") # v5.8.pre2
@bvanassche
Copy link
Contributor

Please retest with the latest version of the master branch.

@sthen
Copy link
Contributor Author

sthen commented Feb 22, 2020

Thanks Bart, it mostly helps but -lm is still present in LIBS from the exp() test in config_os_libs1.

configure:25205: checking for library containing ceil
configure:25233: cc -o conftest -DNETSNMP_ENABLE_IPV6 -fno-strict-aliasing -DNETSNMP_REMOVE_U64 -O2 -pipe -g -Uopenbsd6 -Dopenbsd6=openbsd6   conftest.c   -lm  >&5
conftest.c:233:6: warning: incompatible redeclaration of library function 'ceil' [-Wincompatible-library-redeclaration]
char ceil ();
     ^
conftest.c:233:6: note: 'ceil' is a builtin with type 'double (double)'
1 warning generated.
configure:25233: $? = 0
configure:25269: result: none required

I guess the simplest fix is to save/clear libs when running the ceil test? With this change it works for me:

+netsnmp_save_LIBS="$LIBS"
+LIBS=
 NETSNMP_SEARCH_LIBS(ceil, m,,,, LNETSNMPLIBS)
+LIBS="$netsnmp_save_LIBS"

bvanassche added a commit to bvanassche/net-snmp that referenced this issue Feb 22, 2020
This change allows the ceil() test to add -lm to LNETSNMPLIBS.

This patch is a follow-up for commit d63e35a ("configure: Add -lm to
LNETSNMPLIBS if ceil() exists in libm").

See also net-snmp#72.
@bvanassche
Copy link
Contributor

An additional fix has been checked in on the v5.8 and master branches. Please retest.

@sthen
Copy link
Contributor Author

sthen commented Feb 22, 2020

Many thanks, that works nicely here.

@sthen sthen closed this as completed Feb 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants