This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>, Alex Lindsay <alexlindsay239 at gmail dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 9 Aug 2017 13:31:08 +0100
- Subject: Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com EC9C23CB28E
- References: <20170807201821.25207-1-alexlindsay239@gmail.com> <86k22dyno6.fsf@gmail.com> <2c88c7cb-dca8-7bd7-fe01-ee0c4a9d0e94@redhat.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