[PATCH] Resolve xmethods over dynamic types

Siva Chandra sivachandra@google.com
Fri Oct 31 19:50:00 GMT 2014


[I am not sure if the title is really appropriate.]

Currently, if an xmethod is defined for a virtual method, then the
dynamic type of the object is ignored. The attached patch removes this
limitation. That is, if the xmethod of the static type has an equal
method in C++ which is virtual, then further look up is done to check
if the dynamic type also has an xmethod for the virtual method. If so,
then the xmethod of the dynamic type is invoked.

[Note that xmethods cannot be virtual.]

gdb/ChangeLog:

2014-10-31  Siva Chandra Reddy  <sivachandra@google.com>

        * valops.c (find_overload_match): Lookup xmethods on the
        dynamic type if the best match is a virtual method.
        (value_has_indirect_dynamic_type, cast_args_to_param_types)
        (equal_param_types_p, derived_hides_base_method): New functions.

gdb/testsuite/ChangeLog:

2014-10-31  Siva Chandra Reddy  <sivachandra@google.com>

        * gdb.python/py-xmethods.cc: New test cases.
        * gdb.python/py-xmethods.exp: New tests.
        * gdb.python/py-xmethods.py: New XMethods.
-------------- next part --------------
diff --git a/gdb/testsuite/gdb.python/py-xmethods.cc b/gdb/testsuite/gdb.python/py-xmethods.cc
index 96389fd..bb374f5 100644
--- a/gdb/testsuite/gdb.python/py-xmethods.cc
+++ b/gdb/testsuite/gdb.python/py-xmethods.cc
@@ -27,6 +27,8 @@ public:
   int operator+ (const A &obj);
   int operator- (const A &obj);
   virtual int geta (void);
+  virtual int adda (int);
+  virtual int suba (int);
 };
 
 A::~A () { }
@@ -58,16 +60,38 @@ A::geta (void)
   return a;
 }
 
+int a_adda = 0;
+
+int
+A::adda (int i)
+{
+  a_adda++;
+  return a + i;
+}
+
+int a_suba = 0;
+
+int
+A::suba (int i)
+{
+  a_suba++;
+  return a - i;
+}
+
 class B : public A
 {
 public:
-  virtual int geta (void);
+
+  /* This class implements the virtual method geta but there is no
+     xmethod for this function.  Hence, when called from GDB on an object
+     whose dynamic type is B, B's C++ version should be called.  */
+  virtual int geta ();
 };
 
 int b_geta = 0;
 
 int
