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 livelocking 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 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.
(cherry picked from commit 22cc8d813e)

Conflicts:

	NEWS
	bootstrap.conf
	lib/.cvsignore
	lib/.gitignore
	m4/.cvsignore
	m4/.gitignore
	src/output.c
This commit is contained in:
Joel E. Denny
2010-02-22 18:09:19 -05:00
parent d67e37a74b
commit 47fa574761
19 changed files with 298 additions and 239 deletions

View File

@@ -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

View File

@@ -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);

View File

@@ -24,9 +24,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"
@@ -568,17 +569,17 @@ 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);
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");
@@ -588,14 +589,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);
}

View File

@@ -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;
}