In libnbd we use sys_errlist[] to make a perror function that is safe to call after fork and before exec:
https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...
It looks as if glibc removed this list, which is annoying. Two questions:
(1) How can we write a fork-safe perror function?
(2) Why is the configure not working (it detects that sys_errlist is still available)?
https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...
Rich.
* Richard W. M. Jones:
In libnbd we use sys_errlist[] to make a perror function that is safe to call after fork and before exec:
https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...
It looks as if glibc removed this list, which is annoying. Two questions:
(1) How can we write a fork-safe perror function?
From the glibc NEWS file:
* The functions strerrorname_np and strerrordesc_np have been added. The strerroname_np returns error number name (e.g. "EINVAL" for EINVAL) while strerrordesc_np returns string describing error number (e.g "Invalid argument" for EINVAL). Different than strerror, strerrordesc_np does not attempt to translate the return description, both functions return NULL for an invalid error number.
(2) Why is the configure not working (it detects that sys_errlist is still available)?
https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...
The test is broken (two type mismatches), but linking still fails for me in rawhide.
Thanks, Florian
On Fri, Jul 17, 2020 at 01:13:00PM +0200, Florian Weimer wrote:
- Richard W. M. Jones:
In libnbd we use sys_errlist[] to make a perror function that is safe to call after fork and before exec:
https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...
It looks as if glibc removed this list, which is annoying. Two questions:
(1) How can we write a fork-safe perror function?
From the glibc NEWS file:
- The functions strerrorname_np and strerrordesc_np have been added. The strerroname_np returns error number name (e.g. "EINVAL" for EINVAL) while strerrordesc_np returns string describing error number (e.g "Invalid argument" for EINVAL). Different than strerror, strerrordesc_np does not attempt to translate the return description, both functions return NULL for an invalid error number.
I see these defined in <string.h>. Are they async-safe (ie. safe after fork and before exec)? If so we can use this as an alternative.
(2) Why is the configure not working (it detects that sys_errlist is still available)?
https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...
The test is broken (two type mismatches), but linking still fails for me in rawhide.
[Type mismatches are intentional - see how other autoconf tests are implemented]. Well, it works for me in Rawhide :-(
configure:12994: checking for sys_errlist configure:13007: gcc -o conftest -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld conftest.c >&5 conftest.c: In function 'main': conftest.c:40:35: warning: initialization of 'char *' from incompatible pointer type 'int *' [-Wincompatible-pointer-types] 40 | extern int sys_errlist; char *p = &sys_errlist; | ^ conftest.c:40:31: warning: unused variable 'p' [-Wunused-variable] 40 | extern int sys_errlist; char *p = &sys_errlist; | ^ configure:13007: $? = 0 configure:13012: result: yes
This is with glibc-2.31.9000-19.fc33.x86_64. I can see that sys_errlist is a symbol still in /lib64/libc.so.
If strerrordesc_np etc are async-safe then we can prefer those in the configure script.
Rich.
* Richard W. M. Jones:
On Fri, Jul 17, 2020 at 01:13:00PM +0200, Florian Weimer wrote:
- Richard W. M. Jones:
In libnbd we use sys_errlist[] to make a perror function that is safe to call after fork and before exec:
https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...
It looks as if glibc removed this list, which is annoying. Two questions:
(1) How can we write a fork-safe perror function?
From the glibc NEWS file:
- The functions strerrorname_np and strerrordesc_np have been added. The strerroname_np returns error number name (e.g. "EINVAL" for EINVAL) while strerrordesc_np returns string describing error number (e.g "Invalid argument" for EINVAL). Different than strerror, strerrordesc_np does not attempt to translate the return description, both functions return NULL for an invalid error number.
I see these defined in <string.h>. Are they async-safe (ie. safe after fork and before exec)? If so we can use this as an alternative.
Yes, although the manual doesn't really say so.
(2) Why is the configure not working (it detects that sys_errlist is still available)?
https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...
The test is broken (two type mismatches), but linking still fails for me in rawhide.
[Type mismatches are intentional - see how other autoconf tests are implemented]. Well, it works for me in Rawhide :-(
configure:12994: checking for sys_errlist configure:13007: gcc -o conftest -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld conftest.c >&5 conftest.c: In function 'main': conftest.c:40:35: warning: initialization of 'char *' from incompatible pointer type 'int *' [-Wincompatible-pointer-types] 40 | extern int sys_errlist; char *p = &sys_errlist; | ^ conftest.c:40:31: warning: unused variable 'p' [-Wunused-variable] 40 | extern int sys_errlist; char *p = &sys_errlist; | ^ configure:13007: $? = 0 configure:13012: result: yes
This is with glibc-2.31.9000-19.fc33.x86_64. I can see that sys_errlist is a symbol still in /lib64/libc.so.
Yes, but it's a compat symbol, so not available for linking.
What is the test file autoconf feeds to the compiler?
Thanks, Florian
On Fri, Jul 17, 2020 at 01:31:25PM +0200, Florian Weimer wrote:
- Richard W. M. Jones:
I see these defined in <string.h>. Are they async-safe (ie. safe after fork and before exec)? If so we can use this as an alternative.
Yes, although the manual doesn't really say so.
I've implemented this:
https://github.com/libguestfs/libnbd/commit/814e597c4ed49ca59b8f24a5f16a4874...
which appears to work as far back as RHEL 7 and also in Rawhide.
This is with glibc-2.31.9000-19.fc33.x86_64. I can see that sys_errlist is a symbol still in /lib64/libc.so.
Yes, but it's a compat symbol, so not available for linking.
What is the test file autoconf feeds to the compiler?
I believe the test file is actually just
extern int sys_errlist; char *p = &sys_errlist;
(see https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...)
However it doesn't really matter now since we ignore the outcome of this test if the new function is available.
Thanks,
Rich.
* Richard W. M. Jones:
This is with glibc-2.31.9000-19.fc33.x86_64. I can see that sys_errlist is a symbol still in /lib64/libc.so.
Yes, but it's a compat symbol, so not available for linking.
What is the test file autoconf feeds to the compiler?
I believe the test file is actually just
extern int sys_errlist; char *p = &sys_errlist;
(see https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...)
That can't work because there's no main function, so linking that would always fail.
I suspect that the fragment lands in the body of main, where the reference to sys_errlist is simply optimized away.
Thanks, Florian
On Fri, Jul 17, 2020 at 01:43:12PM +0200, Florian Weimer wrote:
- Richard W. M. Jones:
This is with glibc-2.31.9000-19.fc33.x86_64. I can see that sys_errlist is a symbol still in /lib64/libc.so.
Yes, but it's a compat symbol, so not available for linking.
What is the test file autoconf feeds to the compiler?
I believe the test file is actually just
extern int sys_errlist; char *p = &sys_errlist;
(see https://github.com/libguestfs/libnbd/blob/a6ba108309d9250e5f7fe0a5d41a996b7a...)
That can't work because there's no main function, so linking that would always fail.
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 ...
---------------------------------------------------------------------- /* confdefs.h */ #define PACKAGE_NAME "libnbd" #define PACKAGE_TARNAME "libnbd" #define PACKAGE_VERSION "1.3.8" #define PACKAGE_STRING "libnbd 1.3.8" #define PACKAGE_BUGREPORT "" #define PACKAGE_URL "" #define STDC_HEADERS 1 #define HAVE_SYS_TYPES_H 1 #define HAVE_SYS_STAT_H 1 #define HAVE_STDLIB_H 1 #define HAVE_STRING_H 1 #define HAVE_MEMORY_H 1 #define HAVE_STRINGS_H 1 #define HAVE_INTTYPES_H 1 #define HAVE_STDINT_H 1 #define HAVE_UNISTD_H 1 #define __EXTENSIONS__ 1 #define _ALL_SOURCE 1 #define _GNU_SOURCE 1 #define _POSIX_PTHREAD_SEMANTICS 1 #define _TANDEM_SOURCE 1 #define PACKAGE "libnbd" #define VERSION "1.3.8" #define HAVE_DLFCN_H 1 #define LT_OBJDIR ".libs/" #define PROTOTYPES 1 #define __PROTOTYPES 1 #define HAVE_PTHREAD_PRIO_INHERIT 1 #define HAVE_PTHREAD 1 #define HAVE_BYTESWAP_H 1 #define HAVE_ENDIAN_H 1 #define HAVE_STDATOMIC_H 1 #define HAVE_LINUX_VM_SOCKETS_H 1 #define HAVE_STRERRORDESC_NP 1 /* end confdefs.h. */
int main () { extern int sys_errlist; char *p = &sys_errlist; ; return 0; } ----------------------------------------------------------------------
Rich.
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). Perhaps enhancing the test to do:
char *p = &sys_errlist; return p[0];
bypasses the chance for link optimization?
But as pointed out elsewhere, if another test for new functions succeeds, getting this test correct is less important.
* 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.
Thanks, Florian
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@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/Generic... --- 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
On 7/17/20 10:40 AM, Richard W.M. Jones wrote:
Here's my proposed patch:
From 3907eae3e14087d741b1b1b10b3c42e9cb9b55a3 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" rjones@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:
Do you mind if I clone some of this into here? https://sourceware.org/glibc/wiki/Release/2.32#Packaging_Changes
The release notes try to collect packaging changes and suggestions affect downstream.
On Fri, Jul 17, 2020 at 11:03:59AM -0400, Carlos O'Donell wrote:
On 7/17/20 10:40 AM, Richard W.M. Jones wrote:
Here's my proposed patch:
From 3907eae3e14087d741b1b1b10b3c42e9cb9b55a3 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" rjones@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:
Do you mind if I clone some of this into here? https://sourceware.org/glibc/wiki/Release/2.32#Packaging_Changes
The release notes try to collect packaging changes and suggestions affect downstream.
Go ahead, no problem.
Rich.
On 7/17/20 10:24 AM, Richard W.M. Jones wrote:
Do you mind if I clone some of this into here? https://sourceware.org/glibc/wiki/Release/2.32#Packaging_Changes
The release notes try to collect packaging changes and suggestions affect downstream.
Go ahead, no problem.
While at it, fix the typos in the headings for 2.1 and 2.2: s/Deprectation/Deprecation of/