Re: Test 52 failure on AIX, HP-UX, Solaris

[Joel E. Denny]

To the Cc, I'm adding Didier Godefroy, who reported the Tru64 bug 
originally, and Juan Manuel Guerrero, who maintains Bison's djgpp support.

On Wed, 3 Feb 2010, Joel E. Denny wrote:

> On Wed, 3 Feb 2010, Akim Demaille wrote:

> > I once started a thread on gnulib about getting a module that would 
> > replace our subpipe.

> So, if I read that thread correctly (I confess that I skimmed), after Eric 
> Blake's patches to create_pipe_bidi, the original Tru64 problem that you 
> were trying to fix can be handled fine by switching from our subpipe to 
> gnulib's create_pipe_bidi.  Given that the Bison bug was reported many 
> months ago and that gnulib already offers a likely well tested fix, I'm 
> thinking this fix could be part of 2.4.2 as well.  It sounds like you've 
> already written the patch.  Could you post it?

Never mind.  As it turns out, create_pipe_bidi solves the broken-pipe 
problem too and more cleanly, so below is yet another revision of my 
patch.

> I'm thinking of putting off 2.4.2 about another week because of the recent 
> bug reports.  I'd really like to be done with the 2.4 series at that point 
> if possible.

Now I'm thinking I might roll a 2.4.1b to give at least the bug reporters 
a chance to try out the fixes.  I think djgpp is also affected given that 
it has its own version of our old subpipe module.  I know nothing about 
djgpp, so hopefully someone can take care of that as well during this 
delay.

Another reason to roll 2.4.1b: po/POTFILES.in doesn't include at least 
muscle_tab.c, so there are translations missing.

On Thu, 4 Feb 2010, Joel E. Denny wrote:

> @@ -128,6 +129,8 @@ void
>  fatal_at (location loc, const char *message, ...)
>  {
>    ERROR_MESSAGE (&loc, _("fatal error"), message);
> +  if (m4_pipe != NULL)
> +    while (EOF != getc (m4_pipe)) ;
>    exit (EXIT_FAILURE);
>  }
>  
> @@ -135,5 +138,7 @@ void
>  fatal (const char *message, ...)
>  {
>    ERROR_MESSAGE (NULL, _("fatal error"), message);
> +  if (m4_pipe != NULL)
> +    while (EOF != getc (m4_pipe)) ;
>    exit (EXIT_FAILURE);
>  }

I now realize that it would have been more efficient to just kill the 
child.  Fortunately, create_pipe_bidi can arrange for that to happen on 
exit so that Bison doesn't have to.

Here's the new patch.  Akim, because you've already looked at 
create_pipe_bidi, I figure you might want a chance to review this before I 
push.  Hopefully I can get out a 2.4.1b tarball quickly after that.

>From a47eeff13e6105e678a1cc3031ed8f33fa141fc3 Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Thu, 4 Feb 2010 18:20:12 -0500
Subject: [PATCH] portability: fix several issues with M4 subprocess.

M4's output pipe was not being drained upon fatal errors during
scan_skel.  As a result, broken-pipe messages from M4 were seen
on at least AIX, HP-UX, Solaris, and RHEL4, and this caused a
failure in the test suite.  The problem was that, on platforms
where the default disposition for SIGPIPE is ignore instead of
terminate, M4 sometimes saw fwrite fail with errno=EPIPE and
then reported it.  However, there's some sort of race condition,
because the new test group occasionally succeeded.
Reported by Albert Chin at
<http://lists.gnu.org/archive/html/bug-bison/2010-02/msg00004.html>.

There were also problems with the test suite deadlocking on
Tru64 5.1b.  Reported by Didier Godefroy at
<http://lists.gnu.org/archive/html/bug-bison/2009-05/msg00005.html>.
Switching to create_pipe_bidi suggested by Akim Demaille.

To solve both of these problems, switch to gnulib's
create_pipe_bidi and register M4 process as a slave.  Along the
way, clean up file name conflict handling, which was affected by
the broken-pipe problem before the switch.
* NEWS (2.4.2): Document.
* THANKS (Didier Godefroy): Add.
* bootstrap.conf (gnulib_modules): Add pipe.
* gnulib: Update to latest to make sure we have all the latest
fixes.
* lib/Makefile.am (libbison_a_SOURCES): Remove subpipe.h and
subpipe.c.
* po/POTFILES.in (lib/subpipe.c): Remove.
* src/files.c (compute_output_file_names): Update invocations
of output_file_name_check.
(output_file_name_check): In the case that the grammar file
would be overwritten, use complain instead of fatal, but replace
the output file name with /dev/null.  Use the /dev/null solution
for case of two conflicting output files as well because it
seems safer in case Bison one day tries to open both files at
the same time.
* src/files.h (output_file_name_check): Update prototype.
* src/output.c (output_skeleton): Use create_pipe_bidi and
wait_subprocess.  Assert that scan_skel completely drains the
pipe.
* src/scan-skel.l (at_directive_perform): Update
output_file_name_check invocation.
* tests/output.at (AT_CHECK_CONFLICTING_OUTPUT): Check that the
grammar file actually isn't overwritten.
(Conflicting output files: -o foo.y): Update expected output.
* tests/skeletons.at (Fatal errors but M4 continues producing
output): New test group.
---
 ChangeLog          |   52 +++++++++++++++
 NEWS               |    7 ++-
 THANKS             |    1 +
 bootstrap.conf     |    7 +-
 gnulib             |    2 +-
 lib/.cvsignore     |   51 +++++++++++++++
 lib/.gitignore     |   51 +++++++++++++++
 lib/Makefile.am    |    1 -
 lib/subpipe.c      |  177 ----------------------------------------------------
 lib/subpipe.h      |   28 --------
 m4/.cvsignore      |   22 +++++++
 m4/.gitignore      |   22 +++++++
 po/POTFILES.in     |    1 -
 src/files.c        |   47 ++++++++++----
 src/files.h        |    2 +-
 src/output.c       |   19 ++++--
 src/scan-skel.l    |   12 ++--
 tests/output.at    |    4 +-
 tests/skeletons.at |   42 ++++++++++++
 19 files changed, 306 insertions(+), 242 deletions(-)
 delete mode 100644 lib/subpipe.c
 delete mode 100644 lib/subpipe.h

diff --git a/ChangeLog b/ChangeLog
index b54880b..3e1b2a6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,55 @@
+2010-02-04  Joel E. Denny  <address@hidden>
+
+       portability: fix several issues with M4 subprocess.
+
+       M4's output pipe was not being drained upon fatal errors during
+       scan_skel.  As a result, broken-pipe messages from M4 were seen
+       on at least AIX, HP-UX, Solaris, and RHEL4, and this caused a
+       failure in the test suite.  The problem was that, on platforms
+       where the default disposition for SIGPIPE is ignore instead of
+       terminate, M4 sometimes saw fwrite fail with errno=EPIPE and
+       then reported it.  However, there's some sort of race condition,
+       because the new test group occasionally succeeded.
+       Reported by Albert Chin at
+       <http://lists.gnu.org/archive/html/bug-bison/2010-02/msg00004.html>.
+
+       There were also problems with the test suite deadlocking on
+       Tru64 5.1b.  Reported by Didier Godefroy at
+       <http://lists.gnu.org/archive/html/bug-bison/2009-05/msg00005.html>.
+       Switching to create_pipe_bidi suggested by Akim Demaille.
+
+       To solve both of these problems, switch to gnulib's
+       create_pipe_bidi and register M4 process as a slave.  Along the
+       way, clean up file name conflict handling, which was affected by
+       the broken-pipe problem before the switch.
+       * NEWS (2.4.2): Document.
+       * THANKS (Didier Godefroy): Add.
+       * bootstrap.conf (gnulib_modules): Add pipe.
+       * gnulib: Update to latest to make sure we have all the latest
+       fixes.
+       * lib/Makefile.am (libbison_a_SOURCES): Remove subpipe.h and
+       subpipe.c.
+       * po/POTFILES.in (lib/subpipe.c): Remove.
+       * src/files.c (compute_output_file_names): Update invocations
+       of output_file_name_check.
+       (output_file_name_check): In the case that the grammar file
+       would be overwritten, use complain instead of fatal, but replace
+       the output file name with /dev/null.  Use the /dev/null solution
+       for case of two conflicting output files as well because it
+       seems safer in case Bison one day tries to open both files at
+       the same time.
+       * src/files.h (output_file_name_check): Update prototype.
+       * src/output.c (output_skeleton): Use create_pipe_bidi and
+       wait_subprocess.  Assert that scan_skel completely drains the
+       pipe.
+       * src/scan-skel.l (at_directive_perform): Update
+       output_file_name_check invocation.
+       * tests/output.at (AT_CHECK_CONFLICTING_OUTPUT): Check that the
+       grammar file actually isn't overwritten.
+       (Conflicting output files: -o foo.y): Update expected output.
+       * tests/skeletons.at (Fatal errors but M4 continues producing
+       output): New test group.
+
 2010-02-01  Joel E. Denny  <address@hidden>
 
        tests: link lib/libbison.a for gnulib.
