This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.


Why not just:
   *target = *src->tree;

Changed.


You can calculate LAST only if it gets used - move the code more down.

That block got moved around based on Tom's comments. And obstacks cleaned a lot of it up, too.


In some cases type == otype here, such as if there was only one typedef of
anonymous type or of typedef failed to be resolved.  Here could be also just
return 0.

Done.


double-free, NAME is already in FREE_LIST.
LEN is not set here for NAME.  And CANON is leaked.

Yup. Cleaned up with the move to an obstack.


ui_file_rewind should happen even if N is NULL, somehow probably to rather
abort the operation on NULL N.

Agreed.


IMO here is missing:
	ret_comp->type = DEMANGLE_COMPONENT_NAME;

Indeed!


If it returns NULL I would prefer some sort of abort.  It risks now to quietly
corrupt the name.  The later operations will probably fail not modifying
anything but still it is a bit fragile.

Done.


+	case DEMANGLE_COMPONENT_TYPED_NAME:
+	  {
+	    struct demangle_component *comp = d_right (ret_comp);
+
+	    while (comp != NULL
+		&&  (comp->type == DEMANGLE_COMPONENT_VOLATILE
+		       || comp->type == DEMANGLE_COMPONENT_RESTRICT
+		       || comp->type == DEMANGLE_COMPONENT_CONST
+		       || comp->type == DEMANGLE_COMPONENT_VOLATILE_THIS
+		       || comp->type == DEMANGLE_COMPONENT_RESTRICT_THIS
+		       || comp->type == DEMANGLE_COMPONENT_CONST_THIS))
+	      comp = d_left (comp);
+
+	    if (d_left (ret_comp)->type != DEMANGLE_COMPONENT_NAME
+		|| comp->type != DEMANGLE_COMPONENT_FUNCTION_TYPE)
+	      replace_typedefs (info, d_left (ret_comp), free_list);

I admit I do not understand the goal of the COMP computation and the conditional. replace_typedefs unconditionally does not break gdb.cp/meth-typedefs.exp. At least a comment of the purpose would be nice.

I've removed this to unconditionally call replace_typedefs. IIRC, that was needed for some reason that escapes me at the moment. The real need for it is being hidden by the minsym fallback. I'll follow-up on this after this patchset is finalized. [That entails a lot of fixes to c-typeprint.c and whatnot called by dwarf2_physname when physnames are computed.]


You are now using cp_canonicalize_string_no_typedefs at the consumers but it
is not used at the producer - at dwarf2_canonicalize_name.  It is not needed
there as long as GDB depends on DW_AT_MIPS_linkage_name.  But do you plan to
use it at dwarf2_canonicalize_name?  That is it would fix
the `set debug check-physname yes' warnings.

When physnames are computed, no typedefs should appear in them (although apparently exceptions should exist for std::string and others that are excepted in libiberty). This is a bunch of bugs which I will address immediately after this is finalized.


Thank you for reviewing this (again)!

New patch attached.

Keith

ChangeLog
2011-08-09  Keith Seitz  <keiths@redhat.com>

	* cp-support.h (cp_canonicalize_string_no_typedefs): Declare.
	(cp_merge_demangle_parse_infos): Declare.
	* cp-support.c (ignore_typedefs): New file global.
	(copy_string_to_obstack): New function.
	(inspect_type): New function.
	(replace_typedefs): New function.
	(replace_typedefs_qualified_name): New function.
	(cp_canonicalize_string_no_typedefs): New function.
	* cp-name-parser.y (cp_merge_demangle_parse_infos): New function.
	(cp_new_demangle__parse_info): Allocate and initialize the obstack.
	* linespec.c (find_methods): Use cp_canonicalize_string_no_typedefs
	instead of cp_canonicalize_string.
	(find_method): Likewise.
	(decode_compound): Before looking up the name, call
	cp_canonicalize_string_no_typedefs.
	(decode_variable): Likewise.

2011-08-09 Keith Seitz <keiths@redhat.com>

	* gdb.cp/meth-typedefs.cc: New file.
	* gdb.cp/meth-typedefs.exp: New file.

Attachment: cp_canonicalize_no_typedefs-5.patch
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]