Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)
@ 2017-08-07 20:18 Alex Lindsay
  2017-08-09 11:40 ` Yao Qi
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Lindsay @ 2017-08-07 20:18 UTC (permalink / raw)
  To: gdb-patches

Formerly, in cp_canonicalize_string in cp-support.c, the return value of
cp_comp_to_string was never freed, creating a sizable memory leak detectable
with valgrind. This patch fixes the leak. However, a longer term solution
would be to change the return type of cp_comp_to_string to
gdb::unique_xmalloc_ptr<char>.
---
 gdb/ChangeLog    | 5 +++++
 gdb/cp-support.c | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 92c573d76b..e0c43c6185 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-08-07 Alex Lindsay <alexlindsay239@gmail.com>
+
+	* cp-support.c (cp_canonicalize_string): Free return value of
+	cp_comp_to_string
+
 2017-08-03  Tom Tromey  <tom@tromey.com>
 
 	* utils.c (make_cleanup_freeargv, do_freeargv, gdb_buildargv):
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index df9a563508..ffbfd62ee0 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -566,7 +566,9 @@ cp_canonicalize_string (const char *string)
     return std::string ();
 
   estimated_len = strlen (string) * 2;
-  std::string ret = cp_comp_to_string (info->tree, estimated_len);
+  char *ret_char = cp_comp_to_string (info->tree, estimated_len);
+  std::string ret = ret_char;
+  xfree (ret_char);
 
   if (ret.empty ())
     {
-- 
2.11.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)
  2017-08-07 20:18 [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string) Alex Lindsay
@ 2017-08-09 11:40 ` Yao Qi
  2017-08-09 12:09   ` Pedro Alves
  2017-08-09 13:24   ` Alex Lindsay
  0 siblings, 2 replies; 8+ messages in thread
From: Yao Qi @ 2017-08-09 11:40 UTC (permalink / raw)
  To: Alex Lindsay; +Cc: gdb-patches

Alex Lindsay <alexlindsay239@gmail.com> writes:

> Formerly, in cp_canonicalize_string in cp-support.c, the return value of
> cp_comp_to_string was never freed, creating a sizable memory leak detectable
> with valgrind. This patch fixes the leak. However, a longer term solution
> would be to change the return type of cp_comp_to_string to
> gdb::unique_xmalloc_ptr<char>.

Hi Alex,
Thanks a lot for the investigation and the patch.  I revise it a little
to use gdb::unique_xmalloc_ptr<char>, and fix another leak somewhere else.
Patch below is pushed in.

-- 
Yao (齐尧)

From e88e8651cf3415ba440ee17eb1b22b7d2e8368be Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Wed, 9 Aug 2017 12:39:16 +0100
Subject: [PATCH] Fix memory leak in cp-support.c

The return value of cp_comp_to_string was never freed, creating a
sizable memory leak detectable with valgrind.

==21225== 8 bytes in 1 blocks are definitely lost in loss record 4,599 of 10,949^M
==21225==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)^M
==21225==    by 0x4C2FDEF: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)^M
==21225==    by 0x76CB31: d_growable_string_resize (cp-demangle.c:3963)^M
==21225==    by 0x76CB31: d_growable_string_init (cp-demangle.c:3942)^M
==21225==    by 0x76CB31: cplus_demangle_print (cp-demangle.c:4308)^M
==21225==    by 0x4C9535: cp_comp_to_string(demangle_component*, int) (cp-name-parser.y:1972)^M
==21225==    by 0x53E1D4: cp_canonicalize_string_full[abi:cxx11](char const*, char const* (*)(type*, void*), void*) (cp-support.c:530)^M
==21225==    by 0x53E360: cp_canonicalize_string_no_typedefs[abi:cxx11](char const*) (cp-support.c:548)^M
==21225==    by 0x5D51D2: find_linespec_symbols(linespec_state*, VEC_symtab_ptr*, char const*, VEC_symbolp**, VEC_bound_minimal_symbol_d**) (linespec.c:4030)^M
==21225==    by 0x5D6CF6: linespec_parse_basic (linespec.c:1907)