diff --git a/NEWS b/NEWS
index 9b33d13..fd30819 100644
--- a/NEWS
+++ b/NEWS
@@ -3,8 +3,11 @@ Bison News
 
 * Changes in version 2.4.2 (????-??-??):
 
-** Some portability problems in the testsuite that resulted in failures
-   on at least Solaris 2.7 have been fixed.
+** Some portability problems that resulted in testsuite failures or
+   deadlocks on some versions of at least Solaris, AIX, HP-UX, RHEL4,
+   and Tru64 have been fixed.  As part of those fixes, fatal Bison
+   errors no longer cause M4 to report a broken pipe on the affected
+   platforms.
 
 ** `%prec IDENTIFIER' requires IDENTIFIER to be defined separately.
 
diff --git a/THANKS b/THANKS
index d4f3994..41b08c9 100644
--- a/THANKS
+++ b/THANKS
@@ -29,6 +29,7 @@ David J. MacKenzie        address@hidden
 Derek M. Jones            address@hidden
 Di-an Jan                 address@hidden
 Dick Streefland           address@hidden
+Didier Godefroy           address@hidden
 Enrico Scholz             address@hidden
 Eric Blake                address@hidden
 Evgeny Stambulchik        address@hidden
diff --git a/bootstrap.conf b/bootstrap.conf
index 0c42107..fc5a0e9 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -20,9 +20,10 @@ gnulib_modules='
   announce-gen argmatch assert config-h c-strcase configmake dirname
   error extensions fopen-safer gendocs getopt-gnu gettext
   git-version-gen hash inttypes javacomp-script javaexec-script
-  maintainer-makefile malloc mbswidth obstack quote quotearg stdbool
-  stpcpy strerror strtoul strverscmp unistd unistd-safer unlocked-io
-  update-copyright unsetenv verify warnings xalloc xalloc-die xstrndup
+  maintainer-makefile malloc mbswidth obstack pipe quote quotearg
+  stdbool stpcpy strerror strtoul strverscmp unistd unistd-safer
+  unlocked-io update-copyright unsetenv verify warnings xalloc
+  xalloc-die xstrndup
 '
 
 # Additional xgettext options to use.  Use "\\\newline" to break lines.
diff --git a/gnulib b/gnulib
index 102c411..9d0ad65 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 102c411be0dfd9ac6aa0bb6450415e87bf0842ca
+Subproject commit 9d0ad652de159d08e5f679842f8a2a5658196361
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 77ca664..c2b4ec2 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -37,7 +37,6 @@ timevars_sources = \
 # Non-gnulib sources in Bison's internal library.
 libbison_a_SOURCES += \
   get-errno.h get-errno.c \
-  subpipe.h subpipe.c \
   $(bitsets_sources) $(additional_bitsets_sources) $(timevars_sources)
 
 # The Yacc compatibility library.
diff --git a/lib/subpipe.c b/lib/subpipe.c
deleted file mode 100644
index 1b7c36d..0000000
--- a/lib/subpipe.c
+++ /dev/null
@@ -1,177 +0,0 @@
-/* Subprocesses with pipes.
-
-   Copyright (C) 2002, 2004-2006, 2009-2010 Free Software Foundation,
-   Inc.
-
-   This program is free software: you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation, either version 3 of the License, or
-   (at your option) any later version.
-
-   This program 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 General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-/* Written by Paul Eggert <address@hidden>
-   and Florian Krohm <address@hidden>.  */
-
-#include <config.h>
-
-#include "subpipe.h"
-
-#include <errno.h>
-
-#include <signal.h>
-#if ! defined SIGCHLD && defined SIGCLD
-# define SIGCHLD SIGCLD
-#endif
-
-#include <stdlib.h>
-
-#include <unistd.h>
-#ifndef STDIN_FILENO
-# define STDIN_FILENO 0
-#endif
-#ifndef STDOUT_FILENO
-# define STDOUT_FILENO 1
-#endif
-#if ! HAVE_DUP2 && ! defined dup2
-# include <fcntl.h>
-# define dup2(f, t) (close (t), fcntl (f, F_DUPFD, t))
-#endif
-
-#if HAVE_SYS_WAIT_H
-# include <sys/wait.h>
-#endif
-#ifndef WEXITSTATUS
-# define WEXITSTATUS(stat_val) ((unsigned int) (stat_val) >> 8)
-#endif
-#ifndef WIFEXITED
-# define WIFEXITED(stat_val) (((stat_val) & 255) == 0)
-#endif
-
-#if HAVE_VFORK_H
-# include <vfork.h>
-#endif
-#if ! HAVE_WORKING_VFORK
-# define vfork fork
-#endif
-
-#include "error.h"
-#include "unistd-safer.h"
-
-#include "gettext.h"
-#define _(Msgid)  gettext (Msgid)
-
-#ifndef __attribute__
-/* This feature is available in gcc versions 2.5 and later.  */
-# if ! defined __GNUC__ || __GNUC__ < 2 || \
-(__GNUC__ == 2 && __GNUC_MINOR__ < 5) || __STRICT_ANSI__
-#  define __attribute__(Spec) /* empty */
-# endif
-#endif
-
-#ifndef ATTRIBUTE_UNUSED
-# define ATTRIBUTE_UNUSED __attribute__ ((__unused__))
-#endif
-
-
-/* Initialize this module.  */
-
-void
-init_subpipe (void)
-{
-#ifdef SIGCHLD
-  /* System V fork+wait does not work if SIGCHLD is ignored.  */
-  signal (SIGCHLD, SIG_DFL);
-#endif
-}
-
-
-/* Create a subprocess that is run as a filter.  ARGV is the
-   NULL-terminated argument vector for the subprocess.  Store read and
-   write file descriptors for communication with the subprocess into
-   FD[0] and FD[1]: input meant for the process can be written into
-   FD[0], and output from the process can be read from FD[1].  Return
-   the subprocess id.
-
-   To avoid deadlock, the invoker must not let incoming data pile up
-   in FD[1] while writing data to FD[0].  */
-
-pid_t
-create_subpipe (char const * const *argv, int fd[2])
-{
-  int pipe_fd[2];
-  int child_fd[2];
-  pid_t pid;
-
-  if (pipe_safer (child_fd) != 0 || pipe_safer (pipe_fd) != 0)
-    error (EXIT_FAILURE, errno, "pipe");
-  fd[0] = child_fd[1];
-  fd[1] = pipe_fd[0];
-  child_fd[1] = pipe_fd[1];
-
-  pid = vfork ();
-  if (pid < 0)
-    error (EXIT_FAILURE, errno, "fork");
-
-  if (! pid)
-    {
-      /* Child.  */
-      close (fd[0]);
-      close (fd[1]);
-      dup2 (child_fd[0], STDIN_FILENO);
-      close (child_fd[0]);
-      dup2 (child_fd[1], STDOUT_FILENO);
-      close (child_fd[1]);
-
-      /* The cast to (char **) rather than (char * const *) is needed
-        for portability to older hosts with a nonstandard prototype
-        for execvp.  */
-      execvp (argv[0], (char **) argv);
-
-      _exit (errno == ENOENT ? 127 : 126);
-    }
-
-  /* Parent.  */
-  close (child_fd[0]);
-  close (child_fd[1]);
-  return pid;
-}
-
-
-/* Wait for the subprocess to exit.  */
-
-void
-reap_subpipe (pid_t pid, char const *program)
-{
-#if HAVE_WAITPID || defined waitpid
-  int wstatus;
-  if (waitpid (pid, &wstatus, 0) < 0)
-    error (EXIT_FAILURE, errno, "waitpid");
-  else
-    {
-      int status = WIFEXITED (wstatus) ? WEXITSTATUS (wstatus) : -1;
-      if (status)
-       error (EXIT_FAILURE, 0,
-              _(status == 126
-                ? "subsidiary program `%s' could not be invoked"
-                : status == 127
-                ? "subsidiary program `%s' not found"
-                : status < 0
-                ? "subsidiary program `%s' failed"
-                : "subsidiary program `%s' failed (exit status %d)"),
-              program, status);
-    }
-#endif
-}
-
-void
-end_of_output_subpipe (pid_t pid ATTRIBUTE_UNUSED,
-                      int fd[2] ATTRIBUTE_UNUSED)
-{
-}
diff --git a/lib/subpipe.h b/lib/subpipe.h
deleted file mode 100644
index ff791a8..0000000
--- a/lib/subpipe.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/* Subprocesses with pipes.
-   Copyright (C) 2002, 2004-2005, 2009-2010 Free Software Foundation,
-   Inc.
-
-   This program is free software: you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation, either version 3 of the License, or
-   (at your option) any later version.
-
-   This program 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 General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-/* Written by Paul Eggert <address@hidden>
-   and Florian Krohm <address@hidden>.  */
-
-#if HAVE_SYS_TYPES_H
-# include <sys/types.h>
-#endif
-
-void init_subpipe (void);
-pid_t create_subpipe (char const * const *, int[2]);
-void end_of_output_subpipe (pid_t, int[2]);
-void reap_subpipe (pid_t, char const *);
diff --git a/po/POTFILES.in b/po/POTFILES.in
index d39ca4c..86253f3 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -20,6 +20,5 @@ lib/error.c
 lib/getopt.c
 lib/obstack.c
 lib/quotearg.c
