From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13537 invoked by alias); 9 Sep 2002 23:32:24 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 13509 invoked from network); 9 Sep 2002 23:32:21 -0000 Received: from unknown (HELO localhost.redhat.com) (66.30.197.194) by sources.redhat.com with SMTP; 9 Sep 2002 23:32:21 -0000 Received: by localhost.redhat.com (Postfix, from userid 469) id 84554106CC; Mon, 9 Sep 2002 19:30:27 -0400 (EDT) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <15741.12051.396773.949592@localhost.redhat.com> Date: Mon, 09 Sep 2002 16:32:00 -0000 To: Daniel Jacobowitz Cc: gdb-patches@sources.redhat.com, ezannoni@redhat.com Subject: Re: RFA: Correct field names for class methods In-Reply-To: <20020827031346.GA16591@nevyn.them.org> References: <20020827031346.GA16591@nevyn.them.org> X-SW-Source: 2002-09/txt/msg00135.txt.bz2 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 > > * 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 > > @@ -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