| Message ID | AANLkTim95pxeQ5B3ERo+0JXE_ZRD7wGKxErpNLeMz_=i@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 11/13/2010 06:21 PM, H.J. Lu wrote: > On Sat, Nov 13, 2010 at 8:51 AM, Paolo Bonzini<bonzini@gnu.org> wrote: >> On 11/13/2010 05:25 PM, H.J. Lu wrote: >>> >>> There is one ICE: >>> >>> FAIL: gcc.dg/pr14963.c (internal compiler error) >>> FAIL: gcc.dg/pr14963.c (test for excess errors) >>> >>> It looks like a real bug. >> >> It is PR45062, it's just exposed by this patch because the problematic >> statement is parsed instead of skipped. So the ICE is "correct", it's an >> improvement in coverage. > > Like this? I have no idea, I didn't investigate it much. Maybe it's possible to avoid the TREE_LIST altogether, or use a VEC somewhere. I'd leave it to Nathan, who said he'd fix it at the beginning of stage3. :) Paolo
On Sat, Nov 13, 2010 at 09:21:15AM -0800, H.J. Lu wrote: > On Sat, Nov 13, 2010 at 8:51 AM, Paolo Bonzini <bonzini@gnu.org> wrote: > > It is PR45062, it's just exposed by this patch because the problematic > > statement is parsed instead of skipped. So the ICE is "correct", it's an > > improvement in coverage. > > > > > Like this? > > -- > H.J. > --- > diff --git a/gcc/c-decl.c b/gcc/c-decl.c > index c0d5a49..2b0c0e4 100644 > --- a/gcc/c-decl.c > +++ b/gcc/c-decl.c > @@ -4046,7 +4046,11 @@ start_decl (struct c_declarator *declarator, > struct c_declspecs *declspecs, > if (ce->kind == cdk_function) > { > tree args = ce->u.arg_info->parms; > - for (; args; args = DECL_CHAIN (args)) > + /* ARGS may contain a mixture of DECLs and TREE_LISTs due to > + invalid input, so be careful. */ > + for (; args; args = (DECL_P (args) > + ? DECL_CHAIN (args) > + : TREE_CHAIN (args))) > { > tree type = TREE_TYPE (args); > if (type && INTEGRAL_TYPE_P (type) I agree that this fixes the bug; I don't think it's the right fix. Everyplace else we grovel through arg_info->parms, it's a chain of DECLs...why not here? The reason we're getting TREE_LISTs here is this bit of code in grokparms: else if (arg_types && TREE_CODE (TREE_VALUE (arg_types)) == IDENTIFIER_NODE) { if (!funcdef_flag) pedwarn (input_location, 0, "parameter names (without types) in function declaration"); arg_info->parms = arg_info->types; arg_info->types = 0; return 0; } which is somewhat dubious (and would be invalid code if we had a more statically typed IR). I think the right fix is to zero out arg_info->parms here if !funcdef_flag. Like I said, I'll work on this one, unless you beat me to it. :) -Nathan
On Sat, Nov 13, 2010 at 9:34 AM, Nathan Froyd <froydnj@codesourcery.com> wrote: > On Sat, Nov 13, 2010 at 09:21:15AM -0800, H.J. Lu wrote: >> On Sat, Nov 13, 2010 at 8:51 AM, Paolo Bonzini <bonzini@gnu.org> wrote: >> > It is PR45062, it's just exposed by this patch because the problematic >> > statement is parsed instead of skipped. So the ICE is "correct", it's an >> > improvement in coverage. >> > >> >> >> Like this? >> >> -- >> H.J. >> --- >> diff --git a/gcc/c-decl.c b/gcc/c-decl.c >> index c0d5a49..2b0c0e4 100644 >> --- a/gcc/c-decl.c >> +++ b/gcc/c-decl.c >> @@ -4046,7 +4046,11 @@ start_decl (struct c_declarator *declarator, >> struct c_declspecs *declspecs, >> if (ce->kind == cdk_function) >> { >> tree args = ce->u.arg_info->parms; >> - for (; args; args = DECL_CHAIN (args)) >> + /* ARGS may contain a mixture of DECLs and TREE_LISTs due to >> + invalid input, so be careful. */ >> + for (; args; args = (DECL_P (args) >> + ? DECL_CHAIN (args) >> + : TREE_CHAIN (args))) >> { >> tree type = TREE_TYPE (args); >> if (type && INTEGRAL_TYPE_P (type) > > I agree that this fixes the bug; I don't think it's the right fix. > Everyplace else we grovel through arg_info->parms, it's a chain of > DECLs...why not here? > > The reason we're getting TREE_LISTs here is this bit of code in > grokparms: > > else if (arg_types && TREE_CODE (TREE_VALUE (arg_types)) == IDENTIFIER_NODE) > { > if (!funcdef_flag) > pedwarn (input_location, 0, "parameter names (without types) in function declaration"); > > arg_info->parms = arg_info->types; > arg_info->types = 0; > return 0; > } > > which is somewhat dubious (and would be invalid code if we had a more > statically typed IR). I think the right fix is to zero out > arg_info->parms here if !funcdef_flag. Like I said, I'll work on this > one, unless you beat me to it. :) > No, I have no plan to work on it.
diff --git a/gcc/c-decl.c b/gcc/c-decl.c index c0d5a49..2b0c0e4 100644 --- a/gcc/c-decl.c +++ b/gcc/c-decl.c @@ -4046,7 +4046,11 @@ start_decl (struct c_declarator *declarator, struct c_declspecs *declspecs, if (ce->kind == cdk_function) { tree args = ce->u.arg_info->parms; - for (; args; args = DECL_CHAIN (args)) + /* ARGS may contain a mixture of DECLs and TREE_LISTs due to + invalid input, so be careful. */ + for (; args; args = (DECL_P (args) + ? DECL_CHAIN (args) + : TREE_CHAIN (args))) { tree type = TREE_TYPE (args);