Bug 14137 - [pch] ICE in cgraph_finalize_compilation_unit, at cgraphunit.c:407
Summary: [pch] ICE in cgraph_finalize_compilation_unit, at cgraphunit.c:407
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: pch (show other bugs)
Version: 3.4.0
: P2 normal
Target Milestone: 4.0.0
Assignee: Geoff Keating
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2004-02-13 06:22 UTC by MattyT
Modified: 2004-04-08 23:45 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2004-04-08 19:55:31


Attachments
patch to add flag setting checks (1.26 KB, patch)
2004-03-21 21:59 UTC, Steven Bosscher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description MattyT 2004-02-13 06:22:04 UTC
The following code:

#include <string>
std::string str;

gives an ICE when compiled as:

>>>g++ -funit-at-a-time -include bits/stdc++.h ~/prepare/ice2reduced.cc

/home/matty/prepare/ice2reduced.cc:216: internal compiler error: in
cgraph_finalize_compilation_unit, at cgraphunit.c:407

on Linux x86 with G++ 3.4.
Comment 1 Andrew Pinski 2004-02-13 06:36:03 UTC
Confirmed.  This is an unit-at-a-time and a PCH problem as not doing either will cause this to pass.  I 
do not know which one is causing the exact problem, a cgraph data structor not being marked as GC or 
something in PCH which is causing it.
Comment 2 Andrew Pinski 2004-02-13 06:42:09 UTC
This is a PCH problem, it should have rejected the PCH file as unit-at-a-time was not turned on when 
the file was compiled and not really an unit-at-a-time problem at all.
Comment 3 geoffk@desire.geoffk.org 2004-02-13 18:55:00 UTC
Subject: Re:  [pch] ICE in cgraph_finalize_compilation_unit, at cgraphunit.c:407

> Date: 13 Feb 2004 06:42:10 -0000
> From: "pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org>

> This is a PCH problem, it should have rejected the PCH file as
> unit-at-a-time was not turned on when the file was compiled and not
> really an unit-at-a-time problem at all.

That sounds plausible.  Would you like to produce a patch?

Comment 4 Steven Bosscher 2004-03-21 21:59:43 UTC
Created attachment 5966 [details]
patch to add flag setting checks

I've bootstrapped this on i686 and x86_64 with multilib, tested it on x86_64,
and verified that it fixes the bug:

