This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA] Fix calling of static C++ member functions
- From: "Peter.Schauer" <Peter dot Schauer at regent dot e-technik dot tu-muenchen dot de>
- To: drow at mvista dot com (Daniel Jacobowitz)
- Cc: gdb-patches at sources dot redhat dot com
- Date: Sun, 12 May 2002 21:48:56 MET DST
- Subject: Re: [RFA] Fix calling of static C++ member functions
> 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:
--- ./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