grep-3.5 fails to build on Solaris when libsigsegv is installed

grep-3.5 fails to build on Solaris when libsigsegv is installed

[Bruno Haible]

Hi Paul,

On Solaris 10, when libsigsegv is installed with the same --prefix as
passed to grep's configure:
  - grep 3.4 builds fine.
  - grep 3.5 fails to build:

  CCLD     grep
ld: warning: file /PREFIX/lib/libiconv.so: attempted multiple inclusion of file
Undefined                       first referenced
 symbol                             in file
stackoverflow_install_handler       ../lib/libgreputils.a(c-stack.o)
sigsegv_install_handler             ../lib/libgreputils.a(c-stack.o)
ld: fatal: symbol referencing errors. No output written to grep
*** Error code 1

Also, a build error is seen in the gnulib-tests directory:
$ gmake check
...
  CCLD     test-c-stack
Undefined                       first referenced
 symbol                             in file
stackoverflow_install_handler       ../lib/libgreputils.a(c-stack.o)
sigsegv_install_handler             ../lib/libgreputils.a(c-stack.o)
ld: fatal: symbol referencing errors. No output written to test-c-stack
gmake[3]: *** [Makefile:3301: test-c-stack] Error 1

In both versions, config.status contains
S["LIBSIGSEGV"]="/PREFIX/lib/libsigsegv.a -lc"

The difference is that in grep-3.4
S["LIBCSTACK"]="/PREFIX/lib/libsigsegv.a -lc"
whereas in grep-3.5
S["LIBCSTACK"]=""

I can reproduce the problem with a testdir of module 'c-stack'.

This patch fixes it. BUT... see below.


2020-09-28  Bruno Haible  <bruno@clisp.org>

	c-stack: Fix link error when libsigsegv is used (regression 2020-09-20).
	* m4/c-stack.m4 (gl_PREREQ_C_STACK): Link with libsigsegv when
	libsigsegv is installed and stack overflow heuristics work.

