[PATCH v2 2/5] signal: Move sys_siglist to a compat symbol

Carlos O'Donell carlos@redhat.com
Tue May 12 17:36:58 GMT 2020


On 5/7/20 10:23 AM, Adhemerval Zanella via Libc-alpha wrote:
> The symbol was deprecated by strsignal and its usage imposes issues
> such as copy relocations.
> 
> Its internal name is changed to __sys_siglist_internal to avoid static
> linking usage.  The compat code is also refactored, since both Linux
> and Hurd usage the same strategy: export the same array with different
> object sizes.
> 
> The libSegfault change avoids calling strsignal on the SIGFAULT signal
> handler (the current usage is already sketchy, adding a call that
> potentially issue locale internal function is even sketchier).
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu. I also run a check-abi
> on all affected platforms.

I'm reviewing.

In the meantime testing on 64-bit and 32-bit x86 was clean.

Tested-by: Carlos O'Donell <carlos@redhat.com>


> ---
>  NEWS                                          |  6 ++
>  debug/segfault.c                              | 18 ++---
>  include/signal.h                              |  2 +-
>  signal/signal.h                               |  6 --
>  stdio-common/psiginfo.c                       |  2 +-
>  stdio-common/psignal.c                        |  2 +-
>  stdio-common/siglist.c                        |  7 +-
>  string/strsignal.c                            |  2 +-
>  sysdeps/generic/siglist-compat.c              |  1 +
>  sysdeps/generic/siglist-compat.h              | 47 +++++++++++
>  sysdeps/gnu/siglist.c                         | 78 -------------------
>  .../mach/hurd/{siglist.h => siglist-compat.c} | 13 +++-
>  .../linux/{siglist.h => siglist-compat.c}     | 17 ++--
>  13 files changed, 90 insertions(+), 111 deletions(-)
>  create mode 100644 sysdeps/generic/siglist-compat.c
>  create mode 100644 sysdeps/generic/siglist-compat.h
>  delete mode 100644 sysdeps/gnu/siglist.c
>  rename sysdeps/mach/hurd/{siglist.h => siglist-compat.c} (68%)
>  rename sysdeps/unix/sysv/linux/{siglist.h => siglist-compat.c} (62%)
> 
> diff --git a/NEWS b/NEWS
> index 141078c319..055248928d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -35,6 +35,12 @@ Deprecated and removed features, and other changes affecting compatibility:
>    Its implementation always returned with a failure, and the function
>    was not declared in any header file.
>  
> +* The deprecated sys_siglist, _sys_siglist, and sys_sigabbrev arrays are
> +  non longer available to newly linked binaries, and their declarations
> +  have removed from from <string.h>.  They are exported solely as
> +  compatibility symbols to support old binaries.  All programs should use
> +  strsignal instead.
> +
>  Changes to build and runtime requirements:
>  
>  * powerpc64le requires GCC 7.4 or newer.  This is required for supporting
> diff --git a/debug/segfault.c b/debug/segfault.c
> index 14c64cd0bd..8b59783c9e 100644
> --- a/debug/segfault.c
> +++ b/debug/segfault.c
> @@ -49,20 +49,16 @@
>  static const char *fname;
>  
>  
> -/* We better should not use `strerror' since it can call far too many
> -   other functions which might fail.  Do it here ourselves.  */
> +/* Print the signal number SIGNAL.  Either strerror or strsignal might
> +   call local internal functions and these in turn call far too many
> +   other functions and might even allocate memory which might fail.  */
>  static void
>  write_strsignal (int fd, int signal)
>  {
> -  if (signal < 0 || signal >= _NSIG || _sys_siglist[signal] == NULL)
> -    {
> -      char buf[30];
> -      char *ptr = _itoa_word (signal, &buf[sizeof (buf)], 10, 0);
> -      WRITE_STRING ("signal ");
> -      write (fd, buf, &buf[sizeof (buf)] - ptr);
> -    }
> -  else
> -    WRITE_STRING (_sys_siglist[signal]);
> +  char buf[30];
> +  char *ptr = _itoa_word (signal, &buf[sizeof (buf)], 10, 0);
> +  WRITE_STRING ("signal ");
> +  write (fd, buf, &buf[sizeof (buf)] - ptr);
>  }
>  
>  
> diff --git a/include/signal.h b/include/signal.h
> index 293258ad65..ce511cfe60 100644
> --- a/include/signal.h
> +++ b/include/signal.h
> @@ -12,7 +12,7 @@ libc_hidden_proto (__sigpause)
>  libc_hidden_proto (raise)
>  libc_hidden_proto (__libc_current_sigrtmin)
>  libc_hidden_proto (__libc_current_sigrtmax)
> -libc_hidden_proto (_sys_siglist)
> +extern const char *const __sys_siglist_internal[_NSIG] attribute_hidden;
>  
>  /* Now define the internal interfaces.  */
>  extern __sighandler_t __bsd_signal (int __sig, __sighandler_t __handler);
> diff --git a/signal/signal.h b/signal/signal.h
> index fa8de963f8..3739550e5f 100644
> --- a/signal/signal.h
> +++ b/signal/signal.h
> @@ -281,12 +281,6 @@ extern int sigqueue (__pid_t __pid, int __sig, const union sigval __val)
>  
>  #ifdef __USE_MISC
>  
> -/* Names of the signals.  This variable exists only for compatibility.
> -   Use `strsignal' instead (see <string.h>).  */
> -extern const char *const _sys_siglist[_NSIG];
> -extern const char *const sys_siglist[_NSIG];
> -
> -
>  /* Get machine-dependent `struct sigcontext' and signal subcodes.  */
>  # include <bits/sigcontext.h>
>  
> diff --git a/stdio-common/psiginfo.c b/stdio-common/psiginfo.c
> index 4d498d00aa..e7c3b6137e 100644
> --- a/stdio-common/psiginfo.c
> +++ b/stdio-common/psiginfo.c
> @@ -80,7 +80,7 @@ psiginfo (const siginfo_t *pinfo, const char *s)
>  
>    const char *desc;
>    if (pinfo->si_signo >= 0 && pinfo->si_signo < NSIG
> -      && ((desc = _sys_siglist[pinfo->si_signo]) != NULL
> +      && ((desc = __sys_siglist_internal[pinfo->si_signo]) != NULL
>  #ifdef SIGRTMIN
>  	  || (pinfo->si_signo >= SIGRTMIN && pinfo->si_signo < SIGRTMAX)
>  #endif
> diff --git a/stdio-common/psignal.c b/stdio-common/psignal.c
> index de45e52734..20459a3bb0 100644
> --- a/stdio-common/psignal.c
> +++ b/stdio-common/psignal.c
> @@ -34,7 +34,7 @@ psignal (int sig, const char *s)
>    else
>      colon = ": ";
>  
> -  if (sig >= 0 && sig < NSIG && (desc = _sys_siglist[sig]) != NULL)
> +  if (sig >= 0 && sig < NSIG && (desc = __sys_siglist_internal[sig]) != NULL)
>      (void) __fxprintf (NULL, "%s%s%s\n", s, colon, _(desc));
>    else
>      {
> diff --git a/stdio-common/siglist.c b/stdio-common/siglist.c
> index 04082594a0..34c32f9bc0 100644
> --- a/stdio-common/siglist.c
> +++ b/stdio-common/siglist.c
> @@ -20,17 +20,18 @@
>  #include <signal.h>
>  #include <libintl.h>
>  
> -const char *const _sys_siglist[NSIG] =
> +const char *const __sys_siglist_internal[NSIG] =
>  {
>  #define init_sig(sig, abbrev, desc)   [sig] = desc,
>  #include <siglist.h>
>  #undef init_sig
>  };
>  
> -
> -const char *const _sys_sigabbrev[NSIG] =
> +const char *const __sys_sigabbrev_internal[NSIG] =
>  {
>  #define init_sig(sig, abbrev, desc)   [sig] = abbrev,
>  #include <siglist.h>
>  #undef init_sig
>  };
> +
> +#include <siglist-compat.c>
> diff --git a/string/strsignal.c b/string/strsignal.c
> index 2843ffe39b..1551480026 100644
> --- a/string/strsignal.c
> +++ b/string/strsignal.c
> @@ -51,7 +51,7 @@ strsignal (int signum)
>        (signum >= SIGRTMIN && signum <= SIGRTMAX) ||
>  #endif
>        signum < 0 || signum >= NSIG
> -      || (desc = _sys_siglist[signum]) == NULL)
> +      || (desc = __sys_siglist_internal[signum]) == NULL)
>      {
>        char *buffer = getbuffer ();
>        int len;
> diff --git a/sysdeps/generic/siglist-compat.c b/sysdeps/generic/siglist-compat.c
> new file mode 100644
> index 0000000000..6e25b021ab
> --- /dev/null
> +++ b/sysdeps/generic/siglist-compat.c
> @@ -0,0 +1 @@
> +/* Empty.  */
> diff --git a/sysdeps/generic/siglist-compat.h b/sysdeps/generic/siglist-compat.h
> new file mode 100644
> index 0000000000..b857efb9d7
> --- /dev/null
> +++ b/sysdeps/generic/siglist-compat.h
> @@ -0,0 +1,47 @@
> +/* Generic siglist compatibility macro definitions.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SIGLIST_COMPAT_H
> +#define _SIGLIST_COMPAT_H
> +
> +#include <shlib-compat.h>
> +#include <limits.h>
> +
> +/* Define new compat symbols for _sys_siglist, sys_siglist, and sys_sigabbrev
> +   for version VERSION with NUMBERSIG times number of bytes per long int size.
> +   Both _sys_siglist and sys_siglist alias to __sys_siglist_internal while
> +   sys_sigabbrev alias to _sys_sigabbrev_internal.  Both target alias are
> +   define in siglist.c.  */
> +#define DEFINE_COMPAT_SIGLIST(NUMBERSIG, VERSION) 			     \
> +  declare_symbol_alias (__ ## VERSION ## _sys_siglist,			     \
> +			__sys_siglist_internal,				     \
> +			object,	NUMBERSIG * (ULONG_WIDTH / UCHAR_WIDTH));    \
> +  declare_symbol_alias (__ ## VERSION ## sys_siglist,			     \
> +			__sys_siglist_internal,				     \
> +			object,	NUMBERSIG * (ULONG_WIDTH / UCHAR_WIDTH));    \
> +  declare_symbol_alias (__ ## VERSION ## _sys_sigabbrev,		     \
> +			__sys_sigabbrev_internal,			     \
> +			object, NUMBERSIG * (ULONG_WIDTH / UCHAR_WIDTH));    \
> +  compat_symbol (libc, __## VERSION ## _sys_siglist,   _sys_siglist,	     \
> +		 VERSION);						     \
> +  compat_symbol (libc, __## VERSION ## sys_siglist,    sys_siglist,	     \
> +		 VERSION);						     \
> +  compat_symbol (libc, __## VERSION ## _sys_sigabbrev, sys_sigabbrev,	     \
> +		 VERSION);						     \
> +
> +#endif
> diff --git a/sysdeps/gnu/siglist.c b/sysdeps/gnu/siglist.c
> deleted file mode 100644
> index c24f356f21..0000000000
> --- a/sysdeps/gnu/siglist.c
> +++ /dev/null
> @@ -1,78 +0,0 @@
> -/* Define list of all signal numbers and their names.
> -   Copyright (C) 1997-2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#include <stddef.h>
> -#include <signal.h>
> -#include <libintl.h>
> -#include <shlib-compat.h>
> -#include <bits/wordsize.h>
> -
> -const char *const __new_sys_siglist[NSIG] =
> -{
> -#define init_sig(sig, abbrev, desc)   [sig] = desc,
> -#include <siglist.h>
> -#undef init_sig
> -};
> -libc_hidden_ver (__new_sys_siglist, _sys_siglist)
> -
> -const char *const __new_sys_sigabbrev[NSIG] =
> -{
> -#define init_sig(sig, abbrev, desc)   [sig] = abbrev,
> -#include <siglist.h>
> -#undef init_sig
> -};
> -
> -#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> -declare_symbol_alias (__old_sys_siglist, __new_sys_siglist, object,
> -		      OLD_SIGLIST_SIZE * __WORDSIZE / 8)
> -
> -declare_symbol_alias (__old_sys_sigabbrev, __new_sys_sigabbrev, object,
> -		      OLD_SIGLIST_SIZE * __WORDSIZE / 8)
> -
> -declare_symbol_alias (_old_sys_siglist, __new_sys_siglist, object,
> -		      OLD_SIGLIST_SIZE * __WORDSIZE / 8)
> -
> -compat_symbol (libc, __old_sys_siglist, _sys_siglist, GLIBC_2_0);
> -compat_symbol (libc, _old_sys_siglist, sys_siglist, GLIBC_2_0);
> -compat_symbol (libc, __old_sys_sigabbrev, sys_sigabbrev, GLIBC_2_0);
> -#endif
> -
> -#if SHLIB_COMPAT (libc, GLIBC_2_1, GLIBC_2_3_3) && defined OLD2_SIGLIST_SIZE
> -declare_symbol_alias (__old2_sys_siglist, __new_sys_siglist, object,
> -		      OLD2_SIGLIST_SIZE * __WORDSIZE / 8)
> -
> -declare_symbol_alias (__old2_sys_sigabbrev, __new_sys_sigabbrev, object,
> -		      OLD2_SIGLIST_SIZE * __WORDSIZE / 8)
> -
> -declare_symbol_alias (_old2_sys_siglist, __new_sys_siglist, object,
> -		      OLD2_SIGLIST_SIZE * __WORDSIZE / 8)
> -
> -compat_symbol (libc, __old2_sys_siglist, _sys_siglist, GLIBC_2_1);
> -compat_symbol (libc, _old2_sys_siglist, sys_siglist, GLIBC_2_1);
> -compat_symbol (libc, __old2_sys_sigabbrev, sys_sigabbrev, GLIBC_2_1);
> -
> -strong_alias (__new_sys_siglist, _new_sys_siglist)
> -versioned_symbol (libc, __new_sys_siglist, _sys_siglist, GLIBC_2_3_3);
> -versioned_symbol (libc, _new_sys_siglist, sys_siglist, GLIBC_2_3_3);
> -versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_3_3);
> -#else
> -strong_alias (__new_sys_siglist, _new_sys_siglist)
> -versioned_symbol (libc, __new_sys_siglist, _sys_siglist, GLIBC_2_1);
> -versioned_symbol (libc, _new_sys_siglist, sys_siglist, GLIBC_2_1);
> -versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_1);
> -#endif
> diff --git a/sysdeps/mach/hurd/siglist.h b/sysdeps/mach/hurd/siglist-compat.c
> similarity index 68%
> rename from sysdeps/mach/hurd/siglist.h
> rename to sysdeps/mach/hurd/siglist-compat.c
> index 2eee091610..c93f12366b 100644
> --- a/sysdeps/mach/hurd/siglist.h
> +++ b/sysdeps/mach/hurd/siglist-compat.c
> @@ -1,4 +1,5 @@
> -/* Copyright (C) 1999-2020 Free Software Foundation, Inc.
> +/* Compatibility signal numbers and their names symbols.  Hurd version.
> +   Copyright (C) 1997-2020 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -15,8 +16,12 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -/* This file is included multiple times.  */
> +#include <siglist-compat.h>
>  
> -#include_next <siglist.h>	/* Get the canonical list.  */
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> +DEFINE_COMPAT_SIGLIST (33, GLIBC_2_0)
> +#endif
>  
> -#define	OLD_SIGLIST_SIZE	33 /* For GLIBC_2.0 binary compatibility.  */
> +#if SHLIB_COMPAT (libc, GLIBC_2_1, GLIBC_2_32)
> +DEFINE_COMPAT_SIGLIST (NSIG, GLIBC_2_1)
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/siglist.h b/sysdeps/unix/sysv/linux/siglist-compat.c
> similarity index 62%
> rename from sysdeps/unix/sysv/linux/siglist.h
> rename to sysdeps/unix/sysv/linux/siglist-compat.c
> index 6ff2c613ad..c322326a99 100644
> --- a/sysdeps/unix/sysv/linux/siglist.h
> +++ b/sysdeps/unix/sysv/linux/siglist-compat.c
> @@ -1,4 +1,5 @@
> -/* Copyright (C) 1996-2020 Free Software Foundation, Inc.
> +/* Compatibility signal numbers and their names symbols.  Linux version.
> +   Copyright (C) 1997-2020 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -15,10 +16,16 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -/* This file is included multiple times.  */
> +#include <siglist-compat.h>
>  
> -#include_next <siglist.h>	/* Get the canonical list.  */
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> +DEFINE_COMPAT_SIGLIST (32, GLIBC_2_0)
> +#endif
>  
> -#define	OLD_SIGLIST_SIZE	32 /* For GLIBC_2.0 binary compatibility.  */
> +#if SHLIB_COMPAT (libc, GLIBC_2_1, GLIBC_2_3_3)
> +DEFINE_COMPAT_SIGLIST (64, GLIBC_2_1)
> +#endif
>  
> -#define OLD2_SIGLIST_SIZE	64 /* For GLIBC_2.1 binary compatibility.  */
> +#if SHLIB_COMPAT (libc, GLIBC_2_3_3, GLIBC_2_32)
> +DEFINE_COMPAT_SIGLIST (NSIG, GLIBC_2_3_3)
> +#endif
> 


-- 
Cheers,
Carlos.



More information about the Libc-alpha mailing list