Re: [PATCH] Start abstraction of C++ abi's

Elena Zannoni
2001-02-19T08:48:00Z

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.
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 <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] == '_')
\f
-
/* External variables and functions for the objects described above. */

/* This symtab variable specifies the current file for printing source lines */
--- 1055,1061 ----

OK.

Elena