Bug 17023 - [3.3/3.4/4.0 Regression] ICE with nested functions in parameter declaration
Summary: [3.3/3.4/4.0 Regression] ICE with nested functions in parameter declaration
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.0.0
: P2 normal
Target Milestone: 3.4.3
Assignee: Richard Henderson
URL:
Keywords: ice-on-invalid-code
Depends on:
Blocks:
 
Reported: 2004-08-14 00:12 UTC by Joseph S. Myers
Modified: 2004-10-14 23:25 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 2.95.3
Known to fail: 3.0.4 3.3.3 3.2.3 3.4.0 4.0.0
Last reconfirmed: 2004-10-07 23:09:18


Attachments
patch to allow nested functions, for mainline (1.66 KB, patch)
2004-10-14 20:40 UTC, Richard Henderson
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph S. Myers 2004-08-14 00:12:44 UTC
While converting the C front end to use proper structures instead of
trees for declarators, I came up with the following testcase which
ICEs with CVS mainline and most other GCC versions I tried it with.
(With 2.95.3, 2.7.2.3 and EGCS 1.0.3a, instead it gives bogus errors.)
The code is undoubtedly invalid, but no compiler version rejects it
with a meaningful error.  (I might look into this later.)

void
f(a, b)
     int a;
     int b[({ void h() {} 1; })];
{
}

nested-func-mess.c: In function `f':
nested-func-mess.c:4: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.

Nested functions clearly should not be allowed outside the body of a
function.
Comment 1 Andrew Pinski 2004-08-14 02:19:04 UTC
Confirned, considered a regression because of the ICE.
Comment 2 Andrew Pinski 2004-08-14 02:27:13 UTC
ICEing since at least 2000-12-31.
Comment 3 Mark Mitchell 2004-08-29 18:06:18 UTC
Postponed all ice-on-invalid bugs to GCC 3.4.3.
Comment 4 Richard Henderson 2004-10-14 18:08:31 UTC
Looks to me as if the nested function has nothing to do with it.  Rather, 
it's the statement expression.  Just "int b[({ 1; })]" is enough to crash.
Comment 5 Joseph S. Myers 2004-10-14 18:25:43 UTC
Subject: Re:  [3.3/3.4/4.0 Regression] ICE with nested functions
 in parameter declaration

On Thu, 14 Oct 2004, rth at gcc dot gnu dot org wrote:

> Looks to me as if the nested function has nothing to do with it.  Rather, 
> it's the statement expression.  Just "int b[({ 1; })]" is enough to crash.

I think the plain statement expression case is actually valid code (the 
statement expression should be evaluated on function entry just like the 
other array size expressions in the parameters).

Comment 6 Richard Henderson 2004-10-14 19:55:09 UTC
You do?  Hm, in which case I may need to persue a different solution than 
the one I'm currently testing.  Also, if true, I don't see why a nested
function wouldn't be acceptable there:

  int b[({ int h() { return 1; } h(); })]
Comment 7 Joseph S. Myers 2004-10-14 20:05:36 UTC
Subject: Re:  [3.3/3.4/4.0 Regression] ICE with nested functions
 in parameter declaration

On Thu, 14 Oct 2004, rth at gcc dot gnu dot org wrote:

> You do?  Hm, in which case I may need to persue a different solution than 
> the one I'm currently testing.  Also, if true, I don't see why a nested
> function wouldn't be acceptable there:
> 
>   int b[({ int h() { return 1; } h(); })]

Perhaps you can make nested functions work there, but they seem very 
dubious when not actually within a function body.  Whereas since array 
size expressions can include calls to other functions, or recursively to 
the same function, or indeed jump out of the evaluation of array size 
expressions with longjmp, statement expressions seem more reasonable there 
(though if they attempt to jump into the body of the function that might 
be problematic).

Comment 8 Joseph S. Myers 2004-10-14 20:12:37 UTC
Subject: Re:  [3.3/3.4/4.0 Regression] ICE with nested functions
 in parameter declaration

On Thu, 14 Oct 2004, jsm at polyomino dot org dot uk wrote:

> expressions with longjmp, statement expressions seem more reasonable there 
> (though if they attempt to jump into the body of the function that might 
> be problematic).

Actually, given that we disallow statement expressions in prototypes 
outside a function (even where they only mean [*] and have no further 
significance) perhaps it does make sense to be stricter about saying that 
old-style parameter declarations don't count as inside a function for this 
purpose, and disallowing statement expressions there in general.  But if 
we do that then we should consider if declarations of parameters to nested 
functions likewise are restricted and can't include statement expressions.

Comment 9 Richard Henderson 2004-10-14 20:40:56 UTC
Created attachment 7352 [details]
patch to allow nested functions, for mainline

I can see arguments either way.  Allowing statement expressions to me means
allowing effectively arbitrary code.  If we can handle *any* statement expr,
then we can handle nested functions there as well.

The only thing that would bother me about disallowing statement expressions
here, when we do allow arbitrary function calls, is that glibc headers tend
to have statement expressions hanging about in various macros that implement
various well-known library functions.

Anyway, the attached patch works for mainline.	If we are to allow statement
expressions (or nested functions), I don't know if I can make it work for 3.4.
There's a related pr that we closed WONTFIX that is a prerequisite for this.

What do you think?
Comment 10 Joseph S. Myers 2004-10-14 20:48:34 UTC
Subject: Re:  [3.3/3.4/4.0 Regression] ICE with nested functions
 in parameter declaration

On Thu, 14 Oct 2004, rth at gcc dot gnu dot org wrote:

> Created an attachment (id=7352)
>  --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=7352&action=view)
> patch to allow nested functions, for mainline

Seems reasonable enough to me as a fix for now if this now works sensibly.  
We may need to revisit some of this later if there are other problems (and 
it does still seem a little odd in language definition terms that these 
things are allowed in old-style function parameter declarations but not in 
prototype declarations in the definition, although it makes sense in 
implementation terms of not knowing whether the prototype is in a 
definition until it is all parsed).

Comment 11 Richard Henderson 2004-10-14 21:09:51 UTC
Hum.  That is a good point about this patch only mattering for K&R function
definitions.  In which case I don't really care at all, and so we should 
probably just go with the first choice to disallow stmt exprs here.

That also lets me fix 3.4 with an absolute minimum of effort, which is happy.
Comment 12 GCC Commits 2004-10-14 23:13:03 UTC
Subject: Bug 17023

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	rth@gcc.gnu.org	2004-10-14 23:12:53

Modified files:
	gcc            : ChangeLog c-parse.in 

Log message:
	PR c/17023
	* c-parse.in (compstmt_primary_start): Check last_tree non-null,
	not current_function_decl non-null.

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.661&r2=2.2326.2.662
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-parse.in.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.194.2.5&r2=1.194.2.6

Comment 13 GCC Commits 2004-10-14 23:21:07 UTC
Subject: Bug 17023

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	rth@gcc.gnu.org	2004-10-14 23:20:58

Modified files:
	gcc            : ChangeLog c-decl.c c-parse.in 
Added files:
	gcc/testsuite/gcc.dg: 20041014-1.c 

Log message:
	PR c/17023
	* c-decl.c (store_parm_decls_oldstyle): Care for parameter type
	as error_mark_node.
	* c-parse.in (compstmt_primary_start): Check cur_stmt_list non-null
	instaed of current_function_decl non-null.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.5889&r2=2.5890
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-decl.c.diff?cvsroot=gcc&r1=1.601&r2=1.602
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-parse.in.diff?cvsroot=gcc&r1=1.247&r2=1.248
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/20041014-1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 14 Richard Henderson 2004-10-14 23:25:16 UTC
Fixed.