From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1988 invoked by alias); 9 Aug 2011 20:23:08 -0000 Received: (qmail 1978 invoked by uid 22791); 9 Aug 2011 20:23:05 -0000 X-SWARE-Spam-Status: No, hits=-6.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 09 Aug 2011 20:22:46 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p79KMkid016613 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 9 Aug 2011 16:22:46 -0400 Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p79KMgTM005377 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Tue, 9 Aug 2011 16:22:44 -0400 Message-ID: <4E419712.3070709@redhat.com> Date: Tue, 09 Aug 2011 20:23:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20110707 Thunderbird/5.0 MIME-Version: 1.0 To: Jan Kratochvil CC: gdb-patches@sourceware.org Subject: Re: [RFA] c++/12266 (again) [cp_demangled_name_parse_free-4.patch] References: <4E31EE1A.5040204@redhat.com> <20110802202825.GA13092@host1.jankratochvil.net> In-Reply-To: <20110802202825.GA13092@host1.jankratochvil.net> Content-Type: multipart/mixed; boundary="------------050503050605010901030600" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-08/txt/msg00210.txt.bz2 This is a multi-part message in MIME format. --------------050503050605010901030600 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2663 On 08/02/2011 01:28 PM, Jan Kratochvil wrote: > $ make test-cp-name-parser > cp-name-parser.y:2018: undefined reference to `make_cleanup' Fixed. >> + result = ((struct demangle_parse_info *) >> + malloc (sizeof (struct demangle_parse_info))); > > This cast seems redundant to me. Removed. GDB is a mish-mash of usage discrepancies like this. [Mind you, a C++ compiler will complain about this.] >> + info = ((struct demangle_parse_info *) >> + xmalloc (sizeof (struct demangle_parse_info))); > > Redundant cast. Removed. > I guess you have been considering it but anyway: > > Its ->tree items can reference for s_name the original string being demangled, > as you even state in some comments. Due to it it also requires later explicit > handling of `free_list'. Could this struct already have the original string > duplicated and this struct would track its freeing? > cp_merge_demangle_parse_infos would then track all the source strings in the > merged destination demangle_parse_info. That is actually something that didn't occur to me for some reason, but I've made the necessary changes to use an obstack. [This change can be seen in the next patch, since it is not strictly necessary for this cleanup/refactoring.] New patch attached. Keith ChangeLog 2011-08-09 Keith Seitz * cp-name-parser.y (struct demangle_info): Remove unused member PREV. (d_grab): Likewise. (allocate_info): Change return type to struct demangle_info *. Always allocate a new demangle_info. Remove unused PREV pointer. (cp_new_demangle_parse_info): New function. (cp_demangled_name_parse_free): New function. (do_demangled_name_parse_free_cleanup): New function. (make_cleanup_cp_demangled_name_parse_free): New function. (cp_demangled_name_to_comp): Change return type to struct demangle_parse_info *. Allocate a new storage for each call. (main): Update usage for cp_demangled_name_to_comp API change. * cp-support.h (struct demangle_parse_info): New structure. (cp_demangled_name_to_comp): Update API change for return type. (cp_new_demangle_parse_info): Declare. (make_cleanup_cp_demangled_name_parse_free): New declaration. (cp_demangled_name_parse_free): Declare. * cp-support.c (cp_canonicalize_string): Update API change for cp_demangled_name_to_comp. (mangled_name_to_comp): Likewise. Return struct demangle_parse_info, too. (cp_class_name_from_physname): Update mangled_name_to_comp API change. (method_name_from_physname): Likewise. (cp_func_name): Update API change for cp_demangled_name_to_comp. (cp_remove_params): Likewise. * python/py-type.c (typy_legacy_template_argument): Likewise. --------------050503050605010901030600 Content-Type: text/plain; name="cp_demangled_name_parse_free-5.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="cp_demangled_name_parse_free-5.patch" Content-length: 13678 diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y index 8736777..6d23c9d 100644 --- a/gdb/cp-name-parser.y +++ b/gdb/cp-name-parser.y @@ -60,7 +60,7 @@ static const char *lexptr, *prev_lexptr, *error_lexptr, *global_errmsg; struct demangle_info { int used; - struct demangle_info *prev, *next; + struct demangle_info *next; struct demangle_component comps[ALLOC_CHUNK]; }; @@ -76,7 +76,6 @@ d_grab (void) if (demangle_info->next == NULL) { more = malloc (sizeof (struct demangle_info)); - more->prev = demangle_info; more->next = NULL; demangle_info->next = more; } @@ -1935,20 +1934,14 @@ yyerror (char *msg) generally allocate too many components, but the extra memory usage doesn't hurt because the trees are temporary and the storage is reused. More may be allocated later, by d_grab. */ -static void +static struct demangle_info * allocate_info (void) { - if (demangle_info == NULL) - { - demangle_info = malloc (sizeof (struct demangle_info)); - demangle_info->prev = NULL; - demangle_info->next = NULL; - } - else - while (demangle_info->prev) - demangle_info = demangle_info->prev; + struct demangle_info *info = malloc (sizeof (struct demangle_info)); - demangle_info->used = 0; + info->next = NULL; + info->used = 0; + return info; } /* Convert RESULT to a string. The return value is allocated @@ -1966,23 +1959,60 @@ cp_comp_to_string (struct demangle_component *result, int estimated_len) &err); } +/* A convenience function to allocate and initialize a new struct + demangled_parse_info. */ + +struct demangle_parse_info * +cp_new_demangle_parse_info (void) +{ + struct demangle_parse_info *info; + + info = malloc (sizeof (struct demangle_parse_info)); + info->info = NULL; + info->tree = NULL; + return info; +} + +/* Free any memory associated with the given PARSE_INFO. */ + +void +cp_demangled_name_parse_free (struct demangle_parse_info *parse_info) +{ + struct demangle_info *info = parse_info->info; + + /* Free any allocated chunks of memory for the parse. */ + while (info != NULL) + { + struct demangle_info *next = info->next; + + free (info); + info = next; + } + + /* Free the parser info. */ + free (parse_info); +} + /* Convert a demangled name to a demangle_component tree. On success, - the root of the new tree is returned; it is valid until the next - call to this function and should not be freed. On error, NULL is + a structure containing the root of the new tree is returned; it must + be freed by calling cp_demangled_name_parse_free. On error, NULL is returned, and an error message will be set in *ERRMSG (which does not need to be freed). */ -struct demangle_component * +struct demangle_parse_info * cp_demangled_name_to_comp (const char *demangled_name, const char **errmsg) { static char errbuf[60]; - struct demangle_component *result; + struct demangle_parse_info *result; prev_lexptr = lexptr = demangled_name; error_lexptr = NULL; global_errmsg = NULL; - allocate_info (); + demangle_info = allocate_info (); + + result = cp_new_demangle_parse_info (); + result->info = demangle_info; if (yyparse ()) { @@ -1993,10 +2023,11 @@ cp_demangled_name_to_comp (const char *demangled_name, const char **errmsg) strcat (errbuf, "'"); *errmsg = errbuf; } + cp_demangled_name_parse_free (result); return NULL; } - result = global_result; + result->tree = global_result; global_result = NULL; return result; @@ -2052,7 +2083,7 @@ main (int argc, char **argv) char buf[65536]; int arg; const char *errmsg; - struct demangle_component *result; + struct demangle_parse_info *result; arg = 1; if (argv[arg] && strcmp (argv[arg], "--debug") == 0) @@ -2086,7 +2117,8 @@ main (int argc, char **argv) continue; } - cp_print (result); + cp_print (result->tree); + cp_demangled_name_parse_free (result); free (str2); if (c) @@ -2105,7 +2137,8 @@ main (int argc, char **argv) fputc ('\n', stderr); return 0; } - cp_print (result); + cp_print (result->tree); + cp_demangled_name_parse_free (result); putchar ('\n'); } return 0; diff --git a/gdb/cp-support.c b/gdb/cp-support.c index a479067..8cda2b4 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -86,6 +86,25 @@ static const char *operator_tokens[] = /* new[] and delete[] require special whitespace handling */ }; + +/* A cleanup wrapper for cp_demangled_name_parse_free. */ + +static void +do_demangled_name_parse_free_cleanup (void *data) +{ + struct demangle_parse_info *info = (struct demangle_parse_info *) data; + + cp_demangled_name_parse_free (info); +} + +/* Create a cleanup for C++ name parsing. */ + +struct cleanup * +make_cleanup_cp_demangled_name_parse_free (struct demangle_parse_info *info) +{ + return make_cleanup (do_demangled_name_parse_free_cleanup, info); +} + /* Return 1 if STRING is clearly already in canonical form. This function is conservative; things which it does not recognize are assumed to be non-canonical, and the parser will sort them out @@ -124,19 +143,20 @@ cp_already_canonical (const char *string) char * cp_canonicalize_string (const char *string) { - struct demangle_component *ret_comp; + struct demangle_parse_info *info; unsigned int estimated_len; char *ret; if (cp_already_canonical (string)) return NULL; - ret_comp = cp_demangled_name_to_comp (string, NULL); - if (ret_comp == NULL) + info = cp_demangled_name_to_comp (string, NULL); + if (info == NULL) return NULL; estimated_len = strlen (string) * 2; - ret = cp_comp_to_string (ret_comp, estimated_len); + ret = cp_comp_to_string (info->tree, estimated_len); + cp_demangled_name_parse_free (info); if (strcmp (string, ret) == 0) { @@ -153,23 +173,27 @@ cp_canonicalize_string (const char *string) freed when finished with the tree, or NULL if none was needed. OPTIONS will be passed to the demangler. */ -static struct demangle_component * +static struct demangle_parse_info * mangled_name_to_comp (const char *mangled_name, int options, void **memory, char **demangled_p) { - struct demangle_component *ret; char *demangled_name; + struct demangle_parse_info *info; /* If it looks like a v3 mangled name, then try to go directly to trees. */ if (mangled_name[0] == '_' && mangled_name[1] == 'Z') { + struct demangle_component *ret; + ret = cplus_demangle_v3_components (mangled_name, options, memory); if (ret) { + info = cp_new_demangle_parse_info (); + info->tree = ret; *demangled_p = NULL; - return ret; + return info; } } @@ -181,16 +205,16 @@ mangled_name_to_comp (const char *mangled_name, int options, /* If we could demangle the name, parse it to build the component tree. */ - ret = cp_demangled_name_to_comp (demangled_name, NULL); + info = cp_demangled_name_to_comp (demangled_name, NULL); - if (ret == NULL) + if (info == NULL) { xfree (demangled_name); return NULL; } *demangled_p = demangled_name; - return ret; + return info; } /* Return the name of the class containing method PHYSNAME. */ @@ -201,14 +225,16 @@ cp_class_name_from_physname (const char *physname) void *storage = NULL; char *demangled_name = NULL, *ret; struct demangle_component *ret_comp, *prev_comp, *cur_comp; + struct demangle_parse_info *info; int done; - ret_comp = mangled_name_to_comp (physname, DMGL_ANSI, - &storage, &demangled_name); - if (ret_comp == NULL) + info = mangled_name_to_comp (physname, DMGL_ANSI, + &storage, &demangled_name); + if (info == NULL) return NULL; done = 0; + ret_comp = info->tree; /* First strip off any qualifiers, if we have a function or method. */ @@ -277,8 +303,8 @@ cp_class_name_from_physname (const char *physname) } xfree (storage); - if (demangled_name) - xfree (demangled_name); + xfree (demangled_name); + cp_demangled_name_parse_free (info); return ret; } @@ -348,13 +374,14 @@ method_name_from_physname (const char *physname) void *storage = NULL; char *demangled_name = NULL, *ret; struct demangle_component *ret_comp; + struct demangle_parse_info *info; - ret_comp = mangled_name_to_comp (physname, DMGL_ANSI, - &storage, &demangled_name); - if (ret_comp == NULL) + info = mangled_name_to_comp (physname, DMGL_ANSI, + &storage, &demangled_name); + if (info == NULL) return NULL; - ret_comp = unqualified_name_from_comp (ret_comp); + ret_comp = unqualified_name_from_comp (info->tree); ret = NULL; if (ret_comp != NULL) @@ -363,8 +390,8 @@ method_name_from_physname (const char *physname) ret = cp_comp_to_string (ret_comp, 10); xfree (storage); - if (demangled_name) - xfree (demangled_name); + xfree (demangled_name); + cp_demangled_name_parse_free (info); return ret; } @@ -379,17 +406,19 @@ cp_func_name (const char *full_name) { char *ret; struct demangle_component *ret_comp; + struct demangle_parse_info *info; - ret_comp = cp_demangled_name_to_comp (full_name, NULL); - if (!ret_comp) + info = cp_demangled_name_to_comp (full_name, NULL); + if (!info) return NULL; - ret_comp = unqualified_name_from_comp (ret_comp); + ret_comp = unqualified_name_from_comp (info->tree); ret = NULL; if (ret_comp != NULL) ret = cp_comp_to_string (ret_comp, 10); + cp_demangled_name_parse_free (info); return ret; } @@ -402,16 +431,18 @@ cp_remove_params (const char *demangled_name) { int done = 0; struct demangle_component *ret_comp; + struct demangle_parse_info *info; char *ret = NULL; if (demangled_name == NULL) return NULL; - ret_comp = cp_demangled_name_to_comp (demangled_name, NULL); - if (ret_comp == NULL) + info = cp_demangled_name_to_comp (demangled_name, NULL); + if (info == NULL) return NULL; /* First strip off any qualifiers, if we have a function or method. */ + ret_comp = info->tree; while (!done) switch (ret_comp->type) { @@ -433,6 +464,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); + cp_demangled_name_parse_free (info); return ret; } diff --git a/gdb/cp-support.h b/gdb/cp-support.h index d23f19e..f333ffa 100644 --- a/gdb/cp-support.h +++ b/gdb/cp-support.h @@ -46,6 +46,17 @@ struct demangle_component; #define CP_ANONYMOUS_NAMESPACE_LEN 21 +/* The result of parsing a name. */ + +struct demangle_parse_info +{ + /* The memory used during the parse. */ + struct demangle_info *info; + + /* The result of the parse. */ + struct demangle_component *tree; +}; + /* This struct is designed to store data from using directives. It says that names from namespace IMPORT_SRC should be visible within namespace IMPORT_DEST. These form a linked list; NEXT is the next @@ -214,12 +225,18 @@ struct type *cp_lookup_transparent_type (const char *name); /* Functions from cp-name-parser.y. */ -extern struct demangle_component *cp_demangled_name_to_comp - (const char *demangled_name, const char **errmsg); +extern struct 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 void cp_demangled_name_parse_free (struct demangle_parse_info *); +extern struct cleanup *make_cleanup_cp_demangled_name_parse_free + (struct demangle_parse_info *); + +extern struct demangle_parse_info *cp_new_demangle_parse_info (void); + /* The list of "maint cplus" commands. */ extern struct cmd_list_element *maint_cplus_cmd_list; diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c index 8ab18cf..335342e 100644 --- a/gdb/python/py-type.c +++ b/gdb/python/py-type.c @@ -577,8 +577,10 @@ typy_legacy_template_argument (struct type *type, struct block *block, { int i; struct demangle_component *demangled; + struct demangle_parse_info *info; const char *err; struct type *argtype; + struct cleanup *cleanup; if (TYPE_NAME (type) == NULL) { @@ -587,12 +589,14 @@ typy_legacy_template_argument (struct type *type, struct block *block, } /* Note -- this is not thread-safe. */ - demangled = cp_demangled_name_to_comp (TYPE_NAME (type), &err); - if (! demangled) + info = cp_demangled_name_to_comp (TYPE_NAME (type), &err); + if (! info) { PyErr_SetString (PyExc_RuntimeError, err); return NULL; } + demangled = info->tree; + cleanup = make_cleanup_cp_demangled_name_parse_free (info); /* Strip off component names. */ while (demangled->type == DEMANGLE_COMPONENT_QUAL_NAME @@ -601,6 +605,7 @@ typy_legacy_template_argument (struct type *type, struct block *block, if (demangled->type != DEMANGLE_COMPONENT_TEMPLATE) { + do_cleanups (cleanup); PyErr_SetString (PyExc_RuntimeError, _("Type is not a template.")); return NULL; } @@ -613,12 +618,14 @@ typy_legacy_template_argument (struct type *type, struct block *block, if (! demangled) { + do_cleanups (cleanup); PyErr_Format (PyExc_RuntimeError, _("No argument %d in template."), argno); return NULL; } argtype = typy_lookup_type (demangled->u.s_binary.left, block); + do_cleanups (cleanup); if (! argtype) return NULL; --------------050503050605010901030600--