Re: interrupt causes parse error in configure script

[Ralf Wildenhues]

* Jim Meyering wrote on Mon, Aug 18, 2008 at 11:57:45PM CEST:
> Ralf Wildenhues <address@hidden> wrote:
> >> I think we can factor it into a simpler testcase.  I just tried this:
> >>
> >> $ sh -c 'if test `sleep 5; echo hi` = hi ; then echo yes; fi'
> >
> > and found only pdksh and OpenBSD sh (which is a pdksh descendant) to
> > have this issue.
> 
> I'm pretty sure this happened with recent bash on my primary desktop
> system.  You can reproduce it with any shell like this:
> 
>     if test `sleep 5 && echo f` = f; then echo yes; fi
> 
> Then kill the sleep process in under 5 seconds.  I get parse
> errors from the latest versions of bash, dash, and zsh.

Indeed.

> So this *can* happen with other shells, if either the eval or
> the $as_echo from the expansion of AS_VAR_GET fails.
> Admittedly, this isn't the same as a simple ^C, so maybe
> there's a race condition somewhere in there and the signal
> has to hit a small window... or maybe it did happen on
> some other type of system.

Looks like it.

Luckily parse errors don't seem to happen with
  case `sleep 5 && echo f` in ...
  for x in `sleep 5 && echo f`; do ...

(better: I can't reproduce them with that, with a number of shells),
which means the number of currently known-problematic places in Autoconf
code is quite small, I found one in status.m4 and one in the test suite.

This is what I've got now.  Thoughts?

I'm pretty sure that any remaining test failure I'm seeing are not
introduced by this patch, so if you are ok with it I'll apply.

Cheers,
Ralf


2008-08-19  Jim Meyering <address@hidden>
            Eric Blake <address@hidden>
            Ralf Wildenhues  <address@hidden>

        Avoid shell parse errors after interrupt due to empty command
        substitutions.

        * doc/autoconf.texi (Shell Substitutions): Document the issue.
        * lib/m4sugar/m4sh.m4 (AS_VAR_IF): New function.
        * lib/autoconf/functions.m4 (AC_CHECK_FUNC): Use it in place of
        "test AS_VAR_GET([...]) = yes"
        * lib/autoconf/general.m4 (AC_CHECK_FILE, AC_CHECK_DECL): Likewise.
        * lib/autoconf/headers.m4 (_AC_CHECK_HEADER_MONGREL): Likewise.
        (_AC_CHECK_HEADER_NEW, _AC_CHECK_HEADER_OLD): Likewise.
        (_AC_CHECK_HEADER_DIRENT): Likewise.
        * lib/autoconf/libs.m4 (AC_CHECK_LIB): Likewise.
        * lib/autoconf/types.m4 (_AC_CHECK_TYPE_NEW, AC_CHECK_MEMBER): Likewise.
        * lib/autoconf/status.m4 (_AC_OUTPUT_FILES_PREPARE): Use
        temporary variable to work around the issue.
        * tests/foreign.at (Libtool): Quote result of command
        substitution.

diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 2ce88f8..739bd47 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -13479,6 +13479,26 @@ Shell Substitutions
 - broken differ: char 4, line 1
 @end example
 
+Upon interrupt or SIGTERM, some shells may abort a command substitution,
+replace it with a null string, and wrongly evaluate the enclosing
+command before entering the trap or ending the script.  This can lead to
+spurious errors:
+
address@hidden
+$ @kbd{sh -c 'if test `sleep 5; echo hi` = hi; then echo yes; fi'}
+$ @kbd{^C}
+sh: test: hi: unexpected operator/operand
address@hidden example
+
address@hidden
+You can avoid this by assigning the command substitution to a temporary
+variable:
+
address@hidden
+$ @kbd{sh -c 'res=`sleep 5; echo hi`
+         if test "x$res" = xhi; then echo yes; fi'}
+$ @kbd{^C}
address@hidden example
 
 @item $(@var{commands})
 @cindex $(@var{commands})
diff --git a/lib/autoconf/functions.m4 b/lib/autoconf/functions.m4
index 78c7678..c325f30 100644
--- a/lib/autoconf/functions.m4
+++ b/lib/autoconf/functions.m4
@@ -70,7 +70,7 @@ AC_CACHE_CHECK([for $1], [ac_var],
 [AC_LINK_IFELSE([AC_LANG_FUNC_LINK_TRY([$1])],
                [AS_VAR_SET([ac_var], [yes])],
                [AS_VAR_SET([ac_var], [no])])])
-AS_IF([test AS_VAR_GET([ac_var]) = yes], [$2], [$3])dnl
+AS_VAR_IF([ac_var], [yes], [$2], [$3])dnl
 AS_VAR_POPDEF([ac_var])dnl
 ])# AC_CHECK_FUNC
 
diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
index 8af0dc4..265d78b 100644
--- a/lib/autoconf/general.m4
+++ b/lib/autoconf/general.m4
@@ -2614,7 +2614,7 @@ if test -r "$1"; then
 else
   AS_VAR_SET([ac_File], [no])
 fi])
-AS_IF([test AS_VAR_GET([ac_File]) = yes], [$2], [$3])[]dnl
+AS_VAR_IF([ac_File], [yes], [$2], [$3])[]dnl
 AS_VAR_POPDEF([ac_File])dnl
 ])# AC_CHECK_FILE
 
@@ -2651,7 +2651,7 @@ AC_CACHE_CHECK([whether $1 is declared], [ac_Symbol],
 ])],
                   [AS_VAR_SET([ac_Symbol], [yes])],
                   [AS_VAR_SET([ac_Symbol], [no])])])
-AS_IF([test AS_VAR_GET([ac_Symbol]) = yes], [$2], [$3])[]dnl
+AS_VAR_IF([ac_Symbol], [yes], [$2], [$3])[]dnl
 AS_VAR_POPDEF([ac_Symbol])dnl
 ])# AC_CHECK_DECL
 
diff --git a/lib/autoconf/headers.m4 b/lib/autoconf/headers.m4
index 476df37..16b2737 100644
--- a/lib/autoconf/headers.m4
+++ b/lib/autoconf/headers.m4
@@ -143,7 +143,7 @@ esac
 AC_CACHE_CHECK([for $1], [ac_Header],
               [AS_VAR_SET([ac_Header], [$ac_header_preproc])])
 ])dnl ! set ac_HEADER
-AS_IF([test AS_VAR_GET([ac_Header]) = yes], [$2], [$3])[]dnl
+AS_VAR_IF([ac_Header], [yes], [$2], [$3])[]dnl
 AS_VAR_POPDEF([ac_Header])dnl
 ])# _AC_CHECK_HEADER_MONGREL
 
@@ -160,7 +160,7 @@ AC_CACHE_CHECK([for $1], [ac_Header],
 @%:@include <$1>])],
                                  [AS_VAR_SET([ac_Header], [yes])],
                                  [AS_VAR_SET([ac_Header], [no])])])
-AS_IF([test AS_VAR_GET([ac_Header]) = yes], [$2], [$3])[]dnl
+AS_VAR_IF([ac_Header], [yes], [$2], [$3])[]dnl
 AS_VAR_POPDEF([ac_Header])dnl
 ])# _AC_CHECK_HEADER_NEW
 
@@ -175,7 +175,7 @@ AC_CACHE_CHECK([for $1], [ac_Header],
               [AC_PREPROC_IFELSE([AC_LANG_SOURCE(address@hidden:@include 
<$1>])],
                                         [AS_VAR_SET([ac_Header], [yes])],
                                         [AS_VAR_SET([ac_Header], [no])])])
-AS_IF([test AS_VAR_GET([ac_Header]) = yes], [$2], [$3])[]dnl
+AS_VAR_IF([ac_Header], [yes], [$2], [$3])[]dnl
 AS_VAR_POPDEF([ac_Header])dnl
 ])# _AC_CHECK_HEADER_OLD
 
@@ -403,7 +403,7 @@ AC_CACHE_CHECK([for $1 that defines DIR], [ac_Header],
 return 0;])],
                   [AS_VAR_SET([ac_Header], [yes])],
                   [AS_VAR_SET([ac_Header], [no])])])
-AS_IF([test AS_VAR_GET([ac_Header]) = yes], [$2], [$3])[]dnl
+AS_VAR_IF([ac_Header], [yes], [$2], [$3])[]dnl
 AS_VAR_POPDEF([ac_Header])dnl
 ])# _AC_CHECK_HEADER_DIRENT
 
