Re: [RFA/c++] Fix printing classes with virtual base classes

Re: [RFA/c++] Fix printing classes with virtual base classes

[Michael Elizabeth Chastain]

And I would be happy to run tests against v2 and v3 C++ compilers
and report the results.  Most of my time is only on weekends though.

The trouble with the gdb C++ code is that it's not logic bugs, it's
perverse data structures where each bug fix may not affect the
test results much.  So Jim's judgement is much more useful than
my mechanical test results.

Michael C

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Berlin]

On Tue, 27 Nov 2001, Michael Elizabeth Chastain wrote:

> And I would be happy to run tests against v2 and v3 C++ compilers
> and report the results.  Most of my time is only on weekends though.
>
> The trouble with the gdb C++ code is that it's not logic bugs, it's
> perverse data structures where each bug fix may not affect the
> test results much.
Almost.
It's perverse logic combined with shoehorned data structures.

Or was that shoehorned logic combined with perverse data structures.

Hmmm.

> So Jim's judgement is much more useful than
> my mechanical test results.
>
> Michael C
>

Re: [RFA/c++] Fix printing classes with virtual base classes

[Michael Elizabeth Chastain]

And I would be happy to run tests against v2 and v3 C++ compilers
and report the results.  Most of my time is only on weekends though.

The trouble with the gdb C++ code is that it's not logic bugs, it's
perverse data structures where each bug fix may not affect the
test results much.  So Jim's judgement is much more useful than
my mechanical test results.

Michael C

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Berlin]

On Tue, 27 Nov 2001, Michael Elizabeth Chastain wrote:

> And I would be happy to run tests against v2 and v3 C++ compilers
> and report the results.  Most of my time is only on weekends though.
>
> The trouble with the gdb C++ code is that it's not logic bugs, it's
> perverse data structures where each bug fix may not affect the
> test results much.
Almost.
It's perverse logic combined with shoehorned data structures.

Or was that shoehorned logic combined with perverse data structures.

Hmmm.

> So Jim's judgement is much more useful than
> my mechanical test results.
>
> Michael C
>

Re: [RFA/c++] Fix printing classes with virtual base classes

[Jim Blandy]

I typo'd in CC'ing the message below to gdb-patches.  Daniel had my
approval to commit his change to gnu-v3-abi.c.

To: Daniel Jacobowitz <drow@mvista.com>
Cc: gdb-patches@sources.redhat.comb
Subject: Re: [RFA/c++] Fix printing classes with virtual base classes
References: < 20011126201945.A27754@nevyn.them.org >
	< np667wket5.fsf@zwingli.cygnus.com >
	< 20011127020634.A10010@nevyn.them.org >
	< npd723j4lc.fsf@zwingli.cygnus.com >
	< 20011130014034.A29999@nevyn.them.org >
Bcc: jimb
From: Jim Blandy <jimb@zwingli.cygnus.com>
Date: 30 Nov 2001 12:12:51 -0500
In-Reply-To: Daniel Jacobowitz's message of Fri, 30 Nov 2001 01:40:34 -0500
Message-ID: <np4rncdvxo.fsf@zwingli.cygnus.com>
Lines: 23
X-Mailer: Gnus v5.3/Emacs 19.34
--text follows this line--

Daniel Jacobowitz <drow@mvista.com> writes:
> Jim, can I commit these?  It'll make it easier for me to post the
> following batch.  Now that GCC emits the information I need (on HEAD at
> least) I'd like to finish this up.

Yes, go ahead.

> I guess that I can commit the gnu-v3-abi bits on my own initiative,
> since no one objected... actually, I guess the values stuff is
> unmaintained too?

Well, officially, yes, but my sense is that values.c is a high-traffic
area, so people would squonk more if you just commit stuff there.
I'll let Andrew tell you the Official Position. 

> MAINTAINERS says:
>   If there is no maintainer for a given domain then the responsibility
>   falls to the head maintainer.
> So I guess I need approval from one of Ye Divine Entities first.

... I'm clearly suffering from a divinity deficiency.  But I'll
approve your changes.

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Fri, Nov 30, 2001 at 11:00:26AM -0600, Michael Elizabeth Chastain wrote:
> Hi Daniel,
> 
> Just to make sure, you are talking about this patch, right?
> 
>   http://sources.redhat.com/ml/gdb-patches/2001-11/msg00487.html
> 
> I will give it some intensive testing tonight and tomorrow morning.

That's the one.  I just committed it; more to follow.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Michael Elizabeth Chastain]

Hi Daniel,

Just to make sure, you are talking about this patch, right?

  http://sources.redhat.com/ml/gdb-patches/2001-11/msg00487.html

I will give it some intensive testing tonight and tomorrow morning.

Michael C

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Fri, Nov 30, 2001 at 11:48:49AM -0500, Andrew Cagney wrote:
> >Jim, can I commit these?  It'll make it easier for me to post the
> >following batch.  Now that GCC emits the information I need (on HEAD at
> >least) I'd like to finish this up.
> >
> >I guess that I can commit the gnu-v3-abi bits on my own initiative,
> >since no one objected... actually, I guess the values stuff is
> >unmaintained too?
> 
> I wouldn't describe it as un-maintained.  It is fundamental core code, 
> JimB and I would both be keeping a very keen eye on it.

Thus confusion :)  MAINTAINERS lists a past maintainer for it and no
current maintainer... so I assumed it was lacking.

> >MAINTAINERS says:
> >  If there is no maintainer for a given domain then the responsibility
> >  falls to the head maintainer.
> >So I guess I need approval from one of Ye Divine Entities first.
> 
> Yes.
> 
> >[Would someone more familiar with the state of affairs than I
> >explicitly list the unmaintained parts in MAINTAINERS?  Quite a few
> >things seem to have slipped down that path.]
> 
> Nothing is is really unmaintained.  The buck (unfortunatly :-) stops 
> here.  Did you have any comments on my recent proposal to change how 
> targets (and natives) can get a change approved?

I don't remember seeing it.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Andrew Cagney]

> Jim, can I commit these?  It'll make it easier for me to post the
> following batch.  Now that GCC emits the information I need (on HEAD at
> least) I'd like to finish this up.
> 
> I guess that I can commit the gnu-v3-abi bits on my own initiative,
> since no one objected... actually, I guess the values stuff is
> unmaintained too?

I wouldn't describe it as un-maintained.  It is fundamental core code, 
JimB and I would both be keeping a very keen eye on it.

> MAINTAINERS says:
>   If there is no maintainer for a given domain then the responsibility
>   falls to the head maintainer.
> So I guess I need approval from one of Ye Divine Entities first.

Yes.

> [Would someone more familiar with the state of affairs than I
> explicitly list the unmaintained parts in MAINTAINERS?  Quite a few
> things seem to have slipped down that path.]

Nothing is is really unmaintained.  The buck (unfortunatly :-) stops 
here.  Did you have any comments on my recent proposal to change how 
targets (and natives) can get a change approved?

Andrew

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

Jim, can I commit these?  It'll make it easier for me to post the
following batch.  Now that GCC emits the information I need (on HEAD at
least) I'd like to finish this up.

I guess that I can commit the gnu-v3-abi bits on my own initiative,
since no one objected... actually, I guess the values stuff is
unmaintained too?

MAINTAINERS says:
  If there is no maintainer for a given domain then the responsibility
  falls to the head maintainer.
So I guess I need approval from one of Ye Divine Entities first.

[Would someone more familiar with the state of affairs than I
explicitly list the unmaintained parts in MAINTAINERS?  Quite a few
things seem to have slipped down that path.]


On Tue, Nov 27, 2001 at 04:17:51PM -0500, Jim Blandy wrote:
> 
> These are two independent fixes, right?  I understand GDB may need
> them both before it works correctly; I'm asking if each of them is a
> correct change in its own right.  If so, could you show me a test case
> that each change fixes?
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > 2001-11-26  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* values.c (value_primitive_field): Add embedded_offset to the
> > 	address of structure members.
> > 	* gnu-v3-abi.c (gnuv3_rtti_type): Cast to base type before
> > 	attempting to access vtable pointer.  Set using_enc_p if we cast.
> > 	(gnuv3_virtual_fn_field): Call value_cast with structure rather than
> > 	structure pointer.  Cast to base type before attempting to access
> > 	vtable pointer.
> 

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

