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