==21279== 32 bytes in 1 blocks are definitely lost in loss record 6,066 of 10,947^M
==21279==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)^M
==21279==    by 0x4C2FDEF: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)^M
==21279==    by 0x76CB31: d_growable_string_resize (cp-demangle.c:3963)^M
==21279==    by 0x76CB31: d_growable_string_init (cp-demangle.c:3942)^M
==21279==    by 0x76CB31: cplus_demangle_print (cp-demangle.c:4308)^M
==21279==    by 0x4C9535: cp_comp_to_string(demangle_component*, int) (cp-name-parser.y:1972)^M
==21279==    by 0x53EF14: cp_canonicalize_string[abi:cxx11](char const*) (cp-support.c:569)^M
==21279==    by 0x561B75: dwarf2_canonicalize_name(char const*, dwarf2_cu*, obstack*) [clone .isra.210] (dwarf2read.c:20159)

This patch fixes the leak.  It is a regression by 2f408ecb.

gdb:

2017-08-09  Alex Lindsay  <alexlindsay239@gmail.com>
	    Yao Qi  <yao.qi@linaro.org>

	* cp-support.c (cp_canonicalize_string_full): Use
	gdb::unique_xmalloc_ptr<char>.
	(cp_canonicalize_string): Likewise.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 97c39d7..209d0b6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-08-09  Alex Lindsay  <alexlindsay239@gmail.com>
+	    Yao Qi  <yao.qi@linaro.org>
+
+	* cp-support.c (cp_canonicalize_string_full): Use
+	gdb::unique_xmalloc_ptr<char>.
+	(cp_canonicalize_string): Likewise.
+
 2017-08-09  Yao Qi  <yao.qi@linaro.org>
 
 	* features/Makefile (WHICH): Remove i386/ non-linux stuff.
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index df9a563..f6557ab 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -527,9 +527,11 @@ cp_canonicalize_string_full (const char *string,
       replace_typedefs (info.get (), info->tree, finder, data);
 
       /* Convert the tree back into a string.  */
-      ret = cp_comp_to_string (info->tree, estimated_len);
-      gdb_assert (!ret.empty ());
+      gdb::unique_xmalloc_ptr<char> us (cp_comp_to_string (info->tree,
+							   estimated_len));
+      gdb_assert (us);
 
+      ret = us.get ();
       /* Finally, compare the original string with the computed
 	 name, returning NULL if they are the same.  */
       if (ret == string)
@@ -566,15 +568,18 @@ cp_canonicalize_string (const char *string)
     return std::string ();
 
   estimated_len = strlen (string) * 2;
-  std::string ret = cp_comp_to_string (info->tree, estimated_len);
+  gdb::unique_xmalloc_ptr<char> us (cp_comp_to_string (info->tree,
+						       estimated_len));
 
-  if (ret.empty ())
+  if (!us)
     {
       warning (_("internal error: string \"%s\" failed to be canonicalized"),
 	       string);
       return std::string ();
     }
 
+  std::string ret (us.get ());
+
   if (ret == string)
     return std::string ();
 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)
  2017-08-09 11:40 ` Yao Qi
@ 2017-08-09 12:09   ` Pedro Alves
  2017-08-09 12:31     ` Pedro Alves
  2017-08-09 13:24   ` Alex Lindsay
  1 sibling, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2017-08-09 12:09 UTC (permalink / raw)
  To: Yao Qi, Alex Lindsay; +Cc: gdb-patches

On 08/09/2017 12:40 PM, Yao Qi wrote:
> Alex Lindsay <alexlindsay239@gmail.com> writes:
> 
>> Formerly, in cp_canonicalize_string in cp-support.c, the return value of
>> cp_comp_to_string was never freed, creating a sizable memory leak detectable
>> with valgrind. This patch fixes the leak. However, a longer term solution
>> would be to change the return type of cp_comp_to_string to
>> gdb::unique_xmalloc_ptr<char>.
> 
> Hi Alex,
> Thanks a lot for the investigation and the patch.  I revise it a little
> to use gdb::unique_xmalloc_ptr<char>, and fix another leak somewhere else.
> Patch below is pushed in.
> 

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.

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)
  2017-08-09 12:09   ` Pedro Alves
@ 2017-08-09 12:31     ` Pedro Alves
  2017-08-09 13:27       ` Pedro Alves
  2017-08-09 13:41       ` Yao Qi
  0 siblings, 2 replies; 8+ messages in thread
From: Pedro Alves @ 2017-08-09 12:31 UTC (permalink / raw)
  To: Yao Qi, Alex Lindsay; +Cc: gdb-patches

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)
  2017-08-09 11:40 ` Yao Qi
  2017-08-09 12:09   ` Pedro Alves
@ 2017-08-09 13:24   ` Alex Lindsay
  1 sibling, 0 replies; 8+ messages in thread
From: Alex Lindsay @ 2017-08-09 13:24 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

Thanks Yao!

On 08/09/2017 06:40 AM, Yao Qi wrote:
> Alex Lindsay <alexlindsay239@gmail.com> writes:
>
>> Formerly, in cp_canonicalize_string in cp-support.c, the return value of
>> cp_comp_to_string was never freed, creating a sizable memory leak detectable
>> with valgrind. This patch fixes the leak. However, a longer term solution
>> would be to change the return type of cp_comp_to_string to
>> gdb::unique_xmalloc_ptr<char>.
> Hi Alex,
> Thanks a lot for the investigation and the patch.  I revise it a little
> to use gdb::unique_xmalloc_ptr<char>, and fix another leak somewhere else.
> Patch below is pushed in.
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)
  2017-08-09 12:31     ` Pedro Alves
@ 2017-08-09 13:27       ` Pedro Alves
  2017-08-09 13:41       ` Yao Qi
  1 sibling, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2017-08-09 13:27 UTC (permalink / raw)
  To: Yao Qi, Alex Lindsay; +Cc: gdb-patches

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

>  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).

FTR, that regression was caused by the
"Fix dwarf2_string_attr for -gsplit-dwarf" patch, and
H.J.'s patch fixed it already:

  https://sourceware.org/ml/gdb-patches/2017-08/msg00168.html

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)
  2017-08-09 12:31     ` Pedro Alves
  2017-08-09 13:27       ` Pedro Alves
@ 2017-08-09 13:41       ` Yao Qi
  2017-08-09 14:10         ` Pedro Alves
  1 sibling, 1 reply; 8+ messages in thread
From: Yao Qi @ 2017-08-09 13:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Alex Lindsay, gdb-patches

Pedro Alves <palves@redhat.com> writes:

Patch is good to me.

> -	      n = cp_comp_to_string (&newobj, 100);
> +	      gdb::unique_xmalloc_ptr<char> n = cp_comp_to_string (&newobj, 100);

Nit, it is too long.

-- 
Yao (齐尧)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)
  2017-08-09 13:41       ` Yao Qi
@ 2017-08-09 14:10         ` Pedro Alves
  0 siblings, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2017-08-09 14:10 UTC (permalink / raw)
  To: Yao Qi; +Cc: Alex Lindsay, gdb-patches


On 08/09/2017 02:41 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
> Patch is good to me.
> 
>> -	      n = cp_comp_to_string (&newobj, 100);
>> +	      gdb::unique_xmalloc_ptr<char> n = cp_comp_to_string (&newobj, 100);
> 
> Nit, it is too long.

Fixed and pushed, thanks.

Pedro Alves


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-08-09 14:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 20:18 [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string) Alex Lindsay
2017-08-09 11:40 ` Yao Qi
2017-08-09 12:09   ` Pedro Alves
2017-08-09 12:31     ` Pedro Alves
2017-08-09 13:27       ` Pedro Alves
2017-08-09 13:41       ` Yao Qi
2017-08-09 14:10         ` Pedro Alves
2017-08-09 13:24   ` Alex Lindsay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox