[PATCH] m32c-tdep.c: Don't choke on virtual functions in push_dummy_call

[PATCH] m32c-tdep.c: Don't choke on virtual functions in push_dummy_call

[Corinna Vinschen]

Hi,

the below patch fixes a couple of GDB internal errors when calling
virtual C++ methods.  Virtual methods are not represented as type
TYPE_CODE_METHOD, but as TYPE_CODE_PTR pointing to TYPE_CODE_METHOD.
The m32c_push_dummy_call function only checks the function type for
TYPE_CODE_FUNC or TYPE_CODE_METHOD, which results in a failed assertion
in case of virtual methods.  The below patch skips TYPE_CODE_PTR types
until it points to the actual TYPE_CODE_METHOD to evaluate further
necessary data.

Ok to apply?


Thanks,
Corinna


	* m32c-tdep.c (m32c_push_dummy_call): Skip over virtual method
	pointer type.


Index: m32c-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m32c-tdep.c,v
retrieving revision 1.1
diff -u -p -r1.1 m32c-tdep.c
--- m32c-tdep.c	20 Apr 2006 23:18:48 -0000	1.1
+++ m32c-tdep.c	28 Jun 2006 14:14:11 -0000
@@ -2008,6 +2008,10 @@ m32c_push_dummy_call (struct gdbarch *gd
   {
     struct type *func_type = value_type (function);
 
+    /* Skip over virtual function pointers */
+    while (TYPE_CODE (func_type) == TYPE_CODE_PTR)
+      func_type = TYPE_TARGET_TYPE (func_type);
+
     gdb_assert (TYPE_CODE (func_type) == TYPE_CODE_FUNC ||
 		TYPE_CODE (func_type) == TYPE_CODE_METHOD);
 

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat

Re: [PATCH] m32c-tdep.c: Don't choke on virtual functions in push_dummy_call

[Mark Kettenis]

> Date: Wed, 28 Jun 2006 16:18:39 +0200
> From: Corinna Vinschen <vinschen@redhat.com>
> 
> Hi,
> 
> the below patch fixes a couple of GDB internal errors when calling
> virtual C++ methods.  Virtual methods are not represented as type
> TYPE_CODE_METHOD, but as TYPE_CODE_PTR pointing to TYPE_CODE_METHOD.
> The m32c_push_dummy_call function only checks the function type for
> TYPE_CODE_FUNC or TYPE_CODE_METHOD, which results in a failed assertion
> in case of virtual methods.  The below patch skips TYPE_CODE_PTR types
> until it points to the actual TYPE_CODE_METHOD to evaluate further
> necessary data.
> 
> Ok to apply?

Hmm, I don't understand why this is a m32c-specific problem.
Shouldn't this be handled in generic code instead?

Mark

Re: [PATCH] m32c-tdep.c: Don't choke on virtual functions in push_dummy_call

[Daniel Jacobowitz]

On Wed, Jun 28, 2006 at 04:18:39PM +0200, Corinna Vinschen wrote:
> Hi,
> 
> the below patch fixes a couple of GDB internal errors when calling
> virtual C++ methods.  Virtual methods are not represented as type
> TYPE_CODE_METHOD, but as TYPE_CODE_PTR pointing to TYPE_CODE_METHOD.
> The m32c_push_dummy_call function only checks the function type for
> TYPE_CODE_FUNC or TYPE_CODE_METHOD, which results in a failed assertion
> in case of virtual methods.  The below patch skips TYPE_CODE_PTR types
> until it points to the actual TYPE_CODE_METHOD to evaluate further
> necessary data.
> 
> Ok to apply?

In addition to Mark's question, which I am also curious about - did
anything in the testsuite catch this, or if not, how can we reproduce
it? - I am currently (i.e. right this moment) rewriting lots of this
stuff.  You may wish to wait a little while - though be sure to poke me
about it if I don't have something to check in soon!  It is all broken
anyway; GDB is not going to succeed at calling a virtual pointer, it's
going to go off into neverland.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] m32c-tdep.c: Don't choke on virtual functions in push_dummy_call

[Corinna Vinschen]

On Jun 28 17:32, Daniel Jacobowitz wrote:
> On Wed, Jun 28, 2006 at 04:18:39PM +0200, Corinna Vinschen wrote:
> > Hi,
> > 
> > the below patch fixes a couple of GDB internal errors when calling
> > virtual C++ methods.  Virtual methods are not represented as type
> > TYPE_CODE_METHOD, but as TYPE_CODE_PTR pointing to TYPE_CODE_METHOD.
> > The m32c_push_dummy_call function only checks the function type for
> > TYPE_CODE_FUNC or TYPE_CODE_METHOD, which results in a failed assertion
> > in case of virtual methods.  The below patch skips TYPE_CODE_PTR types
> > until it points to the actual TYPE_CODE_METHOD to evaluate further
> > necessary data.
> > 
> > Ok to apply?

Mark Kettenis asked:
> Hmm, I don't understand why this is a m32c-specific problem.
> Shouldn't this be handled in generic code instead?

[Mark, could you please not quote raw email addresses in the mail body?
 Thanks.]

This isn't a m32c specific problem.  m32c is right now the only
architecture which needs access to the function type, so m32c is
the only affected architecture.

I don't know how this has to be handled in generaic code, actually.
The function pointer which is given to the push_dummy_call function
is a struct value, which points to the function value.  The type
stored here is a TYPE_CODE_PTR pointing to TYPE_CODE_METHOD.  This
makes sort of sense to me.

> In addition to Mark's question, which I am also curious about - did
> anything in the testsuite catch this, or if not, how can we reproduce
> it?

gdb.cp/virtfunc.exp.  All tests calling virtual functions by hand.

> - I am currently (i.e. right this moment) rewriting lots of this
> stuff.  You may wish to wait a little while - though be sure to poke me
> about it if I don't have something to check in soon!  It is all broken
> anyway; GDB is not going to succeed at calling a virtual pointer, it's
> going to go off into neverland.

This I don't understand.  Calling virtual methods works quite nicely,
at least in my m32c case here.  After dereferencing the method type,
the m32c_push_dummy_call function passes all args correctly and the
return value of the call is correct, too.

As for the problem of having to dereference the virtual function type,
Maybe it makes sense to do this in call_function_by_hand already.
Or are you suggesting that the representation of virtual methods is
incorrect in GDB?


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat

Re: [PATCH] m32c-tdep.c: Don't choke on virtual functions in push_dummy_call

[Daniel Jacobowitz]

On Thu, Jun 29, 2006 at 01:01:36PM +0200, Corinna Vinschen wrote:
> This I don't understand.  Calling virtual methods works quite nicely,
> at least in my m32c case here.  After dereferencing the method type,
> the m32c_push_dummy_call function passes all args correctly and the
> return value of the call is correct, too.
> 
> As for the problem of having to dereference the virtual function type,
> Maybe it makes sense to do this in call_function_by_hand already.
> Or are you suggesting that the representation of virtual methods is
> incorrect in GDB?

This is what a pointer to a virtual function looks like in the Itanium
(GNU v3) C++ ABI:

struct {
  int (*__pfn) (A *this);
  int __delta;
};

If it's a virtual function, __pfn is actually an offset to the virtual
table.  This is what a pointer to method is supposed to look like.  GDB
botches this.  The house of cards it has now more or less works, but
that's it.

I think you should be getting a TYPE_CODE_METHOD, not a pointer to
method, in target specific code.

-- 
Daniel Jacobowitz
CodeSourcery