Re: [Patch] [MI] Out-of-scope varObjects no longer trigger a var-update change

[Vladimir Prus <vladimir@codesourcery.com>]

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

Marc Khouzam wrote:

> Hi,
> 
> I believe a small bug slipped in the refactoring of varobj_update
> interface from:
> http://sourceware.org/ml/gdb-patches/2008-05/msg00106.html
> 
> From what I see, varobj that are not in scope don't get flagged
> as changed, because nothing was being pushed on the result vector.
> The attached patch fixes this.
> 
> The MI part of the testsuite passed ok.
> I have an test to trigger the bug, if you care to see it.
> 
> Ok?
> 
> 2009-04-28  Marc Khouzam  <marc.khouzam@ericsson.com>
> 
> * varobj.c (varobj_update): Push an out-of-scope
> variable object on the result vector.
> 
> Index: gdb/varobj.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/varobj.c,v
> retrieving revision 1.126
> diff -u -r1.126 varobj.c
> --- gdb/varobj.c        10 Apr 2009 16:00:49 -0000      1.126
> +++ gdb/varobj.c        28 Apr 2009 19:49:24 -0000
> @@ -1188,7 +1188,7 @@
>        if (new == NULL)
>         r.status = VAROBJ_NOT_IN_SCOPE;
>  
> -      if (r.type_changed || r.changed)
> +      if (r.type_changed || r.changed || r.status ==
> VAROBJ_NOT_IN_SCOPE)
>         VEC_safe_push (varobj_update_result, result, &r);

I am afraid this patch is not right for two reasons:

1. A varobj that is not in scope will be always reported
as changed:

        (gdb)
        -var-update S
        ^done,changelist=[{name="S",in_scope="false",type_changed="false"}]
        (gdb)
        -var-update S
        ^done,changelist=[{name="S",in_scope="false",type_changed="false"}]
        (gdb)

This behaviour almost patches GDB 6.8 (it did not print 'type_changed'), but is
not right.

2. When a varobj enters the scope again, it won't be reported as changed:

        (gdb)
        s
        &"s\n"
        ^running
        *running,thread-id="1"
        (gdb)
        ~"foo () at a.c:7\n"
        ~"7\t    s.i = 10;\n"
        *stopped,....
        (gdb)
        -var-update S
        ^done,changelist=[]
        (gdb)


This does not match GDB 6.8 -- which continues to claim 'S' is out of scope. Both
the above, and GDB 6.8 are clearly in error.

I have checked in the below that makes -var-update bahave right in both cases. Let
me know if that works for you.

Thanks,
Volodya









[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: final.diff --]
[-- Type: text/x-diff; name="final.diff", Size: 4300 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.10474
diff -u -p -r1.10474 ChangeLog
--- ChangeLog	15 May 2009 16:53:41 -0000	1.10474
+++ ChangeLog	17 May 2009 07:12:46 -0000
@@ -1,3 +1,10 @@
+2009-05-17  Vladimir Prus  <vladimir@codesourcery.com>
+
+	Always report varobj as changed when in_scope attribute changes.
+
+	* varobj.c (install_new_value): If non-NULL-ness of value
+	changed, return 1.
+
 2009-05-15  Paul Pluzhnikov  <ppluzhnikov@google.com>
 	
 	* NEWS: Mention set/show libthread-db-search-path.
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.127
diff -u -p -r1.127 varobj.c
--- varobj.c	22 Apr 2009 17:50:54 -0000	1.127
+++ varobj.c	17 May 2009 07:12:46 -0000
@@ -982,9 +982,12 @@ varobj_list (struct varobj ***varlist)
    this is the first assignement after the variable object was just
    created, or changed type.  In that case, just assign the value 
    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.  
+   Otherwise, assign the new value, and return 1 if the value is different
+   from the current one, 0 otherwise. The comparison is done on textual
+   representation of value. Therefore, some types need not be compared. E.g.
+   for structures the reported value is always "{...}", so no comparison is
+   necessary here. If the old value was NULL and new one is not, or vice versa,
+   we always return 1.
 
    The VALUE parameter should not be released -- the function will
    take care of releasing it when needed.  */
@@ -1105,6 +1108,15 @@ install_new_value (struct varobj *var, s
 	}
     }
 
+  if (!initial && !changeable)
+    {
+      /* For values that are not changeable, we don't compare the values.
+	 However, we want to notice if a value was not NULL and now is NULL,
+	 or vise versa, so that we report when top-level varobjs come in scope
+	 and leave the scope.  */
+      changed = (var->value != NULL) != (value != NULL);
+    }
+
   /* We must always keep the new value, since children depend on it.  */
   if (var->value != NULL && var->value != value)
     value_free (var->value);
Index: testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.1868
diff -u -p -r1.1868 ChangeLog
--- testsuite/ChangeLog	11 May 2009 16:24:13 -0000	1.1868
+++ testsuite/ChangeLog	17 May 2009 07:12:47 -0000
@@ -1,3 +1,9 @@
+2009-05-17  Vladimir Prus  <vladimir@codesourcery.com>
+
+	* gdb.mi/mi-cmd-var.exp: Check that when varobj
+	of structure type enters or leaves the scope, it
+	is reported by -var-update.
+
 2009-05-11  Doug Evans  <dje@sebabeach.org>
 
 	* gdb.mi/nsintrall.c (main): Fix off-by-one error.
Index: testsuite/gdb.mi/mi-var-cmd.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-cmd.exp,v
retrieving revision 1.41
diff -u -p -r1.41 mi-var-cmd.exp
--- testsuite/gdb.mi/mi-var-cmd.exp	3 Jan 2009 05:58:06 -0000	1.41
+++ testsuite/gdb.mi/mi-var-cmd.exp	17 May 2009 07:12:47 -0000
@@ -631,5 +631,32 @@ mi_gdb_test "-var-delete endvar" \
     "\\^done,ndeleted=\"1\"" \
     "delete endvar"
 
+mi_delete_breakpoints
+
+mi_runto do_locals_tests
+
+mi_create_varobj "L" "lsimple" "in-and-out-of-scope: create varobj"
+mi_check_varobj_value "L" "{...}" "in-and-out-of-scope: check initial value"
+
+mi_runto main
+
+mi_gdb_test "-var-update L" \
+    {\^done,changelist=\[{name="L",in_scope="false",type_changed="false"}\]} \
+    "in-and-out-of-scope: out of scope now"
+
+mi_gdb_test "-var-update L" \
+    {\^done,changelist=\[]} \
+    "in-and-out-of-scope: out of scope now, not changed"
+
+mi_continue_to do_locals_tests
+
+mi_gdb_test "-var-update L" \
+    {\^done,changelist=\[{name="L",in_scope="true",type_changed="false"}\]} \
+    "in-and-out-of-scope: in scope now"
+
+mi_gdb_test "-var-update L" \
+    {\^done,changelist=\[\]} \
+    "in-and-out-of-scope: in scope now, not changed"
+
 mi_gdb_exit
 return 0