Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Re: [rfc/cp] method stub assertions
@ 2004-01-06  4:28 Michael Elizabeth Chastain
  2004-01-06  4:43 ` Daniel Jacobowitz
  2004-01-06 17:05 ` Daniel Jacobowitz
  0 siblings, 2 replies; 15+ messages in thread
From: Michael Elizabeth Chastain @ 2004-01-06  4:28 UTC (permalink / raw)
  To: drow, mec.gnu; +Cc: gdb-patches

Hi Daniel,

I think we both have to stare at the wall and soak in the reference
books a bit more.  Here's a long message with my understanding.

> Those are static methods.  Don't confuse them with normal methods! 
> They're basically just functions that live in a class.

Right.  There are really three types here:

  plain function (non-class function or static function)
  method function
  pointer to method function

> Do
>   class A { int bar (int); }
> and
>   class B { int baz (int); }
> 
> have the same DNTT type?  If they do, then aCC is so hideously busted
> that I don't know what to do.

I'm pretty sure they don't, because when I step through gdb,
I see that the first parameter of A::bar is "class A * this",
as it should be.

> Wait a sec... this doesn't make sense... if the domain type is only
> needed for non-static members we could just get it from the first
> argument.... something is wrong here.

Yeah.  Forget about gdb for a moment.  Just think about C++
on the C++ type level.

Plain functions:

  extern int f1 (int);
  extern int f2 (int);
  extern int bad3 (double);
  extern double bad4 (int);

  int (*pfii) (int);
  pfii = &f1;    // okay
  pfii = &f2;    // okay
  pfii = &bad3;  // BAD
  pfii = &bad4;  // BAD

The last two assignments are bad because the call signatures do not
match.

Now forget about pointer-to-method function for a minute and just try to
do this:

  class A {
    public:
      static int f5 (int);
      int bad6 (int);
  };

  pfii = &A::f5; // okay
  pfii = &A::bad6; // BAD

That last assignment has to fail because the call signatures do not
match at all.

  pfii     push int; call; pop int
  A::bad6  push A&; push int call; pop init

Suppose we try:

  int (*pfiAi) (A&, int);
  pfiAi = &A::bad6;

That would work with gcc (if it were legal code), but only because gcc
uses an ABI where the "this" pointer interoperates with the first normal
parameter.  But imagine an ABI where "this" goes in a register and every
other parameter goes on the stack.  At the language level, the "this"
parameter is not interchangeable with the normal parameters.

So now let's re-invent pointer-to-member-function.  Start with the right
hand side:

  &A::bad6

The value of &A::bad6 is just like the value of &f1, it's a text address
such as 0x80483ac (this is only true because A::bad6 is not virtual,
leave virtual pmf's for later).

But the type of &A::bad6 is inherently different from the type of &f1.

The type of f1 encodes this information:

  return = {type:int}
  parameter-list = [ {type:int} ]

The type of A::bad6 encodes this information:

  class = {class:A}
  return-type = {type:int}
  parameter-list = [ {type:int} ]

And of course, the real way to write this in C++ is:

  int (A::*pfAii) (int);
  pfAii = &A::bad6;

Okay, let's see what gdb says:

  (gdb) ptype pfii
  type = int (*)(int)

  (gdb) ptype pfiAi
  type = int (*)(A &, int)

  (gdb) ptype pfAii
  type = struct {
      int (*__pfn)(A *, int);
      int __delta;
  }

  (gdb) ptype A::bad6
  type = int (A * const, int)

  (gdb) ptype &A::bad6
  type = int (*)(A * const, int)

The first four of those are okay with me.  pfAii is not only a
different type from a pointer to an ordinary function,
but the type has a different size!

The last one, &A::bad6, bothers me a lot.  The address-of operator
applied to a (non-static) member function should *not* return a
plain pointer-to-function.  Output #5 should *not* look like #2.
It should look like:

  (gdb) ptype &A::bad6
  type = int (A::*)(int)

I'm willing to slide on the "A * const" hidden parameter,
but not on the "A::*" instead of "*".

I think that gdb does not distinguish between pointer-to-function and
PMF.  PMF's always carry a class argument with them, but plain PF's
never need them.  I suspect gdb is mixing the TYPE_CODE's instead of using
separate TYPE_CODE's.

The existing scheme is something like:

  TYPE_CODE_MEMBER
    TYPE_CODE_METHOD
      domain type

But I want it to be more:

  TYPE_CODE_MEMBER
    domain type
    TYPE_CODE_METHOD

Every time you create a TYPE_CODE_MEMBER, you should know what the
domain type is, and store it up one node, not down in the
TYPE_CODE_METHOD node.  And every time you use a TYPE_CODE_MEMBER, you
have to use the domain type.  And if you're not dealing with
pointer-to-member, you don't need the domain type.  It's an attribute
of the pointer-to-member, not an attribute of the member!

Bleagh.  I'm probably butchering the actual gdb data structures here.
Need to read more.

> Do static methods have TYPE_CODE_METHOD, and should they?
> That's the question.

I think they currently do.  And then they have TYPE_FLAG_STATIC to
distinguish them from other types.

Static methods have to appear with other methods because they
participate in overload resolution.

I think that when gdb evaluates UNOP_ADDR, evaluate_subexp_for_address
needs to be more careful to return the appropriate type.  There are
four cases: plain function, static member function, non-static member
function, virtual member function.  They all have different types,
and they even have different bit-representations in their values.

Right now I don't understand the existing TYPE_CODE_* well enough.
I want to stare at the TYPE_CODE_* for a while.  I think that if we
assign and use them properly that some code will actually get simpler
and some of the hp-specific "pointer to member" code will actually just
generalize to everybody's "pointer to member".

Michael C


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [rfc/cp] method stub assertions
@ 2004-01-06 18:24 Michael Elizabeth Chastain
  2004-01-06 19:02 ` Daniel Jacobowitz
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Elizabeth Chastain @ 2004-01-06 18:24 UTC (permalink / raw)
  To: drow; +Cc: gdb-patches

> That's a nice hypothesis.  Unfortunately it's completely wrong :)
> First of all, TYPE_CODE_MEMBER and TYPE_CODE_METHOD are siblings. 
> MEMBER is used for data variables, not to wrap methods.

I think you mean: TYPE_CODE_MEMBER is used for pointers to data
members.

It's a really bad name.  How about:

  TYPE_CODE_PTR		# pointer to memory
  TYPE_CODE_PMD		# pointer to member data
  TYPE_CODE_PMF_PLAIN 	# pointer to non-static non-virtual function
  TYPE_CODE_PMF_VIRTUAL	# pointer to virtual function

TYPE_CODE_PTR has a raw CORE_ADDR, just like it does now.
TYPE_CODE_PMD has a class type and a data offset.
TYPE_CODE_PMF_PLAIN has a class type and a raw CORE_ADDR.
TYPE_CODE_PMF_VIRTUAL has a class type and a vtbl offset.

> The debug information for A::bad6 does not specify that it is a method. 
> Rather only the debug info for class A specifies that it has a method
> named A::bad6.  Take a look at a readelf -wi dump of your testcase to
> see how this works.

Ouch.

How can we make &A::bad6 have a different type than &f1 ?

> Currently they do appear as TYPE_CODE_METHOD.  I think that they
> probably shouldn't.  A pointer to a static method is a function
> pointer, not a pointer-to-member.  Similarly static variables should
> probably not be TYPE_CODE_MEMBER.

I agree that &A::static_function should be TYPE_CODE_PTR.
It's easy to figure that out even if A::static_function is
TYPE_CODE_METHOD, because we can look at TYPE_FLAG_STATIC
at the time we evalue the "&" operator.

Michael C


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [rfc/cp] method stub assertions
@ 2004-01-06  0:12 Michael Elizabeth Chastain
  2004-01-06  2:51 ` Daniel Jacobowitz
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Elizabeth Chastain @ 2004-01-06  0:12 UTC (permalink / raw)
  To: drow; +Cc: gdb-patches

Hi Daniel,

> Right now we assume that methods have a TYPE_DOMAIN_TYPE.  This patch
> pushes more knowledge of limited debug readers out into the rest of
> GDB.  I'd rather go the other direction - set a domain type.

I'm going to push back on this and argue that a C++ method should
not need to have a domain type.

A pointer-to-member needs to have a domain type because it's
explicitly associated with a domain:

  int (A::*PMF)(int);

The debug information for "PMF" says that it's in class A,
and that becomes the domain type.  It's all good.

But an ordinary member does not need to have a domain:

  class A
  {
    static int foo (int);
  };

  class B
  {
    static int bar (int);
  };

  static int bletch (int);

Here, the types of "foo", bar", and "bletch" are exactly the same.
Adding a "domain A" to the first and "domain B" to the second
makes them not the same, and will cause me a big problem.

In HP debug format, the DNTT records for A::foo, B::bar, and bletch can
be the same record.  hp-read.c has a 1-1 map from DNTT records to gdb
types.  This is the dntt_type_vector in 'struct hpread_symfile_info'.

When the HP reader sees a DNTT type, it autovivifies dntt_type_vector
and uses the dntt_type_vector[hp_type.dnttp.index] for the gdb type.
So the same DNTT type always maps to the same gdb type,
and all three functions can share a type entry.

But different DNTT type will need to have different gdb types because
they have different domain types.  That means I have to mess with a lot
of code that translates DNTT types: (1) pass in context information that
is not part of the DNTT record to begin with, and (2) change the
dntt-to-gdbtype mapping array to index off of [domain type, index]
instead of just [index].

All of this for a field which is not used in expression evaluation
anyways!

c_type_print_varspec_prefix does contain this code:

  case TYPE_CODE_METHOD:
      if (passed_a_ptr)
	fprintf_filtered (stream, "(");
      c_type_print_varspec_prefix (TYPE_TARGET_TYPE (type), stream, show, 0, 0);
      if (passed_a_ptr)
	{
	  fprintf_filtered (stream, " ");
	  c_type_print_base (TYPE_DOMAIN_TYPE (type), stream, 0, passed_a_ptr);
	  fprintf_filtered (stream, "::");
	}
      break;

However, I don't think it's actually possible to have a C++ type which
is a pointer to a TYPE_CODE_METHOD.  If the C++ type is "pointer to
function returning int", it can point to a non-method function or to a
static method function.  If the C++ type is "pointer to class method
returning int", then that is a pointer-to-member, and a

pointer-to-member is already required to have a class type
along with the method signature.

How about it, can you re-think your requirement that each method
type has a domain type?

Michael C


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [rfc/cp] method stub assertions
@ 2004-01-05 20:51 Michael Elizabeth Chastain
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Elizabeth Chastain @ 2004-01-05 20:51 UTC (permalink / raw)
  To: drow; +Cc: gdb-patches

drow> No, that's not correct.  hpread.c sets it the same way that
drow> dwarf2read.c does, by calling smash_to_method_type and
drow> smash_to_member_type.

Ah, okay.

> I'm reading dwarf2read correctly it simply uses the enclosing class, not
> walking back to find the class containing the vtable as you describe
> above.  It's for _classes_ that it behaves as described above.

Okay.  I think evaluate_subexp_standard understands this --
that it has to go from method to domain type, and from domain
type to some other base-class type that actually defines the
vtable.

> You will need to find some way to autodetect this.  Does aCC still
> produce SOM output, even for hppa64?  If so you can use
> hp_som_object_present, but that's a gross hack.  It would be better to
> find another way.
> 
> See where we autodetect gnu_v3.

That will be the next problem.  I just wanted to be sure that I was
actually fixing this problem.

> No, please try to set TYPE_DOMAIN_TYPE in hpread instead.  I think
> around line 3950 is the only place you'll need to.  Hmm... or perhaps
> calling smash_to_method_type at 3861.  That may work.

Rats.  Okay.  I'll withdraw my patch, and work on that instead.

Michael C

  2004-01-04  Michael Chastain  <mec.gnu@mindpsring.com>

	* valops.c (find_overload_match): Assert that methods are not
	stubs.  Do not assert that methods have domain types.
	* eval.c (evaluate_subexp_standard): Assert basetype and
	domain_type as needed.


^ permalink raw reply	[flat|nested] 15+ messages in thread
* [rfc/cp] take 2: method stubs asserts
@ 2004-01-05 11:50 Michael Elizabeth Chastain
  2004-01-05  0:50 ` [rfc/cp] method stub assertions Michael Elizabeth Chastain
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Elizabeth Chastain @ 2004-01-05 11:50 UTC (permalink / raw)
  To: dan, gdb-patches

Here's the slightly re-worked method stub patched.

Tested again on:

  native i686-pc-linux-gnu, gcc 2.95.3 3.3.2 HEAD, dwarf-2 stabs+
    full regression test
  native hppa2.0w-hp-hpux11.11, HP aCC++ B3910B A.03.45
    one little test by hand

Same analysis as before.  Briefly, hpread.c does not set the
domain type on a method, but methods don't actually need domain
types unless they are used in pointer-to-member expressions,
which are not implemented with hp SOM anyways.

Is this the right idea?
If it is -- okay to apply this patch?

Michael C

2004-01-04  Michael Chastain  <mec.gnu@mindpsring.com>

	* valops.c (find_overload_match): Assert that methods are not
	stubs.  Do not assert that methods have domain types.
	* eval.c (evaluate_subexp_standard): Assert basetype and
	domain_type as needed.

Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.119
diff -c -3 -p -r1.119 valops.c
*** valops.c	8 Nov 2003 00:13:03 -0000	1.119
--- valops.c	4 Jan 2004 23:05:43 -0000
***************
*** 1,6 ****
  /* Perform non-arithmetic operations on values, for GDB.
     Copyright 1986, 1987, 1988, 1989, 1990, 1991, 1992, 1993, 1994,
!    1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003
     Free Software Foundation, Inc.
  
     This file is part of GDB.
--- 1,6 ----
  /* Perform non-arithmetic operations on values, for GDB.
     Copyright 1986, 1987, 1988, 1989, 1990, 1991, 1992, 1993, 1994,
!    1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004
     Free Software Foundation, Inc.
  
     This file is part of GDB.
*************** find_overload_match (struct type **arg_t
*** 1939,1945 ****
        /* If we are dealing with stub method types, they should have
  	 been resolved by find_method_list via value_find_oload_method_list
  	 above.  */
!       gdb_assert (TYPE_DOMAIN_TYPE (fns_ptr[0].type) != NULL);
      }
    else
      {
--- 1939,1946 ----
        /* If we are dealing with stub method types, they should have
  	 been resolved by find_method_list via value_find_oload_method_list
  	 above.  */
!       for (ix = 0; ix < num_fns; ix++)
! 	gdb_assert (fns_ptr[ix].is_stub == 0);
      }
    else
      {
Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.39
diff -c -3 -p -r1.39 eval.c
*** eval.c	23 Nov 2003 20:41:16 -0000	1.39
--- eval.c	4 Jan 2004 23:05:44 -0000
***************
*** 1,8 ****
  /* Evaluate expressions for GDB.
  
     Copyright 1986, 1987, 1988, 1989, 1990, 1991, 1992, 1993, 1994,
!    1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003 Free Software
!    Foundation, Inc.
  
     This file is part of GDB.
  
--- 1,8 ----
  /* Evaluate expressions for GDB.
  
     Copyright 1986, 1987, 1988, 1989, 1990, 1991, 1992, 1993, 1994,
!    1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004
!    Free Software Foundation, Inc.
  
     This file is part of GDB.
  
***************
*** 36,41 ****
--- 36,42 ----
  #include "objc-lang.h"
  #include "block.h"
  #include "parser-defs.h"
+ #include "gdb_assert.h"
  
  /* Defined in symtab.c */
  extern int hp_som_som_object_present;
*************** evaluate_subexp_standard (struct type *e
*** 996,1006 ****
  	  if (METHOD_PTR_IS_VIRTUAL (fnptr))
  	    {
  	      int fnoffset = METHOD_PTR_TO_VOFFSET (fnptr);
! 	      struct type *basetype;
  	      struct type *domain_type =
! 	      TYPE_DOMAIN_TYPE (TYPE_TARGET_TYPE (VALUE_TYPE (arg1)));
  	      int i, j;
! 	      basetype = TYPE_TARGET_TYPE (VALUE_TYPE (arg2));
  	      if (domain_type != basetype)
  		arg2 = value_cast (lookup_pointer_type (domain_type), arg2);
  	      basetype = TYPE_VPTR_BASETYPE (domain_type);
--- 997,1011 ----
  	  if (METHOD_PTR_IS_VIRTUAL (fnptr))
  	    {
  	      int fnoffset = METHOD_PTR_TO_VOFFSET (fnptr);
! 	      struct type *basetype =
! 		TYPE_TARGET_TYPE (VALUE_TYPE (arg2));
  	      struct type *domain_type =
! 		TYPE_DOMAIN_TYPE (TYPE_TARGET_TYPE (VALUE_TYPE (arg1)));
  	      int i, j;
! 
! 	      gdb_assert (basetype != NULL);
! 	      gdb_assert (domain_type != NULL);
! 
  	      if (domain_type != basetype)
  		arg2 = value_cast (lookup_pointer_type (domain_type), arg2);
  	      basetype = TYPE_VPTR_BASETYPE (domain_type);


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [rfc/cp] method stub assertions
@ 2004-01-05  2:32 Michael Elizabeth Chastain
  2004-01-05  2:33 ` Daniel Jacobowitz
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Elizabeth Chastain @ 2004-01-05  2:32 UTC (permalink / raw)
  To: drow, mec.gnu; +Cc: gdb-patches

> ! 	      struct type *basetype =
> ! 		basetype = TYPE_TARGET_TYPE (VALUE_TYPE (arg2));

Argh!  But the compilers didn't error out.
I wonder if that is legal Ansi C?

I'll re-do the patch.

Michael C


^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2004-01-06 19:05 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-01-06  4:28 [rfc/cp] method stub assertions Michael Elizabeth Chastain
2004-01-06  4:43 ` Daniel Jacobowitz
2004-01-06 17:05 ` Daniel Jacobowitz
2004-01-06 18:41   ` David Carlton
2004-01-06 19:05     ` Daniel Jacobowitz
  -- strict thread matches above, loose matches on Subject: below --
2004-01-06 18:24 Michael Elizabeth Chastain
2004-01-06 19:02 ` Daniel Jacobowitz
2004-01-06  0:12 Michael Elizabeth Chastain
2004-01-06  2:51 ` Daniel Jacobowitz
2004-01-05 20:51 Michael Elizabeth Chastain
2004-01-05 11:50 [rfc/cp] take 2: method stubs asserts Michael Elizabeth Chastain
2004-01-05  0:50 ` [rfc/cp] method stub assertions Michael Elizabeth Chastain
2004-01-05  1:56   ` Daniel Jacobowitz
2004-01-05 19:23   ` Daniel Jacobowitz
2004-01-05  2:32 Michael Elizabeth Chastain
2004-01-05  2:33 ` Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox