This is the mail archive of the gdb-patches@sourceware.org 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: [RFC] [patch] 'p->x' vs. 'p.x' and 'print object on'


On Mon, Aug 11, 2008 at 8:08 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:
>
> Paul> C) Do what the language does: lookup field 'x' in the static type,
> Paul>    and only try dynamic type if the first lookup failed:
>
> It occurred to me last night that we must be careful not to do this in
> the overload resolution case.

Actually, I think there are 2 separate cases: virtual and non-virtual
function.

We must not do this in the non-virtual case, but *should* do that
in virtual case. IOW, 'print basep->virtfn()' and 'print
basep.virtfn()' should be the same for consistency.

> Looking at extra overloads from the
> dynamic type will yield the wrong answer.  I think these code paths
> are separate in gdb, so that should not be a big deal, but I thought
> it would be good to be explicit about it.

I've added tests for these cases.

Unfortunately, as I was adding tests, I discovered 4 new bugs :(
The patch does not introduce any new failures for me.

-- 
Paul Pluzhnikov


gdb/ChangeLog

2008-08-19  Paul Pluzhnikov  <ppluzhnikov@google.com>

	Changes to treat 'p.x' and 'p->x' the same.
	* eval.c (value_static_or_dynamic_member): New.
	(evaluate_subexp_standard): Call it.
	
gdb/testsuite/ChangeLog

2008-08-19  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.cp/class3.exp: New test case.
	* gdb.cp/class3.cc: New source file.
	
diff -Npur gdb.orig/eval.c gdb/eval.c
--- gdb.orig/eval.c	2008-08-19 09:48:39.000000000 -0700
+++ gdb/eval.c	2008-08-19 10:14:04.000000000 -0700
@@ -44,10 +44,6 @@
 /* This is defined in valops.c */
 extern int overload_resolution;

-/* JYG: lookup rtti type of STRUCTOP_PTR when this is set to continue
-   on with successful lookup for member/method of the rtti type. */
-extern int objectprint;
-
 /* Prototypes for local functions. */

 static struct value *evaluate_subexp_for_sizeof (struct expression *, int *);
@@ -438,6 +434,51 @@ value_f90_subarray (struct value *array,
   return value_slice (array, low_bound, high_bound - low_bound + 1);
 }

+static struct value *
+value_static_or_dynamic_member(struct value *arg, char *string,
+			       char *name, enum noside noside)
+{
+  struct type *type = value_type (arg);
+  struct value *temp;
+
+  if (noside == EVAL_AVOID_SIDE_EFFECTS)
+    return value_zero (lookup_struct_elt_type (type, string, 0),
+		       lval_memory);
+  temp = arg;
+  {
+    volatile struct gdb_exception except;
+    struct value *v = NULL;
+    TRY_CATCH (except, RETURN_MASK_ERROR)
+      {
+	v = value_struct_elt (&temp, NULL, string, NULL, name);
+      }
+    if (v)
+      return v;
+  }
+
+  /* If we got here, value_struct_elt() above must have thrown,
+     and there is no field 'name' in 'type'.
+     Try dynamic type of 'arg' instead. */
+
+  if (TYPE_TARGET_TYPE(type)
+      && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_CLASS))
+    {
+      struct type *real_type;
+      int full, top, using_enc;
+      real_type = value_rtti_target_type (arg, &full, &top, &using_enc);
+      if (real_type)
+	{
+	  if (TYPE_CODE (type) == TYPE_CODE_PTR)
+	    real_type = lookup_pointer_type (real_type);
+	  else
+	    real_type = lookup_reference_type (real_type);
+
+	  temp = arg = value_cast (real_type, arg);
+	}
+    }
+  return value_struct_elt (&temp, NULL, string, NULL, name);
+}
+
 struct value *
 evaluate_subexp_standard (struct type *expect_type,
 			  struct expression *exp, int *pos,
@@ -1374,23 +1415,6 @@ evaluate_subexp_standard (struct type *e
       return value_literal_complex (arg1, arg2, builtin_type_f_complex_s16);

     case STRUCTOP_STRUCT:
-      tem = longest_to_int (exp->elts[pc + 1].longconst);
-      (*pos) += 3 + BYTES_TO_EXP_ELEM (tem + 1);
-      arg1 = evaluate_subexp (NULL_TYPE, exp, pos, noside);
-      if (noside == EVAL_SKIP)
-	goto nosideret;
-      if (noside == EVAL_AVOID_SIDE_EFFECTS)
-	return value_zero (lookup_struct_elt_type (value_type (arg1),
-						   &exp->elts[pc + 2].string,
-						   0),
-			   lval_memory);
-      else
-	{
-	  struct value *temp = arg1;
-	  return value_struct_elt (&temp, NULL, &exp->elts[pc + 2].string,
-				   NULL, "structure");
-	}
-
     case STRUCTOP_PTR:
       tem = longest_to_int (exp->elts[pc + 1].longconst);
       (*pos) += 3 + BYTES_TO_EXP_ELEM (tem + 1);
@@ -1398,41 +1422,10 @@ evaluate_subexp_standard (struct type *e
       if (noside == EVAL_SKIP)
 	goto nosideret;

-      /* JYG: if print object is on we need to replace the base type
-	 with rtti type in order to continue on with successful
-	 lookup of member / method only available in the rtti type. */
-      {
-        struct type *type = value_type (arg1);
-        struct type *real_type;
-        int full, top, using_enc;
-
-        if (objectprint && TYPE_TARGET_TYPE(type) &&
-            (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_CLASS))
-          {
-            real_type = value_rtti_target_type (arg1, &full, &top, &using_enc);
-            if (real_type)
-              {
-                if (TYPE_CODE (type) == TYPE_CODE_PTR)
-                  real_type = lookup_pointer_type (real_type);
-                else
-                  real_type = lookup_reference_type (real_type);
-
-                arg1 = value_cast (real_type, arg1);
-              }
-          }
-      }
-
-      if (noside == EVAL_AVOID_SIDE_EFFECTS)
-	return value_zero (lookup_struct_elt_type (value_type (arg1),
-						   &exp->elts[pc + 2].string,
-						   0),
-			   lval_memory);
-      else
-	{
-	  struct value *temp = arg1;
-	  return value_struct_elt (&temp, NULL, &exp->elts[pc + 2].string,
-				   NULL, "structure pointer");
-	}
+      return value_static_or_dynamic_member(arg1, &exp->elts[pc + 2].string,
+					    (op == STRUCTOP_STRUCT)
+					    ? "structure" : "structure pointer",
+					    noside);

     case STRUCTOP_MEMBER:
     case STRUCTOP_MPTR:
diff -Npur gdb.orig/testsuite/gdb.cp/class3.cc gdb/testsuite/gdb.cp/class3.cc
--- gdb.orig/testsuite/gdb.cp/class3.cc	1969-12-31 16:00:00.000000000 -0800
+++ gdb/testsuite/gdb.cp/class3.cc	2008-08-19 10:06:13.000000000 -0700
@@ -0,0 +1,64 @@
+struct Base {
+  int x, y;
+  Base() : x(1), y(2) { }
+  virtual ~Base() { }
+          int  fn() { return x + y; }
+  virtual int vfn() { return x + y; }
+};
+
+struct D1 : public Base {
+  D1() : y(20), d1(30) { }
+          int  fn() { return x + y; }
+  virtual int vfn() { return x + y; }
+  int y, d1;
+};
+
+
+struct D2 : public Base {
+  D2() : y(200), d2(300) { }
+          int  fn() { return x + y; }
+  virtual int vfn() { return x + y; }
+  int y, d2;
+};
+
+struct D2v : public virtual Base {
+  D2v() : y(220), d2v(330) { }
+          int  fn() { return x + y; }
+  virtual int vfn() { return x + y; }
+  int y, d2v;
+};
+
+struct D3: public virtual D1, public virtual D2 {
+  D3() : y(2000), d3(3000) { }
+          int  fn() { return this->D1::x + y; }
+  virtual int vfn() { return this->D1::x + y; }
+  D1 *getD1() { return (D1*)this; }
+  D2 *getD2() { return (D2*)this; }
+  int y, d3;
+};
+
+int fn(Base *b)
+{
+  return b->y;
+}
+
+int main()
+{
+  Base b;
+  D1  d1;
+  D2  d2;
+  D2v d2v;
+  D3  d3;
+  return (fn(&b)            == 2
+	  && fn(&d1)        == 2
+	  && fn(&d2)        == 2
+	  && fn(&d2v)       == 2
+	  && fn(d3.getD1()) == 2
+	  && fn(d3.getD2()) == 2) ? 0 : 1;
+}
+
+int no_optimize(Base *b, D1 *d1, D2 *d2, D3 *d3)
+{
+  return b->fn() + d1->fn() + d2->fn() + d3->fn() +
+    b->vfn() + d1->vfn() + d2->vfn() + d3->vfn();
+}
diff -Npur gdb.orig/testsuite/gdb.cp/class3.exp gdb/testsuite/gdb.cp/class3.exp
--- gdb.orig/testsuite/gdb.cp/class3.exp	1969-12-31 16:00:00.000000000 -0800
+++ gdb/testsuite/gdb.cp/class3.exp	2008-08-19 10:12:52.000000000 -0700
@@ -0,0 +1,114 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+    strace $tracelevel
+    }
+
+if { [skip_cplus_tests] } { continue }
+
+set prms_id 0
+set bug_id 0
+
+set testfile "class3"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+# Create and source the file that provides information about the compiler
+# used to compile the test case.
+if [get_compiler_info ${binfile} "c++"] {
+    return -1
+}
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
executable {debug c++}] != "" } {
+    untested ${testfile}.exp
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# ARGS is a list of quads:
+#   field
+#   its expected value
+#   whether b.field  is a known failure
+#   whether b->field is a known failure
+proc one_test {name args} {
+    foreach on_off {"on" "off"} {
+	gdb_test "set print object $on_off" ""
+	foreach extra $args {
+	    set f [lindex $extra 0]
+	    set v [lindex $extra 1]
+	    set kfail1 [lindex $extra 2]
+	    set kfail2 [lindex $extra 3]
+
+	    if {$kfail1 != ""} {
+		setup_kfail "*-*-*" $kfail1
+	    }
+	    gdb_test "print b.$f"  ".* = $v" "$name b.$f (print object $on_off)"
+
+	    if {$kfail2 != ""} {
+		setup_kfail "*-*-*" $kfail2
+	    }
+	    gdb_test "print b->$f" ".* = $v" "$name b->$f (print object $on_off)"
+	}
+    }
+}
+
+
+if ![runto_main] then {
+    perror "couldn't run to main"
+    continue
+}
+
+gdb_test "break fn" \
+    "Breakpoint.*at.* file .*" ""
+
+gdb_test "continue" "Breakpoint .* at .*" ""
+
+# Looking at Base*
+one_test "Base" {x 1} {y 2} { "fn()" 3 "gdb/NNN0" } \
+    { "vfn()" 3 "gdb/NNN0" }
+gdb_test "continue" "Breakpoint .* at .*" ""
+
+# Looking at D1*
+one_test "D1" {x 1} {y 2} {"d1" 30} {"D1::y" 20 "gdb/NNN1"} \
+    { "fn()" 3 "gdb/NNN0" } { "vfn()" 21 "gdb/NNN0" }
+gdb_test "continue" "Breakpoint .* at .*" ""
+
+# Looking at D2*
+one_test "D2" {x 1} {y 2} {"d2" 300} {"D2::y" 200 "gdb/NNN1"} \
+    { "fn()" 3 "gdb/NNN0" } { "vfn()" 201 "gdb/NNN0" }
+gdb_test "continue" "Breakpoint .* at .*" ""
+
+# Looking at D2v*
+one_test "D2v" {x 1} {y 2} {"d2v" 330 "gdb/NNN2" "gdb/NNN2"} \
+    {"D2::y" 220 "gdb/NNN2" "gdb/NNN2"} \
+    { "fn()" 3 "gdb/NNN0" } { "vfn()" 221 "gdb/NNN0" }
+gdb_test "continue" "Breakpoint .* at .*" ""
+
+# Looking at D1* part of D3
+one_test "D3::D1" {x 1} {y 2} {"d1" 30 "gdb/NNN3" "gdb/NNN3"} \
+    {"D1::y" 20 "gdb/NNN3" "gdb/NNN3"} \
+    { "fn()" 3 "gdb/NNN0" } { "vfn()" 21 "gdb/NNN0" "gdb/NNN0a" }
+gdb_test "continue" "Breakpoint .* at .*" ""
+
+# Looking at D2* part of D3
+one_test "D3::D2" {x 1} {y 2} {"d2" 300 "gdb/NNN3" "gdb/NNN3"} \
+    {"D2::y" 200 "gdb/NNN3" "gdb/NNN3"} \
+    { "fn()" 3 "gdb/NNN0" } { "vfn()" 201 "gdb/NNN0" "gdb/NNN0a" }
+gdb_test "continue" "Program exited normally\." ""


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