public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [RFC 0/4] Make strsignal and strerror async-signal-safe
@ 2020-05-13 20:26 Adhemerval Zanella
  2020-05-13 20:26 ` [RFC 1/4] string: Make strsignal async-signal-safe Adhemerval Zanella
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-13 20:26 UTC (permalink / raw)
  To: libc-alpha

This is an attempt to improve both strsignal and strerror usability
and provide the same semantic as direct access to the
_sys_{err,sig}list arrays.  The patchset is based on the patch to
move both _sys_siglist and_sys_errlist to compat symbol.

To fully make both symbols asyn-signal-safe it is required to both:

  1. Allocate a per-thread bounded buffer and

  2. Avoid to try translate the returned message.

To setup the per-thread static buffer, two strategies are used:

  1. The default one uses a TLS structure, which will be place in the
     static TLS space.

  2. Linux allocates on struct pthread and access it through THREAD_*
     macros.

The default strategy has the disadvantage of increasing libc.so static
TLS consumption and thus descreasing the possible surplus used in
some scenarios (which might be mitigated by BZ#25051 fix).

It is used only on Hurd, where accessing the thread point in single
thread case is not straightforward (afaiu, Hurd developers could
correct me here).

The translation avoidance is more tricky because it is a semantic
change and that's why the patchset also add a new strsignal_l
symbol.  Some systems (bionic) does provide async-signal-safe
version of the symbols, other system (FreeBSD) it is configurable.
Also, some system also does not translate the messags (bionic
and AIX7).

I have not added compat symbols on this RFC, however I don't have
a strong opinion regarding it.

Adhemerval Zanella (4):
  string: Make strsignal async-signal-safe
  string: Add strsignal_l
  string: Make strerror async-signal-safe
  string: Move strerror_l pointer to tls_internal.h

 include/string.h                              |   2 +
 malloc/thread-freeres.c                       |   1 +
 manual/errno.texi                             |   2 +-
 manual/signal.texi                            |   6 +-
 nptl/descr.h                                  |   5 +-
 string/Makefile                               |   2 +-
 string/Versions                               |   3 +
 string/_strerror.c                            |  80 ++++++------
 string/strerror.c                             |  26 ++--
 string/strerror_l.c                           |  34 +++--
 string/string.h                               |   5 +
 string/strsignal.c                            | 119 +++++-------------
 string/strsignal_l.c                          |  68 ++++++++++
 sysdeps/generic/Makefile                      |   1 +
 sysdeps/generic/tls_internal-struct.h         |  39 ++++++
 sysdeps/generic/tls_internal.c                |   3 +
 sysdeps/generic/tls_internal.h                |  32 +++++
 sysdeps/mach/_strerror.c                      |  48 +++++--
 sysdeps/mach/strerror_l.c                     |  24 ++--
 sysdeps/unix/sysv/linux/aarch64/libc.abilist  |   1 +
 sysdeps/unix/sysv/linux/alpha/libc.abilist    |   1 +
 sysdeps/unix/sysv/linux/arm/be/libc.abilist   |   1 +
 sysdeps/unix/sysv/linux/arm/le/libc.abilist   |   1 +
 sysdeps/unix/sysv/linux/csky/libc.abilist     |   1 +
 sysdeps/unix/sysv/linux/hppa/libc.abilist     |   1 +
 sysdeps/unix/sysv/linux/i386/libc.abilist     |   1 +
 sysdeps/unix/sysv/linux/ia64/libc.abilist     |   1 +
 .../sysv/linux/m68k/coldfire/libc.abilist     |   1 +
 .../unix/sysv/linux/m68k/m680x0/libc.abilist  |   1 +
 .../sysv/linux/microblaze/be/libc.abilist     |   1 +
 .../sysv/linux/microblaze/le/libc.abilist     |   1 +
 .../sysv/linux/mips/mips32/fpu/libc.abilist   |   1 +
 .../sysv/linux/mips/mips32/nofpu/libc.abilist |   1 +
 .../sysv/linux/mips/mips64/n32/libc.abilist   |   1 +
 .../sysv/linux/mips/mips64/n64/libc.abilist   |   1 +
 sysdeps/unix/sysv/linux/nios2/libc.abilist    |   1 +
 .../linux/powerpc/powerpc32/fpu/libc.abilist  |   1 +
 .../powerpc/powerpc32/nofpu/libc.abilist      |   1 +
 .../linux/powerpc/powerpc64/be/libc.abilist   |   1 +
 .../linux/powerpc/powerpc64/le/libc.abilist   |   1 +
 .../unix/sysv/linux/riscv/rv64/libc.abilist   |   1 +
 .../unix/sysv/linux/s390/s390-32/libc.abilist |   1 +
 .../unix/sysv/linux/s390/s390-64/libc.abilist |   1 +
 sysdeps/unix/sysv/linux/sh/be/libc.abilist    |   1 +
 sysdeps/unix/sysv/linux/sh/le/libc.abilist    |   1 +
 .../sysv/linux/sparc/sparc32/libc.abilist     |   1 +
 .../sysv/linux/sparc/sparc64/libc.abilist     |   1 +
 sysdeps/unix/sysv/linux/tls_internal.c        |   1 +
 sysdeps/unix/sysv/linux/tls_internal.h        |  30 +++++
 .../unix/sysv/linux/x86_64/64/libc.abilist    |   1 +
 .../unix/sysv/linux/x86_64/x32/libc.abilist   |   1 +
 time/strftime_l.c                             |  10 +-
 52 files changed, 367 insertions(+), 204 deletions(-)
 create mode 100644 string/strsignal_l.c
 create mode 100644 sysdeps/generic/tls_internal-struct.h
 create mode 100644 sysdeps/generic/tls_internal.c
 create mode 100644 sysdeps/generic/tls_internal.h
 create mode 100644 sysdeps/unix/sysv/linux/tls_internal.c
 create mode 100644 sysdeps/unix/sysv/linux/tls_internal.h

-- 
2.25.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC 1/4] string: Make strsignal async-signal-safe
  2020-05-13 20:26 [RFC 0/4] Make strsignal and strerror async-signal-safe Adhemerval Zanella
@ 2020-05-13 20:26 ` Adhemerval Zanella
  2020-05-14  7:15   ` Andreas Schwab
  2020-05-13 20:26 ` [RFC 2/4] string: Add strsignal_l Adhemerval Zanella
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-13 20:26 UTC (permalink / raw)
  To: libc-alpha

To accomplish it a per-thread static buffer is used for unknown
signals and the resulting message is not translated (this will
be covered by a new function).

To setup the per-thread static buffer, two strategies are used:

  1. The default one uses a TLS structure, which will be place in the
     static TLS space.

  2. Linux allocates on struct pthread and access it through THREAD_*
     macros.

The default strategy has the disadvantage of increasing libc.so static
TLS consumption and thus descreasing the possible surplus used in
some scenarios (which might be mitigated by BZ#25051 fix).

It is used only on Hurd, where accessing the thread point in single
thread case is not straightforward (afaiu, Hurd developers could
correct me here).

Checked on x86-64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
and s390x-linux-gnu.
---
 manual/signal.texi                     |   6 +-
 nptl/descr.h                           |   5 +-
 string/strsignal.c                     | 119 +++++++------------------
 sysdeps/generic/Makefile               |   1 +
 sysdeps/generic/tls_internal-struct.h  |  32 +++++++
 sysdeps/generic/tls_internal.c         |   3 +
 sysdeps/generic/tls_internal.h         |  32 +++++++
 sysdeps/unix/sysv/linux/tls_internal.c |   1 +
 sysdeps/unix/sysv/linux/tls_internal.h |  30 +++++++
 time/strftime_l.c                      |  10 +--
 10 files changed, 139 insertions(+), 100 deletions(-)
 create mode 100644 sysdeps/generic/tls_internal-struct.h
 create mode 100644 sysdeps/generic/tls_internal.c
 create mode 100644 sysdeps/generic/tls_internal.h
 create mode 100644 sysdeps/unix/sysv/linux/tls_internal.c
 create mode 100644 sysdeps/unix/sysv/linux/tls_internal.h

diff --git a/manual/signal.texi b/manual/signal.texi
index 33e6646975..08f0edb306 100644
--- a/manual/signal.texi
+++ b/manual/signal.texi
@@ -830,8 +830,8 @@ termination status of a child process (@pxref{Process Completion}) or it
 may come from a signal handler in the same process.
 
 @deftypefun {char *} strsignal (int @var{signum})
-@standards{GNU, string.h}
-@safety{@prelim{}@mtunsafe{@mtasurace{:strsignal} @mtslocale{}}@asunsafe{@asuinit{} @ascuintl{} @asucorrupt{} @ascuheap{}}@acunsafe{@acuinit{} @acucorrupt{} @acsmem{}}}
+@standards{POSIX-1.2008, string.h}
+@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
 @c strsignal @mtasurace:strsignal @mtslocale @asuinit @ascuintl @asucorrupt @ascuheap @acucorrupt @acsmem
 @c   uses a static buffer if tsd key creation fails
 @c  [once] init
@@ -852,7 +852,7 @@ rewritten on subsequent calls, you should save a copy of it if you need
 to reference it later.
 
 @pindex string.h
-This function is a GNU extension, declared in the header file
+This function is declared in the header file
 @file{string.h}.
 @end deftypefun
 
diff --git a/nptl/descr.h b/nptl/descr.h
index e1c7db5473..ee8f4b0e37 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -34,6 +34,7 @@
 #include <unwind.h>
 #include <bits/types/res_state.h>
 #include <kernel-features.h>
+#include <tls_internal-struct.h>
 
 #ifndef TCB_ALIGNMENT
 # define TCB_ALIGNMENT	sizeof (double)
@@ -116,7 +117,6 @@ struct priority_protection_data
   unsigned int priomap[];
 };
 
-
 /* Thread descriptor data structure.  */
 struct pthread
 {
@@ -398,6 +398,9 @@ struct pthread
   /* Indicates whether is a C11 thread created by thrd_creat.  */
   bool c11;
 
+  /* Used on strsignal.  */
+  struct tls_internal_t tls_state;
+
   /* This member must be last.  */
   char end_padding[];
 
diff --git a/string/strsignal.c b/string/strsignal.c
index 1551480026..5fbc3d9a60 100644
--- a/string/strsignal.c
+++ b/string/strsignal.c
@@ -16,110 +16,55 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <signal.h>
-#include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
-#include <libintl.h>
-#include <libc-lock.h>
-
-static __libc_key_t key;
-
-/* If nonzero the key allocation failed and we should better use a
-   static buffer than fail.  */
-#define BUFFERSIZ	100
-static char local_buf[BUFFERSIZ];
-static char *static_buf;
+#include <stdlib.h>
+#include <array_length.h>
+#include <tls_internal.h>
 
-/* Destructor for the thread-specific data.  */
-static void init (void);
-static void free_key_mem (void *mem);
-static char *getbuffer (void);
+#define RT_STR  "Real-time signal "
+_Static_assert (sizeof (RT_STR) <= strsignal_str_len,
+		RT_STR " > strsignal_str_len");
 
+#define UNK_STR "Unknown signal "
+_Static_assert (sizeof (UNK_STR) <= strsignal_str_len,
+		UNK_STR " > strsignal_str_len");
 
 /* Return a string describing the meaning of the signal number SIGNUM.  */
 char *
 strsignal (int signum)
 {
-  __libc_once_define (static, once);
-  const char *desc;
+  const char *desc = NULL;
+
+  if (signum >= 0 && signum <= NSIG
+      && signum < array_length (__sys_siglist_internal))
+    desc = __sys_siglist_internal[signum];
 
-  /* If we have not yet initialized the buffer do it now.  */
-  __libc_once (once, init);
+  if (desc != NULL)
+    return (char *) desc;
 
-  if (
+  const char* prefix = UNK_STR;
 #ifdef SIGRTMIN
-      (signum >= SIGRTMIN && signum <= SIGRTMAX) ||
-#endif
-      signum < 0 || signum >= NSIG
-      || (desc = __sys_siglist_internal[signum]) == NULL)
+  if (signum >= SIGRTMIN && signum <= SIGRTMAX)
     {
-      char *buffer = getbuffer ();
-      int len;
-#ifdef SIGRTMIN
-      if (signum >= SIGRTMIN && signum <= SIGRTMAX)
-	len = __snprintf (buffer, BUFFERSIZ - 1, _("Real-time signal %d"),
-			  signum - SIGRTMIN);
-      else
-#endif
-	len = __snprintf (buffer, BUFFERSIZ - 1, _("Unknown signal %d"),
-			  signum);
-      if (len >= BUFFERSIZ)
-	buffer = NULL;
-      else
-	buffer[len] = '\0';
-
-      return buffer;
+      prefix = RT_STR;
+      signum -= SIGRTMIN;
     }
+#endif
 
-  return (char *) _(desc);
-}
-
-
-/* Initialize buffer.  */
-static void
-init (void)
-{
-  if (__libc_key_create (&key, free_key_mem))
-    /* Creating the key failed.  This means something really went
-       wrong.  In any case use a static buffer which is better than
-       nothing.  */
-    static_buf = local_buf;
-}
-
-
-/* Free the thread specific data, this is done if a thread terminates.  */
-static void
-free_key_mem (void *mem)
-{
-  free (mem);
-  __libc_setspecific (key, NULL);
-}
-
+  char *buffer = __glibc_tls_internal ()->strsignal_buf;
 
-/* Return the buffer to be used.  */
-static char *
-getbuffer (void)
-{
-  char *result;
+  char *q = __stpcpy (buffer, prefix);
 
-  if (static_buf != NULL)
-    result = static_buf;
-  else
+  if (signum < 0)
     {
-      /* We don't use the static buffer and so we have a key.  Use it
-	 to get the thread-specific buffer.  */
-      result = __libc_getspecific (key);
-      if (result == NULL)
-	{
-	  /* No buffer allocated so far.  */
-	  result = malloc (BUFFERSIZ);
-	  if (result == NULL)
-	    /* No more memory available.  We use the static buffer.  */
-	    result = local_buf;
-	  else
-	    __libc_setspecific (key, result);
-	}
+      *q++ = '-';
+      signum = abs (signum);
     }
+  for (int d = signum; q++, (d /= 10) != 0; )
+    continue;
+  *q = '\0';
+  for (int d = signum; *--q = '0' + d % 10, (d /= 10) != 0; )
+    continue;
 
-  return result;
+  return buffer;
 }
diff --git a/sysdeps/generic/Makefile b/sysdeps/generic/Makefile
index bd4f9425ca..9bed5d7fa2 100644
--- a/sysdeps/generic/Makefile
+++ b/sysdeps/generic/Makefile
@@ -16,6 +16,7 @@
 # <https://www.gnu.org/licenses/>.
 
 ifeq ($(subdir),string)
+sysdep_routines += tls_internal
 CFLAGS-wordcopy.c += -Wno-uninitialized
 endif
 
diff --git a/sysdeps/generic/tls_internal-struct.h b/sysdeps/generic/tls_internal-struct.h
new file mode 100644
index 0000000000..dac967b2f3
--- /dev/null
+++ b/sysdeps/generic/tls_internal-struct.h
@@ -0,0 +1,32 @@
+/* Per-thread state.  Generic version.
+   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 _TLS_INTERNAL_STRUCT_H
+#define _TLS_INTERNAL_STRUCT_H 1
+
+#include <intprops.h>
+
+enum { strsignal_str_len = 20 };
+
+struct tls_internal_t
+{
+  /* Used on strsignal.c.  */
+  char strsignal_buf[strsignal_str_len + INT_STRLEN_BOUND (int) + 1];
+};
+
+#endif
diff --git a/sysdeps/generic/tls_internal.c b/sysdeps/generic/tls_internal.c
new file mode 100644
index 0000000000..e988a608b2
--- /dev/null
+++ b/sysdeps/generic/tls_internal.c
@@ -0,0 +1,3 @@
+#include <tls_internal.h>
+
+__thread struct tls_internal_t tls_internal;
diff --git a/sysdeps/generic/tls_internal.h b/sysdeps/generic/tls_internal.h
new file mode 100644
index 0000000000..6edc4e574c
--- /dev/null
+++ b/sysdeps/generic/tls_internal.h
@@ -0,0 +1,32 @@
+/* Per-thread state.  Generic version.
+   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 _TLS_INTERNAL_H
+#define _TLS_INTERNAL_H 1
+
+#include <tls_internal-struct.h>
+
+extern __thread struct tls_internal_t tls_internal attribute_hidden;
+
+static inline struct tls_internal_t *
+__glibc_tls_internal (void)
+{
+  return &tls_internal;
+}
+
+#endif
diff --git a/sysdeps/unix/sysv/linux/tls_internal.c b/sysdeps/unix/sysv/linux/tls_internal.c
new file mode 100644
index 0000000000..6e25b021ab
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tls_internal.c
@@ -0,0 +1 @@
+/* Empty.  */
diff --git a/sysdeps/unix/sysv/linux/tls_internal.h b/sysdeps/unix/sysv/linux/tls_internal.h
new file mode 100644
index 0000000000..f1d3161e08
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tls_internal.h
@@ -0,0 +1,30 @@
+/* Per-thread state.  Linux version.
+   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 _TLS_INTERNAL_H
+#define _TLS_INTERNAL_H 1
+
+#include <nptl/pthreadP.h>
+
+static inline struct tls_internal_t *
+__glibc_tls_internal (void)
+{
+  return &THREAD_SELF->tls_state;
+}
+
+#endif
diff --git a/time/strftime_l.c b/time/strftime_l.c
index a9aae42191..83a62dbb6b 100644
--- a/time/strftime_l.c
+++ b/time/strftime_l.c
@@ -136,15 +136,7 @@ extern char *tzname[];
 # define NULL 0
 #endif
 
-#define TYPE_SIGNED(t) ((t) -1 < 0)
-
-/* Bound on length of the string representing an integer value of type t.
-   Subtract one for the sign bit if t is signed;
-   302 / 1000 is log10 (2) rounded up;
-   add one for integer division truncation;
-   add one more for a minus sign if t is signed.  */
-#define INT_STRLEN_BOUND(t) \
- ((sizeof (t) * CHAR_BIT - TYPE_SIGNED (t)) * 302 / 1000 + 1 + TYPE_SIGNED (t))
+#include <intprops.h>
 
 #define TM_YEAR_BASE 1900
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC 2/4] string: Add strsignal_l
  2020-05-13 20:26 [RFC 0/4] Make strsignal and strerror async-signal-safe Adhemerval Zanella
  2020-05-13 20:26 ` [RFC 1/4] string: Make strsignal async-signal-safe Adhemerval Zanella
@ 2020-05-13 20:26 ` Adhemerval Zanella
  2020-05-14  0:16   ` Joseph Myers
  2020-05-13 20:26 ` [RFC 3/4] string: Make strerror async-signal-safe Adhemerval Zanella
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-13 20:26 UTC (permalink / raw)
  To: libc-alpha

Similar to strsignal, but maps the input signal to a locale-dependent
signal description.  The internal buffer is allocated with malloc
(as previously done by strsignal) and its pointer is stored in the
same per-thread structure used on strsignal (making is thread-safe).

The behavior of strsignal_l is undefined if locale is the special
locale object LC_GLOBAL_LOCALE or is not a valid locale object handle.

