Re: [RFA] c++/12266 (again) [cp_canonicalize_no_typedefs-4.patch]

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

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