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

[Daniel Berlin <dberlin@redhat.com>]

Andrew Cagney <ac131313@cygnus.com> writes:

> Daniel Berlin wrote:
> > 
> > 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.
> 
> FYI,
> 
> Moving stuff into a sub directory is a significant change and should
> really be discussed separatly - at present the only think in sub
> directories are UIs.  I tend to recommend leaving such cosmetic changes
> as a latter pass.

It's not moving stuff into a subdir, as it didn't exist before. I'm
creating it in a subdir. Why is it necessary to have a long discussion
about creating a directory for a bunch of related files? I'm happy to
see if anyone objects, but I don't see it as a significant change.
Maybe this is why it never gets done.

if you really want me to dirty the root directory with these new
files, i will, but i felt it made a lot more sense not to.There are
currently over 400 files in the root gdb directory. Besides the -nat
and -tdep files, I can't tell what a file does from it's name, because
they aren't logically divided into subdirs by functionality, and there
are just too damn many files.  Not that I expect to see this happen,
but to prove a point: 
Quick, what's kod.h?
ocd.c?
stuff.c?
Can you tell that ppc-bdm.c is related to ocd.c?

> 
> Also remember that it is always best to figure out what the issues with
> this change are likely to be and then discuss them publically.

None, as of yet. I've simply replaced macros with abstracted
functions, and provided the implementations of those functions for the
v2, and v3 abi.


> Can you
> walk people through exactly how you're introducing the C++ abi.

Errr, i'm not introducing a C++ abi, i'm introducing a structure so we
can remove the hardcoded dependencies on the C++ ABI's, by abstracting
what they do, and do differently, into functions that will have
different implemenation specifics, but do the same thing.

Fer instance, determining if some mangled name is a destructor.
We currently have a G++ specific macro used all over the place,
DESTRUCTOR_PREFIX_P.
This won't work for the v3 abi. 

We also have a bunch of code that does one thing if it's HP aCC, and
another if it's G++, but they both are performing the same actual
operation, just doing it in different ways because of abi specifics.

This happens all over the place. For instance, we have duplicate sets
of functions  that have large implementation differences (because the
abi's are very different) to search through virtual base classes for
HP aCC, and  G++. However, they both take the same actual parameters, and
return the same values.  We then have if blocks that say

if (hp_som_object)
{
<Whole bunch of HP specific code>
}
else
{
/* Assume it's G++ */
<Whole bunch of G++ specific code that does the same thing, with G++'s
abi>
}

all over the place (literally)


For a random example, from valops.c: 

        if (TYPE_HAS_VTABLE (type))
            {
              /* HP aCC compiled type, search for virtual base offset
               * according to HP/Taligent runtime spec.  */
              int skip;
              find_rt_vbase_offset (type, TYPE_BASECLASS (type, i),
                                    VALUE_CONTENTS_ALL (*argp),
                                    offset + VALUE_EMBEDDED_OFFSET (*argp),
                                    &base_offset, &skip);
              if (skip >= 0)
                error ("Virtual base class offset not found in vtable");
            }
          else
            {
              /* probably g++ runtime model */
              base_offset = VALUE_OFFSET (*argp) + offset;
              base_offset =
                baseclass_offset (type, i,
                                  VALUE_CONTENTS (*argp) + base_offset,
                                  VALUE_ADDRESS (*argp) + base_offset);
              if (base_offset == -1)
                error ("virtual baseclass botch");
            }

This is pretty dumb, they both do the same thing, just for different
ABI's.
This stuff is rampant.

This type of thing  will also be abstracted away, so it's just

base_offset = baseclass_offset (...)



This is necessary for the addition of the v3 G++ abi, unless you want
me to extend all the if blocks, and add more inline code into fifty
five places. And add another set of functions that do the same thing,
just have different implementations.


> 
> > This fixes some problems with the new-abi already, like not
> > being able to set breakpoints on destructors (if you try it,
> 
> This, unfortunatly, makes it sound like more than a cosmetic change :-(
> 

How so?
It wasn't able to detect destructors before, because it was looking
for the v2 abi stuff.
Now, it has the ability to detect both, without any changes to any
non C++ abi specific source. This is due to the added code in gnu-v3-abi.c.

Currently, you could view the stuff i've submitted so far as cosmetic,
if you really wanted to, but it's necessary to support both ABI's at
the same time. I also didn't want to submit too much at once, so that
nobody complained that i should break the patch down. Now it sounds
like your saying it doesn't do *enough* yet.

> As they say, sounds good in theory.


> 
> 	Andrew