-lib/subpipe.c
 lib/timevar.c
 lib/xalloc-die.c
diff --git a/src/files.c b/src/files.c
index 9761de9..e8bb200 100644
--- a/src/files.c
+++ b/src/files.c
@@ -328,21 +328,21 @@ compute_output_file_names (void)
     {
       if (! spec_graph_file)
        spec_graph_file = concat2 (all_but_tab_ext, ".dot");
-      output_file_name_check (spec_graph_file);
+      output_file_name_check (&spec_graph_file);
     }
 
   if (xml_flag)
     {
       if (! spec_xml_file)
        spec_xml_file = concat2 (all_but_tab_ext, ".xml");
-      output_file_name_check (spec_xml_file);
+      output_file_name_check (&spec_xml_file);
     }
 
   if (report_flag)
     {
       if (!spec_verbose_file)
         spec_verbose_file = concat2 (all_but_tab_ext, OUTPUT_EXT);
-      output_file_name_check (spec_verbose_file);
+      output_file_name_check (&spec_verbose_file);
     }
 
   free (all_but_tab_ext);
@@ -351,18 +351,37 @@ compute_output_file_names (void)
 }
 
 void
-output_file_name_check (char const *file_name)
+output_file_name_check (char **file_name)
 {
-  if (0 == strcmp (file_name, grammar_file))
-    fatal (_("refusing to overwrite the input file %s"), quote (file_name));
-  {
-    int i;
-    for (i = 0; i < file_names_count; i++)
-      if (0 == strcmp (file_names[i], file_name))
-        warn (_("conflicting outputs to file %s"), quote (file_name));
-  }
-  file_names = xnrealloc (file_names, ++file_names_count, sizeof *file_names);
-  file_names[file_names_count-1] = xstrdup (file_name);
+  bool conflict = false;
+  if (0 == strcmp (*file_name, grammar_file))
+    {
+      complain (_("refusing to overwrite the input file %s"),
+                quote (*file_name));
+      conflict = true;
+    }
+  else
+    {
+      int i;
+      for (i = 0; i < file_names_count; i++)
+        if (0 == strcmp (file_names[i], *file_name))
+          {
+            warn (_("conflicting outputs to file %s"),
+                  quote (*file_name));
+            conflict = true;
+          }
+    }
+  if (conflict)
+    {
+      free (*file_name);
+      *file_name = strdup ("/dev/null");
+    }
+  else
+    {
+      file_names = xnrealloc (file_names, ++file_names_count,
+                              sizeof *file_names);
+      file_names[file_names_count-1] = xstrdup (*file_name);
+    }
 }
 
 void
