| Message ID | 20191118190415.GL3791@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] c++/92450 - ICE with invalid nested name specifier. | expand |
On 11/18/19 7:04 PM, Marek Polacek wrote: > The newly added diagnostic causes an ICE because the new grokdeclarator call > returned error_mark_node and DECL_SOURCE_LOCATION crashes on that. So don't > attempt to print the new diagnostic if we've encountered something not > resembling a bit-field. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2019-11-18 Marek Polacek <polacek@redhat.com> > > PR c++/92450 - ICE with invalid nested name specifier. > * parser.c (cp_parser_member_declaration): Bail out for invalid code. > > * g++.dg/parse/crash71.C: New test. > > diff --git gcc/cp/parser.c gcc/cp/parser.c > index c473e7fd92f..caed6946977 100644 > --- gcc/cp/parser.c > +++ gcc/cp/parser.c > @@ -25052,6 +25052,9 @@ cp_parser_member_declaration (cp_parser* parser) > tree d = grokdeclarator (declarator, &decl_specifiers, > BITFIELD, /*initialized=*/false, > &attributes); > + /* Hopelessly invalid code, give up. */ > + if (error_operand_p (d)) > + goto out; > error_at (DECL_SOURCE_LOCATION (d), > "bit-field %qD has non-integral type %qT", > d, TREE_TYPE (d)); Don't we still want the rest of the error-recovery code there, i.e. skipping to the end of the statement? Jason
On Tue, Nov 19, 2019 at 05:34:45PM -0500, Jason Merrill wrote: > On 11/18/19 7:04 PM, Marek Polacek wrote: > > The newly added diagnostic causes an ICE because the new grokdeclarator call > > returned error_mark_node and DECL_SOURCE_LOCATION crashes on that. So don't > > attempt to print the new diagnostic if we've encountered something not > > resembling a bit-field. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > 2019-11-18 Marek Polacek <polacek@redhat.com> > > > > PR c++/92450 - ICE with invalid nested name specifier. > > * parser.c (cp_parser_member_declaration): Bail out for invalid code. > > > > * g++.dg/parse/crash71.C: New test. > > > > diff --git gcc/cp/parser.c gcc/cp/parser.c > > index c473e7fd92f..caed6946977 100644 > > --- gcc/cp/parser.c > > +++ gcc/cp/parser.c > > @@ -25052,6 +25052,9 @@ cp_parser_member_declaration (cp_parser* parser) > > tree d = grokdeclarator (declarator, &decl_specifiers, > > BITFIELD, /*initialized=*/false, > > &attributes); > > + /* Hopelessly invalid code, give up. */ > > + if (error_operand_p (d)) > > + goto out; > > error_at (DECL_SOURCE_LOCATION (d), > > "bit-field %qD has non-integral type %qT", > > d, TREE_TYPE (d)); > > Don't we still want the rest of the error-recovery code there, i.e. skipping > to the end of the statement? Sure, that works too, we just emit fewer diagnostics. But maybe that's not that interesting at this point, because we've already given an error. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-11-20 Marek Polacek <polacek@redhat.com> PR c++/92450 - ICE with invalid nested name specifier. * parser.c (cp_parser_member_declaration): Don't attempt to print erroneous bit-field diagnostic if grokdeclarator returns error_mark_node. * g++.dg/parse/crash71.C: New test. diff --git gcc/cp/parser.c gcc/cp/parser.c index c473e7fd92f..ad35945f954 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -25052,9 +25052,10 @@ cp_parser_member_declaration (cp_parser* parser) tree d = grokdeclarator (declarator, &decl_specifiers, BITFIELD, /*initialized=*/false, &attributes); - error_at (DECL_SOURCE_LOCATION (d), - "bit-field %qD has non-integral type %qT", - d, TREE_TYPE (d)); + if (!error_operand_p (d)) + error_at (DECL_SOURCE_LOCATION (d), + "bit-field %qD has non-integral type %qT", + d, TREE_TYPE (d)); cp_parser_skip_to_end_of_statement (parser); /* Avoid "extra ;" pedwarns. */ if (cp_lexer_next_token_is (parser->lexer, diff --git gcc/testsuite/g++.dg/parse/crash71.C gcc/testsuite/g++.dg/parse/crash71.C new file mode 100644 index 00000000000..13f484801fe --- /dev/null +++ gcc/testsuite/g++.dg/parse/crash71.C @@ -0,0 +1,11 @@ +// PR c++/92450 - ICE with invalid nested name specifier. + +typedef int C2; +struct B1 { + struct B2 { + }; +}; + +struct S6g { + C2 : B1:B2; // { dg-error "" } +};
On 11/20/19 8:49 PM, Marek Polacek wrote: > On Tue, Nov 19, 2019 at 05:34:45PM -0500, Jason Merrill wrote: >> On 11/18/19 7:04 PM, Marek Polacek wrote: >>> The newly added diagnostic causes an ICE because the new grokdeclarator call >>> returned error_mark_node and DECL_SOURCE_LOCATION crashes on that. So don't >>> attempt to print the new diagnostic if we've encountered something not >>> resembling a bit-field. >>> >>> Bootstrapped/regtested on x86_64-linux, ok for trunk? >>> >>> 2019-11-18 Marek Polacek <polacek@redhat.com> >>> >>> PR c++/92450 - ICE with invalid nested name specifier. >>> * parser.c (cp_parser_member_declaration): Bail out for invalid code. >>> >>> * g++.dg/parse/crash71.C: New test. >>> >>> diff --git gcc/cp/parser.c gcc/cp/parser.c >>> index c473e7fd92f..caed6946977 100644 >>> --- gcc/cp/parser.c >>> +++ gcc/cp/parser.c >>> @@ -25052,6 +25052,9 @@ cp_parser_member_declaration (cp_parser* parser) >>> tree d = grokdeclarator (declarator, &decl_specifiers, >>> BITFIELD, /*initialized=*/false, >>> &attributes); >>> + /* Hopelessly invalid code, give up. */ >>> + if (error_operand_p (d)) >>> + goto out; >>> error_at (DECL_SOURCE_LOCATION (d), >>> "bit-field %qD has non-integral type %qT", >>> d, TREE_TYPE (d)); >> >> Don't we still want the rest of the error-recovery code there, i.e. skipping >> to the end of the statement? > > Sure, that works too, we just emit fewer diagnostics. But maybe that's not > that interesting at this point, because we've already given an error. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? OK, thanks. > > 2019-11-20 Marek Polacek <polacek@redhat.com> > > PR c++/92450 - ICE with invalid nested name specifier. > * parser.c (cp_parser_member_declaration): Don't attempt to print > erroneous bit-field diagnostic if grokdeclarator returns > error_mark_node. > > * g++.dg/parse/crash71.C: New test. > > diff --git gcc/cp/parser.c gcc/cp/parser.c > index c473e7fd92f..ad35945f954 100644 > --- gcc/cp/parser.c > +++ gcc/cp/parser.c > @@ -25052,9 +25052,10 @@ cp_parser_member_declaration (cp_parser* parser) > tree d = grokdeclarator (declarator, &decl_specifiers, > BITFIELD, /*initialized=*/false, > &attributes); > - error_at (DECL_SOURCE_LOCATION (d), > - "bit-field %qD has non-integral type %qT", > - d, TREE_TYPE (d)); > + if (!error_operand_p (d)) > + error_at (DECL_SOURCE_LOCATION (d), > + "bit-field %qD has non-integral type %qT", > + d, TREE_TYPE (d)); > cp_parser_skip_to_end_of_statement (parser); > /* Avoid "extra ;" pedwarns. */ > if (cp_lexer_next_token_is (parser->lexer, > diff --git gcc/testsuite/g++.dg/parse/crash71.C gcc/testsuite/g++.dg/parse/crash71.C > new file mode 100644 > index 00000000000..13f484801fe > --- /dev/null > +++ gcc/testsuite/g++.dg/parse/crash71.C > @@ -0,0 +1,11 @@ > +// PR c++/92450 - ICE with invalid nested name specifier. > + > +typedef int C2; > +struct B1 { > + struct B2 { > + }; > +}; > + > +struct S6g { > + C2 : B1:B2; // { dg-error "" } > +}; >
diff --git gcc/cp/parser.c gcc/cp/parser.c index c473e7fd92f..caed6946977 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -25052,6 +25052,9 @@ cp_parser_member_declaration (cp_parser* parser) tree d = grokdeclarator (declarator, &decl_specifiers, BITFIELD, /*initialized=*/false, &attributes); + /* Hopelessly invalid code, give up. */ + if (error_operand_p (d)) + goto out; error_at (DECL_SOURCE_LOCATION (d), "bit-field %qD has non-integral type %qT", d, TREE_TYPE (d)); diff --git gcc/testsuite/g++.dg/parse/crash71.C gcc/testsuite/g++.dg/parse/crash71.C new file mode 100644 index 00000000000..13f484801fe --- /dev/null +++ gcc/testsuite/g++.dg/parse/crash71.C @@ -0,0 +1,11 @@ +// PR c++/92450 - ICE with invalid nested name specifier. + +typedef int C2; +struct B1 { + struct B2 { + }; +}; + +struct S6g { + C2 : B1:B2; // { dg-error "" } +};