Re: MI/C++/references fixup

[Vladimir Prus <vladimir@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 1649 bytes --]

On Wednesday 29 November 2006 17:24, Daniel Jacobowitz wrote:
> On Wed, Nov 29, 2006 at 05:15:04PM +0300, Vladimir Prus wrote:
> > > > +  /* 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.  */
> > >
> > > in the address of a reference, in C++ a reference.
> > >
> > > In C++ it can't meaningfully change.  In a program, though, it can;
> > > once when it's initialized, and again if something scribbles on the
> > > stack.  And that might be what you're trying to debug.  So, I'm
> > > a little wary of this; it seems to me that we ought to check for both
> > > changes in the address and value (sort of like we do for watchpoints).
> >
> > In practice, if the address changes, the value also changes, so the user
> > can notice. Second, if user really wants to get the address, he can do
> > that with "&whatever".
>
> I suppose that's true.  Want to post an updated patch, and we'll see if
> anyone has a reason to keep it?  We're leaving the CLI as it was, this
> time.

Here's it.

- 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.

[-- Attachment #2: 00REFERENCES.diff --]
[-- Type: text/x-diff, Size: 4562 bytes --]

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 14:37:11 -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 the address of references, and given
+     that in C++ a 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 14:37:12 -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"
+}

[-- Attachment #3: mi-cpp.cpp --]
[-- Type: text/x-c++src, Size: 915 bytes --]

/* Copyright 2006 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.  
*/

void reference_update_tests ()
{
  int x = 167;
  int& rx = x;
  x = 567;
  x = 567;
}

int main ()
{
  reference_update_tests ();
  return 0;
}

[-- Attachment #4: mi-cpp.exp --]
[-- Type: text/plain, Size: 2014 bytes --]

# Copyright 2006 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.  

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