This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] [patch] 'p->x' vs. 'p.x' and 'print object on'
- From: "Paul Pluzhnikov" <ppluzhnikov at google dot com>
- To: "Tom Tromey" <tromey at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 19 Aug 2008 11:04:42 -0700
- Subject: Re: [RFC] [patch] 'p->x' vs. 'p.x' and 'print object on'
- References: <20080717214839.6AE253A67B6@localhost> <m3mykapwo5.fsf@fleche.redhat.com> <8ac60eac0807301050id1051q8072925c0d11b96d@mail.gmail.com> <m3d4kfa8sg.fsf@fleche.redhat.com>
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\." ""