[rfc/cp] take 2: method stubs asserts

[rfc/cp] take 2: method stubs asserts

[Michael Elizabeth Chastain]

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);

[rfc/cp] method stub assertions

[Michael Elizabeth Chastain]

I'm working on support for hp-ux aCC.

When I call a C++ method, I get this assertion failure:

  call class_param.Aptr_a (&g_A)
  /house/chastain/gdb/s1/gdb/valops.c:1942: internal-error: Assertion `TYPE_DOMAIN_TYPE (fns_ptr[0].type) != NULL' failed.

This assertion is in find_overload_method:

  /* 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);

The "domain type" is a pointer from the method to the base class
that defines the virtual table for the class that contains this method.

Here is some history about this assertion:

  http://sources.redhat.com/ml/gdb-patches/2002-05/msg00413.html

Unfortunately, hpread.c does not set TYPE_DOMAIN_TYPE.  As far as I can
tell, only dwarf2read.c and stabsread.c set this field.

Fortunately, it appears that TYPE_DOMAIN_TYPE is really only needed for
calls through member pointers or member function pointers.  Such calls
are forbidden with hp-ux som anyways.

This patch adjusts some asserts.  I want to change find_overload_method
so that the code asserts that methods are not stubbed (which is what it
really needs), and then change evaluate_subexp_standard so that it
asserts TYPE_DOMAIN_TYPE when it really uses them.

After I do this, gdb gets further, but then crashes anyways in:

  evaluate_subexp_standard
    find_overload_match
      value_virtual_fn_field
	gnuv2_virtual_fn_field		# WRONG!
	  value_primitive_field
	    check_typedef

So the next problem after this is that the the abi type is set to
gnu-v2.  When I do "set cp-abi hpaCC", gdb calls the function,
although I see further problems after that.

Anyways, my questions are:

  Am I on the right track?
  Can I commit this?

Tested with:

  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

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 =
! 		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);

Re: [rfc/cp] method stub assertions

[Daniel Jacobowitz]

On Sun, Jan 04, 2004 at 07:50:48PM -0500, Michael Chastain wrote:
> I'm working on support for hp-ux aCC.

Good luck :)

I'll re-read your analysis tomorrow when I'm more awake, but:

> *************** 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 =
> ! 		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);
> 

I was trying to figure out what you changed here and why the diff
seemed confusing.  Take another look at the new definition of basetype;
you added a typo.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfc/cp] method stub assertions

[Daniel Jacobowitz]

On Sun, Jan 04, 2004 at 07:50:48PM -0500, Michael Chastain wrote:
> I'm working on support for hp-ux aCC.
> 
> When I call a C++ method, I get this assertion failure:
> 
>   call class_param.Aptr_a (&g_A)
>   /house/chastain/gdb/s1/gdb/valops.c:1942: internal-error: Assertion `TYPE_DOMAIN_TYPE (fns_ptr[0].type) != NULL' failed.
> 
> This assertion is in find_overload_method:
> 
>   /* 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);
> 
> The "domain type" is a pointer from the method to the base class
> that defines the virtual table for the class that contains this method.
> 
> Here is some history about this assertion:
> 
>   http://sources.redhat.com/ml/gdb-patches/2002-05/msg00413.html
> 
> Unfortunately, hpread.c does not set TYPE_DOMAIN_TYPE.  As far as I can
> tell, only dwarf2read.c and stabsread.c set this field.

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

These are only used in hpread for DNTT_TYPE_PTRMEMFUNC and
DNTT_TYPE_PTRMEM.

dwarf2read, however, calls smash_to_method_type when creating a method
type.

> Fortunately, it appears that TYPE_DOMAIN_TYPE is really only needed for
> calls through member pointers or member function pointers.  Such calls
> are forbidden with hp-ux som anyways.
> 
> This patch adjusts some asserts.  I want to change find_overload_method
> so that the code asserts that methods are not stubbed (which is what it
> really needs), and then change evaluate_subexp_standard so that it
> asserts TYPE_DOMAIN_TYPE when it really uses them.

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.  If 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.  See the
comment:

  /* For types with virtual functions (TYPE_CODE_STRUCT), VPTR_BASETYPE
     is the base class which defined the virtual function table pointer.  

     For types that are pointer to member types (TYPE_CODE_MEMBER),
     VPTR_BASETYPE is the type that this pointer is a member of.

     For method types (TYPE_CODE_METHOD), VPTR_BASETYPE is the aggregate
     type that contains the method.

     Unused otherwise.  */

> After I do this, gdb gets further, but then crashes anyways in:
> 
>   evaluate_subexp_standard
>     find_overload_match
>       value_virtual_fn_field
> 	gnuv2_virtual_fn_field		# WRONG!
> 	  value_primitive_field
> 	    check_typedef
> 
> So the next problem after this is that the the abi type is set to
> gnu-v2.  When I do "set cp-abi hpaCC", gdb calls the function,
> although I see further problems after that.

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.

> Anyways, my questions are:
> 
>   Am I on the right track?
>   Can I commit this?

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.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer