| Message ID | 20180404194855.GT8577@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Fix invalid structured binding range for parsing error recovery (PR c++/85194) | expand |
OK. On Wed, Apr 4, 2018 at 3:48 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > The maybe_range_for_decl argument is documented on > cp_parser_simple_declaration as: > If MAYBE_RANGE_FOR_DECL is not NULL, the pointed tree will be set to the > parsed declaration if it is an uninitialized single declarator not followed > by a `;', or to error_mark_node otherwise. Either way, the trailing `;', > if present, will not be consumed. */ > but we initialize *maybe_range_for_decl to NULL_TREE at the beginning, to > detect if we saw more than one decl; in case of a structured binding, we > only set it to non-NULL on success and thus violate what the comment says, > and on testcase like below we return is_range_for from the callers because > it is followed by colon, yet range_decl is NULL and we ICE trying to > dereference it. This patch ensures it is error_mark_node instead, it can > only happen in case of errors. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2018-04-04 Jakub Jelinek <jakub@redhat.com> > > PR c++/85194 > * parser.c (cp_parser_simple_declaration): For structured bindings, > if *maybe_range_for_decl is NULL after parsing it, set it to > error_mark_node. > > * g++.dg/cpp1z/decomp37.C: New test. > > --- gcc/cp/parser.c.jj 2018-04-03 18:05:26.000000000 +0200 > +++ gcc/cp/parser.c 2018-04-04 14:59:53.562859572 +0200 > @@ -12993,8 +12993,14 @@ cp_parser_simple_declaration (cp_parser* > /* The next token should be either a `,' or a `;'. */ > cp_token *token = cp_lexer_peek_token (parser->lexer); > /* If it's a `;', we are done. */ > - if (token->type == CPP_SEMICOLON || maybe_range_for_decl) > + if (token->type == CPP_SEMICOLON) > goto finish; > + else if (maybe_range_for_decl) > + { > + if (*maybe_range_for_decl == NULL_TREE) > + *maybe_range_for_decl = error_mark_node; > + goto finish; > + } > /* Anything else is an error. */ > else > { > --- gcc/testsuite/g++.dg/cpp1z/decomp37.C.jj 2018-04-04 15:12:33.724191835 +0200 > +++ gcc/testsuite/g++.dg/cpp1z/decomp37.C 2018-04-04 15:13:40.803227382 +0200 > @@ -0,0 +1,14 @@ > +// PR c++/85194 > +// { dg-do compile { target c++11 } } > +// { dg-options "" } > + > +struct A { int i; }; > + > +A x[2]; > + > +void > +foo () > +{ > + for (auto [i] = A () : x) // { dg-error "initializer in range-based 'for' loop" } > + ; // { dg-warning "structured bindings only available with" "" { target c++14_down } .-1 } > +} > > Jakub
--- gcc/cp/parser.c.jj 2018-04-03 18:05:26.000000000 +0200 +++ gcc/cp/parser.c 2018-04-04 14:59:53.562859572 +0200 @@ -12993,8 +12993,14 @@ cp_parser_simple_declaration (cp_parser* /* The next token should be either a `,' or a `;'. */ cp_token *token = cp_lexer_peek_token (parser->lexer); /* If it's a `;', we are done. */ - if (token->type == CPP_SEMICOLON || maybe_range_for_decl) + if (token->type == CPP_SEMICOLON) goto finish; + else if (maybe_range_for_decl) + { + if (*maybe_range_for_decl == NULL_TREE) + *maybe_range_for_decl = error_mark_node; + goto finish; + } /* Anything else is an error. */ else { --- gcc/testsuite/g++.dg/cpp1z/decomp37.C.jj 2018-04-04 15:12:33.724191835 +0200 +++ gcc/testsuite/g++.dg/cpp1z/decomp37.C 2018-04-04 15:13:40.803227382 +0200 @@ -0,0 +1,14 @@ +// PR c++/85194 +// { dg-do compile { target c++11 } } +// { dg-options "" } + +struct A { int i; }; + +A x[2]; + +void +foo () +{ + for (auto [i] = A () : x) // { dg-error "initializer in range-based 'for' loop" } + ; // { dg-warning "structured bindings only available with" "" { target c++14_down } .-1 } +}