From 22cc8d813ee57c9631e527a31010ab138f9b7e06 Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" Date: Mon, 22 Feb 2010 18:09:19 -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 . There were also problems with the test suite livelocking on Tru64 5.1b. Reported by Didier Godefroy at . Switching to create_pipe_bidi suggested by Akim Demaille. To attempt 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 the 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 b9fb00a8..5fe369d6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,55 @@ +2010-02-22 Joel E. Denny + + 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 + . + + There were also problems with the test suite livelocking on + Tru64 5.1b. Reported by Didier Godefroy at + . + Switching to create_pipe_bidi suggested by Akim Demaille. + + To attempt 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 the 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-04 Joel E. Denny Update POTFILES. diff --git a/NEWS b/NEWS index 9b33d13f..f11844de 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 failures and livelocks + in the test suite 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 d4f39944..41b08c97 100644 --- a/THANKS +++ b/THANKS @@ -29,6 +29,7 @@ David J. MacKenzie djm@gnu.org Derek M. Jones derek@knosof.co.uk Di-an Jan dianj@freeshell.org Dick Streefland dick.streefland@altium.nl +Didier Godefroy dg@ulysium.net Enrico Scholz enrico.scholz@informatik.tu-chemnitz.de Eric Blake ebb9@byu.net Evgeny Stambulchik fnevgeny@plasma-gate.weizmann.ac.il diff --git a/bootstrap.conf b/bootstrap.conf index 0c421074..fc5a0e96 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 102c411b..9d0ad652 160000 --- a/gnulib +++ b/gnulib @@ -1 +1 @@ -Subproject commit 102c411be0dfd9ac6aa0bb6450415e87bf0842ca +Subproject commit 9d0ad652de159d08e5f679842f8a2a5658196361 diff --git a/lib/.cvsignore b/lib/.cvsignore index 5baf4779..bc139336 100644 --- a/lib/.cvsignore +++ b/lib/.cvsignore @@ -1,10 +1,13 @@ .deps Makefile Makefile.in +alloca.h +alloca.in.h argmatch.c argmatch.h basename-lgpl.c basename.c +binary-io.h bitrotate.h c-ctype.c c-ctype.h @@ -12,6 +15,8 @@ c-strcase.h c-strcasecmp.c c-strncasecmp.c charset.alias +cloexec.c +cloexec.h config.charset config.h config.hin @@ -19,6 +24,7 @@ configmake.h dirname-lgpl.c dirname.c dirname.h +dup-safer-flag.c dup-safer.c dup2.c errno.h @@ -27,12 +33,16 @@ error.c error.h exitfail.c exitfail.h +fatal-signal.c +fatal-signal.h fcntl.c fcntl.h fcntl.in.h +fd-safer-flag.c fd-safer.c fopen-safer.c fopen.c +getdtablesize.c getopt.c getopt.h getopt.in.h @@ -59,16 +69,45 @@ memchr.c memchr.valgrind obstack.c obstack.h +open.c pipe-safer.c +pipe.c +pipe.h +pipe2-safer.c +pipe2.c quote.c quote.h quotearg.c quotearg.h +rawmemchr.c +rawmemchr.valgrind ref-add.sed ref-add.sin ref-del.sed ref-del.sin +sched.h +sched.in.h +sig-handler.h +sigaction.c +signal.h +signal.in.h +sigprocmask.c +spawn.h +spawn.in.h +spawn_faction_addclose.c +spawn_faction_adddup2.c +spawn_faction_addopen.c +spawn_faction_destroy.c +spawn_faction_init.c +spawn_int.h +spawnattr_destroy.c +spawnattr_init.c +spawnattr_setflags.c +spawnattr_setsigmask.c +spawni.c +spawnp.c stamp-h1 +stat.c stdbool.h stdbool.in.h stdbool_.h @@ -86,6 +125,8 @@ stdlib.h stdlib.in.h stdlib_.h stpcpy.c +strchrnul.c +strchrnul.valgrind streq.h strerror.c string.h @@ -98,6 +139,13 @@ strtol.c strtoul.c strverscmp.c strverscmp.h +sys +sys_stat.h +sys_stat.in.h +sys_wait.h +sys_wait.in.h +time.h +time.in.h unistd--.h unistd-safer.h unistd.h @@ -109,6 +157,9 @@ uniwidth.h unlocked-io.h unsetenv.c verify.h +w32spawn.h +wait-process.c +wait-process.h wchar.h wchar.in.h wchar_.h diff --git a/lib/.gitignore b/lib/.gitignore index 38965d69..6746b557 100644 --- a/lib/.gitignore +++ b/lib/.gitignore @@ -4,10 +4,13 @@ /.deps /Makefile /Makefile.in +/alloca.h +/alloca.in.h /argmatch.c /argmatch.h /basename-lgpl.c /basename.c +/binary-io.h /bitrotate.h /c-ctype.c /c-ctype.h @@ -15,6 +18,8 @@ /c-strcasecmp.c /c-strncasecmp.c /charset.alias +/cloexec.c +/cloexec.h /config.charset /config.h /config.hin @@ -22,6 +27,7 @@ /dirname-lgpl.c /dirname.c /dirname.h +/dup-safer-flag.c /dup-safer.c /dup2.c /errno.h @@ -30,12 +36,16 @@ /error.h /exitfail.c /exitfail.h +/fatal-signal.c +/fatal-signal.h /fcntl.c /fcntl.h /fcntl.in.h +/fd-safer-flag.c /fd-safer.c /fopen-safer.c /fopen.c +/getdtablesize.c /getopt.c /getopt.h /getopt.in.h @@ -62,16 +72,45 @@ /memchr.valgrind /obstack.c /obstack.h +/open.c /pipe-safer.c +/pipe.c +/pipe.h +/pipe2-safer.c +/pipe2.c /quote.c /quote.h /quotearg.c /quotearg.h +/rawmemchr.c +/rawmemchr.valgrind /ref-add.sed /ref-add.sin /ref-del.sed /ref-del.sin +/sched.h +/sched.in.h +/sig-handler.h +/sigaction.c +/signal.h +/signal.in.h +/sigprocmask.c +/spawn.h +/spawn.in.h +/spawn_faction_addclose.c +/spawn_faction_adddup2.c +/spawn_faction_addopen.c +/spawn_faction_destroy.c +/spawn_faction_init.c +/spawn_int.h +/spawnattr_destroy.c +/spawnattr_init.c +/spawnattr_setflags.c +/spawnattr_setsigmask.c +/spawni.c +/spawnp.c /stamp-h1 +/stat.c /stdbool.h /stdbool.in.h /stdbool_.h @@ -89,6 +128,8 @@ /stdlib.in.h /stdlib_.h /stpcpy.c +/strchrnul.c +/strchrnul.valgrind /streq.h /strerror.c /string.h @@ -101,6 +142,13 @@ /strtoul.c /strverscmp.c /strverscmp.h +/sys +/sys_stat.h +/sys_stat.in.h +/sys_wait.h +/sys_wait.in.h +/time.h +/time.in.h /unistd--.h /unistd-safer.h /unistd.h @@ -112,6 +160,9 @@ /unlocked-io.h /unsetenv.c /verify.h +/w32spawn.h +/wait-process.c +/wait-process.h /wchar.h /wchar.in.h /wchar_.h diff --git a/lib/Makefile.am b/lib/Makefile.am index 77ca6646..c2b4ec27 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 1b7c36d8..00000000 --- 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 . */ - -/* Written by Paul Eggert - and Florian Krohm . */ - -#include - -#include "subpipe.h" - -#include - -#include -#if ! defined SIGCHLD && defined SIGCLD -# define SIGCHLD SIGCLD -#endif - -#include - -#include -#ifndef STDIN_FILENO -# define STDIN_FILENO 0 -#endif -#ifndef STDOUT_FILENO -# define STDOUT_FILENO 1 -#endif -#if ! HAVE_DUP2 && ! defined dup2 -# include -# define dup2(f, t) (close (t), fcntl (f, F_DUPFD, t)) -#endif - -#if HAVE_SYS_WAIT_H -# include -#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 -#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 ff791a81..00000000 --- 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 . */ - -/* Written by Paul Eggert - and Florian Krohm . */ - -#if HAVE_SYS_TYPES_H -# include -#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/m4/.cvsignore b/m4/.cvsignore index d4016ed6..0ebd70a5 100644 --- a/m4/.cvsignore +++ b/m4/.cvsignore @@ -1,7 +1,9 @@ 00gnulib.m4 absolute-header.m4 +alloca.m4 argmatch.m4 assert.m4 +cloexec.m4 config-h.m4 dirname.m4 dos.m4 @@ -12,10 +14,12 @@ errno_h.m4 error.m4 exitfail.m4 extensions.m4 +fatal-signal.m4 fcntl-o.m4 fcntl.m4 fcntl_h.m4 fopen.m4 +getdtablesize.m4 getopt.m4 getpagesize.m4 gettext.m4 @@ -48,13 +52,26 @@ mbstate_t.m4 mbswidth.m4 memchr.m4 mmap-anon.m4 +mode_t.m4 multiarch.m4 nls.m4 +open.m4 +pipe.m4 +pipe2.m4 po.m4 +posix_spawn.m4 progtest.m4 quote.m4 quotearg.m4 +rawmemchr.m4 +sched_h.m4 setenv.m4 +sig_atomic_t.m4 +sigaction.m4 +signal_h.m4 +signalblocking.m4 +spawn_h.m4 +stat.m4 stdbool.m4 stddef_h.m4 stdint.m4 @@ -63,6 +80,7 @@ stdio-safer.m4 stdio_h.m4 stdlib_h.m4 stpcpy.m4 +strchrnul.m4 strerror.m4 string_h.m4 strndup.m4 @@ -70,10 +88,14 @@ strnlen.m4 strtol.m4 strtoul.m4 strverscmp.m4 +sys_stat_h.m4 +sys_wait_h.m4 threadlib.m4 +time_h.m4 unistd-safer.m4 unistd_h.m4 unlocked-io.m4 +wait-process.m4 warn-on-use.m4 warning.m4 warnings.m4 diff --git a/m4/.gitignore b/m4/.gitignore index 6b147a80..bdae61af 100644 --- a/m4/.gitignore +++ b/m4/.gitignore @@ -1,7 +1,9 @@ /00gnulib.m4 /absolute-header.m4 +/alloca.m4 /argmatch.m4 /assert.m4 +/cloexec.m4 /config-h.m4 /dirname.m4 /dos.m4 @@ -12,10 +14,12 @@ /error.m4 /exitfail.m4 /extensions.m4 +/fatal-signal.m4 /fcntl-o.m4 /fcntl.m4 /fcntl_h.m4 /fopen.m4 +/getdtablesize.m4 /getopt.m4 /getpagesize.m4 /gettext.m4 @@ -48,13 +52,26 @@ /mbswidth.m4 /memchr.m4 /mmap-anon.m4 +/mode_t.m4 /multiarch.m4 /nls.m4 +/open.m4 +/pipe.m4 +/pipe2.m4 /po.m4 +/posix_spawn.m4 /progtest.m4 /quote.m4 /quotearg.m4 +/rawmemchr.m4 +/sched_h.m4 /setenv.m4 +/sig_atomic_t.m4 +/sigaction.m4 +/signal_h.m4 +/signalblocking.m4 +/spawn_h.m4 +/stat.m4 /stdbool.m4 /stddef_h.m4 /stdint.m4 @@ -63,6 +80,7 @@ /stdio_h.m4 /stdlib_h.m4 /stpcpy.m4 +/strchrnul.m4 /strerror.m4 /string_h.m4 /strndup.m4 @@ -70,10 +88,14 @@ /strtol.m4 /strtoul.m4 /strverscmp.m4 +/sys_stat_h.m4 +/sys_wait_h.m4 /threadlib.m4 +/time_h.m4 /unistd-safer.m4 /unistd_h.m4 /unlocked-io.m4 +/wait-process.m4 /warn-on-use.m4 /warning.m4 /warnings.m4 diff --git a/po/POTFILES.in b/po/POTFILES.in index 5d6a216b..b2b8b5d5 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -22,6 +22,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 9761de92..e8bb2003 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 e8f28bff..53667834 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 6a05704b..212fa832 100644 --- a/src/output.c +++ b/src/output.c @@ -25,9 +25,10 @@ #include #include #include +#include #include -#include #include +#include #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 90a52dda..cd30576b 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 f8e16532..999ca184 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 f96d13e2..18acbc01 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 -- 2.45.2