On 9/28/20 7:02 PM, Bruno Haible wrote:
#define USE_LIBSIGSEGV (HAVE_XSI_STACK_OVERFLOW_HEURISTIC && HAVE_LIBSIGSEGV)
There seems to be a logic mistake, here.
You're right, it's a typo: there should be a "!" before the
HAVE_XSI_STACK_OVERFLOW_HEURISTIC. As a result of this mistake, libsigsegv is
used only on Solaris-like platforms, where it's not needed, and libsigsegv is
not used on other platforms like GNU/Linux where it can be helpful.
This mistake causes some configuration failures on Solaris on platforms where
libsigsegv has been installed but grep was not configured correctly. It also
means that the stack-overflow reporting on GNU/Linux is too generous, in that
segmentation violations that are not stack overflows get reported as stack
overflows. I installed the first attached patch to correct this. I would guess
that this is not much of a problem in practice except when installing on Solaris
with badly-configured libsigsegv, since from the user's point of view the
problem is only that when grep crashes the wrong message might be printed.
While looking into this error I noticed that c-stack assumes SIGSTKSZ is an
integer constant expression, but there have been moves to make it non-constant
(and this may already be in effect on some platforms). I installed the second
attached patch to work around this potential portability bug; it has a URL
pointing to recent discussions in this area.
The third patch merely streamlines 'configure' when running on platforms like
Solaris that need not use libsigsegv.
>From f20816f7d25b75c111defa27081b2b614c31dc52 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Tue, 29 Sep 2020 14:11:22 -0700
Subject: [PATCH 1/3] c-stack: fix libsigsegv typo
Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2020-09/msg00175.html
* lib/c-stack.c (USE_LIBSIGSEGV): Fix typo that caused libsigsegv
to be used only on Solaris (exactly where it is not needed!).
---
ChangeLog | 8 ++++++++
lib/c-stack.c | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index e6c8079a8..76a76fbc4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2020-10-03 Paul Eggert <egg...@cs.ucla.edu>
+
+ c-stack: fix libsigsegv typo
+ Problem reported by Bruno Haible in:
+ https://lists.gnu.org/r/bug-gnulib/2020-09/msg00175.html
+ * lib/c-stack.c (USE_LIBSIGSEGV): Fix typo that caused libsigsegv
+ to be used only on Solaris (exactly where it is not needed!).
+
2020-10-03 Thien-Thi Nguyen <t...@gnuvola.org>
MODULES.html.sh: Fix typo.
diff --git a/lib/c-stack.c b/lib/c-stack.c
index 742eb023e..80ebcbf00 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -66,7 +66,7 @@ typedef struct sigaltstack stack_t;
/* Use libsigsegv only if needed; kernels like Solaris can detect
stack overflow without the overhead of an external library. */
-#define USE_LIBSIGSEGV (HAVE_XSI_STACK_OVERFLOW_HEURISTIC && HAVE_LIBSIGSEGV)
+#define USE_LIBSIGSEGV (!HAVE_XSI_STACK_OVERFLOW_HEURISTIC && HAVE_LIBSIGSEGV)
#if USE_LIBSIGSEGV
# include <sigsegv.h>
--
2.25.1
>From f9e2b20a12a230efa30f1d479563ae07d276a94b Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Wed, 30 Sep 2020 13:50:36 -0700
Subject: [PATCH 2/3] c-stack: stop using SIGSTKSZ
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
It’s been proposed to stop making SIGSTKSZ an integer constant:
https://sourceware.org/pipermail/libc-alpha/2020-September/118028.html
Also, using SIGSTKSZ in #if did not conform to current POSIX.
Also, avoiding SIGSTKSZ makes the code simpler and easier to grok.
* lib/c-stack.c (SIGSTKSZ): Remove.
(alternate_signal_stack): Now a 64 KiB array, for simplicity.
All uses changed.
---
ChangeLog | 9 +++++++++
lib/c-stack.c | 42 ++++++++++++++++++------------------------
lib/c-stack.h | 2 +-
3 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 76a76fbc4..7f54b7860 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
2020-10-03 Paul Eggert <egg...@cs.ucla.edu>
+ c-stack: stop using SIGSTKSZ
+ It’s been proposed to stop making SIGSTKSZ an integer constant:
+ https://sourceware.org/pipermail/libc-alpha/2020-September/118028.html
+ Also, using SIGSTKSZ in #if did not conform to current POSIX.
+ Also, avoiding SIGSTKSZ makes the code simpler and easier to grok.
+ * lib/c-stack.c (SIGSTKSZ): Remove.
+ (alternate_signal_stack): Now a 64 KiB array, for simplicity.
+ All uses changed.
+
c-stack: fix libsigsegv typo
Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2020-09/msg00175.html
diff --git a/lib/c-stack.c b/lib/c-stack.c
index 80ebcbf00..cf0fe8da0 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -70,15 +70,6 @@ typedef struct sigaltstack stack_t;
#if USE_LIBSIGSEGV
# include <sigsegv.h>
-/* libsigsegv 2.6 through 2.8 have a bug where some architectures use
- more than the Linux default of an 8k alternate stack when deciding
- if a fault was caused by stack overflow. */
-# if LIBSIGSEGV_VERSION <= 0x0208 && SIGSTKSZ < 16384
-# undef SIGSTKSZ
-# endif
-#endif
-#ifndef SIGSTKSZ
-# define SIGSTKSZ 16384
#endif
#include "exitfail.h"
@@ -95,6 +86,16 @@ typedef struct sigaltstack stack_t;
# endif
#endif
+/* Storage for the alternate signal stack.
+ 64 KiB is not too large for Gnulib-using apps, and is large enough
+ for all known platforms. Smaller sizes may run into trouble.
+ For example, libsigsegv 2.6 through 2.8 have a bug where some
+ architectures use more than the Linux default of an 8 KiB alternate
+ stack when deciding if a fault was caused by stack overflow. */
+static max_align_t alternate_signal_stack[(64 * 1024
+ + sizeof (max_align_t) - 1)
+ / sizeof (max_align_t)];
+
/* The user-specified action to take when a SEGV-related program error
or stack overflow occurs. */
static _GL_ASYNC_SAFE void (* volatile segv_action) (int);
@@ -133,7 +134,7 @@ die (int signo)
size_t prognamelen = strlen (progname);
size_t messagelen = strlen (message);
static char const separator[] = {':', ' '};
- char buf[SIGSTKSZ / 16 + sizeof separator];
+ char buf[sizeof alternate_signal_stack / 16 + sizeof separator];
ptrdiff_t buflen;
if (prognamelen + messagelen < sizeof buf - sizeof separator)
{
@@ -159,13 +160,6 @@ die (int signo)
abort ();
}
-/* Storage for the alternate signal stack. */
-static union
-{
- char buffer[SIGSTKSZ];
- max_align_t align;
-} alternate_signal_stack;
-
static _GL_ASYNC_SAFE void
null_action (int signo _GL_UNUSED)
{
@@ -230,8 +224,8 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
/* Always install the overflow handler. */
if (stackoverflow_install_handler (overflow_handler,
- alternate_signal_stack.buffer,
- sizeof alternate_signal_stack.buffer))
+ alternate_signal_stack,
+ sizeof alternate_signal_stack))
{
errno = ENOTSUP;
return -1;
@@ -323,14 +317,14 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
{
stack_t st;
st.ss_flags = 0;
+ st.ss_sp = alternate_signal_stack;
+ st.ss_size = sizeof alternate_signal_stack;
# if SIGALTSTACK_SS_REVERSED
/* Irix mistakenly treats ss_sp as the upper bound, rather than
lower bound, of the alternate stack. */
- st.ss_sp = alternate_signal_stack.buffer + SIGSTKSZ - sizeof (void *);
- st.ss_size = sizeof alternate_signal_stack.buffer - sizeof (void *);
-# else
- st.ss_sp = alternate_signal_stack.buffer;
- st.ss_size = sizeof alternate_signal_stack.buffer;
+ st.ss_size -= sizeof (void *);
+ char *ss_sp = st.ss_sp;
+ st.ss_sp = ss_sp + st.ss_size;
# endif
int r = sigaltstack (&st, NULL);
if (r != 0)
diff --git a/lib/c-stack.h b/lib/c-stack.h
index a11fa3123..a119ef29e 100644
--- a/lib/c-stack.h
+++ b/lib/c-stack.h
@@ -34,7 +34,7 @@
A null ACTION acts like an action that does nothing.
ACTION must be async-signal-safe. ACTION together with its callees
- must not require more than SIGSTKSZ bytes of stack space. Also,
+ must not require more than 64 KiB of stack space. Also,
ACTION should not call longjmp, because this implementation does
not guarantee that it is safe to return to the original stack.
--
2.25.1
>From e14729f3c9094731844421bb37c542b05a5b6953 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 3 Oct 2020 12:51:08 -0700
Subject: [PATCH 3/3] c-stack: streamline Solaris configuration
* lib/c-stack.c: Omit mention of HAVE_SIGALTSTACK, since
the code is used only if a test for sigaltstack worked
in some other way.
* m4/c-stack.m4 (gl_PREREQ_C_STACK): Do not require gl_LIBSIGSEGV;
instead, execute gl_LIBSIGSEGV only if needed (because the XSI
heuristic does not work).
* modules/c-stack (Files): Add m4/libsigsegv.m4, since
we no longer require the libsigsegv module.
(Depends-on): Depend on havelib, not libsigsegv.
---
ChangeLog | 11 +++++++++++
lib/c-stack.c | 8 +++-----
m4/c-stack.m4 | 15 +++++++--------
modules/c-stack | 3 ++-
4 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 7f54b7860..7b528a73d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
2020-10-03 Paul Eggert <egg...@cs.ucla.edu>
+ c-stack: streamline Solaris configuration
+ * lib/c-stack.c: Omit mention of HAVE_SIGALTSTACK, since
+ the code is used only if a test for sigaltstack worked
+ in some other way.
+ * m4/c-stack.m4 (gl_PREREQ_C_STACK): Do not require gl_LIBSIGSEGV;
+ instead, execute gl_LIBSIGSEGV only if needed (because the XSI
+ heuristic does not work).
+ * modules/c-stack (Files): Add m4/libsigsegv.m4, since
+ we no longer require the libsigsegv module.
+ (Depends-on): Depend on havelib, not libsigsegv.
+
c-stack: stop using SIGSTKSZ
It’s been proposed to stop making SIGSTKSZ an integer constant:
https://sourceware.org/pipermail/libc-alpha/2020-September/118028.html
diff --git a/lib/c-stack.c b/lib/c-stack.c
index cf0fe8da0..187bcafff 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -107,8 +107,7 @@ static char const * volatile program_error_message;
static char const * volatile stack_overflow_message;
#if (USE_LIBSIGSEGV \
- || (HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK \
- && HAVE_STACK_OVERFLOW_HANDLING))
+ || (HAVE_DECL_SIGALTSTACK && HAVE_STACK_OVERFLOW_HANDLING))
/* Output an error message, then exit with status EXIT_FAILURE if it
appears to have been a stack overflow, or with a core dump
@@ -236,7 +235,7 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
return 0;
}
-#elif HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK && HAVE_STACK_OVERFLOW_HANDLING
+#elif HAVE_DECL_SIGALTSTACK && HAVE_STACK_OVERFLOW_HANDLING
# if SIGINFO_WORKS
@@ -361,8 +360,7 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
}
#else /* ! (USE_LIBSIGSEGV
- || (HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK
- && HAVE_STACK_OVERFLOW_HANDLING)) */
+ || (HAVE_DECL_SIGALTSTACK && HAVE_STACK_OVERFLOW_HANDLING)) */
int
c_stack_action (_GL_ASYNC_SAFE void (*action) (int) _GL_UNUSED)
diff --git a/m4/c-stack.m4 b/m4/c-stack.m4
index e98f80fff..1523a724d 100644
--- a/m4/c-stack.m4
+++ b/m4/c-stack.m4
@@ -7,7 +7,7 @@
# Written by Paul Eggert.
-# serial 18
+# serial 19
AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
[
@@ -351,21 +351,20 @@ int main ()
AC_DEFUN([gl_PREREQ_C_STACK],
[AC_REQUIRE([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC])
- AC_REQUIRE([gl_LIBSIGSEGV])
AC_CHECK_FUNCS_ONCE([sigaltstack])
AC_CHECK_DECLS([sigaltstack], , , [[#include <signal.h>]])
AC_CHECK_HEADERS_ONCE([ucontext.h])
- AC_CHECK_TYPES([stack_t], , , [#include <signal.h>])
+ AC_CHECK_TYPES([stack_t], , , [[#include <signal.h>]])
dnl c-stack does not need -lsigsegv if the system has XSI heuristics.
- if test "$gl_cv_lib_sigsegv" = yes \
- && test "$gl_cv_sys_xsi_stack_overflow_heuristic" != yes; then
- AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
- AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])
- fi
+ AS_IF([test "$gl_cv_sys_xsi_stack_overflow_heuristic" != yes],
+ [gl_LIBSIGSEGV
+ AS_IF([test "$gl_cv_lib_sigsegv" = yes],
+ [AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
+ AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])])])
])
AC_DEFUN([gl_C_STACK],
diff --git a/modules/c-stack b/modules/c-stack
index 5ddf6e6f5..77cf6aab8 100644
--- a/modules/c-stack
+++ b/modules/c-stack
@@ -5,6 +5,7 @@ Files:
lib/c-stack.h
lib/c-stack.c
m4/c-stack.m4
+m4/libsigsegv.m4
Depends-on:
c99
@@ -12,10 +13,10 @@ errno
exitfail
getprogname
gettext-h
+havelib
ignore-value
intprops
inttypes
-libsigsegv
mempcpy
raise
sigaction
--
2.25.1