cc1plus: warning:
/opt/experimental/bin/../lib/gcc/i686-pc-linux-gnu/3.4.0/../../../../include/c++/3.4.0/i686-pc-linux-gnu/bits/stdc++.h.gch/O0g:
created using different flags
cc1plus: warning:
/opt/experimental/bin/../lib/gcc/i686-pc-linux-gnu/3.4.0/../../../../include/c++/3.4.0/i686-pc-linux-gnu/bits/stdc++.h.gch/O2g:
not used because `__OPTIMIZE__' not defined
cc1plus: bits/stdc++.h: No such file or directory
cc1plus: one or more PCH files were found, but they were invalid
Comment 5 Steven Bosscher 2004-03-22 17:54:08 UTC
proposed patch here: http://gcc.gnu.org/ml/gcc-patches/2004-03/msg01794.html 
 
Comment 6 Steven Bosscher 2004-03-22 18:34:45 UTC
Also passed testing for 3.4.0 now on i686 and x86_64. 
 
I'll take this bug for now. 
Comment 7 Mark Mitchell 2004-03-23 21:52:50 UTC
I'm not sure what to say about this patch.

You've fixed just one of a tiny number of problems of the same flavor.  (For
example, consider -fabi-version=X!)

Most (all other?) compilers with PCH implementations store the entire
command-line and compare it.  We should do the same, but that would take some
infrastructure.  

One good step would be to organize all of our flags into a single structure;
then that structure could be emitted as part of the PCH file.  For example:

  struct flags {
    int unit_at_a_time;
    ...
  };

(These could also be bitfields of course.)

Then, you wouldn't have to translate between flag_* and bits in the PCH header,
and you could just memcmp the two structures.

Yes, there might be cases where that would incorrect reject a PCH, but that is
the conservative choice.  You could make override the comparison of particular
fields to make the comparison laxer as required.

I'll approve this patch for GCC 3.4.0 and mainline, but I'd like to see a more
comprehensive approach attempted in future.
Comment 8 Steven Bosscher 2004-03-23 22:50:35 UTC
I completely agree, see what I said in my mail to gcc-patches.  But to stuff 
all flags and debug settings (and machine-specific flags??) into one big struct 
and writing that out to the pch looks a bit unsafe for 3.4 ;-)

I was actually hoping that Neil's work on option handling would have included 
moving all flags to some struct, but unfortunately...

I'll commit the patch and keep the bug open, targeted for 3.5.

Comment 9 CVS Commits 2004-03-24 22:28:07 UTC
Subject: Bug 14137

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	steven@gcc.gnu.org	2004-03-24 22:27:56

Modified files:
	gcc            : ChangeLog c-pch.c 

Log message:
	Paper over PR pch/14137
	
	* c-pch.c (struct c_pch_validity): New flags_info field.
	(FLAG_UNIT_AT_A_TIME_SET): New definition.
	(pch_init): Write out the flags_info field to the PCH.  Set the
	FLAG_UNIT_AT_A_TIME_SET bit of the field if flag_unit_at_a_time
	is set.
	(c_common_valid_pch): Make sure the flag settings used for compiling
	the PCH are the same as those used in the current compilation.
	
	This needs revisiting for 3.5.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.368&r2=2.2326.2.369
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-pch.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.19&r2=1.19.4.1

Comment 10 Steven Bosscher 2004-03-24 22:30:41 UTC
Retargeted for 3.5.0. 
 
I hope we can rework our handing of flags, should be doable to 
perhaps autogenerate a flags.h using Neil's flags handling scripts... 
 
Mark, for 3.4.0 you can obviously still add some other flags checks 
with this hack.  There are still a few bits available ;-) 
Comment 11 Geoff Keating 2004-03-26 23:38:10 UTC
Most flags shouldn't affect acceptance/rejection of a PCH.  -funit-at-a-time is somewhat unusual in 
this regard, since it completely changes how compilation is done.  Most warning flags should be OK, for 
instance I just checked that -Winline is safe.  Many optimisation flags will be OK, for instance 
-fschedule-insns.  Most flags that don't affect code generation, like -dH, will be OK.  Some flags can be 
special-cased, like -g.  I don't think it's the best thing to simply assume that every flag will affect PCH 
without actually checking each flag individually, since I believe that for the majority of flags this will be 
incorrect or it will be easy to modify GCC so that the flag does not affect PCH.
Comment 12 Steven Bosscher 2004-03-26 23:53:46 UTC
Subject: Re:  [pch] ICE in cgraph_finalize_compilation_unit, at cgraphunit.c:407

On Saturday 27 March 2004 00:38, geoffk at gcc dot gnu dot org wrote:
> ------- Additional Comments From geoffk at gcc dot gnu dot org  2004-03-26 23:38 -------
> Most flags shouldn't affect acceptance/rejection of a PCH. 

While not all flags affect code generation, it doesn't seem unreasonable
to make sure that at least all -m*, -f*, and -g* flags are the same.  It
doesn't sound very safe, or maintainable, to keep track of which flags
do affect PCH and which don't.  Getting all the GTYs right has turned
out to be hard enough ;-)

Seriously though, how likely is it that people will use different flags
for their PCH than for their normal compilation?  Doesn't Joe User
typicaly just use "-O[123] -g"?

I'd rather be safe and make sure all flags match than chasing flag bugs
for PCH...


Comment 13 geoffk@desire.geoffk.org 2004-03-27 06:16:42 UTC
Subject: Re:  [pch] ICE in cgraph_finalize_compilation_unit, at cgraphunit.c:407


> While not all flags affect code generation

That's not the test for whether a flag will cause problems with PCH.
A good test is "If this flag could be controlled by a #pragma at the
top level (that is, outside declarations), would it work?"

> Seriously though, how likely is it that people will use different flags
> for their PCH than for their normal compilation?

What about for their abnormal compilation, or if the PCH file is not
"their" PCH?  One kind of case I'm thinking of is when someone has
just one file that they need to compile with -fno-some-optimisation,
since that optimisation turns out to make this file run slower (or
crash the compiler or whatever).  Or they have a flag like
-fpermissive that they used to use everywhere, and are trying to
eliminate, and they're about half-done.

> Doesn't Joe User typicaly just use "-O[123] -g"?

No, users as a group don't typically use any particular flag or
combination.

> I'd rather be safe and make sure all flags match than chasing flag bugs
> for PCH...

That would make PCH less useful.  You could just remove PCH
altogether, then you would have no PCH bugs to chase at all.

What I think would be helpful would be to update the documentation
every time a flag is discovered to be safe or unsafe for PCH.  At the
moment, it basically says "I don't know".  I think I will create the
structure for this now.

Comment 14 Steven Bosscher 2004-03-27 10:11:04 UTC
I think this idea is unsafe and don't want to implement it. 
Comment 15 Mark Mitchell 2004-03-30 17:23:01 UTC
Subject: Re:  [pch] ICE in cgraph_finalize_compilation_unit,
 at cgraphunit.c:407

steven at gcc dot gnu dot org wrote:

>------- Additional Comments From steven at gcc dot gnu dot org  2004-03-27 10:11 -------
>I think this idea is unsafe and don't want to implement it. 
>  
>
Since silent miscompilation and/or inexplicable crashses can occur from 
using a wrong PCH, we should err on the side of caution.  If the flags 
used to build the PCH and to compile the main translation unit are not 
known to be compatible, we should reject the PCH.

Sometimes, that will mean that we do not use a PCH when we could, but 
that's hardly as confusing to users as doing something unpredictable 
because our data structures get confused.

This conservative approach is also what is used by other PCH 
implemenations, so it will match user expectations.

Comment 16 geoffk@desire.geoffk.org 2004-03-30 23:06:59 UTC
Subject: Re:  [pch] ICE in cgraph_finalize_compilation_unit, at cgraphunit.c:407

> Date: 30 Mar 2004 17:23:10 -0000
> From: "mark at codesourcery dot com" <gcc-bugzilla@gcc.gnu.org>

> This conservative approach is also what is used by other PCH 
> implemenations, so it will match user expectations.

At least one PCH implementation, Metrowerks' codewarrior, appears to
be able to load a PCH even if optimisation, warning, or preprocessor
flags are changed.  (I am not sure what the behaviour is generally; in
the case of preprocessor flags, you get an inconsistent compilation.)

Comment 17 Wolfgang Bangerth 2004-03-30 23:25:55 UTC
In my view, if compilation time is a problem, then  
a) the project is big, 
b) we are talking about regular builds, not ones where someone 
   is playing with compiler options, and 
b) very likely people are using a makefile to build things. 
It seems very likely that within a Makefile, the number of different 
combinations of flags is relatively small (say: 2, one for debug, one 
for optimized builds), and in this case it is reasonable to ask them 
that they generate one pch file per set of flags. After all, we support 
this, by putting multiple pch files into one directory. 
 
I consider simply requiring the user to use the exact same combination 
of flags as an absolutely acceptable requirement and am not expecting 
that we get many bug reports. On the other hand, if we get reports about 
incorrect code generation due to problems with PCH, this is going to be 
significantly worse. Please also keep in mind here that PCH bugs are us 
bugmasters' worst nightmares, since they often come in huge chunks, 
involve not only a single preprocessed file but a large collection of header 
files, may involve different compilation flags for different parts of the 
build, etc. PCH bugs have been very resistant to our efforts in the past, 
and have most often been analysed by the people who know how PCH is  
implemented, rather than by bugzilla people. I therefore very strongly 
urge you to go the safe route and only allow mixing compilation flags for 
PCH generation and PCH use if we are _absolutely_ (double-checked, verifiably) 
sure that this is safe! 
 
W. 
 
Comment 18 Geoff Keating 2004-04-08 19:55:30 UTC
I'll work on this.
Comment 19 Mark Mitchell 2004-04-08 20:16:07 UTC
Subject: Re:  [pch] ICE in cgraph_finalize_compilation_unit,
 at cgraphunit.c:407

geoffk at gcc dot gnu dot org wrote:

>------- Additional Comments From geoffk at gcc dot gnu dot org  2004-04-08 19:55 -------
>I'll work on this.
>  
>
Thanks!

Comment 20 CVS Commits 2004-04-08 23:41:16 UTC
Subject: Bug 14137

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	geoffk@gcc.gnu.org	2004-04-08 23:41:09

Modified files:
	gcc            : ChangeLog c-pch.c 
	gcc/doc        : invoke.texi 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.dg/pch: valid-1.c valid-1.hs valid-1b.c 
	                          valid-1b.hs valid-2.c valid-2.hs 
	                          valid-3.c valid-3.hs valid-4.c 
	                          valid-4.hs valid-5.c valid-5.hs 
	                          valid-6.c valid-6.hs 

Log message:
	2004-04-08  Geoffrey Keating  <geoffk@apple.com>
	
	PR pch/13419
	PR pch/14137
	Radar #: 3315288
	* doc/invoke.texi (Precompiled Headers): Suggest -o
	to put an output file in a particular place.  Be more detailed
	about which options affect PCH validity and which options
	might not work.
	* c-pch.c (pch_matching): New.
	(MATCH_SIZE): New.
	(struct c_pch_validity): New field 'match'.
	(pch_init): Handle pch_matching.
	(c_common_valid_pch): Check pch_matching.
	
	Index: testsuite/ChangeLog
	2004-04-08  Geoffrey Keating  <geoffk@apple.com>
	
	* gcc.dg/pch/valid-1.c, gcc.dg/pch/valid-2.c, gcc.dg/pch/valid-3.c,
	gcc.dg/pch/valid-4.c, gcc.dg/pch/valid-5.c, gcc.dg/pch/valid-6.c,
	gcc.dg/pch/valid-1.hs, gcc.dg/pch/valid-2.hs, gcc.dg/pch/valid-3.hs,
	gcc.dg/pch/valid-4.hs, gcc.dg/pch/valid-5.hs, gcc.dg/pch/valid-6.hs:
	New tests.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.3370&r2=2.3371
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-pch.c.diff?cvsroot=gcc&r1=1.21&r2=1.22
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/doc/invoke.texi.diff?cvsroot=gcc&r1=1.441&r2=1.442
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.3662&r2=1.3663
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-1.hs.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-1b.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-1b.hs.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-2.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-2.hs.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-3.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-3.hs.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-4.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-4.hs.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-5.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-5.hs.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-6.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/pch/valid-6.hs.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 21 Geoff Keating 2004-04-08 23:45:53 UTC
Fixed by patch (I hope).