From patchwork Wed Aug 9 12:31:08 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 22041 Received: (qmail 4453 invoked by alias); 9 Aug 2017 12:31:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 4438 invoked by uid 89); 9 Aug 2017 12:31:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KAM_STOCKGEN, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 09 Aug 2017 12:31:12 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EC9C23CB28E; Wed, 9 Aug 2017 12:31:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com EC9C23CB28E 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@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 441ED7E5DA; Wed, 9 Aug 2017 12:31:09 +0000 (UTC) Subject: Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string) To: Yao Qi , Alex Lindsay References: <20170807201821.25207-1-alexlindsay239@gmail.com> <86k22dyno6.fsf@gmail.com> <2c88c7cb-dca8-7bd7-fe01-ee0c4a9d0e94@redhat.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <2675604a-4460-81e3-93f3-21e34727fe12@redhat.com> Date: Wed, 9 Aug 2017 13:31:08 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <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 Date: Wed, 9 Aug 2017 12:51:52 +0100 Subject: [PATCH] Make cp_comp_to_string return a gdb::unique_xmalloc_ptr To help avoid issues like the one fixed by e88e8651cf34 ("Fix memory leak in cp-support.c"). gdb/ChangeLog: 2017-08-09 Pedro Alves * cp-name-parser.y (cp_comp_to_string): Return a gdb::unique_xmalloc_ptr. * cp-support.c (replace_typedefs_qualified_name) (replace_typedefs): Adjust to use gdb::unique_xmalloc_ptr. (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. * cp-support.h (cp_comp_to_string): Return a gdb::unique_xmalloc_ptr. * python/py-type.c (typy_lookup_type): Adjust to use gdb::unique_xmalloc_ptr. --- 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 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 (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 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 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 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 us (cp_comp_to_string (info->tree, - estimated_len)); + gdb::unique_xmalloc_ptr 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 ret; struct demangle_component *ret_comp, *prev_comp, *cur_comp; std::unique_ptr 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 ret; struct demangle_component *ret_comp; std::unique_ptr 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 ret; struct demangle_component *ret_comp; std::unique_ptr 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 info; - char *ret = NULL; + gdb::unique_xmalloc_ptr 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 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 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 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