[PATCH] On bulletproofing -fsyntax-only
Nix
nix@esperi.demon.co.uk
Sun May 28 02:53:00 GMT 2000
I've recently been using -fsyntax-only a lot, and I've been having a lot
of trouble with segfaults. These seem to be caused by things (especially
in varasm.c) shoving stuff into asm_out_file without checking
syntax_only.
Checking through the non-`execute' parts of the gcc.torture testsuite,
there are numerous additional failures when -fsyntax-only is on due to
the compiler getting segfaults (I've edited out those due to other
unrelated oddities where the lack of output breaks the testsuite
drivers):
FAIL: gcc.c-torture/compile/921126-1.c, -O0
FAIL: gcc.c-torture/compile/921126-1.c, -O1
FAIL: gcc.c-torture/compile/921126-1.c, -O2
FAIL: gcc.c-torture/compile/921126-1.c, -O3 -fomit-frame-pointer
FAIL: gcc.c-torture/compile/921126-1.c, -O3 -g
FAIL: gcc.c-torture/compile/921126-1.c, -Os
FAIL: gcc.c-torture/compile/931013-3.c, -O0
FAIL: gcc.c-torture/compile/931013-3.c, -O1
FAIL: gcc.c-torture/compile/931013-3.c, -O2
FAIL: gcc.c-torture/compile/931013-3.c, -O3 -fomit-frame-pointer
FAIL: gcc.c-torture/compile/931013-3.c, -O3 -g
FAIL: gcc.c-torture/compile/931013-3.c, -Os
FAIL: gcc.c-torture/compile/950612-1.c, -O0
FAIL: gcc.c-torture/compile/950612-1.c, -O1
FAIL: gcc.c-torture/compile/950612-1.c, -O2
FAIL: gcc.c-torture/compile/950612-1.c, -O3 -fomit-frame-pointer
FAIL: gcc.c-torture/compile/950612-1.c, -O3 -fomit-frame-pointer -funroll-loops
FAIL: gcc.c-torture/compile/950612-1.c, -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions
FAIL: gcc.c-torture/compile/950612-1.c, -O3 -g
FAIL: gcc.c-torture/compile/950612-1.c, -Os
FAIL: gcc.c-torture/compile/950729-1.c, -O0
FAIL: gcc.c-torture/compile/950729-1.c, -O1
FAIL: gcc.c-torture/compile/950729-1.c, -O2
FAIL: gcc.c-torture/compile/950729-1.c, -O3 -fomit-frame-pointer
FAIL: gcc.c-torture/compile/950729-1.c, -O3 -fomit-frame-pointer -funroll-loops
FAIL: gcc.c-torture/compile/950729-1.c, -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions
FAIL: gcc.c-torture/compile/950729-1.c, -O3 -g
FAIL: gcc.c-torture/compile/950729-1.c, -Os
FAIL: gcc.c-torture/compile/981001-2.c, -O0
FAIL: gcc.c-torture/compile/981001-2.c, -O1
FAIL: gcc.c-torture/compile/981001-2.c, -O2
FAIL: gcc.c-torture/compile/981001-2.c, -O3 -fomit-frame-pointer
FAIL: gcc.c-torture/compile/981001-2.c, -O3 -g
FAIL: gcc.c-torture/compile/981001-2.c, -Os
FAIL: gcc.c-torture/special/eeprof-1.c compilation, -O0
FAIL: gcc.c-torture/special/eeprof-1.c compilation, -O1
FAIL: gcc.c-torture/special/eeprof-1.c compilation, -O2
FAIL: gcc.c-torture/special/eeprof-1.c compilation, -O3 -fomit-frame-pointer
FAIL: gcc.c-torture/special/eeprof-1.c compilation, -O3 -g
FAIL: gcc.c-torture/special/eeprof-1.c compilation, -Os
FAIL: gcc.c-torture/unsorted/poor.c, -O1
FAIL: gcc.c-torture/unsorted/poor.c, -O2
FAIL: gcc.c-torture/unsorted/poor.c, -O3 -fomit-frame-pointer
FAIL: gcc.c-torture/unsorted/poor.c, -O3 -g
FAIL: gcc.c-torture/unsorted/signext.c, -O0
FAIL: gcc.c-torture/unsorted/signext.c, -O1
FAIL: gcc.c-torture/unsorted/signext.c, -O2
FAIL: gcc.c-torture/unsorted/signext.c, -O3 -fomit-frame-pointer
FAIL: gcc.c-torture/unsorted/signext.c, -O3 -g
FAIL: gcc.c-torture/unsorted/signext.c, -Os
Looking through the mailing list archives, I note that people have had
trouble with this in the past, and, indeed, Andreas and Craig have
checked in patches to put some checking in around places in varasm.c
where crashes were happening. Since people (like the Java frontend)
might actually *use* -fsyntax-only in the future, I think fixing it is
quite important, and also the sort of thing that a gcc newbie like me
could reasonably be expected to attack :)
I think that the method currently being used to suppress output within
gcc (preventing asm_out_file from being written to when syntax_only is
on if it looks like that code path will be walked down) is fragile,
because unless *every* use of asm_out_file is armoured that way, we'll
eventually end up trying to write to it again. And armouring every use
is tricky and ugly, because ports manipulate asm_out_file, and because
not every use of asm_out_file is via the ASM_*() macros.
I think the fundamental problem here is that the semantics of
asm_out_file are inconsistent. Instead of a nice simple `asm_out_file
always points to a place where you can write stuff', it is `asm_out_file
always points to a place where you can write stuff, except that
sometimes it is NULL so if you write to it you crash'.
I propose bulletproofing the existing code by making asm_out_file point
to /dev/null (or a comparable bitbucket) when syntax_only is on. With
that in place, we'll waste nothing but a little time if we accidentally
try to stick stuff into the asm_out_file, rather than crashing.
(I am wondering if given this patch it is still nice to armour code that
you know does no semantic checking in if `(syntax_only)' at all. If
-fsyntax-only means `do everything just like we would beforehand, only
throwing the output away' then we waste a bit of system time compared
to the current approach, but we *know* that the behaviour of gcc with
-fsyntax-only on is exactly the same as its behaviour with it off with
regard to sematic checking, which we cannot be really sure of now;
someone might later put semantic checking into a code path armoured
several functions up by syntax_only... for all I know this has already
happened. Certainly gcc's behaviour with -fsyntax-only on is *not*
very similar to its behaviour with it off right now ;) )
I foresee only two potential problems with this approach: different OSes
have different bit buckets (some might not have them at all), and you
can't normally seek in a bit bucket.
The first problem can be resolved by writing an autoconf check for
/dev/null and NUL and whatever other bit buckets people can think of ---
thus also removing the grotty code in protoize.c that conditionalizes on
the OS name to work out where the bit bucket is... but what to do on
systems with no bit bucket I am not sure. protoize already doesn't work
on such systems (if there are any) but protoize is comparatively
unimportant compared to all of gcc. This problem may in fact be
illusory, because `configure' requires a /dev/null, and how many systems
without autoconf support do we work on? (VMS? Does that port still
work?) For now I'm working with a hardwired "/dev/null" but that will
probably have to change.
(The second problem is illusory, because we already have -pipe, so we
can't assume that we can seek on the asm_out_file anyway.)
Here's a patch which squashes this problem using the method above
(without the /dev/null autoconf test, as yet). If needed I can mail off
a copyright assignment form (although, especially without the autoconf
part, this fix is probably too small to merit it). I've also got a patch
against 2.95.2, because my original problem was there ;)
This patch fixes the core dumps in all the tests mentioned above (but
only when --tool-opts inclues -fsyntax-only, of course). Some of those
tests still fail (notably special/eeprof-1.c), but that is because
dejagnu triggers on the `linking not done' error, not because the
compiler is still core dumping.
It (unsurprisingly) causes no regressions with a normal bootstrap/check
run.
Most of the changes are indentation changes.
2000-05-28 Nix <nix@esperi.demon.co.uk>
* toplev.c: (compile_file): Point the asm_out_file at /dev/null
when -fsyntax-only is on, not at NULL. Close it even if
-fsyntax-only is on.
--- egcs/gcc/toplev.c.orig Sat May 27 19:03:15 2000
+++ egcs/gcc/toplev.c Sun May 28 01:39:56 2000
@@ -2094,35 +2094,40 @@
/* Open assembler code output file. */
if (flag_syntax_only)
- asm_out_file = NULL;
+ {
+ /* If no assmbler is to be generated, we generate it anyway and throw
+ it away. This ensures that all semantic and syntactic checks
+ happen, even those that are mixed up with producing output. */
+
+ asm_file_name = "/dev/null";
+ name_specified = 1;
+ }
+
+ if (! name_specified && asm_file_name == 0)
+ asm_out_file = stdout;
else
{
- if (! name_specified && asm_file_name == 0)
- asm_out_file = stdout;
+ if (asm_file_name == 0)
+ {
+ int len = strlen (dump_base_name);
+ char *dumpname = (char *) xmalloc (len + 6);
+ memcpy (dumpname, dump_base_name, len + 1);
+ strip_off_ending (dumpname, len);
+ strcat (dumpname, ".s");
+ asm_file_name = dumpname;
+ }
+ if (!strcmp (asm_file_name, "-"))
+ asm_out_file = stdout;
else
- {
- if (asm_file_name == 0)
- {
- int len = strlen (dump_base_name);
- char *dumpname = (char *) xmalloc (len + 6);
- memcpy (dumpname, dump_base_name, len + 1);
- strip_off_ending (dumpname, len);
- strcat (dumpname, ".s");
- asm_file_name = dumpname;
- }
- if (!strcmp (asm_file_name, "-"))
- asm_out_file = stdout;
- else
- asm_out_file = fopen (asm_file_name, "w");
- if (asm_out_file == 0)
- pfatal_with_name (asm_file_name);
- }
+ asm_out_file = fopen (asm_file_name, "w");
+ if (asm_out_file == 0)
+ pfatal_with_name (asm_file_name);
+ }
#ifdef IO_BUFFER_SIZE
- setvbuf (asm_out_file, (char *) xmalloc (IO_BUFFER_SIZE),
- _IOFBF, IO_BUFFER_SIZE);
+ setvbuf (asm_out_file, (char *) xmalloc (IO_BUFFER_SIZE),
+ _IOFBF, IO_BUFFER_SIZE);
#endif
- }
if (ggc_p && name != 0)
name = ggc_alloc_string (name, strlen (name));
@@ -2386,8 +2391,7 @@
finish_parse ();
- if (! flag_syntax_only
- && (ferror (asm_out_file) != 0 || fclose (asm_out_file) != 0))
+ if (ferror (asm_out_file) != 0 || fclose (asm_out_file) != 0)
fatal_io_error (asm_file_name);
/* Do whatever is necessary to finish printing the graphs. */
--
`People's needs are not `finance'. You can't eat a bank.' --- Alan Rosenthal
More information about the Gcc-patches
mailing list