[PATCH] PR c/71171: Fix uninitialized source_range in c_parser_postfix_expression
David Malcolm
dmalcolm@redhat.com
Thu May 19 00:42:00 GMT 2016
PR c/71171 reports yet another instance of the src_range of a
c_expr being used without initialization. Investigation shows
that this was due to error-handling, where the "value" field of
a c_expr is set to error_mark_node without touching the
src_range, leading to complaints from valgrind.
This seems to be a common mistake, so this patch introduces a
new method, c_expr::set_error, which sets the value to
error_mark_node whilst initializing the src_range to
UNKNOWN_LOCATION.
This fixes the valgrind issue seen in PR c/71171, along with various
similar issues seen when running the testsuite using the checker
patch I posted here:
https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00887.html
(this checker still doesn't fully work yet, but it seems to be good
for easily detecting these issues without needing Valgrind).
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for trunk and for gcc-6-branch?
gcc/c/ChangeLog:
PR c/71171
* c-parser.c (c_parser_generic_selection): Use c_expr::set_error
in error-handling.
(c_parser_postfix_expression): Likewise.
* c-tree.h (c_expr::set_error): New method.
* c-typeck.c (parser_build_binary_op): In error-handling, ensure
that result's range is initialized.
---
gcc/c/c-parser.c | 72 ++++++++++++++++++++++++++++----------------------------
gcc/c/c-tree.h | 9 +++++++
gcc/c/c-typeck.c | 7 +++++-
3 files changed, 51 insertions(+), 37 deletions(-)
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 5703989..5edeb64 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -7200,7 +7200,7 @@ c_parser_generic_selection (c_parser *parser)
error_expr.original_code = ERROR_MARK;
error_expr.original_type = NULL;
- error_expr.value = error_mark_node;
+ error_expr.set_error ();
matched_assoc.type_location = UNKNOWN_LOCATION;
matched_assoc.type = NULL_TREE;
matched_assoc.expression = error_expr;
@@ -7511,13 +7511,13 @@ c_parser_postfix_expression (c_parser *parser)
gcc_assert (c_dialect_objc ());
if (!c_parser_require (parser, CPP_DOT, "expected %<.%>"))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
if (c_parser_next_token_is_not (parser, CPP_NAME))
{
c_parser_error (parser, "expected identifier");
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
c_token *component_tok = c_parser_peek_token (parser);
@@ -7531,7 +7531,7 @@ c_parser_postfix_expression (c_parser *parser)
}
default:
c_parser_error (parser, "expected expression");
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
break;
@@ -7553,7 +7553,7 @@ c_parser_postfix_expression (c_parser *parser)
parser->error = true;
c_parser_skip_until_found (parser, CPP_CLOSE_BRACE, NULL);
c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
stmt = c_begin_stmt_expr ();
@@ -7582,7 +7582,7 @@ c_parser_postfix_expression (c_parser *parser)
"expected %<)%>");
if (type_name == NULL)
{
- expr.value = error_mark_node;
+ expr.set_error ();
}
else
expr = c_parser_postfix_expression_after_paren_type (parser,
@@ -7642,7 +7642,7 @@ c_parser_postfix_expression (c_parser *parser)
c_parser_consume_token (parser);
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
e1 = c_parser_expr_no_commas (parser, NULL);
@@ -7651,7 +7651,7 @@ c_parser_postfix_expression (c_parser *parser)
if (!c_parser_require (parser, CPP_COMMA, "expected %<,%>"))
{
c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
loc = c_parser_peek_token (parser)->location;
@@ -7661,7 +7661,7 @@ c_parser_postfix_expression (c_parser *parser)
"expected %<)%>");
if (t1 == NULL)
{
- expr.value = error_mark_node;
+ expr.set_error ();
}
else
{
@@ -7683,7 +7683,7 @@ c_parser_postfix_expression (c_parser *parser)
c_parser_consume_token (parser);
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
t1 = c_parser_type_name (parser);
@@ -7694,7 +7694,7 @@ c_parser_postfix_expression (c_parser *parser)
if (parser->error)
{
c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
@@ -7783,7 +7783,7 @@ c_parser_postfix_expression (c_parser *parser)
&cexpr_list, true,
&close_paren_loc))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
@@ -7791,7 +7791,7 @@ c_parser_postfix_expression (c_parser *parser)
{
error_at (loc, "wrong number of arguments to "
"%<__builtin_choose_expr%>");
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
@@ -7816,25 +7816,25 @@ c_parser_postfix_expression (c_parser *parser)
c_parser_consume_token (parser);
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
t1 = c_parser_type_name (parser);
if (t1 == NULL)
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
if (!c_parser_require (parser, CPP_COMMA, "expected %<,%>"))
{
c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
t2 = c_parser_type_name (parser);
if (t2 == NULL)
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
{
@@ -7846,7 +7846,7 @@ c_parser_postfix_expression (c_parser *parser)
e2 = groktypename (t2, NULL, NULL);
if (e1 == error_mark_node || e2 == error_mark_node)
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
@@ -7871,14 +7871,14 @@ c_parser_postfix_expression (c_parser *parser)
&cexpr_list, false,
&close_paren_loc))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
if (vec_safe_length (cexpr_list) != 2)
{
error_at (loc, "wrong number of arguments to "
"%<__builtin_call_with_static_chain%>");
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
@@ -7913,7 +7913,7 @@ c_parser_postfix_expression (c_parser *parser)
&cexpr_list, false,
&close_paren_loc))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
@@ -7921,7 +7921,7 @@ c_parser_postfix_expression (c_parser *parser)
{
error_at (loc, "wrong number of arguments to "
"%<__builtin_complex%>");
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
@@ -7943,7 +7943,7 @@ c_parser_postfix_expression (c_parser *parser)
{
error_at (loc, "%<__builtin_complex%> operand "
"not of real binary floating-point type");
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
if (TYPE_MAIN_VARIANT (TREE_TYPE (e1_p->value))
@@ -7951,7 +7951,7 @@ c_parser_postfix_expression (c_parser *parser)
{
error_at (loc,
"%<__builtin_complex%> operands of different types");
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
pedwarn_c90 (loc, OPT_Wpedantic,
@@ -7977,7 +7977,7 @@ c_parser_postfix_expression (c_parser *parser)
&cexpr_list, false,
&close_paren_loc))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
@@ -8000,7 +8000,7 @@ c_parser_postfix_expression (c_parser *parser)
{
error_at (loc, "wrong number of arguments to "
"%<__builtin_shuffle%>");
- expr.value = error_mark_node;
+ expr.set_error ();
}
set_c_expr_source_range (&expr, loc, close_paren_loc);
break;
@@ -8010,7 +8010,7 @@ c_parser_postfix_expression (c_parser *parser)
c_parser_consume_token (parser);
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
{
@@ -8027,14 +8027,14 @@ c_parser_postfix_expression (c_parser *parser)
c_parser_consume_token (parser);
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
if (c_parser_next_token_is_not (parser, CPP_NAME))
{
c_parser_error (parser, "expected identifier");
c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
{
@@ -8053,13 +8053,13 @@ c_parser_postfix_expression (c_parser *parser)
c_parser_consume_token (parser);
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
{
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
t1 = c_parser_type_name (parser);
if (t1 == NULL)
{
- expr.value = error_mark_node;
+ expr.set_error ();
c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
break;
}
@@ -8082,7 +8082,7 @@ c_parser_postfix_expression (c_parser *parser)
error_at (loc, "-fcilkplus must be enabled to use "
"%<_Cilk_spawn%>");
expr = c_parser_cast_expression (parser, NULL);
- expr.value = error_mark_node;
+ expr.set_error ();
}
else if (c_parser_peek_token (parser)->keyword == RID_CILK_SPAWN)
{
@@ -8101,7 +8101,7 @@ c_parser_postfix_expression (c_parser *parser)
break;
default:
c_parser_error (parser, "expected expression");
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
break;
@@ -8122,7 +8122,7 @@ c_parser_postfix_expression (c_parser *parser)
/* Else fall through to report error. */
default:
c_parser_error (parser, "expected expression");
- expr.value = error_mark_node;
+ expr.set_error ();
break;
}
return c_parser_postfix_expression_after_primary
@@ -8337,7 +8337,7 @@ c_parser_postfix_expression_after_primary (c_parser *parser,
else
{
c_parser_error (parser, "expected identifier");
- expr.value = error_mark_node;
+ expr.set_error ();
expr.original_code = ERROR_MARK;
expr.original_type = NULL;
return expr;
@@ -8369,7 +8369,7 @@ c_parser_postfix_expression_after_primary (c_parser *parser,
else
{
c_parser_error (parser, "expected identifier");
- expr.value = error_mark_node;
+ expr.set_error ();
expr.original_code = ERROR_MARK;
expr.original_type = NULL;
return expr;
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index d9c4c5d..a00882a 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -169,6 +169,15 @@ struct c_expr
of this expression. */
location_t get_start () const { return src_range.m_start; }
location_t get_finish () const { return src_range.m_finish; }
+
+ /* Set the value to error_mark_node whilst ensuring that src_range
+ is initialized. */
+ void set_error ()
+ {
+ value = error_mark_node;
+ src_range.m_start = UNKNOWN_LOCATION;
+ src_range.m_finish = UNKNOWN_LOCATION;
+ }
};
/* Type alias for struct c_expr. This allows to use the structure
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 8405c3d..390959f 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3533,7 +3533,12 @@ parser_build_binary_op (location_t location, enum tree_code code,
result.original_type = NULL;
if (TREE_CODE (result.value) == ERROR_MARK)
- return result;
+ {
+ set_c_expr_source_range (&result,
+ arg1.get_start (),
+ arg2.get_finish ());
+ return result;
+ }
if (location != UNKNOWN_LOCATION)
protected_set_expr_location (result.value, location);
--
1.8.5.3
More information about the Gcc-patches
mailing list