Checked on x86-64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
and s390x-linux-gnu.
---
 include/string.h                              |  1 +
 malloc/thread-freeres.c                       |  1 +
 string/Makefile                               |  2 +-
 string/Versions                               |  3 +
 string/string.h                               |  5 ++
 string/strsignal_l.c                          | 68 +++++++++++++++++++
 sysdeps/generic/tls_internal-struct.h         |  1 +
 sysdeps/unix/sysv/linux/aarch64/libc.abilist  |  1 +
 sysdeps/unix/sysv/linux/alpha/libc.abilist    |  1 +
 sysdeps/unix/sysv/linux/arm/be/libc.abilist   |  1 +
 sysdeps/unix/sysv/linux/arm/le/libc.abilist   |  1 +
 sysdeps/unix/sysv/linux/csky/libc.abilist     |  1 +
 sysdeps/unix/sysv/linux/hppa/libc.abilist     |  1 +
 sysdeps/unix/sysv/linux/i386/libc.abilist     |  1 +
 sysdeps/unix/sysv/linux/ia64/libc.abilist     |  1 +
 .../sysv/linux/m68k/coldfire/libc.abilist     |  1 +
 .../unix/sysv/linux/m68k/m680x0/libc.abilist  |  1 +
 .../sysv/linux/microblaze/be/libc.abilist     |  1 +
 .../sysv/linux/microblaze/le/libc.abilist     |  1 +
 .../sysv/linux/mips/mips32/fpu/libc.abilist   |  1 +
 .../sysv/linux/mips/mips32/nofpu/libc.abilist |  1 +
 .../sysv/linux/mips/mips64/n32/libc.abilist   |  1 +
 .../sysv/linux/mips/mips64/n64/libc.abilist   |  1 +
 sysdeps/unix/sysv/linux/nios2/libc.abilist    |  1 +
 .../linux/powerpc/powerpc32/fpu/libc.abilist  |  1 +
 .../powerpc/powerpc32/nofpu/libc.abilist      |  1 +
 .../linux/powerpc/powerpc64/be/libc.abilist   |  1 +
 .../linux/powerpc/powerpc64/le/libc.abilist   |  1 +
 .../unix/sysv/linux/riscv/rv64/libc.abilist   |  1 +
 .../unix/sysv/linux/s390/s390-32/libc.abilist |  1 +
 .../unix/sysv/linux/s390/s390-64/libc.abilist |  1 +
 sysdeps/unix/sysv/linux/sh/be/libc.abilist    |  1 +
 sysdeps/unix/sysv/linux/sh/le/libc.abilist    |  1 +
 .../sysv/linux/sparc/sparc32/libc.abilist     |  1 +
 .../sysv/linux/sparc/sparc64/libc.abilist     |  1 +
 .../unix/sysv/linux/x86_64/64/libc.abilist    |  1 +
 .../unix/sysv/linux/x86_64/x32/libc.abilist   |  1 +
 37 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 string/strsignal_l.c

diff --git a/include/string.h b/include/string.h
index 4d622f1c03..24a61bcff7 100644
--- a/include/string.h
+++ b/include/string.h
@@ -51,6 +51,7 @@ extern int __ffs (int __i) __attribute__ ((const));
 extern char *__strerror_r (int __errnum, char *__buf, size_t __buflen);
 
 /* Called as part of the thread shutdown sequence.  */
+void __strsignal_thread_freeres (void) attribute_hidden;
 void __strerror_thread_freeres (void) attribute_hidden;
 
 /* Get _STRING_ARCH_unaligned.  */
diff --git a/malloc/thread-freeres.c b/malloc/thread-freeres.c
index c71ca4fc33..d42eea770b 100644
--- a/malloc/thread-freeres.c
+++ b/malloc/thread-freeres.c
@@ -32,6 +32,7 @@ __libc_thread_freeres (void)
   call_function_static_weak (__rpc_thread_destroy);
   call_function_static_weak (__res_thread_freeres);
   call_function_static_weak (__strerror_thread_freeres);
+  call_function_static_weak (__strsignal_thread_freeres);
 
   /* This should come last because it shuts down malloc for this
      thread and the other shutdown functions might well call free.  */
diff --git a/string/Makefile b/string/Makefile
index c46785f1a1..4ec56c5fce 100644
--- a/string/Makefile
+++ b/string/Makefile
@@ -44,7 +44,7 @@ routines	:= strcat strchr strcmp strcoll strcpy strcspn		\
 				     addsep replace)			\
 		   envz basename					\
 		   strcoll_l strxfrm_l string-inlines memrchr		\
-		   xpg-strerror strerror_l explicit_bzero
+		   xpg-strerror strerror_l explicit_bzero strsignal_l
 
 strop-tests	:= memchr memcmp memcpy memmove mempcpy memset memccpy	\
 		   stpcpy stpncpy strcat strchr strcmp strcpy strcspn	\
diff --git a/string/Versions b/string/Versions
index 9b709d12a9..3830c577f8 100644
--- a/string/Versions
+++ b/string/Versions
@@ -85,4 +85,7 @@ libc {
   GLIBC_2.25 {
     explicit_bzero;
   }
+  GLIBC_2.32 {
+    strsignal_l;
+  }
 }
diff --git a/string/string.h b/string/string.h
index d7ce0f4a1b..f36f3ac69f 100644
--- a/string/string.h
+++ b/string/string.h
@@ -454,6 +454,11 @@ extern char *strsep (char **__restrict __stringp,
 /* Return a string describing the meaning of the signal number in SIG.  */
 extern char *strsignal (int __sig) __THROW;
 
+# ifdef __USE_GNU
+/* Translate the meaning of the signal in SIG according to the locale L.  */
+extern char *strsignal_l (int __sig, locale_t __l) __THROW;
+# endif
+
 /* Copy SRC to DEST, returning the address of the terminating '\0' in DEST.  */
 extern char *__stpcpy (char *__restrict __dest, const char *__restrict __src)
      __THROW __nonnull ((1, 2));
diff --git a/string/strsignal_l.c b/string/strsignal_l.c
new file mode 100644
index 0000000000..054593da34
--- /dev/null
+++ b/string/strsignal_l.c
@@ -0,0 +1,68 @@
+/* Return the translated string describing signal.
+   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/>.  */
+
+#include <string.h>
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <locale.h>
+#include <libintl.h>
+#include <array_length.h>
+#include <tls_internal.h>
+
+static const char *
+translate (const char *str, locale_t loc)
+{
+  locale_t oldloc = __uselocale (loc);
+  const char *res = _(str);
+  __uselocale (oldloc);
+  return res;
+}
+
+char *
+strsignal_l (int signum, locale_t loc)
+{
+  const char *desc = NULL;
+
+  if (signum >= 0 && signum <= NSIG
+      && signum < array_length (__sys_siglist_internal))
+    desc = __sys_siglist_internal[signum];
+
+  if (desc != NULL)
+    return (char *) translate (desc, loc);
+
+  const char* prefix = "Unknown signal ";
+#ifdef SIGRTMIN
+  if (signum >= SIGRTMIN && signum <= SIGRTMAX)
+    prefix = "Real-time signal ";
+#endif
+
+  struct tls_internal_t *tls_internal = __glibc_tls_internal ();
+  free (tls_internal->strsignal_l_buf);
+  if (__asprintf (&tls_internal->strsignal_l_buf, "%s%d",
+      translate (prefix, loc), signum) == -1)
+    tls_internal->strsignal_l_buf = NULL;
+  return tls_internal->strsignal_l_buf;
+}
+
+void
+__strsignal_thread_freeres (void)
+{
+  free (__glibc_tls_internal()->strsignal_l_buf);
+}
+text_set_element (__libc_subfreeres, __strsignal_thread_freeres);
diff --git a/sysdeps/generic/tls_internal-struct.h b/sysdeps/generic/tls_internal-struct.h
index dac967b2f3..ca235a93db 100644
--- a/sysdeps/generic/tls_internal-struct.h
+++ b/sysdeps/generic/tls_internal-struct.h
@@ -27,6 +27,7 @@ struct tls_internal_t
 {
   /* Used on strsignal.c.  */
   char strsignal_buf[strsignal_str_len + INT_STRLEN_BOUND (int) + 1];
+  char *strsignal_l_buf;
 };
 
 #endif
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
index 41bb214bb9..ff9f9cc8b9 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
@@ -2147,3 +2147,4 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist
index 6430af207f..d97e08958f 100644
--- a/sysdeps/unix/sysv/linux/alpha/libc.abilist
+++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist
@@ -2227,6 +2227,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/arm/be/libc.abilist b/sysdeps/unix/sysv/linux/arm/be/libc.abilist
index f4ea1756d5..deafe483fb 100644
--- a/sysdeps/unix/sysv/linux/arm/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arm/be/libc.abilist
@@ -134,6 +134,7 @@ GLIBC_2.31 msgctl F
 GLIBC_2.31 semctl F
 GLIBC_2.31 shmctl F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 _Exit F
 GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
 GLIBC_2.4 _IO_2_1_stdin_ D 0xa0
diff --git a/sysdeps/unix/sysv/linux/arm/le/libc.abilist b/sysdeps/unix/sysv/linux/arm/le/libc.abilist
index f1456b26b2..e92d2f620d 100644
--- a/sysdeps/unix/sysv/linux/arm/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arm/le/libc.abilist
@@ -131,6 +131,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 _Exit F
 GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
 GLIBC_2.4 _IO_2_1_stdin_ D 0xa0
diff --git a/sysdeps/unix/sysv/linux/csky/libc.abilist b/sysdeps/unix/sysv/linux/csky/libc.abilist
index c54aed2f8e..16788d6e72 100644
--- a/sysdeps/unix/sysv/linux/csky/libc.abilist
+++ b/sysdeps/unix/sysv/linux/csky/libc.abilist
@@ -2091,3 +2091,4 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist
index 87373f755b..0df921a1c5 100644
--- a/sysdeps/unix/sysv/linux/hppa/libc.abilist
+++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist
@@ -2048,6 +2048,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist
index 1bd2e02f79..c67bcd575d 100644
--- a/sysdeps/unix/sysv/linux/i386/libc.abilist
+++ b/sysdeps/unix/sysv/linux/i386/libc.abilist
@@ -2214,6 +2214,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist
index 07e51d46bf..19f13dae24 100644
--- a/sysdeps/unix/sysv/linux/ia64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist
@@ -2080,6 +2080,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
index 42ea4c24bf..88c60ac4ce 100644
--- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
@@ -135,6 +135,7 @@ GLIBC_2.31 msgctl F
 GLIBC_2.31 semctl F
 GLIBC_2.31 shmctl F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 _Exit F
 GLIBC_2.4 _IO_2_1_stderr_ D 0x98
 GLIBC_2.4 _IO_2_1_stdin_ D 0x98
diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
index e9358fb092..89a2fe6b96 100644
--- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
@@ -2160,6 +2160,7 @@ GLIBC_2.31 msgctl F
 GLIBC_2.31 semctl F
 GLIBC_2.31 shmctl F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
index 2cefe739c0..ba8119f080 100644
--- a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
@@ -2142,3 +2142,4 @@ GLIBC_2.31 msgctl F
 GLIBC_2.31 semctl F
 GLIBC_2.31 shmctl F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
diff --git a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
index 3474ef1490..e5db46bf46 100644
--- a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
@@ -2139,3 +2139,4 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
index a6f99a7369..9d655367d2 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
@@ -2131,6 +2131,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
index 48222af11c..d5d85ddf7e 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
@@ -2129,6 +2129,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
index 99965cfb0f..7ce3f58728 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
@@ -2137,6 +2137,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
index 2c8bafc669..5ed3392638 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
@@ -2131,6 +2131,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist
index 52cf72052c..fa62ca14c8 100644
--- a/sysdeps/unix/sysv/linux/nios2/libc.abilist
+++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist
@@ -2180,3 +2180,4 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
index 2ca5bbccf3..8ad2ab5a1a 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
@@ -2187,6 +2187,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
index e6c4d002d5..81d1622da2 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
@@ -2220,6 +2220,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
index 82d77b7e48..d289bc9dbc 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
@@ -2050,6 +2050,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
index 0c2513a4b3..cc52e4bc74 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
@@ -2342,3 +2342,4 @@ GLIBC_2.32 __wprintf_chkieee128 F
 GLIBC_2.32 __wprintfieee128 F
 GLIBC_2.32 __wscanfieee128 F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
index 234d34929a..fcee77e8de 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
@@ -2109,3 +2109,4 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
index 1f06cce028..9c776e41c4 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
@@ -2185,6 +2185,7 @@ GLIBC_2.31 msgctl F
 GLIBC_2.31 semctl F
 GLIBC_2.31 shmctl F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
index 26c2ce32e5..797429f31f 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
@@ -2086,6 +2086,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/sh/be/libc.abilist b/sysdeps/unix/sysv/linux/sh/be/libc.abilist
index 7ad2e920c3..1e8bf1ace9 100644
--- a/sysdeps/unix/sysv/linux/sh/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sh/be/libc.abilist
@@ -2055,6 +2055,7 @@ GLIBC_2.31 msgctl F
 GLIBC_2.31 semctl F
 GLIBC_2.31 shmctl F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/sh/le/libc.abilist b/sysdeps/unix/sysv/linux/sh/le/libc.abilist
index d2611bf0a5..4914ec8a8d 100644
--- a/sysdeps/unix/sysv/linux/sh/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sh/le/libc.abilist
@@ -2052,6 +2052,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
index 18a528f0e9..672f5bda1b 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
@@ -2176,6 +2176,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
index a1d48b0f3c..18f0685684 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
@@ -2103,6 +2103,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
index 6418ace78a..581a403365 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
@@ -2061,6 +2061,7 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
index edb9f2f004..5961531257 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
@@ -2160,3 +2160,4 @@ GLIBC_2.30 gettid F
 GLIBC_2.30 tgkill F
 GLIBC_2.30 twalk_r F
 GLIBC_2.32 pthread_sigmask F
+GLIBC_2.32 strsignal_l F
-- 
2.25.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC 3/4] string: Make strerror async-signal-safe
  2020-05-13 20:26 [RFC 0/4] Make strsignal and strerror async-signal-safe Adhemerval Zanella
  2020-05-13 20:26 ` [RFC 1/4] string: Make strsignal async-signal-safe Adhemerval Zanella
  2020-05-13 20:26 ` [RFC 2/4] string: Add strsignal_l Adhemerval Zanella
@ 2020-05-13 20:26 ` Adhemerval Zanella
  2020-05-14  7:12   ` Andreas Schwab
  2020-05-13 20:26 ` [RFC 4/4] string: Move strerror_l pointer to tls_internal.h Adhemerval Zanella
  2020-05-14 12:05 ` [RFC 0/4] Make strsignal and strerror async-signal-safe Florian Weimer
  4 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-13 20:26 UTC (permalink / raw)
  To: libc-alpha

To accomplish it a per-thread static buffer is used for unknown
errnos and the resulting message is not translated (this is already
covered by strerror_l).

The buffer allocation uses the same strategy of strsignal.  Its
size should cover both the generic and Hurd requirements (although
Hurd could use more extensive _Static_assert to check if all its
subsystem specific messages would be handled by the buffer size).

Checked on x86-64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
and s390x-linux-gnu.
---
 include/string.h                      |  1 +
 manual/errno.texi                     |  2 +-
 string/_strerror.c                    | 80 ++++++++++++++-------------
 string/strerror.c                     | 26 +++------
 sysdeps/generic/tls_internal-struct.h |  5 ++
 sysdeps/mach/_strerror.c              | 48 +++++++++++-----
 6 files changed, 91 insertions(+), 71 deletions(-)

diff --git a/include/string.h b/include/string.h
index 24a61bcff7..3c784be997 100644
--- a/include/string.h
+++ b/include/string.h
@@ -49,6 +49,7 @@ extern void __bzero (void *__s, size_t __n) __THROW __nonnull ((1));
 extern int __ffs (int __i) __attribute__ ((const));
 
 extern char *__strerror_r (int __errnum, char *__buf, size_t __buflen);
+extern char *__strerror_lookup (int errnum) attribute_hidden;
 
 /* Called as part of the thread shutdown sequence.  */
 void __strsignal_thread_freeres (void) attribute_hidden;
diff --git a/manual/errno.texi b/manual/errno.texi
index 8cb4ce8b48..dbb0c4a66d 100644
--- a/manual/errno.texi
+++ b/manual/errno.texi
@@ -1147,7 +1147,7 @@ name of the program that encountered the error.
 
 @deftypefun {char *} strerror (int @var{errnum})
 @standards{ISO, string.h}