-B::geta (void)
+B::geta ()
 {
   b_geta++;
   return 2 * a;
@@ -77,6 +101,57 @@ typedef B Bt;
 
 typedef Bt Btt;
 
+class C : public A
+{
+public:
+
+  /* This class implements the virtual method geta and there is also
+     an xmethod available for it.  Hence, when called from GDB on a , the
+     xmethod for c::geta should be called.  */
+  virtual int geta (void);
+};
+
+int c_geta = 0;
+
+int
+C::geta (void)
+{
+  c_geta++;
+  return 3 * a;
+}
+
+class D : public A
+{
+public:
+  /* This function is virtual but does not override the base class' adda
+     as the arg type is different.  Hence, even if this class has an xmethod
+     for adda which takes an int argument (and not unsigned int), A::adda (or
+     its xmethod) should be called even if the dynamic type of the object is D.
+     This is because xmethods cannot be virtual.  */
+  virtual int adda (unsigned int);
+
+  /* Same here.  */
+  virtual int suba (unsigned int);
+};
+
+int d_adda = 0;
+
+int
+D::adda (unsigned int i)
+{
+  d_adda++;
+  return a + i;
+}
+
+int d_suba = 0;
+
+int
+D::suba (unsigned int i)
+{
+  d_suba++;
+  return a - i;
+}
+
 class E : public A
 {
 public:
@@ -151,6 +226,14 @@ int main(void)
   b1.a = 30;
   A *a_ptr = &b1;
 
+  C c1;
+  c1.a = 123;
+  A *a1_ptr = &c1;
+
+  D d1;
+  d1.a = 456;
+  A *a2_ptr = &d1;
+
   Bt bt;
   bt.a = 40;
 
diff --git a/gdb/testsuite/gdb.python/py-xmethods.exp b/gdb/testsuite/gdb.python/py-xmethods.exp
index 093b4bc..888f6ac 100644
--- a/gdb/testsuite/gdb.python/py-xmethods.exp
+++ b/gdb/testsuite/gdb.python/py-xmethods.exp
@@ -52,6 +52,26 @@ gdb_test "p a_minus_a" ".* = 2" "Before: a_minus_a 2"
 gdb_test "p a1.geta()" ".* = 5" "Before: a1.geta()"
 gdb_test "p a_geta" ".* = 1" "Before: a_geta 1"
 
+gdb_test "p a1.adda(340)" ".* = 345" "Before: a1.adda()"
+gdb_test "p a_adda" ".* = 1" "Before: a_adda 1"
+
+gdb_test "p a1.suba(1)" ".* = 4" "Before: a1.suba()"
+gdb_test "p a_suba" ".* = 1" "Before: a_suba 1"
+
+gdb_test "p d1.adda(111)" ".* = 567" "Before: a1.adda()"
+gdb_test "p d_adda" ".* = 1" "Before: d_adda 1"
+
+gdb_test "p d1.suba(111)" ".* = 345" "Before: d1.suba()"
+gdb_test "p d_suba" ".* = 1" "Before: d_suba 1"
+
+gdb_test "p a2_ptr->adda(111)" ".* = 567" \
+  "After: a2_ptr->adda()"
+gdb_test "p a_adda" ".* = 2" "Before: a_adda 2"
+
+gdb_test "p a2_ptr->suba(111)" ".* = 345" \
+  "After: a2_ptr->suba()"
+gdb_test "p a_suba" ".* = 2" "Before: a_suba 2"
+
 gdb_test "p ++a1" "No symbol.*" "Before: ++a1"
 gdb_test "p a1.getarrayind(5)" "Couldn't find method.*" \
   "Before: a1.getarrayind(5)"
@@ -100,10 +120,32 @@ gdb_test "p a1.geta()" "From Python <A_geta>.*5" "After: a1.geta()"
 gdb_test "p ++a1" "From Python <plus_plus_A>.*6" "After: ++a1"
 gdb_test "p a1.getarrayind(5)" "From Python <A_getarrayind>.*5" \
   "After: a1.getarrayind(5)"
-# Note the following test.  Xmethods on dynamc types are not looked up
-# currently.  Hence, even though a_ptr points to a B object, the xmethod
-# defined for A objects is invoked.
-gdb_test "p a_ptr->geta()" "From Python <A_geta>.*30" "After: a_ptr->geta()"
+
+# Even though class A has an xmethod for "geta", since the dynamic type
+# of a_ptr is B, B's C++ implementation of "geta" is called as B's "geta"
+# does not have an xmethod and it is virtual.
+gdb_test "p a_ptr->geta()" ".* = 60" "After: a_ptr->geta()"
+gdb_test "p b_geta" ".* = 2" "After: b_geta 2"
+
+# a1_ptr points to a C object.  Also, C has an xmethod for "geta".  Hence,
+# as it is a virtual method, C's xmethod for "geta" should be invoked.
+gdb_test "p a1_ptr->geta()" "From Python <C_geta>:.*123" "After: a1_ptr->geta()"
+gdb_test "p c_geta" ".* = 0" "After: c_geta 0"
+
+gdb_test "p a1.adda(340)" "From Python <A_adda>:.*345" "After: a1.adda()"
+gdb_test "p d1.adda(-111)" "From Python <D_adda>:.*345" "After: d1.adda()"
+gdb_test "p d1.suba(-111)" "From Python <D_suba>:.*567" "After: d1.suba()"
+
+# a2_ptr points to a D object.  However, D does not implement "adda" but hides
+# it.  In such a case, A's virtual method should be invoked.  Since A's virtual
+# method has an xmethod, that is invoked.  Same behavior should be seen with
+# "suba" but since A's suba does not have an xmethod, the C++ implementation is
+# invoked.
+gdb_test "p a2_ptr->adda(111)" "From Python <A_adda>:.*567" \
+  "After: a2_ptr->adda()"
+gdb_test "p a2_ptr->suba(111)" ".* = 345" "After: a2_ptr->suba()"
+gdb_test "p a_suba" ".* = 3" "After: a_suba 3"
+
 gdb_test "p e.geta()" "From Python <A_geta>.*100" "After: e.geta()"
 gdb_test "p e_ptr->geta()" "From Python <A_geta>.*100" "After: e_ptr->geta()"
 gdb_test "p e_ref.geta()" "From Python <A_geta>.*100" "After: e_ref.geta()"
diff --git a/gdb/testsuite/gdb.python/py-xmethods.py b/gdb/testsuite/gdb.python/py-xmethods.py
index 47fb00b..e100cf1 100644
--- a/gdb/testsuite/gdb.python/py-xmethods.py
+++ b/gdb/testsuite/gdb.python/py-xmethods.py
@@ -39,10 +39,26 @@ def A_geta(obj):
   return obj['a']
 
 
+def A_adda(obj, i):
+  print ('From Python <A_adda>:')
+  return obj['a'] + i
+
 def A_getarrayind(obj, index):
   print('From Python <A_getarrayind>:')
   return obj['array'][index]
 
+def C_geta(obj):
+  print('From Python <C_geta>:')
+  return obj['a']
+
+def D_adda(obj, i):
+  print ('From Python <D_adda>:')
+  return obj['a'] + i
+
+def D_suba(obj, i):
+  print ('From Python <D_suba>:')
+  return obj['a'] - i
+
 
 type_A = gdb.parse_and_eval('(dop::A *) 0').type.target()
 type_B = gdb.parse_and_eval('(dop::B *) 0').type.target()
@@ -203,11 +219,30 @@ global_dm_list = [
                          '^dop::A$',
                          '^geta$',
                          A_geta),
+    SimpleXMethodMatcher('A_adda',
+                         '^dop::A$',
+                         '^adda$',
+                         A_adda,
+                         type_int),
     SimpleXMethodMatcher('A_getarrayind',
                          '^dop::A$',
                          '^getarrayind$',
                          A_getarrayind,
                          type_int),
+    SimpleXMethodMatcher('C_geta',
+                         '^dop::C$',
+                         '^geta$',
+                         C_geta),
+    SimpleXMethodMatcher('D_adda',
+                         '^dop::D$',
+                         '^adda$',
+                         D_adda,
+                         type_int),
+    SimpleXMethodMatcher('D_suba',
+                         '^dop::D$',
+                         '^suba$',
+                         D_suba,
+                         type_int),
 ]
 
 for matcher in global_dm_list:
diff --git a/gdb/valops.c b/gdb/valops.c
index 7f3e4f5..c08f13c 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2414,6 +2414,117 @@ value_find_oload_method_list (struct value **argp, const char *method,
 		    basetype, boffset);
 }
 
