Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)

[Pedro Alves <palves at redhat dot com>]

On 08/09/2017 01:09 PM, Pedro Alves wrote:

> Sorry about that, and thanks for the fix.
> 
> I think I'd be good if cp_comp_to_string was changed to
> return a unique_ptr itself, to avoid similar cases creeping
> back in.
> 
> As penance, I'll give it a try.

Like so.  No regressions on F23.

BTW, I'm seeing:

 Running src/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp ...
 ERROR: Process no longer exists
 ERROR: Couldn't send ptype g to GDB.

but I see it too without this patch.  It passed a couple weeks
ago (before I disappeared).

>From b493a8c241e3e5434bbdff0eecd1b99074bc5779 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Aug 2017 12:51:52 +0100
Subject: [PATCH] Make cp_comp_to_string return a gdb::unique_xmalloc_ptr<char>

To help avoid issues like the one fixed by e88e8651cf34 ("Fix memory
leak in cp-support.c").

gdb/ChangeLog:
2017-08-09  Pedro Alves  <palves@redhat.com>

	* cp-name-parser.y (cp_comp_to_string): Return a
	gdb::unique_xmalloc_ptr<char>.
	* cp-support.c (replace_typedefs_qualified_name)
	(replace_typedefs): Adjust to use gdb::unique_xmalloc_ptr<char>.
	(cp_canonicalize_string_full): Use op= instead of explicit
	convertion.
	(cp_class_name_from_physname, method_name_from_physname)
	(cp_func_name, cp_remove_params): Adjust to use
	gdb::unique_xmalloc_ptr<char>.
	* cp-support.h (cp_comp_to_string): Return a
	gdb::unique_xmalloc_ptr<char>.
	* python/py-type.c (typy_lookup_type): Adjust to use
	gdb::unique_xmalloc_ptr<char>.
---
 gdb/cp-name-parser.y |  7 +++---
 gdb/cp-support.c     | 64 ++++++++++++++++++++++++----------------------------
 gdb/cp-support.h     |  4 ++--
 gdb/python/py-type.c |  8 ++-----
 4 files changed, 38 insertions(+), 45 deletions(-)

diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 78745cb..d430ae7 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -1963,13 +1963,14 @@ allocate_info (void)
    cplus_demangle_print does not, specifically the global destructor
    and constructor labels.  */
 
-char *
+gdb::unique_xmalloc_ptr<char>
 cp_comp_to_string (struct demangle_component *result, int estimated_len)
 {
   size_t err;
 
-  return cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI, result, estimated_len,
-			       &err);
+  char *res = cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI,
+				    result, estimated_len, &err);
+  return gdb::unique_xmalloc_ptr<char> (res);
 }
 
 /* Constructor for demangle_parse_info.  */
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index f6557ab..c7e8750 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -296,8 +296,6 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
 				 canonicalization_ftype *finder,
 				 void *data)
 {
-  long len;
-  char *name;
   string_file buf;
   struct demangle_component *comp = ret_comp;
 
@@ -313,14 +311,14 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
 	  struct demangle_component newobj;
 
 	  buf.write (d_left (comp)->u.s_name.s, d_left (comp)->u.s_name.len);
-	  len = buf.size ();
-	  name = (char *) obstack_copy0 (&info->obstack, buf.c_str (), len);
 	  newobj.type = DEMANGLE_COMPONENT_NAME;
-	  newobj.u.s_name.s = name;
-	  newobj.u.s_name.len = len;
+	  newobj.u.s_name.s
+	    = (char *) obstack_copy0 (&info->obstack,
+				      buf.c_str (), buf.size ());
+	  newobj.u.s_name.len = buf.size ();
 	  if (inspect_type (info, &newobj, finder, data))
 	    {
-	      char *n, *s;
+	      char *s;
 	      long slen;
 
 	      /* A typedef was substituted in NEW.  Convert it to a
@@ -328,15 +326,14 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
 		 node.  */
 
 	      buf.clear ();
-	      n = cp_comp_to_string (&newobj, 100);
+	      gdb::unique_xmalloc_ptr<char> n = cp_comp_to_string (&newobj, 100);
 	      if (n == NULL)
 		{
 		  /* If something went astray, abort typedef substitutions.  */
 		  return;
 		}
 
-	      s = copy_string_to_obstack (&info->obstack, n, &slen);
-	      xfree (n);
+	      s = copy_string_to_obstack (&info->obstack, n.get (), &slen);
 
 	      d_left (ret_comp)->type = DEMANGLE_COMPONENT_NAME;
 	      d_left (ret_comp)->u.s_name.s = s;
@@ -352,14 +349,14 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
 	     typedefs in it.  Then print it to the stream to continue
 	     checking for more typedefs in the tree.  */
 	  replace_typedefs (info, d_left (comp), finder, data);
-	  name = cp_comp_to_string (d_left (comp), 100);
+	  gdb::unique_xmalloc_ptr<char> name
+	    = cp_comp_to_string (d_left (comp), 100);
 	  if (name == NULL)
 	    {
 	      /* If something went astray, abort typedef substitutions.  */
 	      return;
 	    }
-	  buf.puts (name);
-	  xfree (name);
+	  buf.puts (name.get ());
 	}
 
       buf.write ("::", 2);
@@ -373,15 +370,15 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
   if (comp->type == DEMANGLE_COMPONENT_NAME)
     {
       buf.write (comp->u.s_name.s, comp->u.s_name.len);
-      len = buf.size ();
-      name = (char *) obstack_copy0 (&info->obstack, buf.c_str (), len);
 
       /* Replace the top (DEMANGLE_COMPONENT_QUAL_NAME) node
 	 with a DEMANGLE_COMPONENT_NAME node containing the whole
 	 name.  */
       ret_comp->type = DEMANGLE_COMPONENT_NAME;
-      ret_comp->u.s_name.s = name;
-      ret_comp->u.s_name.len = len;
+      ret_comp->u.s_name.s
+	= (char *) obstack_copy0 (&info->obstack,
+				  buf.c_str (), buf.size ());
+      ret_comp->u.s_name.len = buf.size ();
       inspect_type (info, ret_comp, finder, data);
     }
   else
@@ -423,7 +420,8 @@ replace_typedefs (struct demangle_parse_info *info,
 	      || ret_comp->type == DEMANGLE_COMPONENT_TEMPLATE
 	      || ret_comp->type == DEMANGLE_COMPONENT_BUILTIN_TYPE))
 	{
-	  char *local_name = cp_comp_to_string (ret_comp, 10);
+	  gdb::unique_xmalloc_ptr<char> local_name
+	    = cp_comp_to_string (ret_comp, 10);
 
 	  if (local_name != NULL)
 	    {
@@ -432,15 +430,14 @@ replace_typedefs (struct demangle_parse_info *info,
 	      sym = NULL;
 	      TRY
 		{
-		  sym = lookup_symbol (local_name, 0, VAR_DOMAIN, 0).symbol;
+		  sym = lookup_symbol (local_name.get (), 0,
+				       VAR_DOMAIN, 0).symbol;
 		}
 	      CATCH (except, RETURN_MASK_ALL)
 		{
 		}
 	      END_CATCH
 
-	      xfree (local_name);
-
 	      if (sym != NULL)
 		{
 		  struct type *otype = SYMBOL_TYPE (sym);
@@ -527,8 +524,8 @@ cp_canonicalize_string_full (const char *string,
       replace_typedefs (info.get (), info->tree, finder, data);
 
       /* Convert the tree back into a string.  */
-      gdb::unique_xmalloc_ptr<char> us (cp_comp_to_string (info->tree,
-							   estimated_len));
+      gdb::unique_xmalloc_ptr<char> us = cp_comp_to_string (info->tree,
+							    estimated_len);
       gdb_assert (us);
 
       ret = us.get ();
@@ -642,7 +639,8 @@ char *
 cp_class_name_from_physname (const char *physname)
 {
   void *storage = NULL;
-  char *demangled_name = NULL, *ret;
+  char *demangled_name = NULL;
+  gdb::unique_xmalloc_ptr<char> ret;
   struct demangle_component *ret_comp, *prev_comp, *cur_comp;
   std::unique_ptr<demangle_parse_info> info;
   int done;
@@ -711,7 +709,6 @@ cp_class_name_from_physname (const char *physname)
 	break;
       }
 
-  ret = NULL;
   if (cur_comp != NULL && prev_comp != NULL)
     {
       /* We want to discard the rightmost child of PREV_COMP.  */
@@ -723,7 +720,7 @@ cp_class_name_from_physname (const char *physname)
 
   xfree (storage);
   xfree (demangled_name);
-  return ret;
+  return ret.release ();
 }
 
 /* Return the child of COMP which is the basename of a method,
@@ -790,7 +787,8 @@ char *
 method_name_from_physname (const char *physname)
 {
   void *storage = NULL;
-  char *demangled_name = NULL, *ret;
+  char *demangled_name = NULL;
+  gdb::unique_xmalloc_ptr<char> ret;
   struct demangle_component *ret_comp;
   std::unique_ptr<demangle_parse_info> info;
 
@@ -801,7 +799,6 @@ method_name_from_physname (const char *physname)
 
   ret_comp = unqualified_name_from_comp (info->tree);
 
-  ret = NULL;
   if (ret_comp != NULL)
     /* The ten is completely arbitrary; we don't have a good
        estimate.  */
@@ -809,7 +806,7 @@ method_name_from_physname (const char *physname)
 
   xfree (storage);
   xfree (demangled_name);
-  return ret;
+  return ret.release ();
 }
 
 /* If FULL_NAME is the demangled name of a C++ function (including an
@@ -821,7 +818,7 @@ method_name_from_physname (const char *physname)
 char *
 cp_func_name (const char *full_name)
 {
-  char *ret;
+  gdb::unique_xmalloc_ptr<char> ret;
   struct demangle_component *ret_comp;
   std::unique_ptr<demangle_parse_info> info;
 
@@ -831,11 +828,10 @@ cp_func_name (const char *full_name)
 
   ret_comp = unqualified_name_from_comp (info->tree);
 
-  ret = NULL;
   if (ret_comp != NULL)
     ret = cp_comp_to_string (ret_comp, 10);
 
-  return ret;
+  return ret.release ();
 }
 
 /* DEMANGLED_NAME is the name of a function, including parameters and
@@ -848,7 +844,7 @@ cp_remove_params (const char *demangled_name)
   int done = 0;
   struct demangle_component *ret_comp;
   std::unique_ptr<demangle_parse_info> info;
-  char *ret = NULL;
+  gdb::unique_xmalloc_ptr<char> ret;
 
   if (demangled_name == NULL)
     return NULL;
@@ -880,7 +876,7 @@ cp_remove_params (const char *demangled_name)
   if (ret_comp->type == DEMANGLE_COMPONENT_TYPED_NAME)
     ret = cp_comp_to_string (d_left (ret_comp), 10);
 
-  return ret;
+  return ret.release ();
 }
 
 /* Here are some random pieces of trivia to keep in mind while trying
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index 37b281f..9210165 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -150,8 +150,8 @@ struct type *cp_find_type_baseclass_by_name (struct type *parent_type,
 extern std::unique_ptr<demangle_parse_info> cp_demangled_name_to_comp
      (const char *demangled_name, const char **errmsg);
 
-extern char *cp_comp_to_string (struct demangle_component *result,
-				int estimated_len);
+extern gdb::unique_xmalloc_ptr<char> cp_comp_to_string
+  (struct demangle_component *result, int estimated_len);
 
 extern void cp_merge_demangle_parse_infos (struct demangle_parse_info *,
 					   struct demangle_component *,
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index aa20d4c..51184ca 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -761,7 +761,6 @@ typy_lookup_type (struct demangle_component *demangled,
 		  const struct block *block)
 {
   struct type *type, *rtype = NULL;
-  char *type_name = NULL;
   enum demangle_component_type demangled_type;
 
   /* Save the type: typy_lookup_type() may (indirectly) overwrite
@@ -816,11 +815,8 @@ typy_lookup_type (struct demangle_component *demangled,
     return rtype;
 
   /* We don't have a type, so lookup the type.  */
-  type_name = cp_comp_to_string (demangled, 10);
-  type = typy_lookup_typename (type_name, block);
-  xfree (type_name);
-
-  return type;
+  gdb::unique_xmalloc_ptr<char> type_name = cp_comp_to_string (demangled, 10);
+  return typy_lookup_typename (type_name.get (), block);
 }
 
 /* This is a helper function for typy_template_argument that is used
-- 
2.5.5