-@safety{@prelim{}@mtunsafe{@mtasurace{:strerror}}@asunsafe{@ascuheap{} @ascuintl{}}@acunsafe{@acsmem{}}}
+@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
 @c Calls strerror_r with a static buffer allocated with malloc on the
 @c first use.
 The @code{strerror} function maps the error code (@pxref{Checking for
diff --git a/string/_strerror.c b/string/_strerror.c
index 985fd4e3c6..f6a0b89b38 100644
--- a/string/_strerror.c
+++ b/string/_strerror.c
@@ -15,60 +15,64 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <libintl.h>
-#include <stdbool.h>
 #include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
+#include <stdlib.h>
+#include <array_length.h>
 #include <sys/param.h>
-#include <_itoa.h>
+#include <tls_internal-struct.h>
+
+#define UNK_ERR_STR "Unknown error "
+_Static_assert (sizeof (UNK_ERR_STR) <= strerror_str_len,
+		UNK_ERR_STR " > strerror_str_len");
 
-/* It is critical here that we always use the `dcgettext' function for
-   the message translation.  Since <libintl.h> only defines the macro
-   `dgettext' to use `dcgettext' for optimizing programs this is not
-   always guaranteed.  */
-#ifndef dgettext
-# include <locale.h>		/* We need LC_MESSAGES.  */
-# define dgettext(domainname, msgid) dcgettext (domainname, msgid, LC_MESSAGES)
-#endif
+char *
+__strerror_lookup (int errnum)
+{
+  if (__glibc_unlikely (errnum < 0 || errnum >= _sys_nerr_internal))
+    return NULL;
+  return (char *) _sys_errlist_internal[errnum];
+}
 
 /* Return a string describing the errno code in ERRNUM.  */
 char *
 __strerror_r (int errnum, char *buf, size_t buflen)
 {
-  if (__glibc_unlikely (errnum < 0 || errnum >= _sys_nerr_internal
-			|| _sys_errlist_internal[errnum] == NULL))
-    {
-      /* Buffer we use to print the number in.  For a maximum size for
-	 `int' of 8 bytes we never need more than 20 digits.  */
-      char numbuf[21];
-      const char *unk = _("Unknown error ");
-      size_t unklen = strlen (unk);
-      char *p, *q;
-      bool negative = errnum < 0;
+  char *r = __strerror_lookup (errnum);
+  if (r != NULL)
+    return r;
 
-      numbuf[20] = '\0';
-      p = _itoa_word (abs (errnum), &numbuf[20], 10, 0);
+  const size_t unklen = array_length (UNK_ERR_STR) - 1;
+  char *q = __mempcpy (buf, UNK_ERR_STR, MIN (buflen, unklen));
+  if (unklen < buflen)
+    {
+      char numstr[INT_STRLEN_BOUND (int) + 1];
+      size_t numstrlen = 1;
 
-      /* Now construct the result while taking care for the destination
-	 buffer size.  */
-      q = __mempcpy (buf, unk, MIN (unklen, buflen));
-      if (negative && unklen < buflen)
+      char *p = numstr;
+      long int errn = errnum;
+      if (errnum < 0)
 	{
-	  *q++ = '-';
-	  ++unklen;
+	  *p++ = '-';
+	  errn = labs (errnum);
+	  numstrlen++;
 	}
-      if (unklen < buflen)
-	memcpy (q, p, MIN ((size_t) (&numbuf[21] - p), buflen - unklen));
+      for (long int d = errn; p++, (d /= 10) != 0; )
+	continue;
+      *p = '\0';
+      for (long int d = errn;
+	    *--p = '0' + d % 10, (d /= 10) != 0;
+	   numstrlen++)
+	continue;
 
-      /* Terminate the string in any case.  */
-      if (buflen > 0)
-	buf[buflen - 1] = '\0';
-
-      return buf;
+      memcpy (q, numstr, MIN (MIN (numstrlen, INT_STRLEN_BOUND (int)) + 1,
+			      buflen - unklen));
     }
 
-  return (char *) _(_sys_errlist_internal[errnum]);
+  if (buflen > 0)
+    buf[buflen - 1] = '\0';
+
+  return buf;
 }
 weak_alias (__strerror_r, strerror_r)
 libc_hidden_def (__strerror_r)
diff --git a/string/strerror.c b/string/strerror.c
index 283ab70f91..4c6f88c682 100644
--- a/string/strerror.c
+++ b/string/strerror.c
@@ -15,29 +15,17 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <libintl.h>
-#include <stdio.h>
 #include <string.h>
-#include <errno.h>
-
-/* Return a string describing the errno code in ERRNUM.
-   The storage is good only until the next call to strerror.
-   Writing to the storage causes undefined behavior.  */
-libc_freeres_ptr (static char *buf);
+#include <tls_internal.h>
 
 char *
 strerror (int errnum)
 {
-  char *ret = __strerror_r (errnum, NULL, 0);
-  int saved_errno;
-
-  if (__glibc_likely (ret != NULL))
+  char *ret = __strerror_lookup (errnum);
+  if (ret != NULL)
     return ret;
-  saved_errno = errno;
-  if (buf == NULL)
-    buf = malloc (1024);
-  __set_errno (saved_errno);
-  if (buf == NULL)
-    return _("Unknown error");
-  return __strerror_r (errnum, buf, 1024);
+
+  char *buffer = __glibc_tls_internal ()->strerror_buf;
+  __strerror_r (errnum, buffer, TLS_INTERNAL_STRERROR_LEN);
+  return buffer;
 }
diff --git a/sysdeps/generic/tls_internal-struct.h b/sysdeps/generic/tls_internal-struct.h
index ca235a93db..4416228757 100644
--- a/sysdeps/generic/tls_internal-struct.h
+++ b/sysdeps/generic/tls_internal-struct.h
@@ -22,12 +22,17 @@
 #include <intprops.h>
 
 enum { strsignal_str_len = 20 };
+enum { strerror_str_len = 37 };
 
 struct tls_internal_t
 {
   /* Used on strsignal.c.  */
   char strsignal_buf[strsignal_str_len + INT_STRLEN_BOUND (int) + 1];
   char *strsignal_l_buf;
+  char strerror_buf[strerror_str_len + INT_STRLEN_BOUND (int) + 1];
 };
 
+#define TLS_INTERNAL_STRERROR_LEN \
+  sizeof ((struct tls_internal_t){0}.strerror_buf)
+
 #endif
diff --git a/sysdeps/mach/_strerror.c b/sysdeps/mach/_strerror.c
index d932b1bd58..f3969b576f 100644
--- a/sysdeps/mach/_strerror.c
+++ b/sysdeps/mach/_strerror.c
@@ -15,22 +15,45 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <libintl.h>
 #include <stdio.h>
 #include <string.h>
 #include <mach/error.h>
 #include <errorlib.h>
 #include <sys/param.h>
 #include <_itoa.h>
+#include <tls_internal-struct.h>
 
-/* It is critical here that we always use the `dcgettext' function for
-   the message translation.  Since <libintl.h> only defines the macro
-   `dgettext' to use `dcgettext' for optimizing programs this is not
-   always guaranteed.  */
-#ifndef dgettext
-# include <locale.h>		/* We need LC_MESSAGES.  */
-# define dgettext(domainname, msgid) dcgettext (domainname, msgid, LC_MESSAGES)
-#endif
+#define UNK_SYSTEM_ERR_STR "Error in unknown error system: "
+_Static_assert (sizeof (UNK_SYSTEM_ERR_STR) <= strerror_str_len,
+		UNK_SYSTEM_ERR_STR " > strerror_str_len");
+#define UNK_SUBSYSTEM_ERR_STR "Unknown error "
+_Static_assert (sizeof (UNK_SUBSYSTEM_ERR_STR) <= strerror_str_len,
+		UNK_SUBSYSTEM_ERR_STR " > strerror_str_len");
+
+extern void __mach_error_map_compat (int *);
+
+char *
+__strerror_lookup (int errnum)
+{
+  __mach_error_map_compat (&errnum);
+
+  int system = err_get_system (errnum);
+  int sub = err_get_sub (errnum);
+  int code = err_get_code (errnum);
+
+  if (system > err_max_system || ! __mach_error_systems[system].bad_sub)
+    return NULL;
+
+  const struct error_system *es = &__mach_error_systems[system];
+
+  if (sub >= es->max_sub)
+    return (char *) es->bad_sub;
+
+  if (code >= es->subsystem[sub].max_code)
+    return NULL;
+
+  return (char *) es->subsystem[sub].codes[code];
+}
 
 /* Return a string describing the errno code in ERRNUM.  */
 char *
@@ -40,7 +63,6 @@ __strerror_r (int errnum, char *buf, size_t buflen)
   int sub;
   int code;
   const struct error_system *es;
-  extern void __mach_error_map_compat (int *);
 
   __mach_error_map_compat (&errnum);
 
@@ -53,7 +75,7 @@ __strerror_r (int errnum, char *buf, size_t buflen)
       /* Buffer we use to print the number in.  For a maximum size for
 	 `int' of 8 bytes we never need more than 20 digits.  */
       char numbuf[21];
-      const char *unk = _("Error in unknown error system: ");
+      const char *unk = UNK_SYSTEM_ERR_STR;
       const size_t unklen = strlen (unk);
       char *p, *q;
 
@@ -83,7 +105,7 @@ __strerror_r (int errnum, char *buf, size_t buflen)
       /* Buffer we use to print the number in.  For a maximum size for
 	 `int' of 8 bytes we never need more than 20 digits.  */
       char numbuf[21];
-      const char *unk = _("Unknown error ");
+      const char *unk = UNK_SUBSYSTEM_ERR_STR;
       const size_t unklen = strlen (unk);
       char *p, *q;
       size_t len = strlen (es->subsystem[sub].subsys_name);
@@ -114,7 +136,7 @@ __strerror_r (int errnum, char *buf, size_t buflen)
       return buf;
     }
 
-  return (char *) _(es->subsystem[sub].codes[code]);
+  return (char *) es->subsystem[sub].codes[code];
 }
 libc_hidden_def (__strerror_r)
 weak_alias (__strerror_r, strerror_r)
-- 
2.25.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC 4/4] string: Move strerror_l pointer to tls_internal.h
  2020-05-13 20:26 [RFC 0/4] Make strsignal and strerror async-signal-safe Adhemerval Zanella
                   ` (2 preceding siblings ...)
  2020-05-13 20:26 ` [RFC 3/4] string: Make strerror async-signal-safe Adhemerval Zanella
@ 2020-05-13 20:26 ` Adhemerval Zanella
  2020-05-14 12:05 ` [RFC 0/4] Make strsignal and strerror async-signal-safe Florian Weimer
  4 siblings, 0 replies; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-13 20:26 UTC (permalink / raw)
  To: libc-alpha

Similar to strsignal_l, move the per-thread message pointer to
tls_internal.h (which on Linux is allocated on struct per-thread).

Checked on x86-64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
and s390x-linux-gnu.
---
 string/strerror_l.c                   | 34 ++++++++++++---------------
 sysdeps/generic/tls_internal-struct.h |  1 +
 sysdeps/mach/strerror_l.c             | 24 +++++++++----------
 3 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/string/strerror_l.c b/string/strerror_l.c
index 40e7d0e896..baa489dd90 100644
--- a/string/strerror_l.c
+++ b/string/strerror_l.c
@@ -15,15 +15,11 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <stdio.h>
 #include <libintl.h>
 #include <locale.h>
-#include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
-#include <sys/param.h>
-#include <libc-symbols.h>
-
-static __thread char *last_value;
+#include <tls_internal.h>
 
 
 static const char *
@@ -40,23 +36,23 @@ translate (const char *str, locale_t loc)
 char *
 strerror_l (int errnum, locale_t loc)
 {
-  if (__glibc_unlikely (errnum < 0 || errnum >= _sys_nerr_internal
-			|| _sys_errlist_internal[errnum] == NULL))
-    {
-      free (last_value);
-      if (__asprintf (&last_value, "%s%d",
-		      translate ("Unknown error ", loc), errnum) == -1)
-	last_value = NULL;
-
-      return last_value;
-    }
-
-  return (char *) translate (_sys_errlist_internal[errnum], loc);
+  char *r = __strerror_lookup (errnum);
+  if (r != NULL)
+    return (char *) translate (r, loc);
+
+  struct tls_internal_t *tls_internal = __glibc_tls_internal ();
+  free (tls_internal->strerror_l_buf);
+  if (__asprintf (&tls_internal->strerror_l_buf, "%s%d",
+		  translate ("Unknown error ", loc), errnum) == -1)
+    tls_internal->strerror_l_buf = NULL;
+
+  return tls_internal->strerror_l_buf;
 }
 
 void
 __strerror_thread_freeres (void)
 {
-  free (last_value);
+  free (__glibc_tls_internal()->strerror_l_buf);
 }
 text_set_element (__libc_subfreeres, __strerror_thread_freeres);
+
diff --git a/sysdeps/generic/tls_internal-struct.h b/sysdeps/generic/tls_internal-struct.h
index 4416228757..a5ae278f81 100644
--- a/sysdeps/generic/tls_internal-struct.h
+++ b/sysdeps/generic/tls_internal-struct.h
@@ -30,6 +30,7 @@ struct tls_internal_t
   char strsignal_buf[strsignal_str_len + INT_STRLEN_BOUND (int) + 1];
   char *strsignal_l_buf;
   char strerror_buf[strerror_str_len + INT_STRLEN_BOUND (int) + 1];
+  char *strerror_l_buf;
 };
 
 #define TLS_INTERNAL_STRERROR_LEN \
diff --git a/sysdeps/mach/strerror_l.c b/sysdeps/mach/strerror_l.c
index f514af341e..d6a69c3784 100644
--- a/sysdeps/mach/strerror_l.c
+++ b/sysdeps/mach/strerror_l.c
@@ -24,10 +24,7 @@
 #include <mach/error.h>
 #include <errorlib.h>
 #include <sys/param.h>
-#include <libc-symbols.h>
-
-
-static __thread char *last_value;
+#include <tls_internal.h>
 
 
 static const char *
@@ -56,15 +53,16 @@ strerror_l (int errnum, locale_t loc)
   sub = err_get_sub (errnum);
   code = err_get_code (errnum);
 