diff --git a/m4/c-stack.m4 b/m4/c-stack.m4
index e98f80f..b86fc8f 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],
   [
@@ -360,9 +360,10 @@ AC_DEFUN([gl_PREREQ_C_STACK],
 
    AC_CHECK_TYPES([stack_t], , , [#include <signal.h>])
 
-   dnl c-stack does not need -lsigsegv if the system has XSI heuristics.
+   dnl c-stack.c uses libsigsegv when HAVE_XSI_STACK_OVERFLOW_HEURISTIC and
+   dnl HAVE_LIBSIGSEGV are both 1.
    if test "$gl_cv_lib_sigsegv" = yes \
-       && test "$gl_cv_sys_xsi_stack_overflow_heuristic" != yes; then
+       && test "$gl_cv_sys_xsi_stack_overflow_heuristic" = yes; then
      AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
      AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])
    fi


BUT I don't want to push it since there seems to be also a mistake in
c-stack.c. Before the 2020-09-20 patch, the idea of c-stack.c was:
  - If HAVE_XSI_STACK_OVERFLOW_HEURISTIC is defined, use that heuristic.
  - Otherwise, if libsigsegv is present, use it to catch stack overflows.
    (It contains 3 different heuristics.)
  - Otherwise, punt.

There was a comment:

/* Only use libsigsegv if we need it; platforms like Solaris can
   detect stack overflow without the overhead of an external
   library.  */
#if HAVE_LIBSIGSEGV && ! HAVE_XSI_STACK_OVERFLOW_HEURISTIC

In the new code, there is a comment:

/* 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)

There seems to be a logic mistake, here.

Bruno

release process analysis

[Bruno Haible]

Hi Paul,

It's a pity that grep-2.5 was released with such a mistake.

What was the cause, and how could it be avoided?
  * One could say that more testing would have uncovered it. But
    - we have limited testing resources (Jeffrey, Assaf, and I are the only
      people who regularly test on several platforms),
    - we don't have continuous integration on Solaris,
    - the continuous integration on Ubuntu that I am maintaining does not
      include a preinstalled libsigsegv.
  * We send the patches to the list in the hope that they will be reviewed
    even if committed immediately. I usually do review your patches, except
      - when it's code I don't understand (like regex and dfa),
      - when it's too big.
    In this case, I didn't review it because it looked too frightening.
    8ba9126d00bfe1ab77a5c820c58c68933d4df85c contained more than 10 distinct
    changes. It would have been more likely that I review it if it had been
    split across 3 or more patches.

How can we avoid such things?

  (a) By reducing the size / complexity of patches? (Yes, I know, when looking
      at a bunch of old code, it is tempting to make so many improvements at
      once.)

  (b) Introduce a formal checklist of patch reviews? So that at any moment we
      can see which patches have been reviewed and which haven't. And the
      'grep' release manager could use this checklist in order to delay the
      release until the reviews have been completed.

  - or other suggestions?

Bruno

Re: release process analysis

[Dagobert Michelsen]

Hi Bruno,

Am 29.09.2020 um 04:28 schrieb Bruno Haible <bruno@clisp.org>:
>    - we don't have continuous integration on Solaris,

This is not correct: there is continous integration on Solaris
for quite some time now and status changes are reported
automatically to grep-devel@gnu.org as well:
  https://buildfarm.opencsw.org/buildbot/waterfall?category=ggrep
If there are any changes needed please let me know.


Best regards

  — Dago

-- 
"You don't become great by trying to be great, you become great by wanting to do something,
and then doing it so hard that you become great in the process." - xkcd #896

Re: release process analysis

[Paul Eggert]

On 9/29/20 12:00 AM, Dagobert Michelsen wrote:
> This is not correct: there is continous integration on Solaris
> for quite some time now

Yes, and I used that with this patch. Unfortunately the failure mode occurs only 
when libsigsegv is installed 3rd-party on Solaris, which was not the situation 
on the continuous-integration platform.

Re: release process analysis

[Jeffrey Walton]

On Mon, Sep 28, 2020 at 10:28 PM Bruno Haible <bruno@clisp.org> wrote:
>
> It's a pity that grep-2.5 was released with such a mistake.
> ...
> How can we avoid such things?

Just my 2-cents, but I think the technical controls are good. You have
a CI pipeline, and you are testing on multiple platforms. You are also
asking the community to test on their favorite hardware and platforms
during development and before a release. You may be able to increase
platform coverage, but I don't think that is the issue.

I think the next improvement to the release process should engineer
around human behavior. Here, the human behavior is, most folks avoid
pre-release testing, and then jump right to the release. Then they
find there are small issues that made it into the release.

Here's how several projects I work with handle it.

1. CI pipelines to provide assurances on Master
2. Prior to release, create a RC
3. Call for testers
4. Accept feedback on the RC
5. Fix issues, back to (2)

Once you are happy with the state of the code, release. In the case of
Grep, say you release Grep 3.5

6. Release the updated software

Now here are the new steps to consider:

7. Accept feedback on the release
8. Fix issues
9. Release a minor version at T+30 days

Step (7) is the standard feedback loop in a software development
lifecycle. You have two feedback cycles: one at Step (4), and one at
Step (7). That's OK. It is simply stating what you already do - you
fix issues.

At step (9), you simply release Grep 3.5.1 at T+30 days if there are
bugs in Grep 3.5 that are generating mailing list messages and bug
reports. The mailing list messages and bug reports will be recurring
because they happen in a release tarball.

Steps (7) - (9) take into account that many folks will not perform
pre-release testing. But they will update and then report problems
back to the project.

It has been my experience that once you perform the minor version
release, like Grep 3.5.1, that's it, you're done. You won't have to
worry about it again until Grep 3.6 is released.

And what really happened is, you used an administrative control to
engineer around human behavior. You did not add any new technical
controls.

Jeff

Re: release process analysis

[Dagobert Michelsen]

Hi Paul,

Am 29.09.2020 um 22:56 schrieb Paul Eggert <eggert@cs.ucla.edu>:
> On 9/29/20 12:00 AM, Dagobert Michelsen wrote:
>> This is not correct: there is continous integration on Solaris
>> for quite some time now
> 
> Yes, and I used that with this patch. Unfortunately the failure mode occurs only when libsigsegv is installed 3rd-party on Solaris, which was not the situation on the continuous-integration platform.

But libsigsegv is installed on both machines where the CI on Solaris runs.
However, it only seems to show up on i386 and not Sparc (at least on my
installations), but it was visible starting 24th September:
  https://buildfarm.opencsw.org/buildbot/builders/ggrep-solaris10-i386

So the CI build failed but nobody noticed. And indeed when I look at the
mailing list archive for September
  https://lists.gnu.org/archive/html/grep-devel/2020-09/threads.html
I only see messages from the buildbot on Sparc and not i386! When reviewing
the buildbot configuration I noticed a typo in the configuration that
associates email notifications with builders which preventer i386 from
working. I guess I am to blame for this…

For next time I corrected the notification so both platforms will be
reported now. Also it is probably worthwhile to check why only the
i386 installation shows the error but not the sparc builder.


Best regards

  — Dago

-- 
"You don't become great by trying to be great, you become great by wanting to do something,
and then doing it so hard that you become great in the process." - xkcd #896

Re: release process analysis

[Bruno Haible]

Jeffrey Walton wrote:
> I think the next improvement to the release process should engineer
> around human behavior. Here, the human behavior is, most folks avoid
> pre-release testing, and then jump right to the release.
> ...
> It has been my experience that once you perform the minor version
> release, like Grep 3.5.1, that's it, you're done.

Indeed. Thanks for stating it so clearly.

I so much strive for a perfect release, that I often forget about the
"relese early, release often" principle [1].

Bruno

[1] https://en.wikipedia.org/wiki/Release_early,_release_often

Re: grep-3.5 fails to build on Solaris when libsigsegv is installed

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1555 bytes --]

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.

[-- Attachment #2: 0001-c-stack-fix-libsigsegv-typo.patch --]
[-- Type: text/x-patch, Size: 1559 bytes --]

From f20816f7d25b75c111defa27081b2b614c31dc52 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@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  <eggert@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  <ttn@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


[-- Attachment #3: 0002-c-stack-stop-using-SIGSTKSZ.patch --]
[-- Type: text/x-patch, Size: 5555 bytes --]

From f9e2b20a12a230efa30f1d479563ae07d276a94b Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@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  <eggert@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


[-- Attachment #4: 0003-c-stack-streamline-Solaris-configuration.patch --]
[-- Type: text/x-patch, Size: 4659 bytes --]

From e14729f3c9094731844421bb37c542b05a5b6953 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@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  <eggert@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

Re: AS_IF

[Bruno Haible]

Hi Paul,

> The third patch merely streamlines 'configure' when running on platforms like 
> Solaris that need not use libsigsegv.
> 
> -   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])])])

Any particular reason why AS_IF is used here?

I typically don't use AS_IF because
  - I find a line of shell code more readable than a mix between m4 syntax
    and shell syntax,
  - it's yet another step in the learning curve, for someone who wants to
    understand how Autoconf macros work.

The Autoconf documentation says
  AS_IF "ensures any required macros ... are expanded before
  the first test."
AFAICS, this is relevant for code written directly into configure.ac. But
inside an AC_DEFUN it is irrelevant, because required macros are hoisted
before the body of the AC_DEFUN anyway.

Bruno

Re: AS_IF

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

On 10/4/20 4:40 PM, Bruno Haible wrote:
> AFAICS, this is relevant for code written directly into configure.ac. But
> inside an AC_DEFUN it is irrelevant, because required macros are hoisted
> before the body of the AC_DEFUN anyway.

I was playing it safe based on this NEWS item from Autoconf 2.69c:

    - Autoconf macros that use AC_REQUIRE internally, are not safe to
      use inside of hand-written shell control-flow constructs.  Use
      AS_IF, AS_CASE, AS_FOR, etc. instead.  (See the “Prerequisite
      Macros” section of the manual for further explanation.)

Since I didn't know whether the macros in question used AC_REQUIRE internally, I 
played it safe.

But now that you mention it, AS_IF isn't needed here and I installed the 
attached further patch. Also, to help prevent others from making a similar 
mistake, I plan to reword the Autoconf NEWS item to start with something like 
"Autoconf macros that use AC_REQUIRE are not safe to use in hand-written shell 
control-flow constructs that appear outside of macros defined by AC_DEFUN."

[-- Attachment #2: 0001-c-stack-avoid-AS_IF.patch --]
[-- Type: text/x-patch, Size: 1880 bytes --]

From f636f37983b32d8db3abea7ecdda928f3a0f542e Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 4 Oct 2020 18:46:44 -0700
Subject: [PATCH] c-stack: avoid AS_IF

Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2020-10/msg00018.html
* m4/c-stack.m4 (gl_PREREQ_C_STACK): No need for AS_IF.
---
 ChangeLog     |  5 +++++
 m4/c-stack.m4 | 14 ++++++++------
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e0c821f7d..4ec03efe4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2020-10-04  Paul Eggert  <eggert@cs.ucla.edu>
 
+	c-stack: avoid AS_IF
+	Problem reported by Bruno Haible in:
+	https://lists.gnu.org/r/bug-gnulib/2020-10/msg00018.html
+	* m4/c-stack.m4 (gl_PREREQ_C_STACK): No need for AS_IF.
+
 	c-stack: pacify GCC 9.3.1 when using libsigsegv
 	* lib/c-stack.c [USE_LIBSIGSEGV]: Disable --suggest-attribute=pure.
 
diff --git a/m4/c-stack.m4 b/m4/c-stack.m4
index 1523a724d..85107f465 100644
--- a/m4/c-stack.m4
+++ b/m4/c-stack.m4
@@ -7,7 +7,7 @@
 
 # Written by Paul Eggert.
 
-# serial 19
+# serial 20
 
 AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
   [
@@ -360,11 +360,13 @@ AC_DEFUN([gl_PREREQ_C_STACK],
    AC_CHECK_TYPES([stack_t], , , [[#include <signal.h>]])
 
    dnl c-stack does not need -lsigsegv if the system has XSI heuristics.
-   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])])])
+   if test "$gl_cv_sys_xsi_stack_overflow_heuristic" != yes; then
+     gl_LIBSIGSEGV
+     if test "$gl_cv_lib_sigsegv" = yes; then
+       AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
+       AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])
+     fi
+   fi
 ])
 
 AC_DEFUN([gl_C_STACK],
-- 
2.25.1

Re: grep-3.5 fails to build on Solaris when libsigsegv is installed

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 109 bytes --]

Further testing suggested one more patch (attached), which I also installed into 
Gnulib master on savannah.

[-- Attachment #2: 0001-c-stack-pacify-GCC-9.3.1-when-using-libsigsegv.patch --]
[-- Type: text/x-patch, Size: 1259 bytes --]

From 05b2d40dd751dd182f58118f4d3273ad8136b4ee Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 4 Oct 2020 18:12:50 -0700
Subject: [PATCH] c-stack: pacify GCC 9.3.1 when using libsigsegv

* lib/c-stack.c [USE_LIBSIGSEGV]: Disable --suggest-attribute=pure.
---
 ChangeLog     | 5 +++++
 lib/c-stack.c | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 83b92302d..e0c821f7d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2020-10-04  Paul Eggert  <eggert@cs.ucla.edu>
+
+	c-stack: pacify GCC 9.3.1 when using libsigsegv
+	* lib/c-stack.c [USE_LIBSIGSEGV]: Disable --suggest-attribute=pure.
+
 2020-10-04  Bruno Haible  <bruno@clisp.org>
 
 	localename: Fix a couple of "unused parameter" warnings.
diff --git a/lib/c-stack.c b/lib/c-stack.c
index 187bcafff..3aea16acd 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -168,6 +168,11 @@ null_action (int signo _GL_UNUSED)
 
 #if USE_LIBSIGSEGV
 
+/* Pacify GCC 9.3.1, which otherwise would complain about segv_handler.  */
+# if __GNUC_PREREQ (4, 6)
+#  pragma GCC diagnostic ignored "-Wsuggest-attribute=pure"
+# endif
+
 /* Nonzero if general segv handler could not be installed.  */
 static volatile int segv_handler_missing;
 
-- 
2.25.1

Re: AS_IF

[Bruno Haible]

[-- Attachment #1: Type: text/plain, Size: 1497 bytes --]

[CCing autoconf-patches]

Paul Eggert wrote in
<https://lists.gnu.org/archive/html/bug-gnulib/2020-10/msg00019.html>:
> > AFAICS, this is relevant for code written directly into configure.ac. But
> > inside an AC_DEFUN it is irrelevant, because required macros are hoisted
> > before the body of the AC_DEFUN anyway.
> 
> I was playing it safe based on this NEWS item from Autoconf 2.69c:
> 
>     - Autoconf macros that use AC_REQUIRE internally, are not safe to
>       use inside of hand-written shell control-flow constructs.  Use
>       AS_IF, AS_CASE, AS_FOR, etc. instead.  (See the “Prerequisite
>       Macros” section of the manual for further explanation.)
> 
> Since I didn't know whether the macros in question used AC_REQUIRE internally, I 
> played it safe.
> 
> But now that you mention it, AS_IF isn't needed here and I installed the 
> attached further patch. Also, to help prevent others from making a similar 
> mistake, I plan to reword the Autoconf NEWS item to start with something like 
> "Autoconf macros that use AC_REQUIRE are not safe to use in hand-written shell 
> control-flow constructs that appear outside of macros defined by AC_DEFUN."

Thanks. But if you, Paul, make an incorrect assumption about what AS_IF really
does, how many more users (who read only the documentation) will make the same
mistake? I therefore think the documentation should be enhanced to avoid this
misinterpretation.

Proposed patch attached.

Bruno

[-- Attachment #2: 0001-Clarify-that-AS_IF-and-AS_CASE-are-overkill-inside-A.patch --]
[-- Type: text/x-patch, Size: 2960 bytes --]

From 11b827d202aa07b2b8c8f894d8f49844d570dfe9 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Tue, 6 Oct 2020 23:46:42 +0200
Subject: [PATCH] Clarify that AS_IF and AS_CASE are overkill inside AC_DEFUN.

* doc/autoconf.texi (Common Shell Constructs): Document that AS_IF and AS_CASE
provide no benefit regarding required macros when used inside macros defined
by AC_DEFUN or AC_DEFUN_ONCE.
* NEWS: Don't mention the undocumented macro AS_FOR.
---
 NEWS              |  4 ++--
 doc/autoconf.texi | 16 +++++++++++++++-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 55e938b..5335709 100644
--- a/NEWS
+++ b/NEWS
@@ -36,8 +36,8 @@ GNU Autoconf NEWS - User visible changes.
 
    - Autoconf macros that use AC_REQUIRE internally, are not safe to
      use inside of hand-written shell control-flow constructs.  Use
-     AS_IF, AS_CASE, AS_FOR, etc. instead.  (See the “Prerequisite
-     Macros” section of the manual for further explanation.)
+     AS_IF, AS_CASE, etc. instead.  (See the “Prerequisite Macros”
+     section of the manual for further explanation.)
 
      The set of macros that use AC_REQUIRE internally may change from
      release to release.  The only macros that are guaranteed *not* to
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index dea85e4..f8d8f8b 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -13759,7 +13759,15 @@ Expand into a shell @samp{case} statement, where @var{word} is matched
 against one or more patterns.  @var{if-matched} is run if the
 corresponding pattern matched @var{word}, else @var{default} is run.
 Avoids several portability issues (@pxref{case, , Limitations of Shell
-Builtins}).
+Builtins}).  Also, @code{AS_CASE} ensures any macros required by
+@var{if-matched1}, @dots{}, @var{default} are expanded before the
+@samp{case} statement.
+
+The handling of required macros provided by @code{AS_CASE} is useful
+in @file{configure.ac}, outside of macros defined by @code{AC_DEFUN}
+or @code{AC_DEFUN_ONCE}.  Inside macros defined by @code{AC_DEFUN}
+or @code{AC_DEFUN_ONCE}, @code{AS_CASE} and the plain shell @samp{case}
+syntax are equivalent.
 @end defmac
 
 @c Deprecated, to be replaced by a better API
@@ -13881,6 +13889,12 @@ blanks, or expand to a nonempty shell command.  For example,
 argument contains the nonblank characters @code{[]} which expand to
 nothing.  This restriction on @var{run-if-false} also applies to other
 macros with ``if-false'' arguments denoting shell commands.
+
+The handling of required macros provided by @code{AS_IF} is useful
+in @file{configure.ac}, outside of macros defined by @code{AC_DEFUN}
+or @code{AC_DEFUN_ONCE}.  Inside macros defined by @code{AC_DEFUN}
+or @code{AC_DEFUN_ONCE}, @code{AS_IF} and the plain shell @samp{if}
+syntax are equivalent.
 @end defmac
 
 @defmac AS_MKDIR_P (@var{file-name})
-- 
2.7.4

Re: AS_IF

[Zack Weinberg]

On Tue, Oct 6, 2020 at 5:57 PM Bruno Haible <bruno@clisp.org> wrote:
> Paul Eggert wrote in
> <https://lists.gnu.org/archive/html/bug-gnulib/2020-10/msg00019.html>:
> > > AFAICS, this is relevant for code written directly into configure.ac. But
> > > inside an AC_DEFUN it is irrelevant, because required macros are hoisted
> > > before the body of the AC_DEFUN anyway.
...
> > But now that you mention it, AS_IF isn't needed here and I installed the
> > attached further patch. Also, to help prevent others from making a similar
> > mistake, I plan to reword the Autoconf NEWS item to start with something like
> > "Autoconf macros that use AC_REQUIRE are not safe to use in hand-written shell
> > control-flow constructs that appear outside of macros defined by AC_DEFUN."
>
> Thanks. But if you, Paul, make an incorrect assumption about what AS_IF really
> does, how many more users (who read only the documentation) will make the same
> mistake? I therefore think the documentation should be enhanced to avoid this
> misinterpretation.

I agree that this should be clarified; I just had the same
conversation in another thread. I'm not sure we should say AS_IF/CASE
are equivalent to plain shell if/case inside an AC_DEFUN; they do do
more than just make the m4 expansion stack not be empty.

Also, why is AS_FOR not documented? Maybe, instead of avoiding
mentioning it in NEWS, it should be documented?

zw

Re: AS_IF

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

On 10/6/20 5:52 PM, Zack Weinberg wrote:

> I agree that this should be clarified; I just had the same
> conversation in another thread. I'm not sure we should say AS_IF/CASE
> are equivalent to plain shell if/case inside an AC_DEFUN; they do do
> more than just make the m4 expansion stack not be empty.

Yes, that goes a bit too far. I installed the attached doc patch to Savannah 
Autoconf master to try to clarify things a bit. I developed this patch before 
seeing Bruno's, but it should address the same issues (plus a few more, like 
portability of Posix case syntax).

> Also, why is AS_FOR not documented? Maybe, instead of avoiding
> mentioning it in NEWS, it should be documented?

AS_FOR is kind of a mess. I wouldn't document it the way it is. Perhaps it could 
be cleaned up in some future Autoconf version.

[-- Attachment #2: 0001-doc-improve-AS_CASE-AS_IF-doc.patch --]
[-- Type: text/x-patch, Size: 17445 bytes --]

From 3cdc910d229d33e5a98e66b64ee1506a0c2e262c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 11 Oct 2020 23:53:41 -0700
Subject: [PATCH] doc: improve AS_CASE, AS_IF doc
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

See the thread containing:
https://lists.gnu.org/r/bug-gnulib/2020-10/msg00033.html
* doc/autoconf.texi: Distinguish between Solaris 10 and later.
(Balancing Parentheses): Mention the Posix syntax for ‘case’,
typically a better solution nowadays.
(AS_CASE, AS_IF): Mention AC_REQUIRE, portability, parens.
(Prerequisite Macros): Tighten up example and make it less dated.
Say that AS_CASE and AS_IF are not needed outside macros.
* NEWS: Don’t mention AS_FOR.  It’s not documented, and for
good reason since it is so ... quirky.
---
 NEWS              |   8 ++--
 doc/autoconf.texi | 115 +++++++++++++++++++++++++++++-----------------
 2 files changed, 77 insertions(+), 46 deletions(-)

diff --git a/NEWS b/NEWS
index be82eab9..356a28b0 100644
--- a/NEWS
+++ b/NEWS
@@ -34,10 +34,10 @@ GNU Autoconf NEWS - User visible changes.
      variables used later in the configure script, or in generated
      Makefiles.
 
-   - Autoconf macros that use AC_REQUIRE internally, are not safe to
-     use inside of hand-written shell control-flow constructs.  Use
-     AS_IF, AS_CASE, AS_FOR, etc. instead.  (See the “Prerequisite
-     Macros” section of the manual for further explanation.)
+   - Autoconf macros that use AC_REQUIRE are not safe to use in shell
+     control-flow constructs that appear outside of macros defined by
+     AC_DEFUN.  Use AS_IF, AS_CASE, etc. instead.  (See the
+     “Prerequisite Macros” section of the manual for details.)
 
      The set of macros that use AC_REQUIRE internally may change from
      release to release.  The only macros that are guaranteed *not* to
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 9f05cee8..9a52fbdb 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -11024,9 +11024,11 @@ Cambridge University computer lab at the time.
 @cindex parentheses, balancing
 @cindex unbalanced parentheses, managing
 
-One of the pitfalls of portable shell programming is that @command{case}
-statements require unbalanced parentheses (@pxref{case, , Limitations of
-Shell Builtins}).  With syntax highlighting
+One of the pitfalls of portable shell programming is that
+if you intend your script to run with obsolescent shells,
+@command{case} statements require unbalanced parentheses.
+@xref{case, , Limitations of Shell Builtins}.
+With syntax highlighting
 editors, the presence of unbalanced @samp{)} can interfere with editors
 that perform syntax highlighting of macro contents based on finding the
 matching @samp{(}.  Another concern is how much editing must be done
@@ -11066,6 +11068,19 @@ variations for defining @code{my_case} to be more robust, even when used
 without proper quoting, each with some benefits and some drawbacks.
 
 @itemize @w{}
+@item Use left parenthesis before pattern
+@example
+AC_DEFUN([my_case],
+[case $file_name in
+  (*.c) echo "C source code";;
+esac])
+@end example
+@noindent
+This is simple and provides balanced parentheses.  Although this is not
+portable to obsolescent shells (notably Solaris 10 @command{/bin/sh}),
+platforms with these shells invariably have a more-modern shell
+available somewhere so this approach typically suffices nowadays.
+
 @item Creative literal shell comment
 @example
 AC_DEFUN([my_case],
@@ -13758,8 +13773,15 @@ log files to separate distinct phases of script operation.
 Expand into a shell @samp{case} statement, where @var{word} is matched
 against one or more patterns.  @var{if-matched} is run if the
 corresponding pattern matched @var{word}, else @var{default} is run.
-Avoids several portability issues (@pxref{case, , Limitations of Shell
-Builtins}).
+@xref{Prerequisite Macros} for why
+this macro should be used instead of plain @samp{case} in code
+outside of an @code{AC_DEFUN} macro, when the contents of the
+@samp{case} use @code{AC_REQUIRE} directly or indirectly.
+@xref{case, , Limitations of Shell Builtins},
+for how this macro avoids some portability issues.
+@xref{Balancing Parentheses}
+for how this macro lets you write code with balanced parentheses
+even if your code must run on obsolescent shells.
 @end defmac
 
 @c Deprecated, to be replaced by a better API
@@ -13881,6 +13903,10 @@ blanks, or expand to a nonempty shell command.  For example,
 argument contains the nonblank characters @code{[]} which expand to
 nothing.  This restriction on @var{run-if-false} also applies to other
 macros with ``if-false'' arguments denoting shell commands.
+
+This macro should be used instead of plain @samp{if} in code
+outside of an @code{AC_DEFUN} macro, when the contents of the @samp{if}
+use @code{AC_REQUIRE} directly or indirectly (@pxref{Prerequisite Macros}).
 @end defmac
 
 @defmac AS_MKDIR_P (@var{file-name})
@@ -14635,7 +14661,7 @@ In particular, @samp{AC_REQUIRE([FOO])} is not replaced with the body of
 @example
 @group
 AC_DEFUN([TRAVOLTA],
-[test "$body_temperature_in_celsius" -gt "38" &&
+[test "$body_temperature_in_celsius" -gt 38 &&
   dance_floor=occupied])
 AC_DEFUN([NEWTON_JOHN],
 [test "x$hair_style" = xcurly &&
@@ -14644,7 +14670,7 @@ AC_DEFUN([NEWTON_JOHN],
 
 @group
 AC_DEFUN([RESERVE_DANCE_FLOOR],
-[if date | grep '^Sat.*pm' >/dev/null 2>&1; then
+[if test "x`date +%A`" = xSaturday; then
   AC_REQUIRE([TRAVOLTA])
   AC_REQUIRE([NEWTON_JOHN])
 fi])
@@ -14663,17 +14689,18 @@ fi
 @end example
 
 @noindent
-does not leave you with a better chance to meet a kindred soul at
-other times than Saturday night since it expands into:
+does not leave you with a better chance to meet a kindred soul on
+days other than Saturday, since the call to @code{RESERVE_DANCE_FLOOR}
+expands to:
 
 @example
 @group
-test "$body_temperature_in_Celsius" -gt "38" &&
+test "$body_temperature_in_Celsius" -gt 38 &&
   dance_floor=occupied
 test "x$hair_style" = xcurly &&
   dance_floor=occupied
 fi
-if date | grep '^Sat.*pm' >/dev/null 2>&1; then
+if test "x`date +%A`" = xSaturday; then
 
 
 fi
@@ -14787,9 +14814,11 @@ in A
 in C
 @end example
 
-The helper macros @code{AS_IF} and @code{AS_CASE} may be used to
-enforce expansion of required macros outside of shell conditional
-constructs.  You are furthermore encouraged, although not required, to
+You can use the helper macros @code{AS_IF} and @code{AS_CASE} in
+top-level code to enforce expansion of required macros outside of shell
+conditional constructs; these helpers are not needed in the bodies of
+macros defined by @code{AC_DEFUN}.
+You are furthermore encouraged, although not required, to
 put all @code{AC_REQUIRE} calls
 at the beginning of a macro.  You can use @code{dnl} to avoid the empty
 lines they leave.
@@ -15337,15 +15366,16 @@ called @samp{ksh88} and @samp{ksh93}, named after the years of initial
 release.  It is usually called @command{ksh}, but is called @command{sh}
 on some hosts if you set your path appropriately.
 
-Solaris systems have three variants:
+On Solaris 11, @command{/bin/sh} and @command{/usr/bin/ksh} are both
+@samp{ksh93}.  On Solaris 10 and earlier, @command{/bin/sh} is a
+pre-Posix Bourne shell and the Korn shell is found elsewhere:
 @prindex @command{/usr/bin/ksh} on Solaris
-@command{/usr/bin/ksh} is @samp{ksh88}; it is
-standard on Solaris 2.0 and later.
+@command{/usr/bin/ksh} is @samp{ksh88} on Solaris 2.0 through 10,
 @prindex @command{/usr/xpg4/bin/sh} on Solaris
 @command{/usr/xpg4/bin/sh} is a Posix-compliant variant of
-@samp{ksh88}; it is standard on Solaris 9 and later.
+@samp{ksh88} on Solaris 9 and later,
 @prindex @command{/usr/dt/bin/dtksh} on Solaris
-@command{/usr/dt/bin/dtksh} is @samp{ksh93}.
+and @command{/usr/dt/bin/dtksh} is @samp{ksh93}.
 Variants that are not standard may be parts of optional
 packages.  There is no extra charge for these packages, but they are
 not part of a minimal OS install and therefore some installations may
@@ -15714,7 +15744,7 @@ hello
 @end example
 
 Don't rely on duplicating a closed file descriptor to cause an
-error.  With Solaris @command{/bin/sh}, failed duplication is silently
+error.  With Solaris 10 @command{/bin/sh}, failed duplication is silently
 ignored, which can cause unintended leaks to the original file
 descriptor.  In this example, observe the leak to standard output:
 
@@ -15987,7 +16017,7 @@ esac
 
 @noindent
 Make sure you quote the brackets if appropriate and keep the backslash as
-first character (@pxref{case, , Limitations of Shell Builtins}).
+first character.  @xref{case, , Limitations of Shell Builtins}.
 
 Also, because the colon is used as part of a drivespec, these systems don't
 use it as path separator.  When creating or accessing paths, you can use the
@@ -16120,7 +16150,7 @@ esac
 and in fact it is even @emph{more} portable: in the first case of the
 first attempt, the computation of @code{top_srcdir} is not portable,
 since not all shells properly understand @code{"`@dots{}"@dots{}"@dots{}`"},
-for example Solaris 10 ksh:
+for example Solaris 10 @command{ksh}:
 
 @example
 $ @kbd{foo="`echo " bar" | sed 's, ,,'`"}
@@ -16427,7 +16457,7 @@ sys	0m0.003s
 @end example
 
 As with @samp{+} and @samp{-}, @var{value} must be a single shell word,
-otherwise some shells, such as Solaris @command{/bin/sh} or on Digital
+otherwise some shells, such as Solaris 10 @command{/bin/sh} or on Digital
 Unix V 5.0, die because of a ``bad substitution''.  Meanwhile, Posix
 requires that with @samp{=}, quote removal happens prior to the
 assignment, and the expansion be the final contents of @var{var} without
@@ -16463,9 +16493,9 @@ $ @kbd{ksh -c 'x= y=$@{x:=b@} sh -c "echo +\$x+\$y+";echo -$x-'}
 
 @item $@{@var{var}=@var{value}@}
 @cindex @code{$@{@var{var}=@var{literal}@}}
-Solaris @command{/bin/sh} has a frightening bug in its handling of
+Solaris 10 @command{/bin/sh} has a frightening bug in its handling of
 literal assignments.  Imagine you need set a variable to a string containing
-@samp{@}}.  This @samp{@}} character confuses Solaris @command{/bin/sh}
+@samp{@}}.  This @samp{@}} character confuses Solaris 10 @command{/bin/sh}
 when the affected variable was already set.  This bug can be exercised
 by running:
 
@@ -16545,8 +16575,8 @@ list=$@{list="$default"@}
 @end example
 
 @noindent
-@dots{}but beware of the @samp{@}} bug from Solaris (see above).  For safety,
-use:
+@dots{}but beware of the @samp{@}} bug from Solaris 10 (see above).
+For safety, use:
 
 @example
 test $@{var+y@} || var=@var{@{value@}}
@@ -16729,7 +16759,7 @@ Always quote @samp{^}, otherwise traditional shells such as
 
 When setting several variables in a row, be aware that the order of the
 evaluation is undefined.  For instance @samp{foo=1 foo=2; echo $foo}
-gives @samp{1} with Solaris @command{/bin/sh}, but @samp{2} with Bash.
+gives @samp{1} with Solaris 10 @command{/bin/sh}, but @samp{2} with Bash.
 You must use
 @samp{;} to enforce the order: @samp{foo=1; foo=2; echo $foo}.
 
@@ -17376,7 +17406,7 @@ and other options upon function entry and exit.  Inside a function,
 IRIX sh sets @samp{$0} to the function name.
 
 It is not portable to pass temporary environment variables to shell
-functions.  Solaris @command{/bin/sh} does not see the variable.
+functions.  Solaris 10 @command{/bin/sh} does not see the variable.
 Meanwhile, not all shells follow the Posix rule that the assignment must
 affect the current environment in the same manner as special built-ins.
 
@@ -17453,7 +17483,7 @@ $ @kbd{zsh -c '. ./syntax; echo $?'}
 @prindex @command{!}
 The Unix version 7 shell did not support
 negating the exit status of commands with @command{!}, and this feature
-is still absent from some shells (e.g., Solaris @command{/bin/sh}).
+is still absent from some shells (e.g., Solaris 10 @command{/bin/sh}).
 Other shells, such as FreeBSD @command{/bin/sh} or @command{ash}, have
 bugs when using @command{!}:
 
@@ -17559,9 +17589,9 @@ esac
 @end example
 
 @noindent
-but the @code{(} in this example is not portable to many Bourne
+but the @code{(} in this example is not portable to a few obsolescent Bourne
 shell implementations, which is a pity for those of us using tools that
-rely on balanced parentheses.  For instance, with Solaris
+rely on balanced parentheses.  For instance, with Solaris 10
 @command{/bin/sh}:
 
 @example
@@ -17705,7 +17735,8 @@ option.
 
 Do not use backslashes in the arguments, as there is no consensus on
 their handling.  For @samp{echo '\n' | wc -l}, the @command{sh} of
-Solaris outputs 2, but Bash and Zsh (in @command{sh} emulation mode) output 1.
+Solaris 10 outputs 2,
+but Bash and Zsh (in @command{sh} emulation mode) output 1.
 The problem is truly @command{echo}: all the shells
 understand @samp{'\n'} as the string composed of a backslash and an
 @samp{n}.  Within a command substitution, @samp{echo 'string\c'} will
@@ -17855,7 +17886,7 @@ trap to clean up before exiting.  If the last shell command exited with
 nonzero status, the trap also exits with nonzero status so that the
 invoker can tell that an error occurred.
 
-Unfortunately, in some shells, such as Solaris @command{/bin/sh}, an exit
+Unfortunately, in some shells, such as Solaris 10 @command{/bin/sh}, an exit
 trap ignores the @code{exit} command's argument.  In these shells, a trap
 cannot determine whether it was invoked by plain @code{exit} or by
 @code{exit 1}.  Instead of calling @code{exit} directly, use the
@@ -17872,7 +17903,7 @@ of the environment variables.  Conversely, each environment variable
 received by the shell when it is launched should be imported as a shell
 variable marked as exported.
 
-Alas, many shells, such as Solaris @command{/bin/sh},
+Alas, many shells, such as Solaris 10 @command{/bin/sh},
 IRIX 6.3, IRIX 5.2,
 AIX 4.1.5, and Digital Unix 4.0, forget to
 @command{export} the environment variables they receive.  As a result,
@@ -17987,7 +18018,7 @@ word splitting on @samp{$@{1+"$@@"@}}; see @ref{Shell Substitutions},
 item @samp{$@@}, for more.
 
 Posix requires support for a @command{for} loop with no list after
-@code{in}.  However, Solaris @command{/bin/sh} treats that as a syntax
+@code{in}.  However, Solaris 10 @command{/bin/sh} treats that as a syntax
 error.  It is possible to work around this by providing any shell word
 that expands to nothing, or by ignoring an obvious sentinel.
 
@@ -18022,7 +18053,7 @@ a
 b
 @end example
 
-In Solaris @command{/bin/sh}, when the list of arguments of a
+In Solaris 10 @command{/bin/sh}, when the list of arguments of a
 @command{for} loop starts with @emph{unquoted} tokens looking like
 variable assignments, the loop is not executed on those tokens:
 
@@ -18185,7 +18216,7 @@ Also please see the discussion of the @command{cd} command.
 @item @command{read}
 @c -----------------
 @prindex @command{read}
-No options are portable, not even support @option{-r} (Solaris
+No options are portable, not even support @option{-r} (Solaris 10
 @command{/bin/sh} for example).  Tru64/OSF 5.1 @command{sh} treats
 @command{read} as a special built-in, so it may exit if input is
 redirected from a non-existent or unreadable file.
@@ -18287,7 +18318,7 @@ instance of @samp{test -n "$foo" && exit 1} to be @samp{if test -n
 users not to use @samp{sh -e}.
 
 When @samp{set -e} is in effect, a failed command substitution in
-Solaris @command{/bin/sh} cannot be ignored, even with @samp{||}.
+Solaris 10 @command{/bin/sh} cannot be ignored, even with @samp{||}.
 
 @example
 $ @kbd{/bin/sh -c 'set -e; foo=`false` || echo foo; echo bar'}
@@ -18474,7 +18505,7 @@ known-safe string of @samp{y}.
 Posix also says that @samp{test ! "@var{string}"},
 @samp{test -n "@var{string}"} and
 @samp{test -z "@var{string}"} work with any string, but many
-shells (such as Solaris, AIX 3.2, UNICOS 10.0.0.6,
+shells (such as Solaris 10, AIX 3.2, UNICOS 10.0.0.6,
 Digital Unix 4, etc.)@: get confused if
 @var{string} looks like an operator:
 
@@ -18538,7 +18569,7 @@ will invoke the trap at the end of this function.
 
 Posix says that @samp{trap - 1 2 13 15} resets the traps for the
 specified signals to their default values, but many common shells (e.g.,
-Solaris @command{/bin/sh}) misinterpret this and attempt to execute a
+Solaris 10 @command{/bin/sh}) misinterpret this and attempt to execute a
 ``command'' named @command{-} when the specified conditions arise.
 Posix 2008 also added a requirement to support @samp{trap 1 2 13 15} to
 reset traps, as this is supported by a larger set of shells, but there
@@ -18555,7 +18586,7 @@ the @emph{last} command run: that before @command{exit}, or
 @command{exit} itself?''
 
 Bash considers @command{exit} to be the last command, while Zsh and
-Solaris @command{/bin/sh} consider that when the trap is run it is
+Solaris 10 @command{/bin/sh} consider that when the trap is run it is
 @emph{still} in the @command{exit}, hence it is the previous exit status
 that the trap receives:
 
-- 
2.25.1

Re: AS_IF

[Zack Weinberg]

On Mon, Oct 12, 2020 at 3:01 AM Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 10/6/20 5:52 PM, Zack Weinberg wrote:
> > I'm not sure we should say AS_IF/CASE
> > are equivalent to plain shell if/case inside an AC_DEFUN; they do do
> > more than just make the m4 expansion stack not be empty.
>
> Yes, that goes a bit too far. I installed the attached doc patch to Savannah
> Autoconf master to try to clarify things a bit. I developed this patch before
> seeing Bruno's, but it should address the same issues (plus a few more, like
> portability of Posix case syntax).

Looks good, thanks.

> AS_FOR is kind of a mess. I wouldn't document it the way it is. Perhaps it could
> be cleaned up in some future Autoconf version.

Oh, yeah, I see what you mean now.  I didn't actually read the
definition of AS_FOR before writing that NEWS entry.

zw