From 8a40fef3106214c7755971f3e27c715b3968ddf4 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Thu, 19 May 2016 20:29:07 +0000 Subject: [PATCH] PR c/71171: Fix uninitialized source_range in c_parser_postfix_expression A common way for a c_expr to have an uninitialized src_range is in error-handling, where the "value" field is set to error_mark_node without touching the src_range, leading to complaints from valgrind. 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 other related 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). 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. From-SVN: r236488 --- gcc/c/ChangeLog | 10 +++++++ gcc/c/c-parser.c | 72 ++++++++++++++++++++++++------------------------ gcc/c/c-tree.h | 9 ++++++ gcc/c/c-typeck.c | 7 ++++- 4 files changed, 61 insertions(+), 37 deletions(-) diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog index 476718fa631..5731048b087 100644 --- a/gcc/c/ChangeLog +++ b/gcc/c/ChangeLog @@ -1,3 +1,13 @@ +2016-05-19 David Malcolm + + 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. + 2016-05-17 James Greenhalgh * c-typeck.c (parser_build_unary_op): Fix formatting. diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 6523c08d63f..c2c83143c05 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -7194,7 +7194,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; @@ -7505,13 +7505,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); @@ -7525,7 +7525,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; @@ -7547,7 +7547,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 (); @@ -7576,7 +7576,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, @@ -7636,7 +7636,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); @@ -7645,7 +7645,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; @@ -7655,7 +7655,7 @@ c_parser_postfix_expression (c_parser *parser) "expected %<)%>"); if (t1 == NULL) { - expr.value = error_mark_node; + expr.set_error (); } else { @@ -7677,7 +7677,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); @@ -7688,7 +7688,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; } @@ -7777,7 +7777,7 @@ c_parser_postfix_expression (c_parser *parser) &cexpr_list, true, &close_paren_loc)) { - expr.value = error_mark_node; + expr.set_error (); break; } @@ -7785,7 +7785,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; } @@ -7810,25 +7810,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; } { @@ -7840,7 +7840,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; } @@ -7865,14 +7865,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; } @@ -7907,7 +7907,7 @@ c_parser_postfix_expression (c_parser *parser) &cexpr_list, false, &close_paren_loc)) { - expr.value = error_mark_node; + expr.set_error (); break; } @@ -7915,7 +7915,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; } @@ -7937,7 +7937,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)) @@ -7945,7 +7945,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, @@ -7971,7 +7971,7 @@ c_parser_postfix_expression (c_parser *parser) &cexpr_list, false, &close_paren_loc)) { - expr.value = error_mark_node; + expr.set_error (); break; } @@ -7994,7 +7994,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; @@ -8004,7 +8004,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; } { @@ -8021,14 +8021,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; } { @@ -8047,13 +8047,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; } @@ -8076,7 +8076,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) { @@ -8095,7 +8095,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; @@ -8116,7 +8116,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 @@ -8331,7 +8331,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; @@ -8363,7 +8363,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 4a0236d87a9..444e9a4777e 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -143,6 +143,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 8297aae4bd8..30102404284 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);