From f39ab2869cae648b37bd954fedd99dab8d79ce9f 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/local.mk (lib_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. (cherry picked from commit 22cc8d813ee57c9631e527a31010ab138f9b7e06) Conflicts: NEWS bootstrap.conf lib/.cvsignore lib/.gitignore lib/Makefile.am m4/.cvsignore m4/.gitignore src/output.c --- ChangeLog | 52 +++++++++++++ NEWS | 6 ++ THANKS | 1 + bootstrap.conf | 2 +- gnulib | 2 +- lib/.cvsignore | 49 +++++++++++++ lib/.gitignore | 49 +++++++++++++ lib/local.mk | 4 +- lib/subpipe.c | 177 --------------------------------------------- lib/subpipe.h | 28 ------- m4/.cvsignore | 21 ++++++ m4/.gitignore | 21 ++++++ 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, 299 insertions(+), 240 deletions(-) delete mode 100644 lib/subpipe.c delete mode 100644 lib/subpipe.h diff --git a/ChangeLog b/ChangeLog index bca61fc2..379afe21 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/local.mk (lib_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 ae87c0af..83f56c6e 100644 --- a/NEWS +++ b/NEWS @@ -238,6 +238,12 @@ Bison News * Changes in version 2.4.2 (????-??-??): +** 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. POSIX specifies that an error be reported for any identifier that does diff --git a/THANKS b/THANKS index 1cfb47c6..e1628bef 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 2d522e25..b14cabd6 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -20,7 +20,7 @@ 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 + maintainer-makefile malloc mbswidth obstack pipe quote quotearg realloc-posix stdbool stpcpy strerror strtoul strverscmp unistd unistd-safer unlocked-io update-copyright unsetenv verify warnings xalloc xalloc-die xstrndup 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 01087fd4..d3c20e87 100644 --- a/lib/.cvsignore +++ b/lib/.cvsignore @@ -8,6 +8,7 @@ argmatch.h asnprintf.c basename-lgpl.c basename.c +binary-io.h bitrotate.h c-ctype.c c-ctype.h @@ -15,6 +16,8 @@ c-strcase.h c-strcasecmp.c c-strncasecmp.c charset.alias +cloexec.c +cloexec.h config.charset config.h config.hin @@ -22,6 +25,7 @@ configmake.h dirname-lgpl.c dirname.c dirname.h +dup-safer-flag.c dup-safer.c dup2.c errno.h @@ -30,9 +34,12 @@ 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 float+.h float.h @@ -45,6 +52,7 @@ frexp.c frexpl.c fseterr.c fseterr.h +getdtablesize.c getopt.c getopt.h getopt.in.h @@ -80,7 +88,12 @@ memchr.c memchr.valgrind obstack.c obstack.h +open.c pipe-safer.c +pipe.c +pipe.h +pipe2-safer.c +pipe2.c printf-args.c printf-args.h printf-frexp.c @@ -94,18 +107,42 @@ quote.c quote.h quotearg.c quotearg.h +rawmemchr.c +rawmemchr.valgrind realloc.c 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 signbitd.c signbitf.c signbitl.c +sigprocmask.c size_max.h snprintf.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 sprintf.c stamp-h1 +stat.c stdbool.h stdbool.in.h stdbool_.h @@ -124,6 +161,8 @@ stdlib.h stdlib.in.h stdlib_.h stpcpy.c +strchrnul.c +strchrnul.valgrind streq.h strerror.c string.h @@ -136,6 +175,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 @@ -152,6 +198,9 @@ verify.h vfprintf.c vsnprintf.c vsprintf.c +w32spawn.h +wait-process.c +wait-process.h wchar.h wchar.in.h wchar_.h diff --git a/lib/.gitignore b/lib/.gitignore index 44d4102b..b2890169 100644 --- a/lib/.gitignore +++ b/lib/.gitignore @@ -11,6 +11,7 @@ /asnprintf.c /basename-lgpl.c /basename.c +/binary-io.h /bitrotate.h /c-ctype.c /c-ctype.h @@ -18,6 +19,8 @@ /c-strcasecmp.c /c-strncasecmp.c /charset.alias +/cloexec.c +/cloexec.h /config.charset /config.h /config.hin @@ -25,6 +28,7 @@ /dirname-lgpl.c /dirname.c /dirname.h +/dup-safer-flag.c /dup-safer.c /dup2.c /errno.h @@ -33,9 +37,12 @@ /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 /float+.h /float.h @@ -48,6 +55,7 @@ /frexpl.c /fseterr.c /fseterr.h +/getdtablesize.c /getopt.c /getopt.h /getopt.in.h @@ -83,7 +91,12 @@ /memchr.valgrind /obstack.c /obstack.h +/open.c /pipe-safer.c +/pipe.c +/pipe.h +/pipe2-safer.c +/pipe2.c /printf-args.c /printf-args.h /printf-frexp.c @@ -97,18 +110,42 @@ /quote.h /quotearg.c /quotearg.h +/rawmemchr.c +/rawmemchr.valgrind /realloc.c /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 /signbitd.c /signbitf.c /signbitl.c +/sigprocmask.c /size_max.h /snprintf.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 /sprintf.c /stamp-h1 +/stat.c /stdbool.h /stdbool.in.h /stdbool_.h @@ -127,6 +164,8 @@ /stdlib.in.h /stdlib_.h /stpcpy.c +/strchrnul.c +/strchrnul.valgrind /streq.h /strerror.c /string.h @@ -139,6 +178,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 @@ -155,6 +201,9 @@ /vfprintf.c /vsnprintf.c /vsprintf.c +/w32spawn.h +/wait-process.c +/wait-process.h /wchar.h /wchar.in.h /wchar_.h diff --git a/lib/local.mk b/lib/local.mk index e9edefb2..298a8aa2 100644 --- a/lib/local.mk +++ b/lib/local.mk @@ -51,9 +51,7 @@ lib_libbison_a_SOURCES += \ # Non-gnulib sources in Bison's internal library. lib_libbison_a_SOURCES += \ lib/get-errno.h \ - lib/get-errno.c \ - lib/subpipe.h \ - lib/subpipe.c + lib/get-errno.c # The Yacc compatibility library. lib_LIBRARIES = $(YACC_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 5268bd85..23153051 100644 --- a/m4/.cvsignore +++ b/m4/.cvsignore @@ -3,6 +3,7 @@ absolute-header.m4 alloca.m4 argmatch.m4 assert.m4 +cloexec.m4 config-h.m4 dirname.m4 dos.m4 @@ -16,6 +17,7 @@ exponentd.m4 exponentf.m4 exponentl.m4 extensions.m4 +fatal-signal.m4 fcntl-o.m4 fcntl.m4 fcntl_h.m4 @@ -25,6 +27,7 @@ fpieee.m4 fprintf-posix.m4 frexp.m4 frexpl.m4 +getdtablesize.m4 getopt.m4 getpagesize.m4 gettext.m4 @@ -63,10 +66,15 @@ mbstate_t.m4 mbswidth.m4 memchr.m4 mmap-anon.m4 +mode_t.m4 multiarch.m4 nls.m4 nocrash.m4 +open.m4 +pipe.m4 +pipe2.m4 po.m4 +posix_spawn.m4 printf-frexp.m4 printf-frexpl.m4 printf-posix-rpl.m4 @@ -75,12 +83,20 @@ printf.m4 progtest.m4 quote.m4 quotearg.m4 +rawmemchr.m4 realloc.m4 +sched_h.m4 setenv.m4 +sig_atomic_t.m4 +sigaction.m4 +signal_h.m4 +signalblocking.m4 signbit.m4 snprintf-posix.m4 snprintf.m4 +spawn_h.m4 sprintf-posix.m4 +stat.m4 stdbool.m4 stddef_h.m4 stdint.m4 @@ -89,6 +105,7 @@ stdio-safer.m4 stdio_h.m4 stdlib_h.m4 stpcpy.m4 +strchrnul.m4 strerror.m4 string_h.m4 strndup.m4 @@ -96,7 +113,10 @@ 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 @@ -106,6 +126,7 @@ vfprintf-posix.m4 vsnprintf-posix.m4 vsnprintf.m4 vsprintf-posix.m4 +wait-process.m4 warn-on-use.m4 warning.m4 warnings.m4 diff --git a/m4/.gitignore b/m4/.gitignore index 1bb8454a..7ff1d99e 100644 --- a/m4/.gitignore +++ b/m4/.gitignore @@ -3,6 +3,7 @@ /alloca.m4 /argmatch.m4 /assert.m4 +/cloexec.m4 /config-h.m4 /dirname.m4 /dos.m4 @@ -16,6 +17,7 @@ /exponentf.m4 /exponentl.m4 /extensions.m4 +/fatal-signal.m4 /fcntl-o.m4 /fcntl.m4 /fcntl_h.m4 @@ -25,6 +27,7 @@ /fprintf-posix.m4 /frexp.m4 /frexpl.m4 +/getdtablesize.m4 /getopt.m4 /getpagesize.m4 /gettext.m4 @@ -63,10 +66,15 @@ /mbswidth.m4 /memchr.m4 /mmap-anon.m4 +/mode_t.m4 /multiarch.m4 /nls.m4 /nocrash.m4 +/open.m4 +/pipe.m4 +/pipe2.m4 /po.m4 +/posix_spawn.m4 /printf-frexp.m4 /printf-frexpl.m4 /printf-posix-rpl.m4 @@ -75,12 +83,20 @@ /progtest.m4 /quote.m4 /quotearg.m4 +/rawmemchr.m4 /realloc.m4 +/sched_h.m4 /setenv.m4 +/sig_atomic_t.m4 +/sigaction.m4 +/signal_h.m4 +/signalblocking.m4 /signbit.m4 /snprintf-posix.m4 /snprintf.m4 +/spawn_h.m4 /sprintf-posix.m4 +/stat.m4 /stdbool.m4 /stddef_h.m4 /stdint.m4 @@ -89,6 +105,7 @@ /stdio_h.m4 /stdlib_h.m4 /stpcpy.m4 +/strchrnul.m4 /strerror.m4 /string_h.m4 /strndup.m4 @@ -96,7 +113,10 @@ /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 @@ -106,6 +126,7 @@ /vsnprintf-posix.m4 /vsnprintf.m4 /vsprintf-posix.m4 +/wait-process.m4 /warn-on-use.m4 /warning.m4 /warnings.m4 diff --git a/po/POTFILES.in b/po/POTFILES.in index 59f7701c..a9519338 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 243bf069..71d3b042 100644 --- a/src/files.c +++ b/src/files.c @@ -319,21 +319,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); @@ -342,18 +342,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 3c1da649..86d9728b 100644 --- a/src/output.c +++ b/src/output.c @@ -24,9 +24,10 @@ #include #include #include +#include #include -#include #include +#include #include "complain.h" #include "files.h" @@ -639,8 +640,9 @@ output_skeleton (void) aver (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); @@ -648,7 +650,7 @@ output_skeleton (void) if (trace_flag & trace_muscles) muscles_output (stderr); { - FILE *out = fdopen (filter_fd[0], "w"); + FILE *out = fdopen (filter_fd[1], "w"); if (! out) error (EXIT_FAILURE, get_errno (), "fdopen"); @@ -658,14 +660,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 6cf360a8..ab334eb5 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 94c618d8..bb6d4491 100644 --- a/tests/output.at +++ b/tests/output.at @@ -145,7 +145,9 @@ AT_DATA([$1], foo: {}; ]]) +[cp ]$1[ expout] AT_BISON_CHECK([$3 $1], $5, [], [$4]) +AT_CHECK([[cat $1]], [[0]], [expout]) AT_CLEANUP ]) @@ -165,7 +167,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