diff --git a/src/files.h b/src/files.h
index e8f28bf..5366783 100644
--- a/src/files.h
+++ b/src/files.h
@@ -63,7 +63,7 @@ extern char *all_but_ext;
 
 void compute_output_file_names (void);
 void output_file_names_free (void);
-void output_file_name_check (char const *file_name);
+void output_file_name_check (char **file_name);
 
 FILE *xfopen (const char *name, const char *mode);
 void xfclose (FILE *ptr);
diff --git a/src/output.c b/src/output.c
index 6a05704..212fa83 100644
--- a/src/output.c
+++ b/src/output.c
@@ -25,9 +25,10 @@
 #include <configmake.h>
 #include <error.h>
 #include <get-errno.h>
+#include <pipe.h>
 #include <quotearg.h>
-#include <subpipe.h>
 #include <timevar.h>
+#include <wait-process.h>
 
 #include "complain.h"
 #include "files.h"
@@ -551,13 +552,14 @@ output_skeleton (void)
     assert (i <= ARRAY_CARDINALITY (argv));
   }
 
-  init_subpipe ();
-  pid = create_subpipe (argv, filter_fd);
+  /* The ugly cast is because gnulib gets the const-ness wrong.  */
+  pid = create_pipe_bidi ("m4", m4, (char **)(void*)argv, false, true,
+                          true, filter_fd);
   free (full_m4sugar);
   free (full_m4bison);
   free (full_skeleton);
 
