On Fri, Jul 17, 2020 at 04:23:28PM +0200, Florian Weimer wrote:
* Eric Blake:
> On 7/17/20 6:48 AM, Richard W.M. Jones wrote:
>
>>> I suspect that the fragment lands in the body of main, where the
>>> reference to sys_errlist is simply optimized away.
>>
>> Very true. Let's see what the test file actually contains
>> by adding an exit into the configure script ...
>
>> int
>> main ()
>> {
>> extern int sys_errlist; char *p = &sys_errlist;
>> ;
>> return 0;
>
> Indeed, this could be a case of link-time optimization (since p is not
> used, the resolution of &sys_errlist does not matter).
Link-time optimization is not necessary because it's a local variable.
> Perhaps enhancing the test to do:
>
> char *p = &sys_errlist;
> return p[0];
>
> bypasses the chance for link optimization?
Not really, since the type of sys_errlist is wrong. If LTO detects
that, the compiler should probably turn the reference into a trap, and
then it's gone as well.
In any case I think this test should probably be using AC_CHECK_DECLS
instead of an open-coded test.
Here's my proposed patch:
From 3907eae3e14087d741b1b1b10b3c42e9cb9b55a3 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones(a)redhat.com>
Date: Fri, 17 Jul 2020 15:36:24 +0100
Subject: [PATCH] configure: Use AC_CHECK_DECLS to check for sys_errlist.
On Fedora 33 (glibc 2.31.9000):
checking for strerrordesc_np... yes
checking whether sys_errlist is declared... no
On RHEL 8 (glibc-2.28):
checking for strerrordesc_np... no
checking whether sys_errlist is declared... yes
On RHEL 7 (glibc-2.17):
checking for strerrordesc_np... no
checking whether sys_errlist is declared... yes
Note that we mustn't use #ifdef when testing HAVE_SYS_ERRLIST. This
is because AC_CHECK_DECLS works differently from the other macros, as
described here:
https://www.gnu.org/software/autoconf/manual/autoconf-2.65/html_node/Gene...
---
configure.ac | 9 +--------
lib/utils.c | 2 +-
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index a44e3b1..99bddc0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -85,14 +85,7 @@ dnl
https://lists.fedoraproject.org/archives/list/glibc@lists.fedoraproject.org/
AC_CHECK_FUNCS([strerrordesc_np])
dnl Check for sys_errlist (optional).
-AC_MSG_CHECKING([for sys_errlist])
-AC_TRY_LINK([], [extern int sys_errlist; char *p = &sys_errlist;], [
- AC_DEFINE([HAVE_SYS_ERRLIST], [1],
- [Define if libc has a sys_errlist variable.])
- AC_MSG_RESULT([yes])
-], [
- AC_MSG_RESULT([no])
-])
+AC_CHECK_DECLS([sys_errlist])
dnl Check for GnuTLS (optional, for TLS support).
AC_ARG_WITH([gnutls],
diff --git a/lib/utils.c b/lib/utils.c
index 86e9849..06b9b85 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -156,7 +156,7 @@ nbd_internal_fork_safe_perror (const char *s)
#ifdef HAVE_STRERRORDESC_NP
m = strerrordesc_np (errno);
#else
-#ifdef HAVE_SYS_ERRLIST
+#if HAVE_SYS_ERRLIST /* NB Don't use #ifdef */
m = errno >= 0 && errno < sys_nerr ? sys_errlist[errno] : NULL;
#endif
#endif
--
2.27.0
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages.
http://libguestfs.org