This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [PATCH] Start abstraction of C++ abi's
Elena Zannoni <ezannoni@cygnus.com> writes:
> Daniel Berlin writes:
> >
> > This patch, plus the attached files, start the abstraction of the C++
> > ABI's.
> >
> > I've started by replacing the simple things, and will incrementally
> > replace the more complex things, and the things that require real code
> > changes, as time goes on.
> >
> > The cp-abi directory, and it's files, are attached in a gzipped tar file.
> >
> > This fixes some problems with the new-abi already, like not being able
> > to set breakpoints on destructors (if you try it, you'll get:
> >
> > (gdb) b foo::~foo
> > the class `foo' does not have destructor defined
> > Hint: try 'foo::~foo<TAB> or 'foo::~foo<ESC-?>
> > (Note leading single quote.)
> > (gdb)
> > )
> >
> > I haven't added the method for detecting which C++ abi we are using,
> > so it simply defaults to the gnu v2 abi.
> >
> > However, I have verified the gnu v3 abi parts work fine too (that's how i
> > know it fixed breakpoints on destructors).
> >
> > This stuff touches a lot of files, but it's only removing code, or
> > changing a macro call to a function call (IE VTBL_PREFIX_P ->
> > vtbl_prefix_p, DESTRUCTOR_PREFIX_P -> destructor_prefix_p).
> >
> > Who exactly do i need approval from to check this stuff in?
> >
> > As I said, this is an incremental process. This is the minimum number
> > of changes necessary to start abstracting the simple things. There is
> > no way to make this patch smaller, without breaking gdb.
> >
> >
> > I need someone to look at the configure.in change, i'm not positive I
> > did it right.
> >
> > --Dan
> >
>
> Dan, I don't see ChangeLogs entries in your patch.
I'll resend with the changelog entries, and the things you ask for
below fixed.
> Also one file in the cp-abi directory looks empty:
>
> kwikemart.cygnus.com: 15 % tar tvf cp-abi.tar
> -rw-r--r-- dberlin/users 2551 2001-02-18 15:12 cp-abi.h
> drwxr-xr-x dberlin/users 0 2001-02-18 15:45 cp-abi/
> -rw-r--r-- dberlin/users 1967 2001-02-18 15:39 cp-abi/gnu-v3-abi.c
> -rw-r--r-- dberlin/users 0 2001-02-18 15:37 cp-abi/cp-abi.c
> -rw-r--r-- dberlin/users 2244 2001-02-18 14:58 cp-abi/gnu-v2-abi.c
>
Yes, it's temporarily empty.
It'll be filled in very shortly (read: tomorrow at the latest).
>
> Note that since your patch is incomplete, I cannot test it. So the
> 'approved' is conditional.
Okeydokey.
>
> Specific comments:
>
>
>
> *************** c_type_print_base (struct type *type, st
> *** 902,912 ****
> {
> char *physname = TYPE_FN_FIELD_PHYSNAME (f, j);
> int is_full_physname_constructor =
> ! ((physname[0] == '_' && physname[1] == '_'
> ! && strchr ("0123456789Qt", physname[2]))
> ! || STREQN (physname, "__ct__", 6)
> ! || DESTRUCTOR_PREFIX_P (physname)
> ! || STREQN (physname, "__dt__", 6));
>
> QUIT;
> if (TYPE_FN_FIELD_PROTECTED (f, j))
> --- 903,912 ----
> {
> char *physname = TYPE_FN_FIELD_PHYSNAME (f, j);
> int is_full_physname_constructor =
> ! constructor_prefix_p (physname)
> ! || destructor_prefix_p (physname)
> ! || STREQN (method_name, "~", 1);
> !
>
> QUIT;
> if (TYPE_FN_FIELD_PROTECTED (f, j))
>
>
> I agree with Michael Chastain, I would prefer if there was one less
> STREQN around, given that we understand what should be used in its
> place, let's just use method_name[1] == '~'
you mean method_name[0] == '~'
:).
But, Okeydokey.
>
>
> Index: dbxread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dbxread.c,v
> retrieving revision 1.12
> diff -c -3 -p -r1.12 dbxread.c
> *** dbxread.c 2001/01/19 14:53:44 1.12
> --- dbxread.c 2001/02/18 20:42:42
> ***************
> *** 58,63 ****
> --- 58,64 ----
> #include "demangle.h"
> #include "language.h" /* Needed inside partial-stab.h */
> #include "complaints.h"
> + #include "cp-abi.h"
>
> #include "aout/aout64.h"
> #include "aout/stab_gnu.h" /* We always use GNU stabs, not native, now */
> *************** record_minimal_symbol (char *name, CORE_
> *** 514,520 ****
> char *tempstring = name;
> if (tempstring[0] == bfd_get_symbol_leading_char (objfile->obfd))
> ++tempstring;
> ! if (VTBL_PREFIX_P ((tempstring)))
> ms_type = mst_data;
> }
> section = SECT_OFF_DATA (objfile);
> --- 515,521 ----
> char *tempstring = name;
> if (tempstring[0] == bfd_get_symbol_leading_char (objfile->obfd))
> ++tempstring;
> ! if (vtbl_prefix_p ((tempstring)))
> ms_type = mst_data;
> }
> section = SECT_OFF_DATA (objfile);
>
> Approved. Do we really need the extra parenthesis around tempstring?
No, I was just doing search and replace, i'll remove them.
>
> Index: linespec.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/linespec.c,v
> retrieving revision 1.4
> diff -c -3 -p -r1.4 linespec.c
> *** linespec.c 2000/12/15 01:01:48 1.4
> --- linespec.c 2001/02/18 20:42:42
> ***************
> *** 28,34 ****
> #include "demangle.h"
> #include "value.h"
> #include "completer.h"
> !
> /* Prototype for one function in parser-defs.h,
> instead of including that entire file. */
>
> --- 28,35 ----
> #include "demangle.h"
> #include "value.h"
> #include "completer.h"
> ! #include "gdb_regex.h"
> ! #include "cp-abi.h"
> /* Prototype for one function in parser-defs.h,
> instead of including that entire file. */
> *************** find_methods (struct type *t, char *name
> *** 119,126 ****
> int method_counter;
>
> /* FIXME: Shouldn't this just be CHECK_TYPEDEF (t)? */
> ! t = SYMBOL_TYPE (sym_class);
> !
> /* Loop over each method name. At this level, all overloads of a name
> are counted as a single name. There is an inner loop which loops over
> each overload. */
> --- 120,126 ----
> int method_counter;
>
> /* FIXME: Shouldn't this just be CHECK_TYPEDEF (t)? */
> ! CHECK_TYPEDEF (t);
> /* Loop over each method name. At this level, all overloads of a name
> are counted as a single name. There is an inner loop which loops over
> each overload. */
> *************** find_methods (struct type *t, char *name
> *** 143,149 ****
> method_name = dem_opname;
> }
>
> ! if (STREQ (name, method_name))
> /* Find all the overloaded methods with that name. */
> for (field_counter = TYPE_FN_FIELDLIST_LENGTH (t, method_counter) - 1;
> field_counter >= 0;
> --- 143,149 ----
> method_name = dem_opname;
> }
>
> ! if (strcmp_iw (name, method_name) == 0)
> /* Find all the overloaded methods with that name. */
> for (field_counter = TYPE_FN_FIELDLIST_LENGTH (t, method_counter) - 1;
> field_counter >= 0;
> *************** find_methods (struct type *t, char *name
> *** 167,177 ****
> }
> else
> phys_name = TYPE_FN_FIELD_PHYSNAME (f, field_counter);
> !
> /* Destructor is handled by caller, dont add it to the list */
> ! if (DESTRUCTOR_PREFIX_P (phys_name))
> continue;
> -
> sym_arr[i1] = lookup_symbol (phys_name,
> NULL, VAR_NAMESPACE,
> (int *) NULL,
> --- 167,176 ----
> }
> else
> phys_name = TYPE_FN_FIELD_PHYSNAME (f, field_counter);
> !
> /* Destructor is handled by caller, dont add it to the list */
> ! if (destructor_prefix_p (phys_name) != 0)
> continue;
> sym_arr[i1] = lookup_symbol (phys_name,
> NULL, VAR_NAMESPACE,
> (int *) NULL,
>
>
> Could you please leave the blank lines that were there originally?
You mean the one where gdb_regex is now?
I don't know what i did to the one before destructor_prefix_p, it's
still there, it's just "not as blank" or something, I guess.
> Could you remove/update the comment/FIXME before CHECK_TYPEDEF.
Yup.
> Do we need to include gdb_regexp.h? What needs it?
No. I don't know how that slipped in there.
>
>
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.28
> diff -c -3 -p -r1.28 symtab.c
> *** symtab.c 2001/01/30 02:49:36 1.28
> --- symtab.c 2001/02/18 20:42:43
> ***************
> *** 44,50 ****
> #include "gdb_string.h"
> #include "gdb_stat.h"
> #include <ctype.h>
> !
> /* Prototype for one function in parser-defs.h,
> instead of including that entire file. */
>
> --- 44,50 ----
> #include "gdb_string.h"
> #include "gdb_stat.h"
> #include <ctype.h>
> ! #include "cp-abi.h"
> /* Prototype for one function in parser-defs.h,
> instead of including that entire file. */
>
> *************** gdb_mangle_name (struct type *type, int
> *** 287,293 ****
> int is_full_physname_constructor;
>
> int is_constructor;
> ! int is_destructor = DESTRUCTOR_PREFIX_P (physname);
> /* Need a new type prefix. */
> char *const_prefix = method->is_const ? "C" : "";
> char *volatile_prefix = method->is_volatile ? "V" : "";
> --- 287,293 ----
> int is_full_physname_constructor;
>
> int is_constructor;
> ! int is_destructor = destructor_prefix_p (physname);
> /* Need a new type prefix. */
> char *const_prefix = method->is_const ? "C" : "";
> char *volatile_prefix = method->is_volatile ? "V" : "";
> *************** gdb_mangle_name (struct type *type, int
> *** 297,306 ****
> if (OPNAME_PREFIX_P (field_name))
> return xstrdup (physname);
>
> ! is_full_physname_constructor =
> ! ((physname[0] == '_' && physname[1] == '_' &&
> ! (isdigit (physname[2]) || physname[2] == 'Q' || physname[2] == 't'))
> ! || (strncmp (physname, "__ct", 4) == 0));
>
> is_constructor =
> is_full_physname_constructor || (newname && STREQ (field_name, newname));
> --- 297,303 ----
> if (OPNAME_PREFIX_P (field_name))
> return xstrdup (physname);
>
> ! is_full_physname_constructor = constructor_prefix_p (physname);
>
> is_constructor =
> is_full_physname_constructor || (newname && STREQ (field_name, newname));
>
>
> Approved.
>
> Index: symtab.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.h,v
> retrieving revision 1.18
> diff -c -3 -p -r1.18 symtab.h
> *** symtab.h 2001/02/08 06:03:54 1.18
> --- symtab.h 2001/02/18 20:42:43
> *************** struct partial_symtab
> *** 1055,1074 ****
> '_vt$' is the old cfront-style vtables; '_VT$' is the new
> style, using thunks (where '$' is really CPLUS_MARKER). */
>
> - #define VTBL_PREFIX_P(NAME) \
> - (((NAME)[0] == '_' \
> - && (((NAME)[1] == 'V' && (NAME)[2] == 'T') \
> - || ((NAME)[1] == 'v' && (NAME)[2] == 't')) \
> - && is_cplus_marker ((NAME)[3])) || ((NAME)[0]=='_' && (NAME)[1]=='_' \
> - && (NAME)[2]=='v' && (NAME)[3]=='t' && (NAME)[4]=='_'))
> -
> - /* Macro that yields non-zero value iff NAME is the prefix for C++ destructor
> - names. Note that this macro is g++ specific (FIXME). */
> -
> - #define DESTRUCTOR_PREFIX_P(NAME) \
> - ((NAME)[0] == '_' && is_cplus_marker ((NAME)[1]) && (NAME)[2] == '_')
>
> -
> /* External variables and functions for the objects described above. */
>
> /* This symtab variable specifies the current file for printing source lines */
> --- 1055,1061 ----
>
> OK.
>
I forgot to cut the VTBL_PREFIX_P comment out, i'll do this as well.
> Elena