diff --git a/lib/autoconf/libs.m4 b/lib/autoconf/libs.m4
index 429918c..a1b8cfe 100644
--- a/lib/autoconf/libs.m4
+++ b/lib/autoconf/libs.m4
@@ -130,7 +130,7 @@ AC_LINK_IFELSE([AC_LANG_CALL([], [$2])],
               [AS_VAR_SET([ac_Lib], [yes])],
               [AS_VAR_SET([ac_Lib], [no])])
 LIBS=$ac_check_lib_save_LIBS])
-AS_IF([test AS_VAR_GET([ac_Lib]) = yes],
+AS_VAR_IF([ac_Lib], [yes],
       [m4_default([$3], [AC_DEFINE_UNQUOTED(AS_TR_CPP(HAVE_LIB$1))
   LIBS="-l$1 $LIBS"
 ])],
diff --git a/lib/autoconf/status.m4 b/lib/autoconf/status.m4
index 4897461..af16f79 100644
--- a/lib/autoconf/status.m4
+++ b/lib/autoconf/status.m4
@@ -419,7 +419,8 @@ for ac_last_try in false false false false false :; do
     AC_MSG_ERROR([could not make $CONFIG_STATUS])
 
 dnl Do not use grep on conf$$subs.awk, since AIX grep has a line length limit.
-  if test `sed -n "s/.*$ac_delim\$/X/p" conf$$subs.awk | grep -c X` = 
$ac_delim_num; then
+  ac_delim_n=`sed -n "s/.*$ac_delim\$/X/p" conf$$subs.awk | grep -c X`
+  if test $ac_delim_n = $ac_delim_num; then
     break
   elif $ac_last_try; then
     AC_MSG_ERROR([could not make $CONFIG_STATUS])
diff --git a/lib/autoconf/types.m4 b/lib/autoconf/types.m4
index 50a489c..0ab85a5 100644
--- a/lib/autoconf/types.m4
+++ b/lib/autoconf/types.m4
@@ -160,7 +160,7 @@ AC_COMPILE_IFELSE(
          return 0;])],
      [],
      [AS_VAR_SET([ac_Type], [yes])])])])
-AS_IF([test AS_VAR_GET([ac_Type]) = yes], [$2], [$3])[]dnl
+AS_VAR_IF([ac_Type], [yes], [$2], [$3])[]dnl
 AS_VAR_POPDEF([ac_Type])dnl
 ])# _AC_CHECK_TYPE_NEW
 
@@ -834,7 +834,7 @@ if (sizeof ac_aggr.m4_bpatsubst([$1], [^[^.]*\.]))
 return 0;])],
                [AS_VAR_SET([ac_Member], [yes])],
                [AS_VAR_SET([ac_Member], [no])])])])
-AS_IF([test AS_VAR_GET([ac_Member]) = yes], [$2], [$3])dnl
+AS_VAR_IF([ac_Member], [yes], [$2], [$3])dnl
 AS_VAR_POPDEF([ac_Member])dnl
 ])# AC_CHECK_MEMBER
 
diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index 1517630..c3af80a 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -1576,6 +1576,17 @@ m4_define([AS_VAR_SET_IF],
 [AS_IF([AS_VAR_TEST_SET([$1])], [$2], [$3])])
 
 
+# AS_VAR_IF(VARIABLE, [VALUE = yes], IF-TRUE, IF-FALSE)
+# -----------------------------------------------------
+# Implement a shell `if test $VARIABLE = VALUE; then-else'.
+# Polymorphic, and avoids pdksh expansion error upon interrupt.
+m4_define([AS_VAR_IF],
+[AS_LITERAL_IF([$1],
+  [AS_IF([test "x$$1" = x""m4_default([$2], [yes])], [$3], [$4])],
+  [as_val=AS_VAR_GET([$1])
+   AS_IF([test "x$as_val" = x""m4_default([$2], [yes])], [$3], [$4])])])
+
+
 # AS_VAR_PUSHDEF and AS_VAR_POPDEF
 # --------------------------------
 #
diff --git a/tests/foreign.at b/tests/foreign.at
index e4cc89d..43ada94 100644
--- a/tests/foreign.at
+++ b/tests/foreign.at
@@ -69,7 +69,7 @@ AT_CHECK([test -f "`cat stdout`"])
 touch install-sh
 
 # Build the concatenation of libtool.m4 and configure.ac.
-cp `cat stdout` configure.in
+cp "`cat stdout`" configure.in
 cat >>configure.in <<_EOF
 AC_INIT
 AC_CONFIG_AUX_DIR(.)