From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Berlin To: Elena Zannoni Cc: gdb-patches@sources.redhat.com Subject: Re: [PATCH] Start abstraction of C++ abi's Date: Mon, 19 Feb 2001 10:24:00 -0000 Message-id: References: <14993.20052.279440.5377@kwikemart.cygnus.com> X-SW-Source: 2001-02/msg00362.html Elena Zannoni 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 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. 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 > ! > /* 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. > I forgot to cut the VTBL_PREFIX_P comment out, i'll do this as well. > Elena