They are both independent fixes.  The fixes to gnu-v3-abi.c by
themselves fix several tests in virtfunc.exp when using G++ 3.0
(from XFAIL to XPASS, ugh...).  Then if you look at the test logs, you
should find two failure modes for the ones that still fail.  Some are
marked "virtual baseclass botch", and I am trying right now to fix
those; but I believe that one will just be wrong.  That's because my
use of value_field in gnuv3_virtual_fn_field triggers the bug; the
value has not been read in yet, so only its address is copied, and the
embedded offset for the parent type is lost.  We then get the wrong
vtable and call the wrong function.

I can't construct an independent testcase, but I think it should be
possible.  Most things run afoul of the other "botch" bug.

On Tue, Nov 27, 2001 at 04:17:51PM -0500, Jim Blandy wrote:
> 
> These are two independent fixes, right?  I understand GDB may need
> them both before it works correctly; I'm asking if each of them is a
> correct change in its own right.  If so, could you show me a test case
> that each change fixes?
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > 2001-11-26  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* values.c (value_primitive_field): Add embedded_offset to the
> > 	address of structure members.
> > 	* gnu-v3-abi.c (gnuv3_rtti_type): Cast to base type before
> > 	attempting to access vtable pointer.  Set using_enc_p if we cast.
> > 	(gnuv3_virtual_fn_field): Call value_cast with structure rather than
> > 	structure pointer.  Cast to base type before attempting to access
> > 	vtable pointer.
> 

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Jim Blandy]

These are two independent fixes, right?  I understand GDB may need
them both before it works correctly; I'm asking if each of them is a
correct change in its own right.  If so, could you show me a test case
that each change fixes?

Daniel Jacobowitz <drow@mvista.com> writes:
> 2001-11-26  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* values.c (value_primitive_field): Add embedded_offset to the
> 	address of structure members.
> 	* gnu-v3-abi.c (gnuv3_rtti_type): Cast to base type before
> 	attempting to access vtable pointer.  Set using_enc_p if we cast.
> 	(gnuv3_virtual_fn_field): Call value_cast with structure rather than
> 	structure pointer.  Cast to base type before attempting to access
> 	vtable pointer.

Re: [RFA/c++] Fix printing classes with virtual base classes

[Jim Blandy]

Andrew Cagney <ac131313@cygnus.com> writes:
> > I don't have any comments on Daniel's patch, but I am wondering who
> > will approve this.  Given that we currently have no C++ maintainer,
> > should we treat Daniel's patch as an RFC and have him commit it after
> > a few days if there are no objections?
> 
> I'm pretty sure Jim Blandy, as a (most experienced?) language 
> maintainer, will take care of it.

I'm not really qualified to be a C++ maintainer, but I'd be happy to
lend Daniel a second pair of eyes for checking his patches.

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Tue, Nov 27, 2001 at 10:16:56AM -0500, Jim Blandy wrote:
> Daniel Jacobowitz <drow@mvista.com> writes:
> > On Mon, Nov 26, 2001 at 11:39:34PM -0500, Jim Blandy wrote:
> > > 
> > > I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
> > > sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
> > > changes to the representation of subvalues of registers and
> > > convenience variables.
> > 
> > I am exceedingly tempted to do this.
> 
> Yeah, wouldn't it be nice if VALUE_ADDRESS returned, oh, say, the
> value's address?  For register and convenience variable subvalues, use
> the value's address field instead of the offset field.  I'm pretty
> sure VALUE_OFFSET goes away entirely then.

If I get a little spare time, or adequately frustrated with vtables,
I'm going to try for this.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> On Mon, Nov 26, 2001 at 11:39:34PM -0500, Jim Blandy wrote:
> > 
> > I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
> > sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
> > changes to the representation of subvalues of registers and
> > convenience variables.
> 
> I am exceedingly tempted to do this.

Yeah, wouldn't it be nice if VALUE_ADDRESS returned, oh, say, the
value's address?  For register and convenience variable subvalues, use
the value's address field instead of the offset field.  I'm pretty
sure VALUE_OFFSET goes away entirely then.

> Gnu v2 code handles this by casting the Baz to a Foo, at which point
> magic happens, and somehow the vptr is visible.  This suggests that my
> fix is not the best way of doing it, and I should be using
> TYPE_VPTR_BASETYPE somehow instead.  I may need to think some more.

Yeah, I think that's what I was getting at; I wanted to see
TYPE_VPTR_BASETYPE in there somewhere.

> > I wonder if that dereferencing code could be simplified with a
> > judicious use of `lookup_pointer_type (vtable_type)' and
> > `value_deref'...
> 
> I suppose it would read simpler if I took a value_addr () and cast a
> bit.  But magic happens in value casting that I don't want to happen.

:(

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Mon, Nov 26, 2001 at 11:39:34PM -0500, Jim Blandy wrote:
> 
> I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
> sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
> changes to the representation of subvalues of registers and
> convenience variables.

Whew.  I'd like to reiterate my comment on VALUE_EMBEDDED_OFFSET.  See
the patch below.

> Can you explain exactly what TYPE_VPTR_FIELDNO means, and how it works
> in heavily derived classes?  What I think you're basically doing there
> is taking the address of the field indicated by TYPE_VPTR_FIELDNO,
> casting that to a void *, and then casting that to the `struct
> gdb_gnu_v3_abi_vtable' type.  I have this vague memory that maybe
> using TYPE_VPTR_FIELDNO correctly would fix that.

The important things to understand, now that I understand them:
 - TYPE_VPTR_FIELDNO is an index into TYPE_VPTR_BASETYPE.  Not into
 any of the other things we try to index with it.
 - value_cast from "pointer to struct" to "struct's base class" is not
 correct.  The comment about value_cast not finding superclasses
 correctly is also not correct.

How does this patch look?  With this patch, every virtual function call
that we think we know how to make (i.e. that we don't hit the "virtual
baseclass botch" that I'll look at with Daniel Berlin's suggestions
tomorrow) is now correct.  We don't call random offsets in the wrong
vtables any more.

I think these fixes also apply to gnu-v2-abi.c.  I'll check tomorrow. 
I'm tired :)

There are some multiple inheritance issues I'm not quite sure of that
I'd like to check, also.  They may require saving a tree of
superclasses rather than an absolute superclass, and while loops
walking/casting up the vptr_basetype chain.  I'll think about it.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-11-26  Daniel Jacobowitz  <drow@mvista.com>

	* values.c (value_primitive_field): Add embedded_offset to the
	address of structure members.
	* gnu-v3-abi.c (gnuv3_rtti_type): Cast to base type before
	attempting to access vtable pointer.  Set using_enc_p if we cast.
	(gnuv3_virtual_fn_field): Call value_cast with structure rather than
	structure pointer.  Cast to base type before attempting to access
	vtable pointer.

Index: gnu-v3-abi.c
===================================================================
RCS file: /cvs/src/src/gdb/gnu-v3-abi.c,v
retrieving revision 1.4
diff -u -p -r1.4 gnu-v3-abi.c
--- gnu-v3-abi.c	2001/10/16 01:58:07	1.4
+++ gnu-v3-abi.c	2001/11/27 06:57:48
@@ -194,6 +194,7 @@ gnuv3_rtti_type (struct value *value,
   const char *class_name;
   struct symbol *class_symbol;
   struct type *run_time_type;
+  struct type *base_type;
   LONGEST offset_to_top;
 
   /* We only have RTTI for class objects.  */
@@ -206,8 +207,18 @@ gnuv3_rtti_type (struct value *value,
   if (TYPE_VPTR_FIELDNO (value_type) == -1)
     return NULL;
 
+  if (using_enc_p)
+    *using_enc_p = 0;
+
   /* Fetch VALUE's virtual table pointer, and tweak it to point at
-     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.   */
+     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.  */
+  base_type = check_typedef (TYPE_VPTR_BASETYPE (value_type));
+  if (value_type != base_type)
+    {
+      value = value_cast (base_type, value);
+      if (using_enc_p)
+	*using_enc_p = 1;
+    }
   vtable_address
     = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (value_type)));
   vtable = value_at_lazy (vtable_type,
@@ -260,8 +271,6 @@ gnuv3_rtti_type (struct value *value,
                    >= TYPE_LENGTH (run_time_type)));
   if (top_p)
     *top_p = - offset_to_top;
-  if (using_enc_p)
-    *using_enc_p = 0;
 
   return run_time_type;
 }
@@ -303,15 +312,17 @@ gnuv3_virtual_fn_field (struct value **v
      function, cast our value to that baseclass.  This takes care of
      any necessary `this' adjustments.  */
   if (vfn_base != value_type)
-    /* It would be nicer to simply cast the value to the appropriate
-       base class (and I think that is supposed to be legal), but
-       value_cast only does the right magic when casting pointers.  */
-    value = value_ind (value_cast (vfn_base, value_addr (value)));
+    value = value_cast (vfn_base, value);
 
   /* Now value is an object of the appropriate base type.  Fetch its
      virtual table.  */
+  /* It might be possible to do this cast at the same time as the above.
+     Does multiple inheritance affect this?  */
+  if (TYPE_VPTR_BASETYPE (vfn_base) != vfn_base)
+    value = value_cast (TYPE_VPTR_BASETYPE (vfn_base), value);
   vtable_address
     = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (vfn_base)));