-  out = fdopen (filter_fd[0], "w");
+  out = fdopen (filter_fd[1], "w");
   if (! out)
     error (EXIT_FAILURE, get_errno (),
           "fdopen");
@@ -576,14 +578,17 @@ output_skeleton (void)
 
   /* Read and process m4's output.  */
   timevar_push (TV_M4);
-  end_of_output_subpipe (pid, filter_fd);
-  in = fdopen (filter_fd[1], "r");
+  in = fdopen (filter_fd[0], "r");
   if (! in)
     error (EXIT_FAILURE, get_errno (),
           "fdopen");
   scan_skel (in);
+  /* scan_skel should have read all of M4's output.  Otherwise, when we
+     close the pipe, we risk letting M4 report a broken-pipe to the
+     Bison user.  */
+  aver (feof (in));
   xfclose (in);
-  reap_subpipe (pid, m4);
+  wait_subprocess (pid, "m4", false, false, true, true, NULL);
   timevar_pop (TV_M4);
 }
 
diff --git a/src/scan-skel.l b/src/scan-skel.l
index 90a52dd..cd30576 100644
--- a/src/scan-skel.l
+++ b/src/scan-skel.l
@@ -107,7 +107,7 @@ static void fail_for_invalid_at (char const *at);
   "@@" { obstack_1grow (&obstack_for_string, '@'); }
   "@{" { obstack_1grow (&obstack_for_string, '['); }
   "@}" { obstack_1grow (&obstack_for_string, ']'); }