+/* Return the dynamic type of OBJ.  NULL is returned if OBJ does not have any
+   dynamic type.  */
+
+static struct type *
+value_has_indirect_dynamic_type (struct value *obj)
+{
+  struct type *stype, *dtype, *dtype_ind;
+
+  stype = check_typedef (TYPE_TARGET_TYPE (value_type (obj)));
+  dtype_ind = value_rtti_indirect_type (obj, NULL, NULL, NULL);
+  dtype = dtype_ind ? check_typedef (TYPE_TARGET_TYPE (dtype_ind)) : stype;
+
+  if (class_types_same_p (stype, dtype))
+    return NULL;
+  else
+    return dtype_ind;
+}
+
+/* Casts the arguments in the array ARGS to the types of the parameters of
+   the M-th method in FNS_PTR.  The length of the array ARGS is given by
+   NARGS.  If SKIP_THIS is 1, then the first argument ARGS[0] is skipped
+   assuming that it is the C++ "this" pointer.  If it is 0, then the first
+   argument is not skipped.  Passing any other value for SKIP_THIS is an error.
+   This is a helper function for find_overload_match.  */
+
+static void
+cast_args_to_param_types (struct value **args, int nargs,
+			  struct fn_field *fns_ptr, int m, int skip_this)
+{
+  int i;
+
+  gdb_assert (TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr, m)) == nargs);
+  gdb_assert (skip_this == 0 || skip_this == 1);
+
+  for (i = skip_this; i < nargs; i++)
+    {
+      struct type *param_type = TYPE_FN_FIELD_ARGS (fns_ptr, m)[i].type;
+      struct type *arg_type = check_typedef (value_type (args[i]));
+
+      CHECK_TYPEDEF (param_type);
+      if (TYPE_CODE (param_type) == TYPE_CODE_REF
+	  && TYPE_CODE (arg_type) != TYPE_CODE_REF)
+	param_type = TYPE_TARGET_TYPE (param_type);
+
+      args[i] = value_cast (param_type, args[i]);
+    }
+}
+
+/* Checks if the N1-th method in FNS_PTR1 has exactly the same parameters
+   as that of the N2-th method in FNS_PTR2.
+   Returns 1 is equal, zero otherwise.  */
+
+static int
+equal_param_types_p (struct fn_field *fns_ptr1, int n1,
+		     struct fn_field *fns_ptr2, int n2)
+{
+  int i;
+
+  if (TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr1, n1))
+      != TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr2, n2)))
+    return 0;
+
+  for (i = 1; i < TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr1, n1)); i++)
+    {
+      struct type *type1 = TYPE_FN_FIELD_ARGS (fns_ptr1, n1)[i].type;
+      struct type *type2 = TYPE_FN_FIELD_ARGS (fns_ptr2, n2)[i].type;
+
+      if (!types_deeply_equal (type1, type2))
+	return 0;
+    }
+
+  return 1;
+}
+
+/* Checks if the derived type DTYPE hides a method NAME of its base class.
+   The base class method is the M-th method in FNS_PTR.  */
+
+static int
+derived_hides_base_method (struct type *dtype, const char *name,
+			   struct fn_field *fns_ptr, int m)
+{
+  int i, ret_val = 0;
+
+  dtype = check_typedef (dtype);
+  gdb_assert (TYPE_CODE (dtype) == TYPE_CODE_STRUCT);
+
+  for (i = 0; i < TYPE_NFN_FIELDS (dtype); i++)
+    {
+      const char *fn_field_name = TYPE_FN_FIELDLIST_NAME (dtype, i);
+
+      if (fn_field_name && (strcmp_iw (fn_field_name, name) == 0))
+	{
+	  int j;
+
+	  /* If a method with the same name is found in the dynamic type,
+	     then assume that it hides the base method until a method with
+	     matching param types is found.  */
+	  ret_val = 1;
+
+	  for (j = 0; j < TYPE_FN_FIELDLIST_LENGTH (dtype, i); j++)
+	    {
+	      if (equal_param_types_p (TYPE_FN_FIELDLIST1 (dtype, i), j,
+				       fns_ptr, m))
+		return 0;
+	    }
+	}
+    }
+
+  return ret_val;
+}
+
 /* Given an array of arguments (ARGS) (which includes an
    entry for "this" in the case of C++ methods), the number of
    arguments NARGS, the NAME of a function, and whether it's a method or
@@ -2474,6 +2585,7 @@ find_overload_match (struct value **args, int nargs,
   int func_oload_champ = -1;
   int method_oload_champ = -1;
   int src_method_oload_champ = -1;
+  int src_method_oload_champ_orig = -1;
   int ext_method_oload_champ = -1;
   int src_and_ext_equal = 0;
 
@@ -2596,6 +2708,10 @@ find_overload_match (struct value **args, int nargs,
 	      case 2: /* Ext method is champion.  */
 		method_oload_champ = ext_method_oload_champ;
 		method_badness = ext_method_badness;