+
   vtable = value_at_lazy (vtable_type,
                           vtable_address - vtable_address_point_offset (),
                           VALUE_BFD_SECTION (value));
Index: values.c
===================================================================
RCS file: /cvs/src/src/gdb/values.c,v
retrieving revision 1.27
diff -u -p -r1.27 values.c
--- values.c	2001/11/09 16:41:50	1.27
+++ values.c	2001/11/27 06:57:50
@@ -929,7 +929,8 @@ value_primitive_field (register value_pt
 	memcpy (VALUE_CONTENTS_RAW (v),
 		VALUE_CONTENTS_RAW (arg1) + offset,
 		TYPE_LENGTH (type));
-      VALUE_OFFSET (v) = VALUE_OFFSET (arg1) + offset;
+      VALUE_OFFSET (v) = VALUE_OFFSET (arg1) + offset
+			 + VALUE_EMBEDDED_OFFSET (arg1);
     }
   VALUE_LVAL (v) = VALUE_LVAL (arg1);
   if (VALUE_LVAL (arg1) == lval_internalvar)

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Berlin]

On Tue, 27 Nov 2001, Daniel Jacobowitz wrote:

> On Mon, Nov 26, 2001 at 11:39:34PM -0500, Jim Blandy wrote:
> >
> > I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
> > sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
> > changes to the representation of subvalues of registers and
> > convenience variables.
>
> I am exceedingly tempted to do this.
>
> > Can you explain exactly what TYPE_VPTR_FIELDNO means, and how it works
> > in heavily derived classes?  What I think you're basically doing there
> > is taking the address of the field indicated by TYPE_VPTR_FIELDNO,
> > casting that to a void *, and then casting that to the `struct
> > gdb_gnu_v3_abi_vtable' type.  I have this vague memory that maybe
> > using TYPE_VPTR_FIELDNO correctly would fix that.
>
> I certainly can't explain it :)  This code mostly mystifies me.

It's taking advantage of g++ magic.
I didn't add it, and have wanted to rid us of it for years.
It's evil.
It's a a field that g++ uses internally to track where the
virtual function table is.
If you want to see how fragile C++ support is in gdb, change the vfield
name in gcc/cp/cp-tree.h and watch what happens.

While you are there, notice that they don't use $ to begin it because it
confuses gdb (bad), and that it actually won't match the string we test
against, depending on what the assembler supports in terms of label names
(It might be __vptr_a instead of _vptr.a).
Cute, no?

> It seems that the vptr for a given class is always a field of the class,
Yes, it has to be, g++ generates them.

However, be aware that they may not always be correct at all times during
execution.
I see comments like this in the g++ code
/* If this class uses a different vtable than its primary base
         then when we will need to initialize our vptr after the base
         class constructor runs.  */
Giving me the impression that it can be very dangerous to rely on them at
all times in gdb.
GCC knows when it's safe to use them, because it's generating the code.
We don't.


> and may actually overlap where the vptr for its first virtual base
> class.

It's going to depend on a number of factors.
>   TYPE_VPTR_FIELDNO tells us  where it is.  For example, in GCC
> 2.x, this code:
>
> class Foo
> {
>   int bar;
> public:
>   virtual int thug() { return 1; }
> };
>
> class Foo2
> {
>   int bar2;
> };
>
> class Baz : public Foo2, public Foo {
>   int baz;
> public:
>   virtual int thugs() { return 1; }
> };
>
> will cause vptr_fieldno for Baz to be 1, indicating its vptr is stored
> in memory at the beginning of field 1.
>
> Gnu v2 code handles this by casting the Baz to a Foo, at which point
> magic happens, and somehow the vptr is visible.  This suggests that my
> fix is not the best way of doing it, and I should be using
> TYPE_VPTR_BASETYPE somehow instead.  I may need to think some more.
>
> Upon further reflection, TYPE_VPTR_FIELDNO is supposed to be a field
> index in TYPE_VPTR_BASETYPE.

Yes.
>   Interesting.  I think there's something
> wrong here; more comments tomorrow.
Wouldn't surprise me a bit.
>
> > I wonder if that dereferencing code could be simplified with a
> > judicious use of `lookup_pointer_type (vtable_type)' and
> > `value_deref'...
>
> I suppose it would read simpler if I took a value_addr () and cast a
> bit.  But magic happens in value casting that I don't want to happen.

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Mon, Nov 26, 2001 at 11:39:34PM -0500, Jim Blandy wrote:
> 
> I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
> sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
> changes to the representation of subvalues of registers and
> convenience variables.

I am exceedingly tempted to do this.

> Can you explain exactly what TYPE_VPTR_FIELDNO means, and how it works
> in heavily derived classes?  What I think you're basically doing there
> is taking the address of the field indicated by TYPE_VPTR_FIELDNO,
> casting that to a void *, and then casting that to the `struct
> gdb_gnu_v3_abi_vtable' type.  I have this vague memory that maybe
> using TYPE_VPTR_FIELDNO correctly would fix that.

I certainly can't explain it :)  This code mostly mystifies me.  It
seems that the vptr for a given class is always a field of the class,
and may actually overlap where the vptr for its first virtual base
class.  TYPE_VPTR_FIELDNO tells us where it is.  For example, in GCC
2.x, this code:

class Foo
{
  int bar;
public:
  virtual int thug() { return 1; }
};

class Foo2
{
  int bar2;
};

class Baz : public Foo2, public Foo {
  int baz;
public:
  virtual int thugs() { return 1; }
};

will cause vptr_fieldno for Baz to be 1, indicating its vptr is stored
in memory at the beginning of field 1.

Gnu v2 code handles this by casting the Baz to a Foo, at which point
magic happens, and somehow the vptr is visible.  This suggests that my
fix is not the best way of doing it, and I should be using
TYPE_VPTR_BASETYPE somehow instead.  I may need to think some more.

Upon further reflection, TYPE_VPTR_FIELDNO is supposed to be a field
index in TYPE_VPTR_BASETYPE.  Interesting.  I think there's something
wrong here; more comments tomorrow.

> I wonder if that dereferencing code could be simplified with a
> judicious use of `lookup_pointer_type (vtable_type)' and
> `value_deref'...

I suppose it would read simpler if I took a value_addr () and cast a
bit.  But magic happens in value casting that I don't want to happen.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Andrew Cagney]

> I don't have any comments on Daniel's patch, but I am wondering who
> will approve this.  Given that we currently have no C++ maintainer,
> should we treat Daniel's patch as an RFC and have him commit it after
> a few days if there are no objections?

I'm pretty sure Jim Blandy, as a (most experienced?) language 
maintainer, will take care of it.

Andrew

Re: [RFA/c++] Fix printing classes with virtual base classes

[Jim Blandy]

I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
changes to the representation of subvalues of registers and
convenience variables.

