This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: RFA: Correct field names for class methods
- From: Elena Zannoni <ezannoni at redhat dot com>
- To: Daniel Jacobowitz <drow at mvista dot com>
- Cc: gdb-patches at sources dot redhat dot com, ezannoni at redhat dot com
- Date: Mon, 9 Sep 2002 19:30:27 -0400
- Subject: Re: RFA: Correct field names for class methods
- References: <20020827031346.GA16591@nevyn.them.org>
Daniel Jacobowitz writes:
> Right now, the "name" field of a method is not always reliable. There's
> special-cased code all over the linespec, function calling, struct lookup,
> etc. code to handle this. Among the problems:
>
> - v3 stabs emit __comp_ctor etc.
> - v2 stabs emit constructors and destructors in the same fieldlist
> - v2 stabs emit mangled operator names
>
> This patch does not remove any of the special cases, but renders it
> all obsolete, to be cleaned up in a forthcoming patch. This will greatly
> simplify the revised method printing code that I'm working on, a necessary
> cleanup as we move towards namespace support. It pushes the stabs special
> casing back into stabs related code as much as practical.
>
> Elena, this does a bit of its ugliness in read_member_functions, so I'd like
> your approval before I go ahead with it.
See below. I have some questions and some general comments on the
structure of the patch.
>
> Comments, anyone?
>
> --
> Daniel Jacobowitz
> MontaVista Software Debian GNU/Linux Developer
>
> 2002-08-26 Daniel Jacobowitz <drow@mvista.com>
>
> * gdbtypes.c (check_stub_method): Make static.
> (update_method_from_physname, check_stub_method_group)
> (find_last_component, class_name_from_physname)
> (method_name_from_physname): New functions.
> * gdbtypes.h: Update prototypes.
>
> * stabsread.c: Include "cp-abi.h".
Makefile?
> (read_member_functions): Correct method names for operators
> and v3 constructors/destructors. Separate v2 constructors and
> destructors.
>
> * cp-valprint.c (cp_print_class_method): Call
> check_stub_method_group instead of check_stub_method.
> * p-valprint.c (pascal_object_print_class_method): Likewise.
> * valops.c (search_struct_method): Likewise.
> (find_method_list, value_struct_elt_for_reference): Likewise.
>
> Index: cp-valprint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/cp-valprint.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 cp-valprint.c
> --- cp-valprint.c 29 Jul 2002 22:55:26 -0000 1.13
> +++ cp-valprint.c 27 Aug 2002 02:02:55 -0000
> @@ -97,13 +97,12 @@ cp_print_class_method (char *valaddr,
> f = TYPE_FN_FIELDLIST1 (domain, i);
> len2 = TYPE_FN_FIELDLIST_LENGTH (domain, i);
>
> + check_stub_method_group (domain, i);
> for (j = 0; j < len2; j++)
> {
> QUIT;
Did you mean to leave this QUIT?
> if (TYPE_FN_FIELD_VOFFSET (f, j) == offset)
> {
> - if (TYPE_FN_FIELD_STUB (f, j))
> - check_stub_method (domain, i, j);
> kind = "virtual ";
> goto common;
> }
> @@ -129,15 +128,11 @@ cp_print_class_method (char *valaddr,
> f = TYPE_FN_FIELDLIST1 (domain, i);
> len2 = TYPE_FN_FIELDLIST_LENGTH (domain, i);
>
> + check_stub_method_group (f, j);
> for (j = 0; j < len2; j++)
> {
> - QUIT;
> - if (TYPE_FN_FIELD_STUB (f, j))
> - check_stub_method (domain, i, j);
> if (STREQ (SYMBOL_NAME (sym), TYPE_FN_FIELD_PHYSNAME (f, j)))
> - {
> - goto common;
> - }
> + goto common;
> }
> }
> }
> Index: gdbtypes.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtypes.c,v
> retrieving revision 1.56
> diff -u -p -r1.56 gdbtypes.c
> --- gdbtypes.c 20 Aug 2002 19:57:32 -0000 1.56
> +++ gdbtypes.c 27 Aug 2002 02:02:55 -0000
> @@ -1672,7 +1672,7 @@ safe_parse_type (char *p, int length)
> which info used to be in the stab's but was removed to hack back
> the space required for them. */
>
> -void
> +static void
> check_stub_method (struct type *type, int method_id, int signature_id)
> {
> struct fn_field *f;
> @@ -1781,6 +1781,54 @@ check_stub_method (struct type *type, in
> xfree (demangled_name);
> }
>
Could you add some comments about what this function does?
Actually, since it is used only in stabsread.c, why not move it there?
I think a lot of this c++ specific stuff really doesn't belong in gdbtypes.c.
You can move all the new functions, except for check_stub_method_group to
stabsread.c. They are called from there only.
> +void
> +update_method_name_from_physname (char **old_name, char *physname)
> +{
> + char *method_name;
> +
> + method_name = method_name_from_physname (physname);
> +
> + if (method_name == NULL)
> + error ("bad physname %s\n", physname);
> +
> + if (strcmp (*old_name, method_name) != 0)
> + *old_name = method_name;
> + else
> + xfree (method_name);
> +}
> +
Could you add some comments here too?
> +void
> +check_stub_method_group (struct type *type, int method_id)
> +{
> + int len = TYPE_FN_FIELDLIST_LENGTH (type, method_id);
> + struct fn_field *f = TYPE_FN_FIELDLIST1 (type, method_id);
> + int j, found_stub;
> +
> + for (j = 0; j < len; j++)
> + if (TYPE_FN_FIELD_STUB (f, j))
> + {
> + found_stub = 1;
> + check_stub_method (type, method_id, j);
> + }
> +
> + /* We can handle only the v2 case here, because the only stabs with
> + incorrect field names in v3 are constructors and destructors, and
> + the only stub methods in v3 are static methods. */
> + if (found_stub && strncmp (TYPE_FN_FIELD_PHYSNAME (f, 0), "_Z", 2) != 0)
> + {
> + int ret;
> + char dem_opname[256];
> +
> + ret = cplus_demangle_opname (TYPE_FN_FIELDLIST_NAME (type, method_id),
> + dem_opname, DMGL_ANSI);
> + if (!ret)
> + ret = cplus_demangle_opname (TYPE_FN_FIELDLIST_NAME (type, method_id),
> + dem_opname, 0);
> + if (ret)
> + TYPE_FN_FIELDLIST_NAME (type, method_id) = xstrdup (dem_opname);
> + }
> +}
> +
> const struct cplus_struct_type cplus_struct_default;
>
> void
> @@ -3435,6 +3483,118 @@ build_gdbtypes (void)
> "__bfd_vma", (struct objfile *) NULL);
> }
>
> +/* Find the last component of the demangled C++ name NAME.
> +
> + This function return a pointer to the first colon before the
> + last component, or NULL if the name had only one component. */
> +
> +static const char *
> +find_last_component (const char *name)
> +{
> + const char *p;
> + int depth;
> +
> + /* Functions can have local classes, so we need to find the
> + beginning of the last argument list, not the end of the first
> + one. */
> + p = name + strlen (name) - 1;
> + while (p > name && *p != ')')
> + p--;
> +
> + if (p == name)
> + return NULL;
> +
> + /* P now points at the `)' at the end of the argument list. Walk
> + back to the beginning. */
> + p--;
> + depth = 1;
> + while (p > name && depth > 0)
> + {
> + if (*p == '<' || *p == '(')
> + depth--;
> + else if (*p == '>' || *p == ')')
> + depth++;
> + p--;
> + }
> +
> + if (p == name)
> + return NULL;
> +
> + while (p > name && *p != ':')
> + p--;
> +
> + if (p == name || p == name + 1 || p[-1] != ':')
> + return NULL;
> +
> + return p - 1;
> +}
> +
Did you forget to post a piece of the patch? I don't see the function
below being called anywhere.
> +/* Return the name of the class containing method PHYSNAME. */
> +
> +char *
> +class_name_from_physname (const char *physname)
> +{
> + char *ret = NULL;
> + const char *end;
> + int depth = 0;
> + char *demangled_name = cplus_demangle (physname, DMGL_ANSI);
> +
> + if (demangled_name == NULL)
> + return NULL;
> +
> + end = find_last_component (demangled_name);
> + if (end != NULL)
> + {
> + ret = xmalloc (end - demangled_name + 1);
> + memcpy (ret, demangled_name, end - demangled_name);
> + ret[end - demangled_name] = '\0';
> + }
> +
> + xfree (demangled_name);
> + return ret;
> +}
> +
> +/* Return the name of the method whose linkage name is PHYSNAME. */
> +
> +char *
> +method_name_from_physname (const char *physname)
> +{
> + char *ret = NULL;
> + const char *end;
> + int depth = 0;
> + char *demangled_name = cplus_demangle (physname, DMGL_ANSI);
> +
> + if (demangled_name == NULL)
> + return NULL;
> +
> + end = find_last_component (demangled_name);
> + if (end != NULL)
> + {
> + char *args;
> + int len;
> +
> + /* Skip "::". */
> + end = end + 2;
> +
> + /* Find the argument list, if any. */
> + args = strchr (end, '(');
> + if (args == NULL)
> + len = strlen (end + 2);
> + else
> + {
> + args --;
> + while (*args == ' ')
> + args --;
> + len = args - end + 1;
> + }
> + ret = xmalloc (len + 1);
> + memcpy (ret, end, len);
> + ret[len] = 0;
> + }
> +
> + xfree (demangled_name);
> + return ret;
> +}
>
> extern void _initialize_gdbtypes (void);
> void
> Index: gdbtypes.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtypes.h,v
> retrieving revision 1.35
> diff -u -p -r1.35 gdbtypes.h
> --- gdbtypes.h 10 Aug 2002 05:12:40 -0000 1.35
> +++ gdbtypes.h 27 Aug 2002 02:02:56 -0000
> @@ -1124,11 +1124,17 @@ extern struct type *check_typedef (struc
>
> #define CHECK_TYPEDEF(TYPE) (TYPE) = check_typedef (TYPE)
>
> -extern void check_stub_method (struct type *, int, int);
> +extern void check_stub_method_group (struct type *, int);
> +
> +extern void update_method_name_from_physname (char **old_name, char *physname);
>
> extern struct type *lookup_primitive_typename (char *);
>
> extern char *gdb_mangle_name (struct type *, int, int);
> +
> +extern char *class_name_from_physname (const char *physname);
> +
> +extern char *method_name_from_physname (const char *physname);
>
> extern struct type *builtin_type (char **);
>
> Index: p-valprint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/p-valprint.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 p-valprint.c
> --- p-valprint.c 19 Aug 2002 13:12:09 -0000 1.13
> +++ p-valprint.c 27 Aug 2002 02:02:56 -0000
> @@ -620,13 +620,11 @@ pascal_object_print_class_method (char *
> f = TYPE_FN_FIELDLIST1 (domain, i);
> len2 = TYPE_FN_FIELDLIST_LENGTH (domain, i);
>
> + check_stub_method_group (domain, i);
> for (j = 0; j < len2; j++)
> {
> - QUIT;
> if (TYPE_FN_FIELD_VOFFSET (f, j) == offset)
> {
> - if (TYPE_FN_FIELD_STUB (f, j))
> - check_stub_method (domain, i, j);
> kind = "virtual ";
> goto common;
> }
> @@ -646,15 +644,11 @@ pascal_object_print_class_method (char *
> f = TYPE_FN_FIELDLIST1 (domain, i);
> len2 = TYPE_FN_FIELDLIST_LENGTH (domain, i);
>
> + check_stub_method_group (domain, i);
> for (j = 0; j < len2; j++)
> {
> - QUIT;
> - if (TYPE_FN_FIELD_STUB (f, j))
> - check_stub_method (domain, i, j);
> if (STREQ (SYMBOL_NAME (sym), TYPE_FN_FIELD_PHYSNAME (f, j)))
> - {
> - goto common;
> - }
> + goto common;
> }
> }
> }
> Index: stabsread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/stabsread.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 stabsread.c
> --- stabsread.c 1 Aug 2002 17:18:32 -0000 1.38
> +++ stabsread.c 27 Aug 2002 02:02:57 -0000
> @@ -44,6 +44,7 @@
> #include "demangle.h"
> #include "language.h"
> #include "doublest.h"
> +#include "cp-abi.h"
>
> #include <ctype.h>
>
> @@ -3377,6 +3378,127 @@ read_member_functions (struct field_info
> }
> else
> {
> + int has_stub = 0;
> + int has_nondestructor = 0, has_destructor = 0;
> + int is_v3 = 0;
> + struct next_fnfield *tmp_sublist;
> +
> + /* Various versions of GCC emit various mostly-useless
> + strings in the name field for special member functions.
> + For some methods, we have a complete physname, so we
> + could fix this up now. For stub methods we will do this
> + later, in check_stub_method_group. For non-stub methods,
> + we have no clear way to know whether or not the physname
> + is correct; g++ 2.95.x only reliably emits full physnames
> + for operators which do not start with the method name
> + (constructors, destructors fall in this category; there
> + may be others?). But for other methods it may or may not
> + emit a full physname depending on the platform (if
> + CPLUS_MARKER can be `$' or `.', it will use minimal debug
> + information, but not otherwise).
> +
> + Rather than dealing with this, we take a different approach.
> + For v3 mangled names, we can use the full physname; for v2,
> + we use cplus_demangle_opname, because the only interesting
> + names are all operators. Skip if any method in the group
> + is a stub, to prevent our fouling up the workings of
> + gdb_mangle_name.
> +
> + Another thing that we need to clean up here: GCC 2.95.x
> + puts constructors and destructors in the same group. We
> + need to split this into two groups. */
> +
Since you are at it, could you insert examples of mangled names,
physnames, etc in the comment?
> + tmp_sublist = sublist;
> + while (tmp_sublist != NULL)
> + {
> + if (tmp_sublist->fn_field.is_stub)
> + has_stub = 1;
> + if (tmp_sublist->fn_field.physname[0] == '_'
> + && tmp_sublist->fn_field.physname[1] == 'Z')
> + is_v3 = 1;
> +
> + if (is_destructor_name (tmp_sublist->fn_field.physname))
> + has_destructor++;
> + else
> + has_nondestructor++;
Dumb question, but, what is a nondestructor? Maybe a different variable
name would be a bit more enlightening.
> +
> + tmp_sublist = tmp_sublist->next;
> + }
> +
> + if (has_destructor && has_nondestructor)
Comments needed, for the c++ challenged. What does it mean to have both
a destructor and a nondestructor?
> + {
> + struct next_fnfieldlist *destr_fnlist;
> + struct next_fnfield *last_sublist;
> +
> + /* Create a new fn_fieldlist for the destructors. */;
> + destr_fnlist = (struct next_fnfieldlist *)
> + xmalloc (sizeof (struct next_fnfieldlist));
> + make_cleanup (xfree, destr_fnlist);
> + memset (destr_fnlist, 0, sizeof (struct next_fnfieldlist));
> + destr_fnlist->fn_fieldlist.name
> + = obconcat (&objfile->type_obstack, "", "~",
> + new_fnlist->fn_fieldlist.name);
> +
> + destr_fnlist->fn_fieldlist.fn_fields = (struct fn_field *)
> + obstack_alloc (&objfile->type_obstack,
> + sizeof (struct fn_field) * has_destructor);
> + memset (destr_fnlist->fn_fieldlist.fn_fields, 0,
> + sizeof (struct fn_field) * has_destructor);
> + tmp_sublist = sublist;
> + last_sublist = NULL;
> + i = 0;
I am confused. Why is i always 0? Or it isn't?
Elena
> + while (tmp_sublist != NULL)
> + {
> + if (!is_destructor_name (tmp_sublist->fn_field.physname))
> + {
> + tmp_sublist = tmp_sublist->next;
> + continue;
> + }
> +
> + destr_fnlist->fn_fieldlist.fn_fields[i]
> + = tmp_sublist->fn_field;
> + if (last_sublist)
> + last_sublist->next = tmp_sublist->next;
> + else
> + sublist = tmp_sublist->next;
> + last_sublist = tmp_sublist;
> + tmp_sublist = tmp_sublist->next;
> + }
> +
> + destr_fnlist->fn_fieldlist.length = has_destructor;
> + destr_fnlist->next = fip->fnlist;
> + fip->fnlist = destr_fnlist;
> + nfn_fields++;
> + total_length += has_destructor;
> + length -= has_destructor;
> + }
> + else if (is_v3)
> + {
> + /* v3 mangling prevents the use of abbreviated physnames,
> + so we can do this here. There are stubbed methods in v3
> + only:
> + - in -gstabs instead of -gstabs+
> + - or for static methods, who are output as a function type
> + instead of a method type. */
> +
> + update_method_name_from_physname (&new_fnlist->fn_fieldlist.name,
> + sublist->fn_field.physname);
> + }
> + else if (!has_stub)
> + {
> + char dem_opname[256];
> + int ret;
> + ret = cplus_demangle_opname (new_fnlist->fn_fieldlist.name,
> + dem_opname, DMGL_ANSI);
> + if (!ret)
> + ret = cplus_demangle_opname (new_fnlist->fn_fieldlist.name,
> + dem_opname, 0);
> + if (ret)
> + new_fnlist->fn_fieldlist.name
> + = obsavestring (dem_opname, strlen (dem_opname),
> + &objfile->type_obstack);
> + }
> +
> new_fnlist->fn_fieldlist.fn_fields = (struct fn_field *)
> obstack_alloc (&objfile->type_obstack,
> sizeof (struct fn_field) * length);
> Index: valops.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/valops.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 valops.c
> --- valops.c 21 Aug 2002 17:24:31 -0000 1.69
> +++ valops.c 27 Aug 2002 02:02:58 -0000
> @@ -2302,12 +2302,11 @@ search_struct_method (char *name, struct
> struct fn_field *f = TYPE_FN_FIELDLIST1 (type, i);
> name_matched = 1;
>
> + check_stub_method_group (type, i);
> if (j > 0 && args == 0)
> error ("cannot resolve overloaded method `%s': no arguments supplied", name);
> else if (j == 0 && args == 0)
> {
> - if (TYPE_FN_FIELD_STUB (f, j))
> - check_stub_method (type, i, j);
> v = value_fn_field (arg1p, f, j, type, offset);
> if (v != NULL)
> return v;
> @@ -2315,8 +2314,6 @@ search_struct_method (char *name, struct
> else
> while (j >= 0)
> {
> - if (TYPE_FN_FIELD_STUB (f, j))
> - check_stub_method (type, i, j);
> if (!typecmp (TYPE_FN_FIELD_STATIC_P (f, j),
> TYPE_VARARGS (TYPE_FN_FIELD_TYPE (f, j)),
> TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (f, j)),
> @@ -2555,20 +2552,15 @@ find_method_list (struct value **argp, c
> char *fn_field_name = TYPE_FN_FIELDLIST_NAME (type, i);
> if (fn_field_name && (strcmp_iw (fn_field_name, method) == 0))
> {
> - /* Resolve any stub methods. */
> int len = TYPE_FN_FIELDLIST_LENGTH (type, i);
> struct fn_field *f = TYPE_FN_FIELDLIST1 (type, i);
> - int j;
>
> *num_fns = len;
> *basetype = type;
> *boffset = offset;
>
> - for (j = 0; j < len; j++)
> - {
> - if (TYPE_FN_FIELD_STUB (f, j))
> - check_stub_method (type, i, j);
> - }
> + /* Resolve any stub methods. */
> + check_stub_method_group (type, i);
>
> return f;
> }
> @@ -3094,6 +3086,8 @@ value_struct_elt_for_reference (struct t
> int j = TYPE_FN_FIELDLIST_LENGTH (t, i);
> struct fn_field *f = TYPE_FN_FIELDLIST1 (t, i);
>
> + check_stub_method_group (t, i);
> +
> if (intype == 0 && j > 1)
> error ("non-unique member `%s' requires type instantiation", name);
> if (intype)
> @@ -3107,8 +3101,6 @@ value_struct_elt_for_reference (struct t
> else
> j = 0;
>
> - if (TYPE_FN_FIELD_STUB (f, j))
> - check_stub_method (t, i, j);
> if (TYPE_FN_FIELD_VIRTUAL_P (f, j))
> {
> return value_from_longest