c: Fix sizeof error recovery [PR117745]
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
Commit Message
Hi!
Compilation of the following testcase hangs forever after emitting first
error. The problem is that in one place we just return error_mark_node
directly rather than going through c_expr_sizeof_expr or c_expr_sizeof_type.
The parsing of the expression could have called record_maybe_used_decl
though, but nothing calls pop_maybe_used which needs to be called after
parsing of every sizeof/typeof, successful or not.
At the end of the toplevel declaration we free the parser_obstack and in
another function record_maybe_used_decl is called again and due to the
missing pop_maybe_unused we end up with a cycle in the chain.
The following patch fixes it by just setting error and goto to the
sizeof_expr:
c_inhibit_evaluation_warnings--;
in_sizeof--;
mark_exp_read (expr.value);
if (TREE_CODE (expr.value) == COMPONENT_REF
&& DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
error_at (expr_loc, "%<sizeof%> applied to a bit-field");
result = c_expr_sizeof_expr (expr_loc, expr);
where c_expr_sizeof_expr will do:
struct c_expr ret;
if (expr.value == error_mark_node)
{
ret.value = error_mark_node;
ret.original_code = ERROR_MARK;
ret.original_type = NULL;
ret.m_decimal = 0;
pop_maybe_used (false);
}
...
return ret;
which is exactly what the old code did manually except for the missing
pop_maybe_used call. mark_exp_read does nothing on error_mark_node and
error_mark_node doesn't have COMPONENT_REF tree_code.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2024-11-27 Jakub Jelinek <jakub@redhat.com>
PR c/117745
* c-parser.cc (c_parser_sizeof_expression): If type_name is NULL,
just expr.set_error () and goto sizeof_expr instead of doing error
recovery manually.
* gcc.dg/pr117745.c: New test.
Jakub
Comments
On Wed, Nov 27, 2024 at 10:12:25AM +0100, Jakub Jelinek wrote:
> Hi!
>
> Compilation of the following testcase hangs forever after emitting first
> error. The problem is that in one place we just return error_mark_node
> directly rather than going through c_expr_sizeof_expr or c_expr_sizeof_type.
> The parsing of the expression could have called record_maybe_used_decl
> though, but nothing calls pop_maybe_used which needs to be called after
> parsing of every sizeof/typeof, successful or not.
Ah, I see.
> At the end of the toplevel declaration we free the parser_obstack and in
> another function record_maybe_used_decl is called again and due to the
> missing pop_maybe_unused we end up with a cycle in the chain.
>
> The following patch fixes it by just setting error and goto to the
> sizeof_expr:
> c_inhibit_evaluation_warnings--;
> in_sizeof--;
> mark_exp_read (expr.value);
> if (TREE_CODE (expr.value) == COMPONENT_REF
> && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
> error_at (expr_loc, "%<sizeof%> applied to a bit-field");
> result = c_expr_sizeof_expr (expr_loc, expr);
> where c_expr_sizeof_expr will do:
> struct c_expr ret;
> if (expr.value == error_mark_node)
> {
> ret.value = error_mark_node;
> ret.original_code = ERROR_MARK;
> ret.original_type = NULL;
> ret.m_decimal = 0;
> pop_maybe_used (false);
> }
> ...
> return ret;
> which is exactly what the old code did manually except for the missing
> pop_maybe_used call. mark_exp_read does nothing on error_mark_node and
> error_mark_node doesn't have COMPONENT_REF tree_code.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2024-11-27 Jakub Jelinek <jakub@redhat.com>
>
> PR c/117745
> * c-parser.cc (c_parser_sizeof_expression): If type_name is NULL,
> just expr.set_error () and goto sizeof_expr instead of doing error
> recovery manually.
>
> * gcc.dg/pr117745.c: New test.
>
> --- gcc/c/c-parser.cc.jj 2024-11-23 13:00:28.328028242 +0100
> +++ gcc/c/c-parser.cc 2024-11-26 11:59:18.036410746 +0100
> @@ -10405,13 +10405,8 @@ c_parser_sizeof_expression (c_parser *pa
> finish = parser->tokens_buf[0].location;
> if (type_name == NULL)
> {
> - struct c_expr ret;
> - c_inhibit_evaluation_warnings--;
> - in_sizeof--;
> - ret.set_error ();
> - ret.original_code = ERROR_MARK;
> - ret.original_type = NULL;
> - return ret;
> + expr.set_error ();
> + goto sizeof_expr;
Patch is OK, though I'd appreciate a comment explaining why we bother to
call c_expr_sizeof_expr. Maybe
/* Let c_expr_sizeof_expr call pop_maybe_used; the parsing of the
expression could have called record_maybe_used_decl. */
or something like that. c_expr_sizeof_type doesn't do it, so that wouldn't
do.
> }
> if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
> {
> --- gcc/testsuite/gcc.dg/pr117745.c.jj 2024-11-26 12:07:11.120756946 +0100
> +++ gcc/testsuite/gcc.dg/pr117745.c 2024-11-26 12:08:00.031068850 +0100
> @@ -0,0 +1,8 @@
> +/* PR c/117745 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +static int foo (void);
> +void bar (void) { sizeof (int [0 ? foo () : 1); } /* { dg-error "expected" } */
> +static int baz (void);
> +void qux (void) { sizeof (sizeof (int[baz ()])); }
>
> Jakub
>
Marek
@@ -10405,13 +10405,8 @@ c_parser_sizeof_expression (c_parser *pa
finish = parser->tokens_buf[0].location;
if (type_name == NULL)
{
- struct c_expr ret;
- c_inhibit_evaluation_warnings--;
- in_sizeof--;
- ret.set_error ();
- ret.original_code = ERROR_MARK;
- ret.original_type = NULL;
- return ret;
+ expr.set_error ();
+ goto sizeof_expr;
}
if (c_parser_next_token_is (parser, CPP_OPEN_BRACE))
{
@@ -0,0 +1,8 @@
+/* PR c/117745 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+static int foo (void);
+void bar (void) { sizeof (int [0 ? foo () : 1); } /* { dg-error "expected" } */
+static int baz (void);
+void qux (void) { sizeof (sizeof (int[baz ()])); }