-  "@`" /* Emtpy.  Useful for starting an argument
+  "@`" /* Empty.  Useful for starting an argument
           that begins with whitespace. */
   @\n  /* Empty.  */
 
@@ -174,10 +174,10 @@ skel_scanner_free (void)
   yylex_destroy ();
 }
 
-static
-void at_directive_perform (int at_directive_argc,
-                           char *at_directive_argv[],
-                           char **outnamep, int *out_linenop)
+static void
+at_directive_perform (int at_directive_argc,
+                      char *at_directive_argv[],
+                      char **outnamep, int *out_linenop)
 {
   if (0 == strcmp (at_directive_argv[0], "@basename"))
     {
@@ -276,7 +276,7 @@ void at_directive_perform (int at_directive_argc,
           xfclose (yyout);
         }
       *outnamep = xstrdup (at_directive_argv[1]);
-      output_file_name_check (*outnamep);
+      output_file_name_check (outnamep);
       yyout = xfopen (*outnamep, "w");
       *out_linenop = 1;
     }
diff --git a/tests/output.at b/tests/output.at
index f8e1653..999ca18 100644
--- a/tests/output.at
+++ b/tests/output.at
@@ -137,7 +137,9 @@ AT_DATA([$1],
 foo: {};
 ]])
 
+[cp ]$1[ expout]
 AT_BISON_CHECK([$3 $1], $5, [], [$4])
+AT_CHECK([[cat $1]], [[0]], [expout])
 AT_CLEANUP
 ])
 
@@ -157,7 +159,7 @@ AT_CHECK_CONFLICTING_OUTPUT([foo.y],
 ])
 
 AT_CHECK_CONFLICTING_OUTPUT([foo.y], [], [-o foo.y],
-[foo.y: fatal error: refusing to overwrite the input file `foo.y'
+[foo.y: refusing to overwrite the input file `foo.y'
 ], 1)
 
 
diff --git a/tests/skeletons.at b/tests/skeletons.at
index f96d13e..18acbc0 100644
--- a/tests/skeletons.at
+++ b/tests/skeletons.at
@@ -288,3 +288,45 @@ foo.y:1.5-6: fatal error: M4 should exit immediately here
 ]])
 
 AT_CLEANUP
+
+
+## ------------------------------------------------ ##
+## Fatal errors but M4 continues producing output.  ##
+## ------------------------------------------------ ##
+
+# At one time, if Bison encountered a fatal error during M4 processing,
+# Bison failed to drain M4's output pipe.  The result was a SIGPIPE.
+# On some platforms, the default disposition for SIGPIPE is terminate,
+# which was fine.  On others, it's ignore, which caused M4 to report
+# the broken pipe to the user, but we don't want to bother the user with
+# that.
+
+# There is a race condition somewhere.  That is, before the associated
+# fix, running this test group many times in a row would occasionally
+# produce a pass among all the failures.
+
+AT_SETUP([[Fatal errors but M4 continues producing output]])
+
+AT_DATA([[gen-skel.pl]],
+[[use warnings;
+use strict;
+my $M4 = "m4";
+my $DNL = "d"."nl";
+print "${M4}_divert_push(0)$DNL\n";
+print '@output(@,@)', "\n";
+(print "garbage"x10, "\n") for (1..1000);
+print "${M4}_divert_pop(0)\n";
+]])
+AT_CHECK([[perl gen-skel.pl > skel.c || exit 77]])
+
+AT_DATA([[input.y]],
+[[%skeleton "./skel.c"
+%%
+start: ;
+]])
+
+AT_BISON_CHECK([[input.y]], [[1]], [[]],
+[[input.y: fatal error: too many arguments for @output directive in skeleton
+]])
+
+AT_CLEANUP
-- 
1.5.4.3