Can you explain exactly what TYPE_VPTR_FIELDNO means, and how it works
in heavily derived classes?  What I think you're basically doing there
is taking the address of the field indicated by TYPE_VPTR_FIELDNO,
casting that to a void *, and then casting that to the `struct
gdb_gnu_v3_abi_vtable' type.  I have this vague memory that maybe
using TYPE_VPTR_FIELDNO correctly would fix that.

I wonder if that dereferencing code could be simplified with a
judicious use of `lookup_pointer_type (vtable_type)' and
`value_deref'...

Re: [RFA/c++] Fix printing classes with virtual base classes

[Kevin Buettner]

On Nov 26,  9:02pm, Daniel Jacobowitz wrote:

> On Mon, Nov 26, 2001 at 08:19:45PM -0500, Daniel Jacobowitz wrote:
> > Whew.  Finally tracked this down.
> 
> [snip correct explanation]
> 
> [snip incorrect patch]
> 
> I find the use of VALUE_OFFSET and VALUE_EMBEDDED_OFFSET exceedingly
> unintuitive.  Go figure.  This one works a little bit better yet.
> 
> -- 
> Daniel Jacobowitz                           Carnegie Mellon University
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2001-11-26  Daniel Jacobowitz  <drow@mvista.com>
> 
>         * gnu-v3-abi.c (gnuv3_rtti_type): Explicitly cast
>         the vtable pointer to a pointer before loading it.
>         (gnuv3_virtual_fn_field): Likewise.

I don't have any comments on Daniel's patch, but I am wondering who
will approve this.  Given that we currently have no C++ maintainer,
should we treat Daniel's patch as an RFC and have him commit it after
a few days if there are no objections?

Kevin

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Mon, Nov 26, 2001 at 08:19:45PM -0500, Daniel Jacobowitz wrote:
> Whew.  Finally tracked this down.

[snip correct explanation]

[snip incorrect patch]

I find the use of VALUE_OFFSET and VALUE_EMBEDDED_OFFSET exceedingly
unintuitive.  Go figure.  This one works a little bit better yet.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-11-26  Daniel Jacobowitz  <drow@mvista.com>

        * gnu-v3-abi.c (gnuv3_rtti_type): Explicitly cast
        the vtable pointer to a pointer before loading it.
        (gnuv3_virtual_fn_field): Likewise.

Index: gnu-v3-abi.c
===================================================================
RCS file: /cvs/src/src/gdb/gnu-v3-abi.c,v
retrieving revision 1.4
diff -u -p -r1.4 gnu-v3-abi.c
--- gnu-v3-abi.c	2001/10/16 01:58:07	1.4
+++ gnu-v3-abi.c	2001/11/27 02:00:22
@@ -188,7 +188,7 @@ gnuv3_rtti_type (struct value *value,
   struct type *vtable_type = gdbarch_data (vtable_type_gdbarch_data);
   struct type *value_type = check_typedef (VALUE_TYPE (value));
   CORE_ADDR vtable_address;
-  struct value *vtable;
+  struct value *vtable, *vtable_addr_value, *vtable_field;
   struct minimal_symbol *vtable_symbol;
   const char *vtable_symbol_name;
   const char *class_name;
@@ -207,9 +207,20 @@ gnuv3_rtti_type (struct value *value,
     return NULL;
 
   /* Fetch VALUE's virtual table pointer, and tweak it to point at
-     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.   */
+     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.
+     The type of the field may not be a pointer type.  In a derived
+     class with a virtual base class, it will be the type of the base
+     class.  Thus we need to cast.  */
+  vtable_field = value_field (value,
+			      TYPE_VPTR_FIELDNO (value_type));
+  vtable_addr_value =
+    value_at (builtin_type_void_data_ptr,
+	      VALUE_ADDRESS (vtable_field)
+	      + VALUE_OFFSET (vtable_field)
+	      + VALUE_EMBEDDED_OFFSET (vtable_field),
+	      VALUE_BFD_SECTION (value));
   vtable_address
-    = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (value_type)));
+    = value_as_address (vtable_addr_value);
   vtable = value_at_lazy (vtable_type,
                           vtable_address - vtable_address_point_offset (),
                           VALUE_BFD_SECTION (value));
@@ -277,7 +288,7 @@ gnuv3_virtual_fn_field (struct value **v
   struct type *value_type = check_typedef (VALUE_TYPE (value));
   struct type *vfn_base;
   CORE_ADDR vtable_address;
-  struct value *vtable;
+  struct value *vtable, *vtable_addr_value, *vtable_field;
   struct value *vfn;
 
   /* Some simple sanity checks.  */
@@ -309,9 +320,19 @@ gnuv3_virtual_fn_field (struct value **v
     value = value_ind (value_cast (vfn_base, value_addr (value)));
 
   /* Now value is an object of the appropriate base type.  Fetch its
-     virtual table.  */
+     virtual table.  As in gnuv3_rtti_type, the type of the field may
+     not be a pointer type.  In a derived class with a virtual base class,
+     it will be the type of the base class.  Thus we need to cast.  */
+  vtable_field = value_field (value,
+			      TYPE_VPTR_FIELDNO (vfn_base));
+  vtable_addr_value =
+    value_at (builtin_type_void_data_ptr,
+	      VALUE_ADDRESS (vtable_field)
+	      + VALUE_OFFSET (vtable_field)
+	      + VALUE_EMBEDDED_OFFSET (vtable_field),
+	      VALUE_BFD_SECTION (value));
   vtable_address
-    = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (vfn_base)));
+    = value_as_address (vtable_addr_value);
   vtable = value_at_lazy (vtable_type,
                           vtable_address - vtable_address_point_offset (),
                           VALUE_BFD_SECTION (value));

[RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

Whew.  Finally tracked this down.

The problem is in the layout of a class whose base class has a virtual
table.  Given:

class Bar {
  virtual int thunk ();
};

class Foo : public Bar {
  int test;
};

A Foo will look like this:

   /-----------------------\
   | Foo       |    test   |
   \-----------------------/

Its own vptr will be conveniently Foo's vptr.  But the type of Bar's field 0
(which is marked as having the vptr in it) is Foo, not some pointer type. 
If I explicitly cast to pointer, everything works more-or-less OK.

I'm pretty sure of the C++ side of this.  If there's a more appropriate
function for doing the cast to pointer I'd love to hear about it; otherwise,
OK to commit?

Can't really tell what effect it has on the testsuite, as just about
everything else in the relevant tests is failing anyway.  I have a few more
low-hanging bugs in this area that I'm debugging now.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-11-26  Daniel Jacobowitz  <drow@mvista.com>

	* gnu-v3-abi.c (gnuv3_rtti_type): Explicitly cast
	the vtable pointer to a pointer before loading it.
	(gnuv3_virtual_fn_field): Likewise.

Index: gnu-v3-abi.c
===================================================================
RCS file: /cvs/src/src/gdb/gnu-v3-abi.c,v
retrieving revision 1.4
diff -u -p -r1.4 gnu-v3-abi.c
--- gnu-v3-abi.c	2001/10/16 01:58:07	1.4
+++ gnu-v3-abi.c	2001/11/27 00:21:44
@@ -188,7 +188,7 @@ gnuv3_rtti_type (struct value *value,
   struct type *vtable_type = gdbarch_data (vtable_type_gdbarch_data);
   struct type *value_type = check_typedef (VALUE_TYPE (value));
   CORE_ADDR vtable_address;
-  struct value *vtable;
+  struct value *vtable, *vtable_addr_value;
   struct minimal_symbol *vtable_symbol;
   const char *vtable_symbol_name;
   const char *class_name;
@@ -207,9 +207,17 @@ gnuv3_rtti_type (struct value *value,
     return NULL;
 
   /* Fetch VALUE's virtual table pointer, and tweak it to point at
-     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.   */
+     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.
+     The type of the field may not be a pointer type.  In a derived
+     class with a virtual base class, it will be the type of the base
+     class.  Thus we need to cast.  */
+  vtable_addr_value =
+    value_at (builtin_type_void_data_ptr,
+	      VALUE_ADDRESS (value_field (value,
+					  TYPE_VPTR_FIELDNO (value_type))),
+	      VALUE_BFD_SECTION (value));
   vtable_address
-    = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (value_type)));
+    = value_as_address (vtable_addr_value);
   vtable = value_at_lazy (vtable_type,
                           vtable_address - vtable_address_point_offset (),
                           VALUE_BFD_SECTION (value));
@@ -277,7 +285,7 @@ gnuv3_virtual_fn_field (struct value **v
   struct type *value_type = check_typedef (VALUE_TYPE (value));
   struct type *vfn_base;
   CORE_ADDR vtable_address;
-  struct value *vtable;
+  struct value *vtable, *vtable_addr_value;
   struct value *vfn;
 
   /* Some simple sanity checks.  */
@@ -309,9 +317,16 @@ gnuv3_virtual_fn_field (struct value **v
     value = value_ind (value_cast (vfn_base, value_addr (value)));
 
   /* Now value is an object of the appropriate base type.  Fetch its
-     virtual table.  */
+     virtual table.  As in gnuv3_rtti_type, the type of the field may
+     not be a pointer type.  In a derived class with a virtual base class,
+     it will be the type of the base class.  Thus we need to cast.  */
+  vtable_addr_value =
+    value_at (builtin_type_void_data_ptr,
+	      VALUE_ADDRESS (value_field (value,
+					  TYPE_VPTR_FIELDNO (vfn_base))),
+	      VALUE_BFD_SECTION (value));
   vtable_address
-    = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (vfn_base)));
+    = value_as_address (vtable_addr_value);
   vtable = value_at_lazy (vtable_type,
                           vtable_address - vtable_address_point_offset (),
                           VALUE_BFD_SECTION (value));

Re: [RFA/c++] Fix printing classes with virtual base classes

[Jim Blandy]

I typo'd in CC'ing the message below to gdb-patches.  Daniel had my
approval to commit his change to gnu-v3-abi.c.

To: Daniel Jacobowitz <drow@mvista.com>
Cc: gdb-patches@sources.redhat.comb
Subject: Re: [RFA/c++] Fix printing classes with virtual base classes
References: <20011126201945.A27754@nevyn.them.org>
	<np667wket5.fsf@zwingli.cygnus.com>
	<20011127020634.A10010@nevyn.them.org>
	<npd723j4lc.fsf@zwingli.cygnus.com>
	<20011130014034.A29999@nevyn.them.org>
Bcc: jimb
From: Jim Blandy <jimb@zwingli.cygnus.com>
Date: 30 Nov 2001 12:12:51 -0500
In-Reply-To: Daniel Jacobowitz's message of Fri, 30 Nov 2001 01:40:34 -0500
Message-ID: <np4rncdvxo.fsf@zwingli.cygnus.com>
Lines: 23
X-Mailer: Gnus v5.3/Emacs 19.34
--text follows this line--

Daniel Jacobowitz <drow@mvista.com> writes:
> Jim, can I commit these?  It'll make it easier for me to post the
> following batch.  Now that GCC emits the information I need (on HEAD at
> least) I'd like to finish this up.

Yes, go ahead.

> I guess that I can commit the gnu-v3-abi bits on my own initiative,
> since no one objected... actually, I guess the values stuff is
> unmaintained too?

Well, officially, yes, but my sense is that values.c is a high-traffic
area, so people would squonk more if you just commit stuff there.
I'll let Andrew tell you the Official Position. 

> MAINTAINERS says:
>   If there is no maintainer for a given domain then the responsibility
>   falls to the head maintainer.
> So I guess I need approval from one of Ye Divine Entities first.

... I'm clearly suffering from a divinity deficiency.  But I'll
approve your changes.

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Fri, Nov 30, 2001 at 11:00:26AM -0600, Michael Elizabeth Chastain wrote:
> Hi Daniel,
> 
> Just to make sure, you are talking about this patch, right?
> 
>   http://sources.redhat.com/ml/gdb-patches/2001-11/msg00487.html
> 
> I will give it some intensive testing tonight and tomorrow morning.

That's the one.  I just committed it; more to follow.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Fri, Nov 30, 2001 at 11:48:49AM -0500, Andrew Cagney wrote:
> >Jim, can I commit these?  It'll make it easier for me to post the
> >following batch.  Now that GCC emits the information I need (on HEAD at
> >least) I'd like to finish this up.
> >
> >I guess that I can commit the gnu-v3-abi bits on my own initiative,
> >since no one objected... actually, I guess the values stuff is
> >unmaintained too?
> 
> I wouldn't describe it as un-maintained.  It is fundamental core code, 
> JimB and I would both be keeping a very keen eye on it.

Thus confusion :)  MAINTAINERS lists a past maintainer for it and no
current maintainer... so I assumed it was lacking.

> >MAINTAINERS says:
> >  If there is no maintainer for a given domain then the responsibility
> >  falls to the head maintainer.
> >So I guess I need approval from one of Ye Divine Entities first.
> 
> Yes.
> 
> >[Would someone more familiar with the state of affairs than I
> >explicitly list the unmaintained parts in MAINTAINERS?  Quite a few
> >things seem to have slipped down that path.]
> 
> Nothing is is really unmaintained.  The buck (unfortunatly :-) stops 
> here.  Did you have any comments on my recent proposal to change how 
> targets (and natives) can get a change approved?

I don't remember seeing it.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Michael Elizabeth Chastain]

Hi Daniel,

Just to make sure, you are talking about this patch, right?

  http://sources.redhat.com/ml/gdb-patches/2001-11/msg00487.html

I will give it some intensive testing tonight and tomorrow morning.

Michael C

Re: [RFA/c++] Fix printing classes with virtual base classes

[Andrew Cagney]

> Jim, can I commit these?  It'll make it easier for me to post the
> following batch.  Now that GCC emits the information I need (on HEAD at
> least) I'd like to finish this up.
> 
> I guess that I can commit the gnu-v3-abi bits on my own initiative,
> since no one objected... actually, I guess the values stuff is
> unmaintained too?

I wouldn't describe it as un-maintained.  It is fundamental core code, 
JimB and I would both be keeping a very keen eye on it.

> MAINTAINERS says:
>   If there is no maintainer for a given domain then the responsibility
>   falls to the head maintainer.
> So I guess I need approval from one of Ye Divine Entities first.

Yes.

> [Would someone more familiar with the state of affairs than I
> explicitly list the unmaintained parts in MAINTAINERS?  Quite a few
> things seem to have slipped down that path.]

Nothing is is really unmaintained.  The buck (unfortunatly :-) stops 
here.  Did you have any comments on my recent proposal to change how 
targets (and natives) can get a change approved?

Andrew

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

Jim, can I commit these?  It'll make it easier for me to post the
following batch.  Now that GCC emits the information I need (on HEAD at
least) I'd like to finish this up.

I guess that I can commit the gnu-v3-abi bits on my own initiative,
since no one objected... actually, I guess the values stuff is
unmaintained too?

MAINTAINERS says:
  If there is no maintainer for a given domain then the responsibility
  falls to the head maintainer.
So I guess I need approval from one of Ye Divine Entities first.

[Would someone more familiar with the state of affairs than I
explicitly list the unmaintained parts in MAINTAINERS?  Quite a few
things seem to have slipped down that path.]


On Tue, Nov 27, 2001 at 04:17:51PM -0500, Jim Blandy wrote:
> 
> These are two independent fixes, right?  I understand GDB may need
> them both before it works correctly; I'm asking if each of them is a
> correct change in its own right.  If so, could you show me a test case
> that each change fixes?
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > 2001-11-26  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* values.c (value_primitive_field): Add embedded_offset to the
> > 	address of structure members.
> > 	* gnu-v3-abi.c (gnuv3_rtti_type): Cast to base type before
> > 	attempting to access vtable pointer.  Set using_enc_p if we cast.
> > 	(gnuv3_virtual_fn_field): Call value_cast with structure rather than
> > 	structure pointer.  Cast to base type before attempting to access
> > 	vtable pointer.
> 

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

They are both independent fixes.  The fixes to gnu-v3-abi.c by
themselves fix several tests in virtfunc.exp when using G++ 3.0
(from XFAIL to XPASS, ugh...).  Then if you look at the test logs, you
should find two failure modes for the ones that still fail.  Some are
marked "virtual baseclass botch", and I am trying right now to fix
those; but I believe that one will just be wrong.  That's because my
use of value_field in gnuv3_virtual_fn_field triggers the bug; the
value has not been read in yet, so only its address is copied, and the
embedded offset for the parent type is lost.  We then get the wrong
vtable and call the wrong function.

I can't construct an independent testcase, but I think it should be
possible.  Most things run afoul of the other "botch" bug.

On Tue, Nov 27, 2001 at 04:17:51PM -0500, Jim Blandy wrote:
> 
> These are two independent fixes, right?  I understand GDB may need
> them both before it works correctly; I'm asking if each of them is a
> correct change in its own right.  If so, could you show me a test case
> that each change fixes?
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > 2001-11-26  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* values.c (value_primitive_field): Add embedded_offset to the
> > 	address of structure members.
> > 	* gnu-v3-abi.c (gnuv3_rtti_type): Cast to base type before
> > 	attempting to access vtable pointer.  Set using_enc_p if we cast.
> > 	(gnuv3_virtual_fn_field): Call value_cast with structure rather than
> > 	structure pointer.  Cast to base type before attempting to access
> > 	vtable pointer.
> 

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Jim Blandy]

These are two independent fixes, right?  I understand GDB may need
them both before it works correctly; I'm asking if each of them is a
correct change in its own right.  If so, could you show me a test case
that each change fixes?

Daniel Jacobowitz <drow@mvista.com> writes:
> 2001-11-26  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* values.c (value_primitive_field): Add embedded_offset to the
> 	address of structure members.
> 	* gnu-v3-abi.c (gnuv3_rtti_type): Cast to base type before
> 	attempting to access vtable pointer.  Set using_enc_p if we cast.
> 	(gnuv3_virtual_fn_field): Call value_cast with structure rather than
> 	structure pointer.  Cast to base type before attempting to access
> 	vtable pointer.

Re: [RFA/c++] Fix printing classes with virtual base classes

[Jim Blandy]

Andrew Cagney <ac131313@cygnus.com> writes:
> > I don't have any comments on Daniel's patch, but I am wondering who
> > will approve this.  Given that we currently have no C++ maintainer,
> > should we treat Daniel's patch as an RFC and have him commit it after
> > a few days if there are no objections?
> 
> I'm pretty sure Jim Blandy, as a (most experienced?) language 
> maintainer, will take care of it.

I'm not really qualified to be a C++ maintainer, but I'd be happy to
lend Daniel a second pair of eyes for checking his patches.

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Tue, Nov 27, 2001 at 10:16:56AM -0500, Jim Blandy wrote:
> Daniel Jacobowitz <drow@mvista.com> writes:
> > On Mon, Nov 26, 2001 at 11:39:34PM -0500, Jim Blandy wrote:
> > > 
> > > I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
> > > sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
> > > changes to the representation of subvalues of registers and
> > > convenience variables.
> > 
> > I am exceedingly tempted to do this.
> 
> Yeah, wouldn't it be nice if VALUE_ADDRESS returned, oh, say, the
> value's address?  For register and convenience variable subvalues, use
> the value's address field instead of the offset field.  I'm pretty
> sure VALUE_OFFSET goes away entirely then.

If I get a little spare time, or adequately frustrated with vtables,
I'm going to try for this.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> On Mon, Nov 26, 2001 at 11:39:34PM -0500, Jim Blandy wrote:
> > 
> > I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
> > sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
> > changes to the representation of subvalues of registers and
> > convenience variables.
> 
> I am exceedingly tempted to do this.

Yeah, wouldn't it be nice if VALUE_ADDRESS returned, oh, say, the
value's address?  For register and convenience variable subvalues, use
the value's address field instead of the offset field.  I'm pretty
sure VALUE_OFFSET goes away entirely then.

> Gnu v2 code handles this by casting the Baz to a Foo, at which point
> magic happens, and somehow the vptr is visible.  This suggests that my
> fix is not the best way of doing it, and I should be using
> TYPE_VPTR_BASETYPE somehow instead.  I may need to think some more.

Yeah, I think that's what I was getting at; I wanted to see
TYPE_VPTR_BASETYPE in there somewhere.

> > I wonder if that dereferencing code could be simplified with a
> > judicious use of `lookup_pointer_type (vtable_type)' and
> > `value_deref'...
> 
> I suppose it would read simpler if I took a value_addr () and cast a
> bit.  But magic happens in value casting that I don't want to happen.

:(

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Mon, Nov 26, 2001 at 11:39:34PM -0500, Jim Blandy wrote:
> 
> I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
> sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
> changes to the representation of subvalues of registers and
> convenience variables.

Whew.  I'd like to reiterate my comment on VALUE_EMBEDDED_OFFSET.  See
the patch below.

> Can you explain exactly what TYPE_VPTR_FIELDNO means, and how it works
> in heavily derived classes?  What I think you're basically doing there
> is taking the address of the field indicated by TYPE_VPTR_FIELDNO,
> casting that to a void *, and then casting that to the `struct
> gdb_gnu_v3_abi_vtable' type.  I have this vague memory that maybe
> using TYPE_VPTR_FIELDNO correctly would fix that.