+  struct tls_internal_t *tls_internal = __glibc_tls_internal ();
   if (system > err_max_system || ! __mach_error_systems[system].bad_sub)
     {
-      free (last_value);
-      if (__asprintf (&last_value, "%s%X",
+      free (tls_internal->strerror_l_buf);
+      if (__asprintf (&tls_internal->strerror_l_buf, "%s%X",
 		      translate ("Error in unknown error system: ", loc),
 		      errnum) == -1)
-	last_value = NULL;
+	tls_internal->strerror_l_buf = NULL;
 
-      return last_value;
+      return tls_internal->strerror_l_buf;
     }
 
   es = &__mach_error_systems[system];
@@ -74,14 +72,14 @@ strerror_l (int errnum, locale_t loc)
 
   if (code >= es->subsystem[sub].max_code)
     {
-      free (last_value);
-      if (__asprintf (&last_value, "%s%s %d",
+      free (tls_internal->strerror_l_buf);
+      if (__asprintf (&tls_internal->strerror_l_buf, "%s%s %d",
 		      translate ("Unknown error ", loc),
 		      translate (es->subsystem[sub].subsys_name, loc),
 		      errnum) == -1)
-	last_value = NULL;
+	tls_internal->strerror_l_buf = NULL;
 
-      return last_value;
+      return tls_internal->strerror_l_buf;
     }
 
   return (char *) translate (es->subsystem[sub].codes[code], loc);
@@ -91,6 +89,6 @@ strerror_l (int errnum, locale_t loc)
 void
 __strerror_thread_freeres (void)
 {
-  free (last_value);
+  free (__glibc_tls_internal()->strerror_l_buf);
 }
 text_set_element (__libc_subfreeres, __strerror_thread_freeres);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 2/4] string: Add strsignal_l
  2020-05-13 20:26 ` [RFC 2/4] string: Add strsignal_l Adhemerval Zanella
@ 2020-05-14  0:16   ` Joseph Myers
  0 siblings, 0 replies; 21+ messages in thread
From: Joseph Myers @ 2020-05-14  0:16 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

Note that a new public function should get a NEWS entry.  (And normally 
documentation in the manual, though in this case none of the existing *_l 
functions are documented so it may not be necessary to document a single 
new one on its own.)

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 3/4] string: Make strerror async-signal-safe
  2020-05-13 20:26 ` [RFC 3/4] string: Make strerror async-signal-safe Adhemerval Zanella
@ 2020-05-14  7:12   ` Andreas Schwab
  0 siblings, 0 replies; 21+ messages in thread
From: Andreas Schwab @ 2020-05-14  7:12 UTC (permalink / raw)
  To: Adhemerval Zanella via Libc-alpha

On Mai 13 2020, Adhemerval Zanella via Libc-alpha wrote:

> +char *
> +__strerror_lookup (int errnum)
> +{
> +  if (__glibc_unlikely (errnum < 0 || errnum >= _sys_nerr_internal))
> +    return NULL;
> +  return (char *) _sys_errlist_internal[errnum];
> +}
>  
>  /* Return a string describing the errno code in ERRNUM.  */
>  char *
>  __strerror_r (int errnum, char *buf, size_t buflen)
>  {
> -  if (__glibc_unlikely (errnum < 0 || errnum >= _sys_nerr_internal
> -			|| _sys_errlist_internal[errnum] == NULL))
> -    {
> -      /* Buffer we use to print the number in.  For a maximum size for
> -	 `int' of 8 bytes we never need more than 20 digits.  */
> -      char numbuf[21];
> -      const char *unk = _("Unknown error ");
> -      size_t unklen = strlen (unk);
> -      char *p, *q;
> -      bool negative = errnum < 0;
> +  char *r = __strerror_lookup (errnum);
> +  if (r != NULL)
> +    return r;
>  
> -      numbuf[20] = '\0';
> -      p = _itoa_word (abs (errnum), &numbuf[20], 10, 0);
> +  const size_t unklen = array_length (UNK_ERR_STR) - 1;
> +  char *q = __mempcpy (buf, UNK_ERR_STR, MIN (buflen, unklen));
> +  if (unklen < buflen)
> +    {
> +      char numstr[INT_STRLEN_BOUND (int) + 1];
> +      size_t numstrlen = 1;
>  
> -      /* Now construct the result while taking care for the destination
> -	 buffer size.  */
> -      q = __mempcpy (buf, unk, MIN (unklen, buflen));
> -      if (negative && unklen < buflen)
> +      char *p = numstr;
> +      long int errn = errnum;

Why long int?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 1/4] string: Make strsignal async-signal-safe
  2020-05-13 20:26 ` [RFC 1/4] string: Make strsignal async-signal-safe Adhemerval Zanella
@ 2020-05-14  7:15   ` Andreas Schwab
  0 siblings, 0 replies; 21+ messages in thread
From: Andreas Schwab @ 2020-05-14  7:15 UTC (permalink / raw)
  To: Adhemerval Zanella via Libc-alpha

On Mai 13 2020, Adhemerval Zanella via Libc-alpha wrote:

> +  if (signum < 0)
>      {
> -      /* We don't use the static buffer and so we have a key.  Use it
> -	 to get the thread-specific buffer.  */
> -      result = __libc_getspecific (key);
> -      if (result == NULL)
> -	{
> -	  /* No buffer allocated so far.  */
> -	  result = malloc (BUFFERSIZ);
> -	  if (result == NULL)
> -	    /* No more memory available.  We use the static buffer.  */
> -	    result = local_buf;
> -	  else
> -	    __libc_setspecific (key, result);
> -	}
> +      *q++ = '-';
> +      signum = abs (signum);

Since you already know signum < 0 you can just use -signum.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-13 20:26 [RFC 0/4] Make strsignal and strerror async-signal-safe Adhemerval Zanella
                   ` (3 preceding siblings ...)
  2020-05-13 20:26 ` [RFC 4/4] string: Move strerror_l pointer to tls_internal.h Adhemerval Zanella
@ 2020-05-14 12:05 ` Florian Weimer
  2020-05-14 12:47   ` Adhemerval Zanella
  4 siblings, 1 reply; 21+ messages in thread
From: Florian Weimer @ 2020-05-14 12:05 UTC (permalink / raw)
  To: Adhemerval Zanella via Libc-alpha

* Adhemerval Zanella via Libc-alpha:

>   2. Avoid to try translate the returned message.

No more translation is a significant change.  Is this really
appropriate?

Using strerror output in translated strings is very common:

<https://codesearch.debian.net/search?q=_%5C%28.*strerror&literal=0>

I still prefer my approach with new functions which return a string
(with the name of the constant), but only for known error numbers.  I
think the constant names would also be more useful to us in diagnostics.

> The translation avoidance is more tricky because it is a semantic
> change and that's why the patchset also add a new strsignal_l
> symbol.  Some systems (bionic) does provide async-signal-safe
> version of the symbols, other system (FreeBSD) it is configurable.
> Also, some system also does not translate the messags (bionic
> and AIX7).

Isn't bionic basically a loader for Dalvik, with few end-user visible
use cases?  So it might not be the right reference.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 12:05 ` [RFC 0/4] Make strsignal and strerror async-signal-safe Florian Weimer
@ 2020-05-14 12:47   ` Adhemerval Zanella
  2020-05-14 15:11     ` Zack Weinberg
  2020-05-14 15:16     ` Florian Weimer
  0 siblings, 2 replies; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-14 12:47 UTC (permalink / raw)
  To: Florian Weimer, Adhemerval Zanella via Libc-alpha



On 14/05/2020 09:05, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>>   2. Avoid to try translate the returned message.
> 
> No more translation is a significant change.  Is this really
> appropriate?

I take that once we have proper symbols that provides the translation
functionality, making them async-signal-safe is an welcome improvement. 

The glibc itself uses the __sys_* access on libSegFault due this
shortcoming.  And I am not if other programs are really aware of
the shortcoming of the always translatable error strings.

> 
> Using strerror output in translated strings is very common:
> 
> <https://codesearch.debian.net/search?q=_%5C%28.*strerror&literal=0>
> 
> I still prefer my approach with new functions which return a string
> (with the name of the constant), but only for known error numbers.  I
> think the constant names would also be more useful to us in diagnostics.

New non-standard symbols takes time and code effort to be used, and
it would required either to be implemented in other libc to fully
portable or even additional efforts from the projects to use it 
(and there is recent trend to avoid use new glibc symbol that might
break the usage on older glibc, although it is not really supported
from out side).

So I do prefer to improve current interfaces than add newer ones
(unless there are functionalities that current approach does not 
support).  The translation from str{signal,error} is an implementation 
detail and I was done mainly glibc also exports sys_* (which has its
own issues and shortcomings). And I am not if changing really breaks 
thing here, in fact it might provide more robust log message if they
are being generated in signal handler (which seems to be common in
some code).

> 
>> The translation avoidance is more tricky because it is a semantic
>> change and that's why the patchset also add a new strsignal_l
>> symbol.  Some systems (bionic) does provide async-signal-safe
>> version of the symbols, other system (FreeBSD) it is configurable.
>> Also, some system also does not translate the messags (bionic
>> and AIX7).
> 
> Isn't bionic basically a loader for Dalvik, with few end-user visible
> use cases?  So it might not be the right reference.

Not sure, although android does use multiple services in native code.

> 
> Thanks,
> Florian
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 12:47   ` Adhemerval Zanella
@ 2020-05-14 15:11     ` Zack Weinberg
  2020-05-14 16:42       ` Adhemerval Zanella
  2020-05-14 18:39       ` Florian Weimer
  2020-05-14 15:16     ` Florian Weimer
  1 sibling, 2 replies; 21+ messages in thread
From: Zack Weinberg @ 2020-05-14 15:11 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Florian Weimer, Adhemerval Zanella via Libc-alpha

On Thu, May 14, 2020 at 8:47 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
> On 14/05/2020 09:05, Florian Weimer wrote:
> > * Adhemerval Zanella via Libc-alpha:
> >
> >>   2. Avoid to try translate the returned message.
> >
> > No more translation is a significant change.  Is this really
> > appropriate?
> >
> > Using strerror output in translated strings is very common:
> >
> > <https://codesearch.debian.net/search?q=_%5C%28.*strerror&literal=0>
>
> I take that once we have proper symbols that provides the translation
> functionality, making them async-signal-safe is an welcome improvement.
>
> The glibc itself uses the __sys_* access on libSegFault due this
> shortcoming.  And I am not if other programs are really aware of
> the shortcoming of the always translatable error strings.

I think I'm with Florian here.  There is lots of existing code that
expects strerror() to translate, and _most_ of the time it's not being
called from an async signal handler, making everyone change their code
to use strerror_l (and get a locale_t to use it with) just to get back
the behavior they're used to seems like too much churn.

It would be nice if there was a way for strerror to detect that it
wasn't safe for it to call dcgettext or allocate memory.  In principle
we ought to be able to try-lock the "I might need to initialize the
global locale now" lock but, looking at the guts of dcgettext, there's
several different locks involved and it's not clear to me that we
could make this work without a major overhaul of libintl.

(Giant-hammer idea: have sigaction wrap all signal handlers with a
routine that sets a thread-local flag while calling the signal
handler.  siglongjmp and swapcontext would need to restore this flag.)

zw

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 12:47   ` Adhemerval Zanella
  2020-05-14 15:11     ` Zack Weinberg
@ 2020-05-14 15:16     ` Florian Weimer
  2020-05-14 16:28       ` Adhemerval Zanella
  1 sibling, 1 reply; 21+ messages in thread
From: Florian Weimer @ 2020-05-14 15:16 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Adhemerval Zanella via Libc-alpha

* Adhemerval Zanella:

> On 14/05/2020 09:05, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>> 
>>>   2. Avoid to try translate the returned message.
>> 
>> No more translation is a significant change.  Is this really
>> appropriate?
>
> I take that once we have proper symbols that provides the translation
> functionality, making them async-signal-safe is an welcome
> improvement.

But why would non-portable software switch to those interfaces?

It seems more conservative to me to introduce new interfaces that are
async-signal-safe (and thread-safe).

This change also continues the pattern of removal of widely-used
interfaces without a deprecation warning (although these interfaces have
been informally deprecated for a long time, so maybe that's okay).

> The glibc itself uses the __sys_* access on libSegFault due this
> shortcoming.  And I am not if other programs are really aware of
> the shortcoming of the always translatable error strings.

Thread safety for strerror can be a problem.  We can fix that
independently.  Also the use of legacy TLS is problematic and we should
get rid of that.

>> Using strerror output in translated strings is very common:
>> 
>> <https://codesearch.debian.net/search?q=_%5C%28.*strerror&literal=0>
>> 
>> I still prefer my approach with new functions which return a string
>> (with the name of the constant), but only for known error numbers.  I
>> think the constant names would also be more useful to us in diagnostics.
>
> New non-standard symbols takes time and code effort to be used, and
> it would required either to be implemented in other libc to fully
> portable or even additional efforts from the projects to use it 
> (and there is recent trend to avoid use new glibc symbol that might
> break the usage on older glibc, although it is not really supported
> from out side).

Yes, but as I showed you, your proposed change leads to partially
translated strings.  I don't see a compelling reason to do that.  Yes,
intl/ is not async-signal-safe, and it would be some work to make it so,
but that isn't justification enough to drop the translation.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 15:16     ` Florian Weimer
@ 2020-05-14 16:28       ` Adhemerval Zanella
  2020-05-14 16:40         ` Florian Weimer
  0 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-14 16:28 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Adhemerval Zanella via Libc-alpha



On 14/05/2020 12:16, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 14/05/2020 09:05, Florian Weimer wrote:
>>> * Adhemerval Zanella via Libc-alpha:
>>>
>>>>   2. Avoid to try translate the returned message.
>>>
>>> No more translation is a significant change.  Is this really
>>> appropriate?
>>
>> I take that once we have proper symbols that provides the translation
>> functionality, making them async-signal-safe is an welcome
>> improvement.
> 
> But why would non-portable software switch to those interfaces?
> 
> It seems more conservative to me to introduce new interfaces that are
> async-signal-safe (and thread-safe).

The downside is new interface usually takes time to be adopted, they add
even more code bloat/complexity to support old interfaces, and they
does help current usage.

> 
> This change also continues the pattern of removal of widely-used
> interfaces without a deprecation warning (although these interfaces have
> been informally deprecated for a long time, so maybe that's okay).
> 
>> The glibc itself uses the __sys_* access on libSegFault due this
>> shortcoming.  And I am not if other programs are really aware of
>> the shortcoming of the always translatable error strings.
> 
> Thread safety for strerror can be a problem.  We can fix that
> independently.  Also the use of legacy TLS is problematic and we should
> get rid of that.

By legacy TLS do you mean __thread usage?

> 
>>> Using strerror output in translated strings is very common:
>>>
>>> <https://codesearch.debian.net/search?q=_%5C%28.*strerror&literal=0>
>>>
>>> I still prefer my approach with new functions which return a string
>>> (with the name of the constant), but only for known error numbers.  I
>>> think the constant names would also be more useful to us in diagnostics.
>>
>> New non-standard symbols takes time and code effort to be used, and
>> it would required either to be implemented in other libc to fully
>> portable or even additional efforts from the projects to use it 
>> (and there is recent trend to avoid use new glibc symbol that might
>> break the usage on older glibc, although it is not really supported
>> from out side).
> 
> Yes, but as I showed you, your proposed change leads to partially
> translated strings.  I don't see a compelling reason to do that.  Yes,
> intl/ is not async-signal-safe, and it would be some work to make it so,
> but that isn't justification enough to drop the translation.

What do you mean by *partially* translated? 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 16:28       ` Adhemerval Zanella
@ 2020-05-14 16:40         ` Florian Weimer
  2020-05-14 16:46           ` Adhemerval Zanella
  0 siblings, 1 reply; 21+ messages in thread
From: Florian Weimer @ 2020-05-14 16:40 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Adhemerval Zanella via Libc-alpha

* Adhemerval Zanella:

> On 14/05/2020 12:16, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 14/05/2020 09:05, Florian Weimer wrote:
>>>> * Adhemerval Zanella via Libc-alpha:
>>>>
>>>>>   2. Avoid to try translate the returned message.
>>>>
>>>> No more translation is a significant change.  Is this really
>>>> appropriate?
>>>
>>> I take that once we have proper symbols that provides the translation
>>> functionality, making them async-signal-safe is an welcome
>>> improvement.
>> 
>> But why would non-portable software switch to those interfaces?
>> 
>> It seems more conservative to me to introduce new interfaces that are
>> async-signal-safe (and thread-safe).
>
> The downside is new interface usually takes time to be adopted, they add
> even more code bloat/complexity to support old interfaces, and they
> does help current usage.

But dropping features from existing interfaces does not seem a good
alternative in this case.

>> This change also continues the pattern of removal of widely-used
>> interfaces without a deprecation warning (although these interfaces have
>> been informally deprecated for a long time, so maybe that's okay).
>> 
>>> The glibc itself uses the __sys_* access on libSegFault due this
>>> shortcoming.  And I am not if other programs are really aware of
>>> the shortcoming of the always translatable error strings.
>> 
>> Thread safety for strerror can be a problem.  We can fix that
>> independently.  Also the use of legacy TLS is problematic and we should
>> get rid of that.
>
> By legacy TLS do you mean __thread usage?

No, the libc-internal equivalent of pthread_key_create
(__libc_key_create).

>> Yes, but as I showed you, your proposed change leads to partially
>> translated strings.  I don't see a compelling reason to do that.  Yes,
>> intl/ is not async-signal-safe, and it would be some work to make it so,
>> but that isn't justification enough to drop the translation.
>
> What do you mean by *partially* translated?

First hit from the Debian Code Search query I posted:

		die(_("cannot open %s: %s"), device_name, strerror(errno));

Currently, this translates to:

  DEVICE_NAME kann nicht geöffnet werden: Datei oder Verzeichnis nicht gefunden

After removing translation from strerror, this turns into:

  DEVICE_NAME kann nicht geöffnet werden: No such file or directory

That's a clear regression.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 15:11     ` Zack Weinberg
@ 2020-05-14 16:42       ` Adhemerval Zanella
  2020-05-14 18:39       ` Florian Weimer
  1 sibling, 0 replies; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-14 16:42 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Florian Weimer, Adhemerval Zanella via Libc-alpha



On 14/05/2020 12:11, Zack Weinberg wrote:
> On Thu, May 14, 2020 at 8:47 AM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>> On 14/05/2020 09:05, Florian Weimer wrote:
>>> * Adhemerval Zanella via Libc-alpha:
>>>
>>>>   2. Avoid to try translate the returned message.
>>>
>>> No more translation is a significant change.  Is this really
>>> appropriate?
>>>
>>> Using strerror output in translated strings is very common:
>>>
>>> <https://codesearch.debian.net/search?q=_%5C%28.*strerror&literal=0>
>>
>> I take that once we have proper symbols that provides the translation
>> functionality, making them async-signal-safe is an welcome improvement.
>>
>> The glibc itself uses the __sys_* access on libSegFault due this
>> shortcoming.  And I am not if other programs are really aware of
>> the shortcoming of the always translatable error strings.
> 
> I think I'm with Florian here.  There is lots of existing code that
> expects strerror() to translate, and _most_ of the time it's not being
> called from an async signal handler, making everyone change their code
> to use strerror_l (and get a locale_t to use it with) just to get back
> the behavior they're used to seems like too much churn.

Alright, I took this semantic change would to make them async-signal-safe
allow to be more beneficial in the long term (in both code complexity and
more concise API) but looks like we should go to newer symbols instead.

> 
> It would be nice if there was a way for strerror to detect that it
> wasn't safe for it to call dcgettext or allocate memory.  In principle
> we ought to be able to try-lock the "I might need to initialize the
> global locale now" lock but, looking at the guts of dcgettext, there's
> several different locks involved and it's not clear to me that we
> could make this work without a major overhaul of libintl.
> 
> (Giant-hammer idea: have sigaction wrap all signal handlers with a
> routine that sets a thread-local flag while calling the signal
> handler.  siglongjmp and swapcontext would need to restore this flag.)

The sigaction wrap was a suggestion I proposed to Florian some time ago
as a way to detect a code was running in a signal handler.  FreeBSB does
this for the its libpthread analogous and its adds some complexity 
(it uses a static list and it requires some locking to handle concurrent
access), but I think it should be feasible.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 16:40         ` Florian Weimer
@ 2020-05-14 16:46           ` Adhemerval Zanella
  2020-05-14 16:51             ` Florian Weimer
  0 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-14 16:46 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Adhemerval Zanella via Libc-alpha



On 14/05/2020 13:40, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 14/05/2020 12:16, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> On 14/05/2020 09:05, Florian Weimer wrote:
>>>>> * Adhemerval Zanella via Libc-alpha:
>>>>>
>>>>>>   2. Avoid to try translate the returned message.
>>>>>
>>>>> No more translation is a significant change.  Is this really
>>>>> appropriate?
>>>>
>>>> I take that once we have proper symbols that provides the translation
>>>> functionality, making them async-signal-safe is an welcome
>>>> improvement.
>>>
>>> But why would non-portable software switch to those interfaces?
>>>
>>> It seems more conservative to me to introduce new interfaces that are
>>> async-signal-safe (and thread-safe).
>>
>> The downside is new interface usually takes time to be adopted, they add
>> even more code bloat/complexity to support old interfaces, and they
>> does help current usage.
> 
> But dropping features from existing interfaces does not seem a good
> alternative in this case.
> 
>>> This change also continues the pattern of removal of widely-used
>>> interfaces without a deprecation warning (although these interfaces have
>>> been informally deprecated for a long time, so maybe that's okay).
>>>
>>>> The glibc itself uses the __sys_* access on libSegFault due this
>>>> shortcoming.  And I am not if other programs are really aware of
>>>> the shortcoming of the always translatable error strings.
>>>
>>> Thread safety for strerror can be a problem.  We can fix that
>>> independently.  Also the use of legacy TLS is problematic and we should
>>> get rid of that.
>>
>> By legacy TLS do you mean __thread usage?
> 
> No, the libc-internal equivalent of pthread_key_create
> (__libc_key_create).

Right, it seems that dlfcn is the only place that still uses it.

What about moving per-thread state to __thread (generic) / pthread (Linux),
do you have some reservation about it?

> 
>>> Yes, but as I showed you, your proposed change leads to partially
>>> translated strings.  I don't see a compelling reason to do that.  Yes,
>>> intl/ is not async-signal-safe, and it would be some work to make it so,
>>> but that isn't justification enough to drop the translation.
>>
>> What do you mean by *partially* translated?
> 
> First hit from the Debian Code Search query I posted:
> 
> 		die(_("cannot open %s: %s"), device_name, strerror(errno));
> 
> Currently, this translates to:
> 
>   DEVICE_NAME kann nicht geöffnet werden: Datei oder Verzeichnis nicht gefunden
> 
> After removing translation from strerror, this turns into:
> 
>   DEVICE_NAME kann nicht geöffnet werden: No such file or directory
> 
> That's a clear regression.

Alright, as from Zack message I took this semantic change would to make them 
async-signal-safe allow to be more beneficial in the long term (in both code 
complexity and more concise API) but looks like we should go to newer symbols 
instead.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 16:46           ` Adhemerval Zanella
@ 2020-05-14 16:51             ` Florian Weimer
  2020-05-14 16:55               ` Adhemerval Zanella
  0 siblings, 1 reply; 21+ messages in thread
From: Florian Weimer @ 2020-05-14 16:51 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Adhemerval Zanella via Libc-alpha

* Adhemerval Zanella:

> What about moving per-thread state to __thread (generic) / pthread (Linux),
> do you have some reservation about it?

No, that makes sense.  You can free allocations via
malloc/thread-freeres.c.

I think I have a patch somewhere that unifies strerror and strerror_l at
the same time, or I can review yours.  I read POSIX as saying that
strerror and strerror_l can share the same buffer.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 16:51             ` Florian Weimer
@ 2020-05-14 16:55               ` Adhemerval Zanella
  2020-05-14 17:02                 ` Florian Weimer
  0 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-14 16:55 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Adhemerval Zanella via Libc-alpha



On 14/05/2020 13:51, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> What about moving per-thread state to __thread (generic) / pthread (Linux),
>> do you have some reservation about it?
> 
> No, that makes sense.  You can free allocations via
> malloc/thread-freeres.c.
> 
> I think I have a patch somewhere that unifies strerror and strerror_l at
> the same time, or I can review yours.  I read POSIX as saying that
> strerror and strerror_l can share the same buffer.

Alright, I will rework the patch to add new async-signal-safe symbols to
sys_{sig,err}list.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 16:55               ` Adhemerval Zanella
@ 2020-05-14 17:02                 ` Florian Weimer
  2020-05-14 17:34                   ` Adhemerval Zanella
  0 siblings, 1 reply; 21+ messages in thread
From: Florian Weimer @ 2020-05-14 17:02 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Adhemerval Zanella via Libc-alpha

* Adhemerval Zanella:

> On 14/05/2020 13:51, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> What about moving per-thread state to __thread (generic) / pthread (Linux),
>>> do you have some reservation about it?
>> 
>> No, that makes sense.  You can free allocations via
>> malloc/thread-freeres.c.
>> 
>> I think I have a patch somewhere that unifies strerror and strerror_l at
>> the same time, or I can review yours.  I read POSIX as saying that
>> strerror and strerror_l can share the same buffer.
>
> Alright, I will rework the patch to add new async-signal-safe symbols to
> sys_{sig,err}list.

I think it would be more valuable to return the names of the
*constants*, not the strings.  There's currently no interface for that,
and (almost) everybody rolls their own.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 17:02                 ` Florian Weimer
@ 2020-05-14 17:34                   ` Adhemerval Zanella
  0 siblings, 0 replies; 21+ messages in thread
From: Adhemerval Zanella @ 2020-05-14 17:34 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Adhemerval Zanella via Libc-alpha



On 14/05/2020 14:02, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 14/05/2020 13:51, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> What about moving per-thread state to __thread (generic) / pthread (Linux),
>>>> do you have some reservation about it?
>>>
>>> No, that makes sense.  You can free allocations via
>>> malloc/thread-freeres.c.
>>>
>>> I think I have a patch somewhere that unifies strerror and strerror_l at
>>> the same time, or I can review yours.  I read POSIX as saying that
>>> strerror and strerror_l can share the same buffer.
>>
>> Alright, I will rework the patch to add new async-signal-safe symbols to
>> sys_{sig,err}list.
> 
> I think it would be more valuable to return the names of the
> *constants*, not the strings.  There's currently no interface for that,
> and (almost) everybody rolls their own.

For signal we have two list, sys_siglist and sys_sigabbrev and former 
returns the signal name.  We don't an analogous for errnos though.

What I have in mind would be something like:

  const char *strlist_abbrev (int num);
  
    Return the signal NUM name (for instance,'INT' for SIGINT) or 
    NULL if NUM is not present in the internal list.  No translation
    is done neither a string is created to indicate invalid NUM.

  const char *strlist_descr (int num);

    Return the signal NUM description (for instance,'Interrupt' for
    SIGINT) or NULL if NUM is not present in the internal list.  No
    translation is done neither a string is created to indicate invalid
    NUM.

And for strerror analogous: 

  const char *errlist_abbrev (int num);
  
    Return the errno NUM name (for instance,'EINVAL' for EINVAL) or 
    NULL if NUM is not present in the internal list.  No translation
    is done neither a string is created to indicate invalid NUM.

  const char *errlist_descr (int num);

    Return the signal NUM description (for instance,'Invalid argument' 
    for EINVAL) or NULL if NUM is not present in the internal list.  No
    translation is done neither a string is created to indicate invalid
    NUM.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC 0/4] Make strsignal and strerror async-signal-safe
  2020-05-14 15:11     ` Zack Weinberg
  2020-05-14 16:42       ` Adhemerval Zanella
@ 2020-05-14 18:39       ` Florian Weimer
  1 sibling, 0 replies; 21+ messages in thread
From: Florian Weimer @ 2020-05-14 18:39 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Adhemerval Zanella, Adhemerval Zanella via Libc-alpha

* Zack Weinberg:

> It would be nice if there was a way for strerror to detect that it
> wasn't safe for it to call dcgettext or allocate memory.  In principle
> we ought to be able to try-lock the "I might need to initialize the
> global locale now" lock but, looking at the guts of dcgettext, there's
> several different locks involved and it's not clear to me that we
> could make this work without a major overhaul of libintl.

Right.

> (Giant-hammer idea: have sigaction wrap all signal handlers with a
> routine that sets a thread-local flag while calling the signal
> handler.  siglongjmp and swapcontext would need to restore this flag.)

It's hard to ensure that the sigaction flags match the registered signal
handler at all times, though.  Trampolines would work, but they
introduce a lot of complexity.  There is some discussion here:

  <https://sourceware.org/glibc/wiki/SignalHandlerWrapper>

Thanks,
Florian


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2020-05-14 18:39 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 20:26 [RFC 0/4] Make strsignal and strerror async-signal-safe Adhemerval Zanella
2020-05-13 20:26 ` [RFC 1/4] string: Make strsignal async-signal-safe Adhemerval Zanella
2020-05-14  7:15   ` Andreas Schwab
2020-05-13 20:26 ` [RFC 2/4] string: Add strsignal_l Adhemerval Zanella
2020-05-14  0:16   ` Joseph Myers
2020-05-13 20:26 ` [RFC 3/4] string: Make strerror async-signal-safe Adhemerval Zanella
2020-05-14  7:12   ` Andreas Schwab
2020-05-13 20:26 ` [RFC 4/4] string: Move strerror_l pointer to tls_internal.h Adhemerval Zanella
2020-05-14 12:05 ` [RFC 0/4] Make strsignal and strerror async-signal-safe Florian Weimer
2020-05-14 12:47   ` Adhemerval Zanella
2020-05-14 15:11     ` Zack Weinberg
2020-05-14 16:42       ` Adhemerval Zanella
2020-05-14 18:39       ` Florian Weimer
2020-05-14 15:16     ` Florian Weimer
2020-05-14 16:28       ` Adhemerval Zanella
2020-05-14 16:40         ` Florian Weimer
2020-05-14 16:46           ` Adhemerval Zanella
2020-05-14 16:51             ` Florian Weimer
2020-05-14 16:55               ` Adhemerval Zanella
2020-05-14 17:02                 ` Florian Weimer
2020-05-14 17:34                   ` Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).