[PATCH v2] c, c++: Plug -Wduplicated-cond memory leaks [PR99057]
Jason Merrill
jason@redhat.com
Thu Feb 11 02:05:57 GMT 2021
On 2/10/21 7:27 PM, Marek Polacek wrote:
> On Wed, Feb 10, 2021 at 04:48:56PM -0500, Jason Merrill wrote:
>> On 2/10/21 12:35 PM, Marek Polacek wrote:
>>> Freeing the condition chain needs to use vec_free which does ->release,
>>> or we leak memory.
>>
>> OK, but if chain were an auto_vec, delete would work.
>
> Yes.
>
>> Can we merge auto_vec with vec now that we're compiling as C++11?
>
> Yeah, that works too, though I like how vec_free also sets its argument
> to NULL... Up to you.
>
> Joseph, are you OK with this version too?
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> Verified that the leaks valgrind detected are gone.
>
> -- >8 --
> -Wduplicated-cond uses a simple vec for the condition chain and
> it uses delete to destroy the chain, which doesn't call ->release,
> resulting in memory leaks.
>
> This patch makes it use auto_vec instead; we'll now call ->release
> when deleting the chain.
>
> gcc/c-family/ChangeLog:
>
> * c-common.h (warn_duplicated_cond_add_or_warn): Use auto_vec
> for the condition chain.
> * c-warn.c (warn_duplicated_cond_add_or_warn): Likewise.
>
> gcc/c/ChangeLog:
>
> * c-parser.c (c_parser_statement_after_labels): Use auto_vec
> for the condition chain.
> (c_parser_if_statement): Likewise.
> (c_parser_else_body): Likewise.
>
> gcc/cp/ChangeLog:
>
> * parser.c (cp_parser_statement): Use auto_vec for the condition chain.
> (cp_parser_selection_statement): Likewise.
> (cp_parser_implicitly_scoped_statement): Likewise.
Ah, I didn't think so many other functions would need to change their
prototypes. Let's go with your original patch.
> ---
> gcc/c-family/c-common.h | 3 ++-
> gcc/c-family/c-warn.c | 3 ++-
> gcc/c/c-parser.c | 12 ++++++------
> gcc/cp/parser.c | 15 ++++++++-------
> 4 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index f30b6c6ac33..bc02481a11e 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1430,7 +1430,8 @@ extern void maybe_record_typedef_use (tree);
> extern void maybe_warn_unused_local_typedefs (void);
> extern void maybe_warn_bool_compare (location_t, enum tree_code, tree, tree);
> extern bool maybe_warn_shift_overflow (location_t, tree, tree);
> -extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec<tree> **);
> +extern void warn_duplicated_cond_add_or_warn (location_t, tree,
> + auto_vec<tree> **);
> extern bool diagnose_mismatched_attributes (tree, tree);
> extern tree do_warn_duplicated_branches_r (tree *, int *, void *);
> extern void warn_for_multistatement_macros (location_t, location_t,
> diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
> index e6e28d9b139..ce5d4890181 100644
> --- a/gcc/c-family/c-warn.c
> +++ b/gcc/c-family/c-warn.c
> @@ -2499,7 +2499,8 @@ maybe_warn_unused_local_typedefs (void)
> of COND. */
>
> void
> -warn_duplicated_cond_add_or_warn (location_t loc, tree cond, vec<tree> **chain)
> +warn_duplicated_cond_add_or_warn (location_t loc, tree cond,
> + auto_vec<tree> **chain)
> {
> /* No chain has been created yet. Do nothing. */
> if (*chain == NULL)
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index a8df208493c..0153f0805f8 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -1524,10 +1524,10 @@ static location_t c_parser_compound_statement_nostart (c_parser *);
> static void c_parser_label (c_parser *, tree);
> static void c_parser_statement (c_parser *, bool *, location_t * = NULL);
> static void c_parser_statement_after_labels (c_parser *, bool *,
> - vec<tree> * = NULL);
> + auto_vec<tree> * = nullptr);
> static tree c_parser_c99_block_statement (c_parser *, bool *,
> location_t * = NULL);
> -static void c_parser_if_statement (c_parser *, bool *, vec<tree> *);
> +static void c_parser_if_statement (c_parser *, bool *, auto_vec<tree> *);
> static void c_parser_switch_statement (c_parser *, bool *);
> static void c_parser_while_statement (c_parser *, bool, unsigned short, bool *);
> static void c_parser_do_statement (c_parser *, bool, unsigned short);
> @@ -6084,7 +6084,7 @@ c_parser_statement (c_parser *parser, bool *if_p, location_t *loc_after_labels)
>
> static void
> c_parser_statement_after_labels (c_parser *parser, bool *if_p,
> - vec<tree> *chain)
> + auto_vec<tree> *chain)
> {
> location_t loc = c_parser_peek_token (parser)->location;
> tree stmt = NULL_TREE;
> @@ -6373,7 +6373,7 @@ c_parser_if_body (c_parser *parser, bool *if_p,
>
> static tree
> c_parser_else_body (c_parser *parser, const token_indent_info &else_tinfo,
> - vec<tree> *chain)
> + auto_vec<tree> *chain)
> {
> location_t body_loc = c_parser_peek_token (parser)->location;
> tree block = c_begin_compound_stmt (flag_isoc99);
> @@ -6458,7 +6458,7 @@ c_parser_maybe_reclassify_token (c_parser *parser)
> implement -Wparentheses. */
>
> static void
> -c_parser_if_statement (c_parser *parser, bool *if_p, vec<tree> *chain)
> +c_parser_if_statement (c_parser *parser, bool *if_p, auto_vec<tree> *chain)
> {
> tree block;
> location_t loc;
> @@ -6494,7 +6494,7 @@ c_parser_if_statement (c_parser *parser, bool *if_p, vec<tree> *chain)
> {
> /* We've got "if (COND) else if (COND2)". Start the
> condition chain and add COND as the first element. */
> - chain = new vec<tree> ();
> + chain = new auto_vec<tree> ();
> if (!CONSTANT_CLASS_P (cond) && !TREE_SIDE_EFFECTS (cond))
> chain->safe_push (cond);
> }
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 5da8670f0e2..641113d0528 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -2176,7 +2176,8 @@ static void cp_parser_lambda_body
> /* Statements [gram.stmt.stmt] */
>
> static void cp_parser_statement
> - (cp_parser *, tree, bool, bool *, vec<tree> * = NULL, location_t * = NULL);
> + (cp_parser *, tree, bool, bool *, auto_vec<tree> * = nullptr,
> + location_t * = nullptr);
> static void cp_parser_label_for_labeled_statement
> (cp_parser *, tree);
> static tree cp_parser_expression_statement
> @@ -2186,7 +2187,7 @@ static tree cp_parser_compound_statement
> static void cp_parser_statement_seq_opt
> (cp_parser *, tree);
> static tree cp_parser_selection_statement
> - (cp_parser *, bool *, vec<tree> *);
> + (cp_parser *, bool *, auto_vec<tree> *);
> static tree cp_parser_condition
> (cp_parser *);
> static tree cp_parser_iteration_statement
> @@ -2211,7 +2212,7 @@ static void cp_parser_declaration_statement
> (cp_parser *);
>
> static tree cp_parser_implicitly_scoped_statement
> - (cp_parser *, bool *, const token_indent_info &, vec<tree> * = NULL);
> + (cp_parser *, bool *, const token_indent_info &, auto_vec<tree> * = nullptr);
> static void cp_parser_already_scoped_statement
> (cp_parser *, bool *, const token_indent_info &);
>
> @@ -11565,7 +11566,7 @@ add_debug_begin_stmt (location_t loc)
>
> static void
> cp_parser_statement (cp_parser* parser, tree in_statement_expr,
> - bool in_compound, bool *if_p, vec<tree> *chain,
> + bool in_compound, bool *if_p, auto_vec<tree> *chain,
> location_t *loc_after_labels)
> {
> tree statement, std_attrs = NULL_TREE;
> @@ -12186,7 +12187,7 @@ cp_parser_init_statement_p (cp_parser *parser)
>
> static tree
> cp_parser_selection_statement (cp_parser* parser, bool *if_p,
> - vec<tree> *chain)
> + auto_vec<tree> *chain)
> {
> cp_token *token;
> enum rid keyword;
> @@ -12329,7 +12330,7 @@ cp_parser_selection_statement (cp_parser* parser, bool *if_p,
> /* We've got "if (COND) else if (COND2)". Start
> the condition chain and add COND as the first
> element. */
> - chain = new vec<tree> ();
> + chain = new auto_vec<tree> ();
> if (!CONSTANT_CLASS_P (condition)
> && !TREE_SIDE_EFFECTS (condition))
> {
> @@ -13547,7 +13548,7 @@ cp_parser_declaration_statement (cp_parser* parser)
> static tree
> cp_parser_implicitly_scoped_statement (cp_parser* parser, bool *if_p,
> const token_indent_info &guard_tinfo,
> - vec<tree> *chain)
> + auto_vec<tree> *chain)
> {
> tree statement;
> location_t body_loc = cp_lexer_peek_token (parser->lexer)->location;
>
> base-commit: 21c6ad7a12fecc4c85ac26289d9096379b550585
>
More information about the Gcc-patches
mailing list