The important things to understand, now that I understand them:
 - TYPE_VPTR_FIELDNO is an index into TYPE_VPTR_BASETYPE.  Not into
 any of the other things we try to index with it.
 - value_cast from "pointer to struct" to "struct's base class" is not
 correct.  The comment about value_cast not finding superclasses
 correctly is also not correct.

How does this patch look?  With this patch, every virtual function call
that we think we know how to make (i.e. that we don't hit the "virtual
baseclass botch" that I'll look at with Daniel Berlin's suggestions
tomorrow) is now correct.  We don't call random offsets in the wrong
vtables any more.

I think these fixes also apply to gnu-v2-abi.c.  I'll check tomorrow. 
I'm tired :)

There are some multiple inheritance issues I'm not quite sure of that
I'd like to check, also.  They may require saving a tree of
superclasses rather than an absolute superclass, and while loops
walking/casting up the vptr_basetype chain.  I'll think about it.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-11-26  Daniel Jacobowitz  <drow@mvista.com>

	* values.c (value_primitive_field): Add embedded_offset to the
	address of structure members.
	* gnu-v3-abi.c (gnuv3_rtti_type): Cast to base type before
	attempting to access vtable pointer.  Set using_enc_p if we cast.
	(gnuv3_virtual_fn_field): Call value_cast with structure rather than
	structure pointer.  Cast to base type before attempting to access
	vtable pointer.

