public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] plug memory leaks in warn_parm_array_mismatch (PR 99055)
@ 2021-02-10 17:16 Martin Sebor
  2021-02-11  7:59 ` Richard Biener
  0 siblings, 1 reply; 11+ messages in thread
From: Martin Sebor @ 2021-02-10 17:16 UTC (permalink / raw)
  To: gcc-patches, Richard Biener

[-- Attachment #1: Type: text/plain, Size: 3089 bytes --]

The attached patch replaces calls to print_generic_expr_to_str() with
a helper function that returns a std::string and releases the caller
from the responsibility to explicitly free memory.

With the patch installed, Valgrind shows more leaks in this code that
I'm not sure what to do about:

1) A tree built by build_type_attribute_qual_variant() called from
    attr_access::array_as_string() to build a temporary type only
    for the purposes of formatting it.

2) A tree (an attribute list) built by tree_cons() called from
    build_attr_access_from_parms() that's used only for the duration
    of the caller.

Do these temporary trees need to be released somehow or are the leaks
expected?

Martin

Leak 1:
==7112== 56 bytes in 1 blocks are still reachable in loss record 73 of 691
==7112==    at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==7112==    by 0x294ED71: xcalloc (xmalloc.c:162)
==7112==    by 0xB8B21D: alloc_page(unsigned int) (ggc-page.c:918)
==7112==    by 0xB8BAD1: ggc_internal_alloc(unsigned long, void 
(*)(void*), unsigned long, unsigned long) (ggc-page.c:1294)
==7112==    by 0x9E1F33: ggc_internal_alloc(unsigned long) (ggc.h:130)
==7112==    by 0x198AB1A: ggc_alloc_tree_node_stat(unsigned long) 
(ggc.h:309)
==7112==    by 0x193BD18: copy_node(tree_node*) (tree.c:1223)
==7112==    by 0x1953BDD: build_distinct_type_copy(tree_node*) (tree.c:6730)
==7112==    by 0x9A1379: build_type_attribute_qual_variant(tree_node*, 
tree_node*, int) (attribs.c:1161)
==7112==    by 0x9A483D: 
attr_access::array_as_string[abi:cxx11](tree_node*) const (attribs.c:2332)
==7112==    by 0xB7C5A7: warn_parm_array_mismatch(unsigned int, 
tree_node*, tree_node*) (c-warn.c:3449)
==7112==    by 0xA3EB57: c_parser_declaration_or_fndef(c_parser*, bool, 
bool, bool, bool, bool, tree_node**, vec<c_token, va_heap, vl_ptr>, 
bool, tree_node*, oacc_routine_data*, bool*) (c-parser.c:2342)


Leak 2:
==7112== 64 bytes in 1 blocks are still reachable in loss record 150 of 691
==7112==    at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==7112==    by 0x294ED71: xcalloc (xmalloc.c:162)
==7112==    by 0xB8B21D: alloc_page(unsigned int) (ggc-page.c:918)
==7112==    by 0xB8BAD1: ggc_internal_alloc(unsigned long, void 
(*)(void*), unsigned long, unsigned long) (ggc-page.c:1294)
==7112==    by 0x9E1F33: ggc_internal_alloc(unsigned long) (ggc.h:130)
==7112==    by 0x198AB1A: ggc_alloc_tree_node_stat(unsigned long) 
(ggc.h:309)
==7112==    by 0x19433E6: tree_cons(tree_node*, tree_node*, tree_node*) 
(tree.c:3331)
==7112==    by 0xB67188: build_attr_access_from_parms(tree_node*, bool) 
(c-attribs.c:5060)
==7112==    by 0xB7C008: warn_parm_array_mismatch(unsigned int, 
tree_node*, tree_node*) (c-warn.c:3364)
==7112==    by 0xA3EB57: c_parser_declaration_or_fndef(c_parser*, bool, 
bool, bool, bool, bool, tree_node**, vec<c_token, va_heap, vl_ptr>, 
bool, tree_node*, oacc_routine_data*, bool*) (c-parser.c:2342)
==7112==    by 0xA3D5A9: c_parser_external_declaration(c_parser*) 
(c-parser.c:1777)
==7112==    by 0xA3D06A: c_parser_translation_unit(c_parser*) 
(c-parser.c:1650)

[-- Attachment #2: gcc-99055.diff --]
[-- Type: text/x-patch, Size: 3522 bytes --]

PR c/99055 - memory leak in warn_parm_array_mismatch

gcc/c-family/ChangeLog:

	PR c/99055
	* c-warn.c (generic_expr_as_string): New function.
	(warn_parm_array_mismatch): Replace calls to print_generic_expr_to_str
	with generic_expr_as_string.

gcc/ChangeLog:

	* tree-pretty-print.c (print_generic_expr_to_str): Update comment.

diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index e6e28d9b139..8cbc598bf6f 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -3319,6 +3319,20 @@ warn_parm_ptrarray_mismatch (location_t origloc, tree curparms, tree newparms)
     }
 }
 
+/* Same as print_generic_expr_to_str but returning std::string to keep
+   callers from having to free memory.  */
+
+static std::string
+generic_expr_as_string (tree expr, const char *dflt = "")
+{
+  if (!expr)
+    return dflt;
+
+  pretty_printer pp;
+  dump_generic_node (&pp, expr, 0, TDF_VOPS | TDF_MEMSYMS, false);
+  return std::string (pp_formatted_text (&pp));
+}
+
 /* Detect and diagnose a mismatch between an attribute access specification
    on the original declaration of FNDECL and that on the parameters NEWPARMS
    from its refeclaration.  ORIGLOC is the location of the first declaration
@@ -3585,10 +3599,8 @@ warn_parm_array_mismatch (location_t origloc, tree fndecl, tree newparms)
 	       the same.  */
 	    continue;
 
-	  const char* const newbndstr =
-	    newbnd ? print_generic_expr_to_str (newbnd) : "*";
-	  const char* const curbndstr =
-	    curbnd ? print_generic_expr_to_str (curbnd) : "*";
+	  std::string newbndstr = generic_expr_as_string (newbnd, "*");
+	  std::string curbndstr = generic_expr_as_string (curbnd, "*");
 
 	  if (!newpos != !curpos
 	      || (newpos && !tree_int_cst_equal (newpos, curpos)))
@@ -3619,7 +3631,7 @@ warn_parm_array_mismatch (location_t origloc, tree fndecl, tree newparms)
 				     "argument %u of type %s declared "
 				     "with mismatched bound %<%s%>",
 				     parmpos + 1, newparmstr.c_str (),
-				     newbndstr);
+				     newbndstr.c_str ());
 
 	      if (warned)
 		{
@@ -3633,8 +3645,9 @@ warn_parm_array_mismatch (location_t origloc, tree fndecl, tree newparms)
 			      curparmstr.c_str (), plus_one (curpos));
 		    }
 		  else
-		    inform (&richloc, "previously declared as %s with bound "
-			    "%<%s%>", curparmstr.c_str (), curbndstr);
+		    inform (&richloc,
+			    "previously declared as %s with bound %<%s%>",
+			    curparmstr.c_str (), curbndstr.c_str ());
 
 		  continue;
 		}
@@ -3651,9 +3664,10 @@ warn_parm_array_mismatch (location_t origloc, tree fndecl, tree newparms)
 	      if (warning_at (newloc, OPT_Wvla_parameter,
 			      "argument %u of type %s declared with "
 			      "mismatched bound %<%s%>",
-			      parmpos + 1, newparmstr.c_str (), newbndstr))
+			      parmpos + 1, newparmstr.c_str (),
+			      newbndstr.c_str ()))
 		inform (origloc, "previously declared as %s with bound %qs",
-			curparmstr.c_str (), curbndstr);
+			curparmstr.c_str (), curbndstr.c_str ());
 	      continue;
 	    }
 	}
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index aabe6bb23b9..986f75d1d5f 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -169,7 +169,8 @@ print_generic_expr (FILE *file, tree t, dump_flags_t flags)
   pp_flush (tree_pp);
 }
 
-/* Print a single expression T to string, and return it.  */
+/* Print a single expression T to string, and return it.  The caller
+   must free the returned memory.  */
 
 char *
 print_generic_expr_to_str (tree t)

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-08-17 13:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-10 17:16 [PATCH] plug memory leaks in warn_parm_array_mismatch (PR 99055) Martin Sebor
2021-02-11  7:59 ` Richard Biener
2021-02-11 17:29   ` Martin Sebor
2021-02-11 18:35     ` Fwd: " Martin Sebor
2021-02-12  7:35       ` Richard Biener
2021-02-12 18:21         ` Martin Sebor
2021-02-12 19:36           ` Richard Biener
2021-02-12 20:32             ` Martin Sebor
2021-08-06 14:09         ` Valgrind '--show-leak-kinds=all' Thomas Schwinge
2021-08-06 15:10           ` Richard Biener
2021-08-17 13:00             ` Thomas Schwinge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).