This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Fix calling of static C++ member functions


On Sun, May 12, 2002 at 09:48:56PM +0200, Peter.Schauer wrote:
> > I had to make some substantial changes to this.  First of all, I wanted
> > to kill the now-obsolete static_memfuncp argument to find_method_list. 
> > Secondly, the way you were skipping THIS in typecmp caused segfaults
> > for some methods which only expected one argument.  Here's what I
> > checked in.
> > 
> > I -think- I'm caught up on all the patches you sent me now, Peter.
> 
> Yes you are, thank you very much.
> 
> Thanks also for fixing the segfaults, but I'd be interested in an example
> where this happened. I tried to simplify the typcmp logic as much as possible,
> but obviously failed.
> 
> One minor nit, the comments should reflect the elimination of static_memfuncp:

I've committed this.

2002-07-12  Peter Schauer  <Peter.Schauer@Regent.E-Technik.TU-Muenchen.DE>

        * valops.c (find_method_list): Remove comment about
        removed STATIC_MEMFUNCP argument.
        (value_find_oload_method_list): Likewise.

> 
> --- ./valops.c.orig	Sun May 12 17:16:37 2002
> +++ ./valops.c	Sun May 12 17:55:37 2002
> @@ -2515,7 +2516,6 @@ value_struct_elt (struct value **argp, s
>   * ARGP is a pointer to a pointer to a value (the object)
>   * METHOD is a string containing the method name
>   * OFFSET is the offset within the value
> - * STATIC_MEMFUNCP is set if the method is static
>   * TYPE is the assumed type of the object
>   * NUM_FNS is the number of overloaded instances
>   * BASETYPE is set to the actual type of the subobject where the method is found
> @@ -2606,7 +2606,6 @@ find_method_list (struct value **argp, c
>   * ARGP is a pointer to a pointer to a value (the object)
>   * METHOD is the method name
>   * OFFSET is the offset within the value contents
> - * STATIC_MEMFUNCP is set if the method is static
>   * NUM_FNS is the number of overloaded instances
>   * BASETYPE is set to the type of the base subobject that defines the method
>   * BOFFSET is the offset of the base subobject which defines the method */
> 
> Thanks again,
> 
> > On Fri, Mar 15, 2002 at 11:44:46AM +0100, Peter.Schauer wrote:
> > > This one fixes
> > > http://sources.redhat.com/ml/bug-gdb/2001-06/msg00059.html
> > > which is still broken in CVS Head GDB.
> > > 
> > > Actually there are a bunch problems:
> > > 
> > > - The printing of the bad value happens, because find_overload_match does
> > >   not set *staticp for static member functions.
> > > 
> > > - The crash happens because in
> > >         domain = TYPE_DOMAIN_TYPE (fns_ptr[0].type);
> > >   in find_overload_match we don't find the domain, as find_method_list
> > >   doesn't handle stub methods correctly and it's too late in
> > >   find_overload_match. Checking for stub methods in find_method_list
> > >   instead gets rid of the crash.
> > > 
> > > - When trying the provided testcase for dwarf2, more problems arise.
> > >   Until now check_stub_method did not handle static member
> > >   functions correctly, as the this parameter was added unconditionally.
> > >   After fixing this, we also have to adjust typecmp, so that static
> > >   member functions also work with `set overload off'.
> > >   And finally we have to fix find_overload_match, to make it behave properly
> > >   in the `set overload on' case.
> > > 
> > > The patch below fixes the calling of static member functions for
> > > gcc-2.95.2/gcc-3.x and stabs/dwarf2 debugging, ok to commit ?
> > 
> > I had to make some substantial changes to this.  First of all, I wanted
> > to kill the now-obsolete static_memfuncp argument to find_method_list. 
> > Secondly, the way you were skipping THIS in typecmp caused segfaults
> > for some methods which only expected one argument.  Here's what I
> > checked in.
> > 
> > I -think- I'm caught up on all the patches you sent me now, Peter.
> > 
> > > 2002-03-15  Peter Schauer  <pes@regent.e-technik.tu-muenchen.de>
> > > 
> > > 	Fix calling of static member functions.
> > > 	* gdbtypes.c (check_stub_method): Handle static member functions.
> > > 	* valops.c (typecmp): Handle static member functions correctly.
> > > 	(find_overload_match): Ditto, set *staticp for static member
> > > 	functions.
> > > 	(find_method_list): Check for stub functions here, not in
> > > 	find_overload_match.
> > 
> > -- 
> > Daniel Jacobowitz                           Carnegie Mellon University
> > MontaVista Software                         Debian GNU/Linux Developer
> > 
> > 2005-05-11  Daniel Jacobowitz  <drow@mvista.com>
> > 	    Peter Schauer  <pes@regent.e-technik.tu-muenchen.de>
> > 
> > 	* Makefile.in: Update dependencies for valops.c.
> > 	* valops.c: Include "gdb_assert.h".
> > 	(typecmp): Skip THIS parameter to methods.
> > 	(find_method_list): Remove static_memfuncp argument,
> > 	update callers.  Check for stub methods.
> > 	(find_value_oload_method_list): Don't set *static_memfuncp.
> > 	(find_overload_match): Don't check for stub methods.  Assert
> > 	that methods are not stubbed.  Handle static methods.
> > 	(value_find_oload_method_list): Remove static_memfuncp argument.
> > 	* gdbtypes.c (check_stub_method): Do not add THIS pointer
> > 	to the argument list for static stub methods.
> > 	* value.h (value_find_oload_method_list): Update prototype.
> > 
> > Index: valops.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/valops.c,v
> > retrieving revision 1.56
> > diff -u -p -r1.56 valops.c
> > --- valops.c	2 May 2002 19:00:36 -0000	1.56
> > +++ valops.c	12 May 2002 02:08:08 -0000
> > @@ -36,6 +36,7 @@
> >  
> >  #include <errno.h>
> >  #include "gdb_string.h"
> > +#include "gdb_assert.h"
> >  
> >  /* Flag indicating HP compilers were used; needed to correctly handle some
> >     value operations with HP aCC code/runtime. */
> > @@ -66,7 +67,7 @@ static CORE_ADDR allocate_space_in_infer
> >  static struct value *cast_into_complex (struct type *, struct value *);
> >  
> >  static struct fn_field *find_method_list (struct value ** argp, char *method,
> > -					  int offset, int *static_memfuncp,
> > +					  int offset,
> >  					  struct type *type, int *num_fns,
> >  					  struct type **basetype,
> >  					  int *boffset);
> > @@ -1963,10 +1964,13 @@ typecmp (int staticp, struct type *t1[],
> >      return t2[1] != 0;
> >    if (t1 == 0)
> >      return 1;
> > -  if (TYPE_CODE (t1[0]) == TYPE_CODE_VOID)
> > -    return 0;
> >    if (t1[!staticp] == 0)
> >      return 0;
> > +  if (TYPE_CODE (t1[0]) == TYPE_CODE_VOID)
> > +    return 0;
> > +  /* Skip ``this'' argument if applicable.  T2 will always include THIS.  */
> > +  if (staticp)
> > +    t2++;
> >    for (i = !staticp; t1[i] && TYPE_CODE (t1[i]) != TYPE_CODE_VOID; i++)
> >      {
> >        struct type *tt1, *tt2;
> > @@ -2520,7 +2524,7 @@ value_struct_elt (struct value **argp, s
> >  
> >  static struct fn_field *
> >  find_method_list (struct value **argp, char *method, int offset,
> > -		  int *static_memfuncp, struct type *type, int *num_fns,
> > +		  struct type *type, int *num_fns,
> >  		  struct type **basetype, int *boffset)
> >  {
> >    int i;
> > @@ -2536,10 +2540,22 @@ find_method_list (struct value **argp, c
> >        char *fn_field_name = TYPE_FN_FIELDLIST_NAME (type, i);
> >        if (fn_field_name && (strcmp_iw (fn_field_name, method) == 0))
> >  	{
> > -	  *num_fns = TYPE_FN_FIELDLIST_LENGTH (type, i);
> > +	  /* Resolve any stub methods.  */
> > +	  int len = TYPE_FN_FIELDLIST_LENGTH (type, i);
> > +	  struct fn_field *f = TYPE_FN_FIELDLIST1 (type, i);
> > +	  int j;
> > +
> > +	  *num_fns = len;
> >  	  *basetype = type;
> >  	  *boffset = offset;
> > -	  return TYPE_FN_FIELDLIST1 (type, i);
> > +
> > +	  for (j = 0; j < len; j++)
> > +	    {
> > +	      if (TYPE_FN_FIELD_STUB (f, j))
> > +		check_stub_method (type, i, j);
> > +	    }
> > +
> > +	  return f;
> >  	}
> >      }
> >  
> > @@ -2579,7 +2595,8 @@ find_method_list (struct value **argp, c
> >  	  base_offset = TYPE_BASECLASS_BITPOS (type, i) / 8;
> >  	}
> >        f = find_method_list (argp, method, base_offset + offset,
> > -      static_memfuncp, TYPE_BASECLASS (type, i), num_fns, basetype, boffset);
> > +			    TYPE_BASECLASS (type, i), num_fns, basetype,
> > +			    boffset);
> >        if (f)
> >  	return f;
> >      }
> > @@ -2597,8 +2614,8 @@ find_method_list (struct value **argp, c
> >  
> >  struct fn_field *
> >  value_find_oload_method_list (struct value **argp, char *method, int offset,
> > -			      int *static_memfuncp, int *num_fns,
> > -			      struct type **basetype, int *boffset)
> > +			      int *num_fns, struct type **basetype,
> > +			      int *boffset)
> >  {
> >    struct type *t;
> >  
> > @@ -2621,12 +2638,7 @@ value_find_oload_method_list (struct val
> >        && TYPE_CODE (t) != TYPE_CODE_UNION)
> >      error ("Attempt to extract a component of a value that is not a struct or union");
> >  
> > -  /* Assume it's not static, unless we see that it is.  */
> > -  if (static_memfuncp)
> > -    *static_memfuncp = 0;
> > -
> > -  return find_method_list (argp, method, 0, static_memfuncp, t, num_fns, basetype, boffset);
> > -
> > +  return find_method_list (argp, method, 0, t, num_fns, basetype, boffset);
> >  }
> >  
> >  /* Given an array of argument types (ARGTYPES) (which includes an
> > @@ -2685,6 +2697,7 @@ find_overload_match (struct type **arg_t
> >    int boffset;
> >    register int jj;
> >    register int ix;
> > +  int static_offset;
> >  
> >    char *obj_type_name = NULL;
> >    char *func_name = NULL;
> > @@ -2692,9 +2705,6 @@ find_overload_match (struct type **arg_t
> >    /* Get the list of overloaded methods or functions */
> >    if (method)
> >      {
> > -      int i;
> > -      int len;
> > -      struct type *domain;
> >        obj_type_name = TYPE_NAME (VALUE_TYPE (obj));
> >        /* Hack: evaluate_subexp_standard often passes in a pointer
> >           value rather than the object itself, so try again */
> > @@ -2703,7 +2713,6 @@ find_overload_match (struct type **arg_t
> >  	obj_type_name = TYPE_NAME (TYPE_TARGET_TYPE (VALUE_TYPE (obj)));
> >  
> >        fns_ptr = value_find_oload_method_list (&temp, name, 0,
> > -					      staticp,
> >  					      &num_fns,
> >  					      &basetype, &boffset);
> >        if (!fns_ptr || !num_fns)
> > @@ -2711,26 +2720,10 @@ find_overload_match (struct type **arg_t
> >  	       obj_type_name,
> >  	       (obj_type_name && *obj_type_name) ? "::" : "",
> >  	       name);
> > -      domain = TYPE_DOMAIN_TYPE (fns_ptr[0].type);
> > -      len = TYPE_NFN_FIELDS (domain);
> > -      /* NOTE: dan/2000-03-10: This stuff is for STABS, which won't
> > -         give us the info we need directly in the types. We have to
> > -         use the method stub conversion to get it. Be aware that this
> > -         is by no means perfect, and if you use STABS, please move to
> > -         DWARF-2, or something like it, because trying to improve
> > -         overloading using STABS is really a waste of time. */
> > -      for (i = 0; i < len; i++)
> > -	{
> > -	  int j;
> > -	  struct fn_field *f = TYPE_FN_FIELDLIST1 (domain, i);
> > -	  int len2 = TYPE_FN_FIELDLIST_LENGTH (domain, i);
> > -
> > -	  for (j = 0; j < len2; j++)
> > -	    {
> > -	      if (TYPE_FN_FIELD_STUB (f, j) && (!strcmp_iw (TYPE_FN_FIELDLIST_NAME (domain,i),name)))
> > -		check_stub_method (domain, i, j);
> > -	    }
> > -	}
> > +      /* 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
> >      {
> > @@ -2757,10 +2750,11 @@ find_overload_match (struct type **arg_t
> >    /* Consider each candidate in turn */
> >    for (ix = 0; ix < num_fns; ix++)
> >      {
> > +      static_offset = 0;
> >        if (method)
> >  	{
> > -	  /* For static member functions, we won't have a this pointer, but nothing
> > -	     else seems to handle them right now, so we just pretend ourselves */
> > +	  if (TYPE_FN_FIELD_STATIC_P (fns_ptr, ix))
> > +	    static_offset = 1;
> >  	  nparms=0;
> >  
> >  	  if (TYPE_FN_FIELD_ARGS(fns_ptr,ix))
> > @@ -2782,8 +2776,10 @@ find_overload_match (struct type **arg_t
> >  			  ? (TYPE_FN_FIELD_ARGS (fns_ptr, ix)[jj])
> >  			  : TYPE_FIELD_TYPE (SYMBOL_TYPE (oload_syms[ix]), jj));
> >  
> > -      /* Compare parameter types to supplied argument types */
> > -      bv = rank_function (parm_types, nparms, arg_types, nargs);
> > +      /* Compare parameter types to supplied argument types.  Skip THIS for
> > +         static methods.  */
> > +      bv = rank_function (parm_types, nparms, arg_types + static_offset,
> > +			  nargs - static_offset);
> >  
> >        if (!oload_champ_bv)
> >  	{
> > @@ -2821,7 +2817,7 @@ find_overload_match (struct type **arg_t
> >  	    fprintf_filtered (gdb_stderr,"Overloaded method instance %s, # of parms %d\n", fns_ptr[ix].physname, nparms);
> >  	  else
> >  	    fprintf_filtered (gdb_stderr,"Overloaded function instance %s # of parms %d\n", SYMBOL_DEMANGLED_NAME (oload_syms[ix]), nparms);
> > -	  for (jj = 0; jj < nargs; jj++)
> > +	  for (jj = 0; jj < nargs - static_offset; jj++)
> >  	    fprintf_filtered (gdb_stderr,"...Badness @ %d : %d\n", jj, bv->rank[jj]);
> >  	  fprintf_filtered (gdb_stderr,"Overload resolution champion is %d, ambiguous? %d\n", oload_champ, oload_ambiguous);
> >  	}
> > @@ -2844,8 +2840,11 @@ find_overload_match (struct type **arg_t
> >      }
> >  #endif
> >  
> > -  /* Check how bad the best match is */
> > -  for (ix = 1; ix <= nargs; ix++)
> > +  /* Check how bad the best match is.  */
> > +  static_offset = 0;
> > +  if (method && TYPE_FN_FIELD_STATIC_P (fns_ptr, oload_champ))
> > +    static_offset = 1;
> > +  for (ix = 1; ix <= nargs - static_offset; ix++)
> >      {
> >        if (oload_champ_bv->rank[ix] >= 100)
> >  	oload_incompatible = 1;	/* truly mismatched types */
> > @@ -2878,6 +2877,10 @@ find_overload_match (struct type **arg_t
> >  
> >    if (method)
> >      {
> > +      if (staticp && TYPE_FN_FIELD_STATIC_P (fns_ptr, oload_champ))
> > +	*staticp = 1;
> > +      else if (staticp)
> > +	*staticp = 0;
> >        if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, oload_champ))
> >  	*valp = value_virtual_fn_field (&temp, fns_ptr, oload_champ, basetype, boffset);
> >        else
> > Index: gdbtypes.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/gdbtypes.c,v
> > retrieving revision 1.48
> > diff -u -p -r1.48 gdbtypes.c
> > --- gdbtypes.c	8 May 2002 22:58:39 -0000	1.48
> > +++ gdbtypes.c	12 May 2002 02:08:09 -0000
> > @@ -1649,9 +1649,16 @@ check_stub_method (struct type *type, in
> >    argtypes = (struct type **)
> >      TYPE_ALLOC (type, (argcount + 2) * sizeof (struct type *));
> >    p = argtypetext;
> > -  /* FIXME: This is wrong for static member functions.  */
> > -  argtypes[0] = lookup_pointer_type (type);
> > -  argcount = 1;
> > +
> > +  /* Add THIS pointer for non-static methods.  */
> > +  f = TYPE_FN_FIELDLIST1 (type, method_id);
> > +  if (TYPE_FN_FIELD_STATIC_P (f, signature_id))
> > +    argcount = 0;
> > +  else
> > +    {
> > +      argtypes[0] = lookup_pointer_type (type);
> > +      argcount = 1;
> > +    }
> >  
> >    if (*p != ')')		/* () means no args, skip while */
> >      {
> > @@ -1693,8 +1700,6 @@ check_stub_method (struct type *type, in
> >      }
> >  
> >    xfree (demangled_name);
> > -
> > -  f = TYPE_FN_FIELDLIST1 (type, method_id);
> >  
> >    TYPE_FN_FIELD_PHYSNAME (f, signature_id) = mangled_name;
> >  
> > Index: value.h
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/value.h,v
> > retrieving revision 1.30
> > diff -u -p -r1.30 value.h
> > --- value.h	11 May 2002 23:48:23 -0000	1.30
> > +++ value.h	12 May 2002 02:08:09 -0000
> > @@ -375,7 +375,7 @@ extern struct value *value_struct_elt_fo
> >  extern struct value *value_static_field (struct type *type, int fieldno);
> >  
> >  extern struct fn_field *value_find_oload_method_list (struct value **, char *,
> > -						      int, int *, int *,
> > +						      int, int *,
> >  						      struct type **, int *);
> >  
> >  extern int find_overload_match (struct type **arg_types, int nargs,
> > Index: Makefile.in
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/Makefile.in,v
> > retrieving revision 1.185
> > diff -u -p -r1.185 Makefile.in
> > --- Makefile.in	11 May 2002 23:14:25 -0000	1.185
> > +++ Makefile.in	12 May 2002 02:16:43 -0000
> > @@ -2144,7 +2144,7 @@ valarith.o: valarith.c $(bfd_h) $(defs_h
> >  	$(gdb_string_h) $(doublest_h)
> >  
> >  valops.o: valops.c $(defs_h) $(gdbcore_h) $(inferior_h) $(target_h) \
> > -	$(gdb_string_h) $(regcache_h) $(cp_abi_h)
> > +	$(gdb_string_h) $(regcache_h) $(cp_abi_h) $(gdb_assert_h)
> >  
> >  valprint.o: valprint.c $(defs_h) $(expression_h) $(gdbcmd_h) \
> >  	$(gdbcore_h) $(gdbtypes_h) $(language_h) $(symtab_h) $(target_h) \
> > 
> > 
> 
> 
> -- 
> Peter Schauer			pes@regent.e-technik.tu-muenchen.de
> 

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]