From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24847 invoked by alias); 19 Aug 2008 18:05:51 -0000 Received: (qmail 24780 invoked by uid 22791); 19 Aug 2008 18:05:50 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.33.17) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 19 Aug 2008 18:04:54 +0000 Received: from zps75.corp.google.com (zps75.corp.google.com [172.25.146.75]) by smtp-out.google.com with ESMTP id m7JI4iH3022754 for ; Tue, 19 Aug 2008 19:04:45 +0100 Received: from wa-out-1112.google.com (wahv33.prod.google.com [10.114.248.33]) by zps75.corp.google.com with ESMTP id m7JI4h6s016488 for ; Tue, 19 Aug 2008 11:04:44 -0700 Received: by wa-out-1112.google.com with SMTP id v33so14432wah.24 for ; Tue, 19 Aug 2008 11:04:43 -0700 (PDT) Received: by 10.114.159.5 with SMTP id h5mr6972292wae.222.1219169082916; Tue, 19 Aug 2008 11:04:42 -0700 (PDT) Received: by 10.114.122.10 with HTTP; Tue, 19 Aug 2008 11:04:42 -0700 (PDT) Message-ID: <8ac60eac0808191104r38efbd1fi28db42c05ae04d1d@mail.gmail.com> Date: Tue, 19 Aug 2008 18:05:00 -0000 From: "Paul Pluzhnikov" To: "Tom Tromey" Subject: Re: [RFC] [patch] 'p->x' vs. 'p.x' and 'print object on' Cc: gdb-patches@sourceware.org In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20080717214839.6AE253A67B6@localhost> <8ac60eac0807301050id1051q8072925c0d11b96d@mail.gmail.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-08/txt/msg00525.txt.bz2 On Mon, Aug 11, 2008 at 8:08 AM, Tom Tromey wrote: >>>>>> "Paul" == Paul Pluzhnikov 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 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 * 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 . + +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\." ""