From mboxrd@z Thu Jan 1 00:00:00 1970 From: Elena Zannoni To: Daniel Berlin Cc: gdb-patches@sources.redhat.com Subject: Re: [PATCH] Start abstraction of C++ abi's Date: Mon, 19 Feb 2001 08:48:00 -0000 Message-id: <14993.20052.279440.5377@kwikemart.cygnus.com> References: X-SW-Source: 2001-02/msg00361.html 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 or 'foo::~foo > (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. 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 Note that since your patch is incomplete, I cannot test it. So the 'approved' is conditional. 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] == '~' 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? 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? Could you remove/update the comment/FIXME before CHECK_TYPEDEF. Do we need to include gdb_regexp.h? What needs it? 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 ! /* 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 ! #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. Elena