From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3319 invoked by alias); 13 Aug 2011 16:50:46 -0000 Received: (qmail 3307 invoked by uid 22791); 13 Aug 2011 16:50:43 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_CP 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; Sat, 13 Aug 2011 16:50:25 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p7DGoOu3032625 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sat, 13 Aug 2011 12:50:24 -0400 Received: from host1.jankratochvil.net (ovpn-116-17.ams2.redhat.com [10.36.116.17]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p7DGoLEk023415 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 13 Aug 2011 12:50:23 -0400 Received: from host1.jankratochvil.net (localhost [127.0.0.1]) by host1.jankratochvil.net (8.14.4/8.14.4) with ESMTP id p7DGoKtL021364; Sat, 13 Aug 2011 18:50:20 +0200 Received: (from jkratoch@localhost) by host1.jankratochvil.net (8.14.4/8.14.4/Submit) id p7DGoJTa021363; Sat, 13 Aug 2011 18:50:19 +0200 Date: Sat, 13 Aug 2011 16:50:00 -0000 From: Jan Kratochvil To: Keith Seitz Cc: gdb-patches@sourceware.org Subject: Re: [RFA] c++/12266 (again) [cp_canonicalize_no_typedefs-4.patch] Message-ID: <20110813165019.GA2217@host1.jankratochvil.net> References: <4E31EE1A.5040204@redhat.com> <20110802203654.GA8670@host1.jankratochvil.net> <4E419D17.9020404@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4E419D17.9020404@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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/msg00272.txt.bz2 On Tue, 09 Aug 2011 22:48:23 +0200, Keith Seitz wrote: > On 08/02/2011 01:36 PM, Jan Kratochvil wrote: > >>+/* Merge the two parse trees given by DEST and SRC. The parse tree > >>+ in SRC is attached to DEST at the node represented by TARGET. > >>+ SRC is then freed. */ > > > >It would need to state that DEST will then still possibly reference the > >original string of SRC. Commented more at `struct demangle_parse_info'. > > I have updated the comments. I have filed: http://sourceware.org/bugzilla/show_bug.cgi?id=13087 as I find wrong to reference memory it does not own, but I see no easy solution now, so one can fix it by that PR above in the future. [...] > --- a/gdb/cp-name-parser.y > +++ b/gdb/cp-name-parser.y [...] > @@ -1970,6 +1972,9 @@ cp_new_demangle_parse_info (void) > info = malloc (sizeof (struct demangle_parse_info)); > info->info = NULL; > info->tree = NULL; > + info->obstack = malloc (sizeof (struct obstack)); obstack could be "inlined", not being a pointer, you stated it needs #include "gdb_obstack.h" from "cp-support.h" but I do not see it as a problem. > + obstack_init (info->obstack); > + > return info; > } > > @@ -1989,10 +1994,51 @@ cp_demangled_name_parse_free (struct demangle_parse_info *parse_info) > info = next; > } > > + /* Free any memory allocated during typedef replacement. */ > + obstack_free (parse_info->obstack, NULL); > + free (parse_info->obstack); > + > /* Free the parser info. */ > free (parse_info); > } [...] > +void > +cp_merge_demangle_parse_infos (struct demangle_parse_info *dest, > + struct demangle_component *target, > + struct demangle_parse_info *src) > + > +{ [...] > + /* Assert if the SRC obstack is not empty. */ > + gdb_assert (obstack_empty_p (src->obstack)); $ make test-cp-name-parser test-cp-name-parser.o: In function `cp_merge_demangle_parse_infos': .../gdb/cp-name-parser.y:2036: undefined reference to `internal_error' > + > + /* Free SRC. */ > + cp_demangled_name_parse_free (src); > +} > + > /* Convert a demangled name to a demangle_component tree. On success, > 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 > diff --git a/gdb/cp-support.c b/gdb/cp-support.c > index 8cda2b4..2eff1d8 100644 > --- a/gdb/cp-support.c > +++ b/gdb/cp-support.c [...] > +static char * > +copy_string_to_obstack (struct obstack *obstack, const char *string, > + size_t *len) > +{ > + char *s; > + > + *len = strlen (string); > + s = obstack_alloc (obstack, *len); > + memcpy (s, string, *len); obstack_copy is more appropriate here. > + return s; > +} > > /* A cleanup wrapper for cp_demangled_name_parse_free. */ > > @@ -136,6 +162,341 @@ cp_already_canonical (const char *string) > return 0; > } > > +/* Inspect the given RET_COMP for its type. If it is a typedef, > + replace the node with the typedef's tree. > + > + Returns 1 if any typedef substitutions were made, 0 otherwise. */ > + > +static int > +inspect_type (struct demangle_parse_info *info, > + struct demangle_component *ret_comp) > +{ > + int i; > + char *name; > + struct symbol *sym; > + > + /* Copy the symbol's name from RET_COMP and look it up > + in the symbol table. */ > + name = (char *) alloca (ret_comp->u.s_name.len + 1); > + memcpy (name, ret_comp->u.s_name.s, ret_comp->u.s_name.len); > + name[ret_comp->u.s_name.len] = '\0'; > + > + /* Ignore any typedefs that should not be substituted. */ > + for (i = 0; i < ARRAY_SIZE (ignore_typedefs); ++i) > + { > + if (strcmp (name, ignore_typedefs[i]) == 0) > + return 0; > + } > + > + sym = lookup_symbol (name, 0, VAR_DOMAIN, 0); > + if (sym != NULL) > + { > + struct type *otype = SYMBOL_TYPE (sym); > + > + /* If the type is a typedef, replace it. */ > + if (TYPE_CODE (otype) == TYPE_CODE_TYPEDEF) > + { > + long len; > + int is_anon; > + struct type *type; > + struct demangle_parse_info *i; > + struct ui_file *buf; > + struct cleanup *cleanup; > + > + /* Get the real type of the typedef. */ > + type = check_typedef (otype); > + > + is_anon = (TYPE_TAG_NAME (type) == NULL > + && (TYPE_CODE (type) == TYPE_CODE_ENUM > + || TYPE_CODE (type) == TYPE_CODE_STRUCT > + || TYPE_CODE (type) == TYPE_CODE_UNION)); > + if (is_anon) > + { > + struct type *last = otype; > + > + /* Find the last typedef for the type. */ > + while (TYPE_TARGET_TYPE (last) != NULL > + && (TYPE_CODE (TYPE_TARGET_TYPE (last)) > + == TYPE_CODE_TYPEDEF)) > + last = TYPE_TARGET_TYPE (last); > + > + /* If there is only one typedef for this anonymous type, > + do not substitute it. */ > + if (type == otype) > + return 0; > + else > + /* Use the last typedef seen as the type for this > + anonymous type. */ > + type = last; > + } > + > + > + buf = mem_fileopen (); > + cleanup = make_cleanup_ui_file_delete (buf); > + > + type_print (type, "", buf, -1); > + name = ui_file_obsavestring (buf, info->obstack, &len); > + do_cleanups (cleanup); > + > + /* Turn the result into a new tree. Note that this > + tree will contain pointers into NAME, so NAME cannot > + be free'd until all typedef conversion is done and > + the final result is converted into a string. */ > + i = cp_demangled_name_to_comp (name, NULL); > + if (i != NULL) > + { > + /* Merge the two trees. */ > + cp_merge_demangle_parse_infos (info, ret_comp, i); > + > + /* Replace any newly introduced typedefs -- but not > + if the type is anonymous (that would lead to infinite > + looping). */ > + if (!is_anon) > + replace_typedefs (info, ret_comp); > + } > + else > + { > + /* This shouldn't happen unless the type printer has > + output something that the name parser cannot grok. > + Nonetheless, an ounce of prevention... > + > + Canonicalize the name again, and store it in the > + current node (RET_COMP). */ > + char *canon = cp_canonicalize_string_no_typedefs (name); > + > + if (canon != NULL) > + { > + /* Copy the canonicalization into the obstack and > + free CANON. */ > + name = copy_string_to_obstack (info->obstack, canon, &len); > + xfree (canon); > + } > + > + ret_comp->u.s_name.s = name; > + ret_comp->u.s_name.len = len; > + } > + > + return 1; > + } > + } > + > + return 0; > +} > + > +/* Replace any typedefs appearing in the qualified name > + (DEMANGLE_COMPONENT_QUAL_NAME) represented in RET_COMP for the name parse > + given in INFO. */ > + > +static void > +replace_typedefs_qualified_name (struct demangle_parse_info *info, > + struct demangle_component *ret_comp) > +{ > + long len; > + char *name; > + struct ui_file *buf = mem_fileopen (); > + struct demangle_component *comp = ret_comp; > + > + /* Walk each node of the qualified name, reconstructing the name of > + this element. With every node, check for any typedef substitutions. > + If a substitution has occurred, replace the qualified name node > + with a DEMANGLE_COMPONENT_NAME node representing the new, typedef- > + substituted name. */ > + while (comp->type == DEMANGLE_COMPONENT_QUAL_NAME) > + { > + if (d_left (comp)->type == DEMANGLE_COMPONENT_NAME) > + { > + struct demangle_component new; > + > + ui_file_write (buf, d_left (comp)->u.s_name.s, > + d_left (comp)->u.s_name.len); > + name = ui_file_obsavestring (buf, info->obstack, &len); > + new.type = DEMANGLE_COMPONENT_NAME; > + new.u.s_name.s = name; > + new.u.s_name.len = len; > + if (inspect_type (info, &new)) I missed it before but inspect_type calls type_print which is already protected for exceptions inside inspect_type but BUF here is not protected by make_cleanup. It is just a negligible mem leak. > + { > + char *n, *s; > + size_t slen; > + > + /* A typedef was substituted in NEW. Convert it to a > + string and replace the top DEMANGLE_COMPONENT_QUAL_NAME > + node. */ > + > + ui_file_rewind (buf); > + n = cp_comp_to_string (&new, 100); > + if (n == NULL) > + { > + /* If something went astray, abort typedef substitutions. */ > + ui_file_delete (buf); > + return; > + } > + > + s = copy_string_to_obstack (info->obstack, n, &slen); > + xfree (n); > + > + ret_comp->type = DEMANGLE_COMPONENT_NAME; > + d_left (ret_comp)->u.s_name.s = s; > + d_left (ret_comp)->u.s_name.len = slen; > + d_right (ret_comp) = d_right (comp); > + comp = ret_comp; > + continue; > + } > + } > + else > + { > + /* The current node is not a name, so simply replace any > + typedefs in it. Then print it to the stream to continue > + checking for more typedefs in the tree. */ > + replace_typedefs (info, d_left (comp)); > + name = cp_comp_to_string (d_left (comp), 100); > + if (name == NULL) > + { > + /* If something went astray, abort typedef substitutions. */ > + ui_file_delete (buf); > + return; > + } > + fputs_unfiltered (name, buf); > + xfree (name); > + } > + ui_file_write (buf, "::", 2); > + comp = d_right (comp); > + } > + > + /* If the next component is DEMANGLE_COMPONENT_NAME, save the qualified > + name assembled above and append the name given by COMP. Then use this > + reassembled name to check for a typedef. */ > + > + if (comp->type == DEMANGLE_COMPONENT_NAME) > + { > + ui_file_write (buf, comp->u.s_name.s, comp->u.s_name.len); > + name = ui_file_obsavestring (buf, info->obstack, &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; > + inspect_type (info, ret_comp); > + } > + else > + replace_typedefs (info, comp); The same BUF exceptions protection for inspect_type and replace_typedefs > + > + ui_file_delete (buf); > +} > + > + nitpick: Two empty lines, should be one. I find it OK to check it in with the noted changes. Thanks, Jan