+		/* We save the source overload champ index so that it can be
+		   used to determine whether the source method is virtual
+		   later in this function.  */
+		src_method_oload_champ_orig = src_method_oload_champ;
 		src_method_oload_champ = -1;
 		method_match_quality = ext_method_match_quality;
 		break;
@@ -2775,9 +2891,55 @@ find_overload_match (struct value **args, int nargs,
 	  if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ)
 	      && noside != EVAL_AVOID_SIDE_EFFECTS)
 	    {
-	      *valp = value_virtual_fn_field (&temp, fns_ptr,
-					      method_oload_champ, basetype,
-					      boffset);
+	      struct type *dtype;
+
+	      dtype = value_has_indirect_dynamic_type (args[0]);
+	      /* Look for better methods in the dynamic type only if the
+		 dynamic type does not hide the base class virtual method.  */
+	      if (dtype != NULL
+		  && !derived_hides_base_method (TYPE_TARGET_TYPE (dtype),
+						 name, fns_ptr,
+						 method_oload_champ))
+		{
+		  /* If the object has a dynamic type, then look for matching
+		     methods for its dynamic type.  */
+		  args[0] = value_cast (dtype, args[0]);
+		  do_cleanups (all_cleanups);
+		  /* Even if the derived class does not hide the base class
+		     method, it could define another method with the same name
+		     but with compatible parameters.  Avoid the chance of
+		     picking up the wrong function by explicitly casting the
+		     args to the virtual function param types.
+
+		     Example:
+
+		     class base
+		     {
+		     public:
+		       virtual int foo (char i);
+		     };
+
+		     class derived : public base
+		     {
+		     public:
+		       virtual int foo (char i);
+		       int foo (int i);
+		     };
+
+		     If the arg to base::foo was as int, then looking for
+		     matching methods in the dynamic object will match foo(int)
+		     and not foo (char) even though the derived class overrides
+		     (but not hide) the base class virtual method.  */
+		  cast_args_to_param_types (args, nargs, fns_ptr,
+					    method_oload_champ, 1);
+		  return find_overload_match (args, nargs, name, method,
+					      &args[0], fsym, valp, symp,
+					      staticp, no_adl, noside);
+		}
+	      else
+		*valp = value_virtual_fn_field (&temp, fns_ptr,
+						method_oload_champ, basetype,
+						boffset);
 	    }
 	  else
 	    *valp = value_fn_field (&temp, fns_ptr, method_oload_champ,
@@ -2785,6 +2947,32 @@ find_overload_match (struct value **args, int nargs,
 	}
       else
 	{
+	  /* Xmethods cannot be virtual.  However, if an xmethod is as
+	     good as the source method, and if the source method is virtual, we
+	     should look for the possibility of better matching methods defined
+	     for the dynamic type of the object.  */
+	  if (src_and_ext_equal
+	      && TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, src_method_oload_champ_orig)
+	      && noside != EVAL_AVOID_SIDE_EFFECTS)
+	    {
+	      struct type *dtype;
+
+	      dtype = value_has_indirect_dynamic_type (args[0]);
+	      if (dtype != NULL
+		  && !derived_hides_base_method (TYPE_TARGET_TYPE (dtype),
+						 name, fns_ptr,
+						 src_method_oload_champ_orig))
+		{
+		  args[0] = value_cast (dtype, args[0]);
+		  do_cleanups (all_cleanups);
+		  cast_args_to_param_types (args, nargs, fns_ptr,
+					    src_method_oload_champ_orig, 1);
+		  return find_overload_match (args, nargs, name, method,
+					      &args[0], fsym, valp, symp,
+					      staticp, no_adl, noside);
+		}
+	    }
+
 	  *valp = value_of_xmethod (clone_xmethod_worker
 	    (VEC_index (xmethod_worker_ptr, xm_worker_vec,
 			ext_method_oload_champ)));


More information about the Gdb-patches mailing list