Re: [C++ PATCH] Fix invalid structured binding range for parsing error recovery (PR c++/85194)
OK.
On Wed, Apr 4, 2018 at 3:48 PM, Jakub Jelinek 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
>
> 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.0 +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.jj2018-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
[C++ PATCH] Fix invalid structured binding range for parsing error recovery (PR c++/85194)
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
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.0 +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.jj2018-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
