From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10731 invoked by alias); 29 Nov 2006 09:15:42 -0000 Received: (qmail 10716 invoked by uid 22791); 29 Nov 2006 09:15:38 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 29 Nov 2006 09:15:33 +0000 Received: (qmail 13128 invoked from network); 29 Nov 2006 09:15:31 -0000 Received: from unknown (HELO 172.16.unknown.plus.ru) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 29 Nov 2006 09:15:31 -0000 From: Vladimir Prus To: gdb-patches@sources.redhat.com Subject: MI/C++/references fixup Date: Wed, 29 Nov 2006 09:15:00 -0000 User-Agent: KMail/1.9.1 Cc: Nick Roberts MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_p+UbFNTOf0c6AdK" Message-Id: <200611291215.21876.vladimir@codesourcery.com> 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: 2006-11/txt/msg00373.txt.bz2 --Boundary-00=_p+UbFNTOf0c6AdK Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 1770 As noted by Nick, my varobj laziness patch has broken C++ references updates -- now -var-update does not detect the situation when a reference variable just got initialized, for example: int x = 4; int &rx = x; after executing the second line, -var-update does not say that varobj corresponding to rx has change. References were already broken, in a different way, before my change -- -var-update would always list reference variable as changed. That was because gdb was comparing undereferenced old value with dereferenced new value -- that obviously never compare equal. Also, display of references includes the address, like this: -var-evaluate-expression var1 ^done,value="@0xbff72ebc: 4" We discussed this at length some time ago: http://thread.gmane.org/gmane.comp.gdb.patches/28414/ The conclusion was that this "address prefix" is not necessary. In fact, both KDevelop and Eclipse explicitly remove it. Back then, it was decided that change would be hard. But recent varobj refactorings make this change straight-forward. Attached is the patch that: - Makes -var-update property report reference variables -- only when the referred-to value actually changed. - Removes the "@ADDRESS" output for reference variables. - Adds tests for references. The patch and the new files are attached. - Volodya * varobj.c (varobj_create): Don't call release_value. (varobj_set_value): Likewise. (install_new_value): Call coerce_ref and release_value on the value. Add asserts. testsuite/ * lib/mi-support.exp (mi_runto): Accept "()" after function name. (mi_create_varobj): New function. (mi_varobj_update): New function. (mi_Check_varobj_value): New function. * gdb.mi/mi-cpp.exp: New file. * gdb.mi/mi-cpp.cpp: New file. --Boundary-00=_p+UbFNTOf0c6AdK Content-Type: text/x-diff; charset="us-ascii"; name="00REFERENCES.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="00REFERENCES.diff" Content-length: 4582 Index: varobj.c =================================================================== RCS file: /cvs/src/src/gdb/varobj.c,v retrieving revision 1.62 diff -u -p -r1.62 varobj.c --- varobj.c 29 Nov 2006 06:41:13 -0000 1.62 +++ varobj.c 29 Nov 2006 08:52:22 -0000 @@ -512,10 +512,7 @@ varobj_create (char *objname, right type. */ value = evaluate_type (var->root->exp); - release_value (value); - var->type = value_type (value); - install_new_value (var, value, 1 /* Initial assignment */); /* Set language info */ @@ -847,9 +844,7 @@ varobj_set_value (struct varobj *var, ch with catch the exception. */ if (!gdb_value_assign (var->value, value, &val)) return 0; - - release_value (val); - + /* If the value has changed, record it, so that next -var-update can report this change. If a variable had a value of '1', we've set it to '333' and then set again to '1', when -var-update will report this @@ -901,7 +896,10 @@ varobj_list (struct varobj ***varlist) and return 0. Otherwise, assign the value and if type_changeable returns non-zero, find if the new value is different from the current value. - Return 1 if so, and 0 if the values are equal. */ + Return 1 if so, and 0 if the values are equal. + + The VALUE parameter should not be released -- the function will + take care of releasing it when needed. */ static int install_new_value (struct varobj *var, struct value *value, int initial) { @@ -917,6 +915,15 @@ install_new_value (struct varobj *var, s changeable = type_changeable (var); need_to_fetch = changeable; + /* We are not interested in address of references, and given + that in C++ reference is not rebindable, it cannot + meaningfully change. So, get hold of the real value. */ + if (value) + { + value = coerce_ref (value); + release_value (value); + } + if (var->type && TYPE_CODE (var->type) == TYPE_CODE_UNION) /* For unions, we need to fetch the value implicitly because of implementation of union member fetch. When gdb @@ -985,6 +992,8 @@ install_new_value (struct varobj *var, s value_free (var->value); var->value = value; var->updated = 0; + + gdb_assert (!var->value || value_type (var->value)); return changed; } Index: testsuite/lib/mi-support.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/lib/mi-support.exp,v retrieving revision 1.33 diff -u -p -r1.33 mi-support.exp --- testsuite/lib/mi-support.exp 24 Jul 2006 20:38:08 -0000 1.33 +++ testsuite/lib/mi-support.exp 29 Nov 2006 08:52:23 -0000 @@ -869,7 +869,7 @@ proc mi_runto {func} { set test "mi runto $func" mi_gdb_test "200-break-insert $func" \ - "200\\^done,bkpt=\{number=\"\[0-9\]+\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"$func\",file=\".*\",line=\"\[0-9\]*\",times=\"0\"\}" \ + "200\\^done,bkpt=\{number=\"\[0-9\]+\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"$func\(\\\(\\\)\)?\",file=\".*\",line=\"\[0-9\]*\",times=\"0\"\}" \ "breakpoint at $func" if {![regexp {number="[0-9]+"} $expect_out(buffer) str] @@ -998,3 +998,40 @@ proc mi0_continue_to { bkptno func args mi0_execute_to "exec-continue" "breakpoint-hit\",bkptno=\"$bkptno" \ "$func" "$args" "$file" "$line" "" "$test" } + +# Creates varobj named NAME for EXPRESSION. +# Name cannot be "-". +proc mi_create_varobj { name expression } { + mi_gdb_test "-var-create $name * $expression" \ + "\\^done,name=\"$name\",numchild=\"\[0-9\]+\",type=.*" \ + "-var-create $name * $expression" +} + +# Updates varobj named NAME and checks that all varobjs in EXPECTED +# are reported as updated, and no other varobj is updated. +# Assumes that no varobj is out of scope and that no varobj changes +# types. +proc mi_varobj_update { name expected } { + set er "\\^done,changelist=\\\[" + set first 1 + foreach item $expected { + set v "{name=\"$item\",in_scope=\"true\",type_changed=\"false\"}" + if {$first} { + set er "$er$v" + } else { + set er "$er,$v" + } + } + set er "$er\\\]" + + verbose -log "Expecting: $er" 2 + mi_gdb_test "-var-update $name" $er \ + "-var-update $name: expect $expected" +} + +proc mi_check_varobj_value { name value } { + + mi_gdb_test "-var-evaluate-expression $name" \ + "\\^done,value=\"$value\"" \ + "-var-evaluate-expression $name: expect $value" +} \ No newline at end of file --Boundary-00=_p+UbFNTOf0c6AdK Content-Type: text/plain; charset="us-ascii"; name="mi-cpp.exp" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="mi-cpp.exp" Content-length: 2116 # Copyright 2002, 2003 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 2 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, write to the Free Software # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. # Please email any bugs, comments, and/or additions to this file to: # bug-gdb@prep.ai.mit.edu if { [skip_cplus_tests] } { continue } load_lib mi-support.exp set MIFLAGS "-i=mi" gdb_exit if [mi_gdb_start] { continue } set testfile mi-cpp set srcfile "$testfile.cpp" set binfile $objdir/$subdir/$testfile if [get_compiler_info ${binfile} "c++"] { return -1; } if {[gdb_compile $srcdir/$subdir/$srcfile $binfile executable {debug c++}] != ""} { untested $testfile.exp return -1 } mi_gdb_load ${binfile} # Test that children of classes are properly reported # Run to main mi_runto reference_update_tests mi_create_varobj "RX" "rx" set x_assignment [gdb_get_line_number "x = 567;"] mi_next_to "reference_update_tests" {} ".*${srcfile}" [expr $x_assignment-1] \ "step to x assignment" mi_next_to "reference_update_tests" {} ".*${srcfile}" [expr $x_assignment] \ "step to x assignment" mi_varobj_update RX {RX} mi_check_varobj_value RX 167 # Execute the first 'x = 567' line. mi_next_to "reference_update_tests" {} ".*${srcfile}" [expr $x_assignment+1] \ "step to x assignment" mi_varobj_update RX {RX} mi_check_varobj_value RX 567 # Execute the second 'x = 567' line. mi_next_to "reference_update_tests" {} ".*${srcfile}" [expr $x_assignment+2] \ "step to x assignment" mi_varobj_update RX {} mi_gdb_exit return 0 --Boundary-00=_p+UbFNTOf0c6AdK Content-Type: text/x-c++src; charset="us-ascii"; name="mi-cpp.cpp" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="mi-cpp.cpp" Content-length: 146 void reference_update_tests () { int x = 167; int& rx = x; x = 567; x = 567; } int main () { reference_update_tests (); return 0; } --Boundary-00=_p+UbFNTOf0c6AdK--