Index: gnu-v3-abi.c
===================================================================
RCS file: /cvs/src/src/gdb/gnu-v3-abi.c,v
retrieving revision 1.4
diff -u -p -r1.4 gnu-v3-abi.c
--- gnu-v3-abi.c	2001/10/16 01:58:07	1.4
+++ gnu-v3-abi.c	2001/11/27 06:57:48
@@ -194,6 +194,7 @@ gnuv3_rtti_type (struct value *value,
   const char *class_name;
   struct symbol *class_symbol;
   struct type *run_time_type;
+  struct type *base_type;
   LONGEST offset_to_top;
 
   /* We only have RTTI for class objects.  */
@@ -206,8 +207,18 @@ gnuv3_rtti_type (struct value *value,
   if (TYPE_VPTR_FIELDNO (value_type) == -1)
     return NULL;
 
+  if (using_enc_p)
+    *using_enc_p = 0;
+
   /* Fetch VALUE's virtual table pointer, and tweak it to point at
-     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.   */
+     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.  */
+  base_type = check_typedef (TYPE_VPTR_BASETYPE (value_type));
+  if (value_type != base_type)
+    {
+      value = value_cast (base_type, value);
+      if (using_enc_p)
+	*using_enc_p = 1;
+    }
   vtable_address
     = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (value_type)));
   vtable = value_at_lazy (vtable_type,
@@ -260,8 +271,6 @@ gnuv3_rtti_type (struct value *value,
                    >= TYPE_LENGTH (run_time_type)));
   if (top_p)
     *top_p = - offset_to_top;
-  if (using_enc_p)
-    *using_enc_p = 0;
 
   return run_time_type;
 }
@@ -303,15 +312,17 @@ gnuv3_virtual_fn_field (struct value **v
      function, cast our value to that baseclass.  This takes care of
      any necessary `this' adjustments.  */
   if (vfn_base != value_type)
-    /* It would be nicer to simply cast the value to the appropriate
-       base class (and I think that is supposed to be legal), but
-       value_cast only does the right magic when casting pointers.  */
-    value = value_ind (value_cast (vfn_base, value_addr (value)));
+    value = value_cast (vfn_base, value);
 
   /* Now value is an object of the appropriate base type.  Fetch its
      virtual table.  */
+  /* It might be possible to do this cast at the same time as the above.
+     Does multiple inheritance affect this?  */
+  if (TYPE_VPTR_BASETYPE (vfn_base) != vfn_base)
+    value = value_cast (TYPE_VPTR_BASETYPE (vfn_base), value);
   vtable_address
     = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (vfn_base)));
+
   vtable = value_at_lazy (vtable_type,
                           vtable_address - vtable_address_point_offset (),
                           VALUE_BFD_SECTION (value));
Index: values.c
===================================================================
RCS file: /cvs/src/src/gdb/values.c,v
retrieving revision 1.27
diff -u -p -r1.27 values.c
--- values.c	2001/11/09 16:41:50	1.27
+++ values.c	2001/11/27 06:57:50
@@ -929,7 +929,8 @@ value_primitive_field (register value_pt
 	memcpy (VALUE_CONTENTS_RAW (v),
 		VALUE_CONTENTS_RAW (arg1) + offset,
 		TYPE_LENGTH (type));
-      VALUE_OFFSET (v) = VALUE_OFFSET (arg1) + offset;
+      VALUE_OFFSET (v) = VALUE_OFFSET (arg1) + offset
+			 + VALUE_EMBEDDED_OFFSET (arg1);
     }
   VALUE_LVAL (v) = VALUE_LVAL (arg1);
   if (VALUE_LVAL (arg1) == lval_internalvar)

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Berlin]

On Tue, 27 Nov 2001, Daniel Jacobowitz wrote:

> On Mon, Nov 26, 2001 at 11:39:34PM -0500, Jim Blandy wrote:
> >
> > I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
> > sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
> > changes to the representation of subvalues of registers and
> > convenience variables.
>
> I am exceedingly tempted to do this.
>
> > Can you explain exactly what TYPE_VPTR_FIELDNO means, and how it works
> > in heavily derived classes?  What I think you're basically doing there
> > is taking the address of the field indicated by TYPE_VPTR_FIELDNO,
> > casting that to a void *, and then casting that to the `struct
> > gdb_gnu_v3_abi_vtable' type.  I have this vague memory that maybe
> > using TYPE_VPTR_FIELDNO correctly would fix that.
>
> I certainly can't explain it :)  This code mostly mystifies me.

It's taking advantage of g++ magic.
I didn't add it, and have wanted to rid us of it for years.
It's evil.
It's a a field that g++ uses internally to track where the
virtual function table is.
If you want to see how fragile C++ support is in gdb, change the vfield
name in gcc/cp/cp-tree.h and watch what happens.

While you are there, notice that they don't use $ to begin it because it
confuses gdb (bad), and that it actually won't match the string we test
against, depending on what the assembler supports in terms of label names
(It might be __vptr_a instead of _vptr.a).
Cute, no?

> It seems that the vptr for a given class is always a field of the class,
Yes, it has to be, g++ generates them.

However, be aware that they may not always be correct at all times during
execution.
I see comments like this in the g++ code
/* If this class uses a different vtable than its primary base
         then when we will need to initialize our vptr after the base
         class constructor runs.  */
Giving me the impression that it can be very dangerous to rely on them at
all times in gdb.
GCC knows when it's safe to use them, because it's generating the code.
We don't.


> and may actually overlap where the vptr for its first virtual base
> class.

It's going to depend on a number of factors.
>   TYPE_VPTR_FIELDNO tells us  where it is.  For example, in GCC
> 2.x, this code:
>
> class Foo
> {
>   int bar;
> public:
>   virtual int thug() { return 1; }
> };
>
> class Foo2
> {
>   int bar2;
> };
>
> class Baz : public Foo2, public Foo {
>   int baz;
> public:
>   virtual int thugs() { return 1; }
> };
>
> will cause vptr_fieldno for Baz to be 1, indicating its vptr is stored
> in memory at the beginning of field 1.
>
> Gnu v2 code handles this by casting the Baz to a Foo, at which point
> magic happens, and somehow the vptr is visible.  This suggests that my
> fix is not the best way of doing it, and I should be using
> TYPE_VPTR_BASETYPE somehow instead.  I may need to think some more.
>
> Upon further reflection, TYPE_VPTR_FIELDNO is supposed to be a field
> index in TYPE_VPTR_BASETYPE.

Yes.
>   Interesting.  I think there's something
> wrong here; more comments tomorrow.
Wouldn't surprise me a bit.
>
> > I wonder if that dereferencing code could be simplified with a
> > judicious use of `lookup_pointer_type (vtable_type)' and
> > `value_deref'...
>
> I suppose it would read simpler if I took a value_addr () and cast a
> bit.  But magic happens in value casting that I don't want to happen.

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Mon, Nov 26, 2001 at 11:39:34PM -0500, Jim Blandy wrote:
> 
> I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
> sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
> changes to the representation of subvalues of registers and
> convenience variables.

I am exceedingly tempted to do this.

> Can you explain exactly what TYPE_VPTR_FIELDNO means, and how it works
> in heavily derived classes?  What I think you're basically doing there
> is taking the address of the field indicated by TYPE_VPTR_FIELDNO,
> casting that to a void *, and then casting that to the `struct
> gdb_gnu_v3_abi_vtable' type.  I have this vague memory that maybe
> using TYPE_VPTR_FIELDNO correctly would fix that.

I certainly can't explain it :)  This code mostly mystifies me.  It
seems that the vptr for a given class is always a field of the class,
and may actually overlap where the vptr for its first virtual base
class.  TYPE_VPTR_FIELDNO tells us where it is.  For example, in GCC
2.x, this code:

class Foo
{
  int bar;
public:
  virtual int thug() { return 1; }
};

class Foo2
{
  int bar2;
};

class Baz : public Foo2, public Foo {
  int baz;
public:
  virtual int thugs() { return 1; }
};

will cause vptr_fieldno for Baz to be 1, indicating its vptr is stored
in memory at the beginning of field 1.

Gnu v2 code handles this by casting the Baz to a Foo, at which point
magic happens, and somehow the vptr is visible.  This suggests that my
fix is not the best way of doing it, and I should be using
TYPE_VPTR_BASETYPE somehow instead.  I may need to think some more.

Upon further reflection, TYPE_VPTR_FIELDNO is supposed to be a field
index in TYPE_VPTR_BASETYPE.  Interesting.  I think there's something
wrong here; more comments tomorrow.

> I wonder if that dereferencing code could be simplified with a
> judicious use of `lookup_pointer_type (vtable_type)' and
> `value_deref'...

I suppose it would read simpler if I took a value_addr () and cast a
bit.  But magic happens in value casting that I don't want to happen.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/c++] Fix printing classes with virtual base classes

[Andrew Cagney]

> I don't have any comments on Daniel's patch, but I am wondering who
> will approve this.  Given that we currently have no C++ maintainer,
> should we treat Daniel's patch as an RFC and have him commit it after
> a few days if there are no objections?

I'm pretty sure Jim Blandy, as a (most experienced?) language 
maintainer, will take care of it.

Andrew

Re: [RFA/c++] Fix printing classes with virtual base classes

[Jim Blandy]

I'm with you on VALUE_OFFSET and VALUE_EMBEDDED_OFFSET.  I'm pretty
sure VALUE_OFFSET can be eliminated from GDB entirely, with some minor
changes to the representation of subvalues of registers and
convenience variables.

Can you explain exactly what TYPE_VPTR_FIELDNO means, and how it works
in heavily derived classes?  What I think you're basically doing there
is taking the address of the field indicated by TYPE_VPTR_FIELDNO,
casting that to a void *, and then casting that to the `struct
gdb_gnu_v3_abi_vtable' type.  I have this vague memory that maybe
using TYPE_VPTR_FIELDNO correctly would fix that.

I wonder if that dereferencing code could be simplified with a
judicious use of `lookup_pointer_type (vtable_type)' and
`value_deref'...

Re: [RFA/c++] Fix printing classes with virtual base classes

[Kevin Buettner]

On Nov 26,  9:02pm, Daniel Jacobowitz wrote:

> On Mon, Nov 26, 2001 at 08:19:45PM -0500, Daniel Jacobowitz wrote:
> > Whew.  Finally tracked this down.
> 
> [snip correct explanation]
> 
> [snip incorrect patch]
> 
> I find the use of VALUE_OFFSET and VALUE_EMBEDDED_OFFSET exceedingly
> unintuitive.  Go figure.  This one works a little bit better yet.
> 
> -- 
> Daniel Jacobowitz                           Carnegie Mellon University
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2001-11-26  Daniel Jacobowitz  <drow@mvista.com>
> 
>         * gnu-v3-abi.c (gnuv3_rtti_type): Explicitly cast
>         the vtable pointer to a pointer before loading it.
>         (gnuv3_virtual_fn_field): Likewise.

I don't have any comments on Daniel's patch, but I am wondering who
will approve this.  Given that we currently have no C++ maintainer,
should we treat Daniel's patch as an RFC and have him commit it after
a few days if there are no objections?

Kevin

[RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

Whew.  Finally tracked this down.

The problem is in the layout of a class whose base class has a virtual
table.  Given:

class Bar {
  virtual int thunk ();
};

class Foo : public Bar {
  int test;
};

A Foo will look like this:

   /-----------------------\
   | Foo       |    test   |
   \-----------------------/

Its own vptr will be conveniently Foo's vptr.  But the type of Bar's field 0
(which is marked as having the vptr in it) is Foo, not some pointer type. 
If I explicitly cast to pointer, everything works more-or-less OK.

I'm pretty sure of the C++ side of this.  If there's a more appropriate
function for doing the cast to pointer I'd love to hear about it; otherwise,
OK to commit?

Can't really tell what effect it has on the testsuite, as just about
everything else in the relevant tests is failing anyway.  I have a few more
low-hanging bugs in this area that I'm debugging now.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-11-26  Daniel Jacobowitz  <drow@mvista.com>

	* gnu-v3-abi.c (gnuv3_rtti_type): Explicitly cast
	the vtable pointer to a pointer before loading it.
	(gnuv3_virtual_fn_field): Likewise.

Index: gnu-v3-abi.c
===================================================================
RCS file: /cvs/src/src/gdb/gnu-v3-abi.c,v
retrieving revision 1.4
diff -u -p -r1.4 gnu-v3-abi.c
--- gnu-v3-abi.c	2001/10/16 01:58:07	1.4
+++ gnu-v3-abi.c	2001/11/27 00:21:44
@@ -188,7 +188,7 @@ gnuv3_rtti_type (struct value *value,
   struct type *vtable_type = gdbarch_data (vtable_type_gdbarch_data);
   struct type *value_type = check_typedef (VALUE_TYPE (value));
   CORE_ADDR vtable_address;
-  struct value *vtable;
+  struct value *vtable, *vtable_addr_value;
   struct minimal_symbol *vtable_symbol;
   const char *vtable_symbol_name;
   const char *class_name;
@@ -207,9 +207,17 @@ gnuv3_rtti_type (struct value *value,
     return NULL;
 
   /* Fetch VALUE's virtual table pointer, and tweak it to point at
-     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.   */
+     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.
+     The type of the field may not be a pointer type.  In a derived
+     class with a virtual base class, it will be the type of the base
+     class.  Thus we need to cast.  */
+  vtable_addr_value =
+    value_at (builtin_type_void_data_ptr,
+	      VALUE_ADDRESS (value_field (value,
+					  TYPE_VPTR_FIELDNO (value_type))),
+	      VALUE_BFD_SECTION (value));
   vtable_address
-    = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (value_type)));
+    = value_as_address (vtable_addr_value);
   vtable = value_at_lazy (vtable_type,
                           vtable_address - vtable_address_point_offset (),
                           VALUE_BFD_SECTION (value));
@@ -277,7 +285,7 @@ gnuv3_virtual_fn_field (struct value **v
   struct type *value_type = check_typedef (VALUE_TYPE (value));
   struct type *vfn_base;
   CORE_ADDR vtable_address;
-  struct value *vtable;
+  struct value *vtable, *vtable_addr_value;
   struct value *vfn;
 
   /* Some simple sanity checks.  */
@@ -309,9 +317,16 @@ gnuv3_virtual_fn_field (struct value **v
     value = value_ind (value_cast (vfn_base, value_addr (value)));
 
   /* Now value is an object of the appropriate base type.  Fetch its
-     virtual table.  */
+     virtual table.  As in gnuv3_rtti_type, the type of the field may
+     not be a pointer type.  In a derived class with a virtual base class,
+     it will be the type of the base class.  Thus we need to cast.  */
+  vtable_addr_value =
+    value_at (builtin_type_void_data_ptr,
+	      VALUE_ADDRESS (value_field (value,
+					  TYPE_VPTR_FIELDNO (vfn_base))),
+	      VALUE_BFD_SECTION (value));
   vtable_address
-    = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (vfn_base)));
+    = value_as_address (vtable_addr_value);
   vtable = value_at_lazy (vtable_type,
                           vtable_address - vtable_address_point_offset (),
                           VALUE_BFD_SECTION (value));

Re: [RFA/c++] Fix printing classes with virtual base classes

[Daniel Jacobowitz]

On Mon, Nov 26, 2001 at 08:19:45PM -0500, Daniel Jacobowitz wrote:
> Whew.  Finally tracked this down.

[snip correct explanation]

[snip incorrect patch]

I find the use of VALUE_OFFSET and VALUE_EMBEDDED_OFFSET exceedingly
unintuitive.  Go figure.  This one works a little bit better yet.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-11-26  Daniel Jacobowitz  <drow@mvista.com>

        * gnu-v3-abi.c (gnuv3_rtti_type): Explicitly cast
        the vtable pointer to a pointer before loading it.
        (gnuv3_virtual_fn_field): Likewise.

Index: gnu-v3-abi.c
===================================================================
RCS file: /cvs/src/src/gdb/gnu-v3-abi.c,v
retrieving revision 1.4
diff -u -p -r1.4 gnu-v3-abi.c
--- gnu-v3-abi.c	2001/10/16 01:58:07	1.4
+++ gnu-v3-abi.c	2001/11/27 02:00:22
@@ -188,7 +188,7 @@ gnuv3_rtti_type (struct value *value,
   struct type *vtable_type = gdbarch_data (vtable_type_gdbarch_data);
   struct type *value_type = check_typedef (VALUE_TYPE (value));
   CORE_ADDR vtable_address;
-  struct value *vtable;
+  struct value *vtable, *vtable_addr_value, *vtable_field;
   struct minimal_symbol *vtable_symbol;
   const char *vtable_symbol_name;
   const char *class_name;
@@ -207,9 +207,20 @@ gnuv3_rtti_type (struct value *value,
     return NULL;
 
   /* Fetch VALUE's virtual table pointer, and tweak it to point at
-     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.   */
+     an instance of our imaginary gdb_gnu_v3_abi_vtable structure.
+     The type of the field may not be a pointer type.  In a derived
+     class with a virtual base class, it will be the type of the base
+     class.  Thus we need to cast.  */
+  vtable_field = value_field (value,
+			      TYPE_VPTR_FIELDNO (value_type));
+  vtable_addr_value =
+    value_at (builtin_type_void_data_ptr,
+	      VALUE_ADDRESS (vtable_field)
+	      + VALUE_OFFSET (vtable_field)
+	      + VALUE_EMBEDDED_OFFSET (vtable_field),
+	      VALUE_BFD_SECTION (value));
   vtable_address
-    = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (value_type)));
+    = value_as_address (vtable_addr_value);
   vtable = value_at_lazy (vtable_type,
                           vtable_address - vtable_address_point_offset (),
                           VALUE_BFD_SECTION (value));
@@ -277,7 +288,7 @@ gnuv3_virtual_fn_field (struct value **v
   struct type *value_type = check_typedef (VALUE_TYPE (value));
   struct type *vfn_base;
   CORE_ADDR vtable_address;
-  struct value *vtable;
+  struct value *vtable, *vtable_addr_value, *vtable_field;
   struct value *vfn;
 
   /* Some simple sanity checks.  */
@@ -309,9 +320,19 @@ gnuv3_virtual_fn_field (struct value **v
     value = value_ind (value_cast (vfn_base, value_addr (value)));
 
   /* Now value is an object of the appropriate base type.  Fetch its
-     virtual table.  */
+     virtual table.  As in gnuv3_rtti_type, the type of the field may
+     not be a pointer type.  In a derived class with a virtual base class,
+     it will be the type of the base class.  Thus we need to cast.  */
+  vtable_field = value_field (value,
+			      TYPE_VPTR_FIELDNO (vfn_base));
+  vtable_addr_value =
+    value_at (builtin_type_void_data_ptr,
+	      VALUE_ADDRESS (vtable_field)
+	      + VALUE_OFFSET (vtable_field)
+	      + VALUE_EMBEDDED_OFFSET (vtable_field),
+	      VALUE_BFD_SECTION (value));
   vtable_address
-    = value_as_address (value_field (value, TYPE_VPTR_FIELDNO (vfn_base)));
+    = value_as_address (vtable_addr_value);
   vtable = value_at_lazy (vtable_type,
                           vtable_address - vtable_address_point_offset (),
                           VALUE_BFD_SECTION (value));