diff mbox series

[v3,02/13] signal: Move sys_siglist to a compat symbol

Message ID 20200519180518.318733-3-adhemerval.zanella@linaro.org
State New
Headers show
Series Signal and error list refactoring | expand

Commit Message

Adhemerval Zanella Netto May 19, 2020, 6:05 p.m. UTC
Changes from previous version:

  - Fixed NEWS wording.

---

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.

Tested-by: Carlos O'Donell <carlos@redhat.com>
---
 NEWS                                          |  6 ++
 debug/segfault.c                              | 18 ++---
 include/signal.h                              |  2 +-
 manual/signal.texi                            |  5 --
 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 ++--
 14 files changed, 90 insertions(+), 116 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%)

Comments

Florian Weimer May 28, 2020, 2:50 p.m. UTC | #1
* Adhemerval Zanella via Libc-alpha:

> diff --git a/NEWS b/NEWS
> index 10473bd514..f5b8954b12 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -38,6 +38,12 @@ Deprecated and removed features, and other changes affecting compatibility:
>  * ldconfig now defaults to the new format for ld.so.cache. glibc has
>    already supported this format for almost 20 years.
>  
> +* The deprecated arrays sys_siglist, _sys_siglist, and sys_sigabbrev
> +  arrays are no longer available to newly linked binaries, and their
> +  declarations have been removed from from <string.h>.  They are exported
> +  solely as compatibility symbols to support old binaries.  All programs
> +  should use strsignal instead.

Two “arrays”, “from from”.

The mechanics of the patch look okay to me.

Thanks,
Florian
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index 10473bd514..f5b8954b12 100644
--- a/NEWS
+++ b/NEWS
@@ -38,6 +38,12 @@  Deprecated and removed features, and other changes affecting compatibility:
 * ldconfig now defaults to the new format for ld.so.cache. glibc has
   already supported this format for almost 20 years.
 
+* The deprecated arrays sys_siglist, _sys_siglist, and sys_sigabbrev
+  arrays are no longer available to newly linked binaries, and their
+  declarations have been 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/manual/signal.texi b/manual/signal.texi
index 33e6646975..a3ecc7dc76 100644
--- a/manual/signal.texi
+++ b/manual/signal.texi
@@ -880,11 +880,6 @@  to @var{signum}.
 This function is a BSD feature, declared in the header file @file{signal.h}.
 @end deftypefun
 
-@vindex sys_siglist
-There is also an array @code{sys_siglist} which contains the messages
-for the various signal codes.  This array exists on BSD systems, unlike
-@code{strsignal}.
-
 @node Signal Actions
 @section Specifying Signal Actions
 @cindex signal actions
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