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

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

[Marc Khouzam]

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);
 
       if (r.status == VAROBJ_NOT_IN_SCOPE)

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

[Nick Roberts]

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

I'm not sure that I see this.  If the variable goes out of scope then, as the
comment says, then value_of_root returns NULL.  But (*varp)->value is not NULL,
so install_new_value returns 1 and r.changed is set to 1 and r gets pushed onto
the result vector.

 > The MI part of the testsuite passed ok.
 > I have an test to trigger the bug, if you care to see it.

Yes, please post that.  The current testsuite covers varobjs that go out of
scope.  If it is a regression then there needs to be a test that covers this
mode of failure in any case.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

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

[Marc Khouzam]

 

> -----Original Message-----
> From: Nick Roberts [mailto:nickrob@snap.net.nz] 
> Sent: Friday, May 01, 2009 8:00 AM
> To: Marc Khouzam
> Cc: gdb-patches@sourceware.org
> Subject: [Patch] [MI] Out-of-scope varObjects no longer 
> trigger a var-update change
> 
>  > 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.
> 
> I'm not sure that I see this.  If the variable goes out of 
> scope then, as the
> comment says, then value_of_root returns NULL.  But 
> (*varp)->value is not NULL,
> so install_new_value returns 1 and r.changed is set to 1 and 
> r gets pushed onto
> the result vector.

You had me questioning myself...
But in the case I'm looking at
install_new_value must be returning 0.
(Maybe that is the bug?  I didn't look at that.)
You can see below the particularities of my testcase.
 
>  > The MI part of the testsuite passed ok.
>  > I have an test to trigger the bug, if you care to see it.
> 
> Yes, please post that.  The current testsuite covers varobjs 
> that go out of
> scope.  If it is a regression then there needs to be a test 
> that covers this
> mode of failure in any case.

Below is the session.  The testcase is part of my Eclipse
regression testsuite and basically looks for the content
of a variable name the same thing as a previous variable,
which is part of a method named the same thing as where the
previous variable was.  You'll understand better from the code
below :-)

Note that the below passes after I applied my proposed patch.


~"GNU gdb (GDB) 6.8.50.20090420-cvs\n"
~"Copyright (C) 2009 Free Software Foundation, Inc.\n"
~"License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>\n"
~"This is free software: you are free to change and redistribute it.\n"
~"There is NO WARRANTY, to the extent permitted by law.  Type \"show
copying\"\n"
~"and \"show warranty\" for details.\n"
~"This GDB was configured as \"i686-pc-linux-gnu\".\n"~"For bug
reporting instructions, please see:\n"
~"<http://www.gnu.org/software/gdb/bugs/>...\n"
(gdb) 
l 1 
&"l 1\n"
~"1\tstruct Z {\n"
~"2\tpublic:\n"
~"3\t    int x;\n"
~"4\t    int y;\n"
~"5\t};\n"
~"6\t\n"
~"7\tvoid sameName(int newVal) {\n"
~"8\t    Z z;\n"
~"9\t    z.x = newVal;\n"
~"10\t    return;\n"
^done
(gdb) 
l
&"l\n"
~"11\t}\n"
~"12\t\n"
~"13\tvoid sameName(int newVal, int ignore) {\n"
~"14\t    Z z;\n"
~"15\t    z.x = newVal;\n"
~"16\t    return;\n"
~"17\t}\n"
~"18\t\n"
~"19\t\n"
~"20\tint main() {\n"
^done
(gdb) 
l
&"l\n"
~"21\t    sameName(1);\n"
~"22\t    sameName(2, 0);\n"
~"23\t\n"
~"24\t    return 0;\n"
~"25\t}\n"
^done
(gdb)
b 10
&"b 10\n"
~"Breakpoint 1 at 0x8048410: file expression.cc, line 10.\n"
^done
(gdb) 
b 16
&"b 16\n"~"Breakpoint 2 at 0x804841e: file expression.cc, line 16.\n"
^done
(gdb) 
r
&"r\n"
~"Starting program: /local/home/lmckhou/testing/a.out \n"
=thread-group-created,id="31707"
=thread-created,id="1",group-id="31707"
^running
*running,thread-id="all"
(gdb) 
=library-loaded,id="/lib/ld-linux.so.2",target-name="/lib/ld-linux.so.2"
,host-name="/lib/ld-linux.so.2",symbols-loaded="0"
=library-loaded,id="/usr/lib/libstdc++.so.6",target-name="/usr/lib/libst
dc++.so.6",host-name="/usr/lib/libstdc++.so.6",symbols-loaded="0"
=library-loaded,id="/lib/libm.so.6",target-name="/lib/libm.so.6",host-na
me="/lib/libm.so.6",symbols-loaded="0"
=library-loaded,id="/lib/libgcc_s.so.1",target-name="/lib/libgcc_s.so.1"
,host-name="/lib/libgcc_s.so.1",symbols-loaded="0"
=library-loaded,id="/lib/libc.so.6",target-name="/lib/libc.so.6",host-na
me="/lib/libc.so.6",symbols-loaded="0"
~"\n"
~"Breakpoint 1, sameName (newVal=1) at expression.cc:11\n"
~"11\t}\n"
*stopped,frame={addr="0x08048410",func="sameName",args=[{name="newVal",v
alue="1"}],file="expression.cc",fullname="/local/home/lmckhou/testing/ex
pression.cc",line="11
"},thread-id="1",stopped-threads="all"(gdb) 
-var-create - * z
^done,name="var1",numchild="1",value="{...}",type="Z",thread-id="1"
(gdb) 
-var-list-children var1
^done,numchild="1",children=[child={name="var1.public",exp="public",numc
hild="2",thread-id="1"}]
(gdb) 
-var-list-children var1.public
^done,numchild="2",children=[child={name="var1.public.x",exp="x",numchil
d="0",type="int",thread-id="1"},child={name="var1.public.y",exp="y",numc
hild="0",type="int",t
hread-id="1"}]
(gdb) 
-var-info-path-expression var1.public.x
^done,path_expr="((z).x)"
(gdb) 
-var-info-path-expression var1.public.y
^done,path_expr="((z).y)"
(gdb) 
-var-evaluate-expression var1.public.x
^done,value="1"
(gdb) 
c
&"c\n"
~"Continuing.\n"
^running
*running,thread-id="1"
(gdb) 
~"\n"
~"Breakpoint 2, sameName (newVal=2, ignore=0) at expression.cc:17\n"
~"17\t}\n"
*stopped,frame={addr="0x0804841e",func="sameName",args=[{name="newVal",v
alue="2"},{name="ignore",value="0"}],file="expression.cc",fullname="/loc
al/home/lmckhou/testi
ng/expression.cc",line="17"},thread-id="1",stopped-threads="all"
(gdb) 
 -var-update 1 var1
^done,changelist=[]
(gdb)

========================================
== var1 should have gone out of scope ==
========================================

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

[Nick Roberts]

Marc Khouzam writes:
 > Below is the session.  The testcase is part of my Eclipse
 > regression testsuite and basically looks for the content
 > of a variable name the same thing as a previous variable,
 > which is part of a method named the same thing as where the
 > previous variable was.  You'll understand better from the code
 > below :-)

OK, I see this now.  The failure occurs because `public' is considered
an unchangeable field by GDB.

 > Note that the below passes after I applied my proposed patch.

As a general principle, if a regression occurs I try to remove some of the
added logic, rather than add to it, as I find the latter tends to make
the logic more convoluted.

The procedure, varobj_update, used to return a scalar that corresponded to the
status field of the structure. varobj_update_result.  I don't know what the
advantage of returning a vector of structures is but, in any case, the changed
field of varobj_update_result appears not to be used outside varobj_update.  I
would suggest a change something like below.  A full patch would remove the
changed field altogether.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


*** varobj.c	25 Apr 2009 13:04:27 +1200	1.127
--- varobj.c	03 May 2009 03:27:17 +1200	
*************** VEC(varobj_update_result) *varobj_update
*** 1182,1198 ****
        r.varobj = *varp;
  
        r.type_changed = type_changed;
-       if (install_new_value ((*varp), new, type_changed))
- 	r.changed = 1;
-       
-       if (new == NULL)
- 	r.status = VAROBJ_NOT_IN_SCOPE;
  
!       if (r.type_changed || r.changed)
  	VEC_safe_push (varobj_update_result, result, &r);
  
!       if (r.status == VAROBJ_NOT_IN_SCOPE)
! 	return result;
      }
  
    VEC_safe_push (varobj_p, stack, *varp);
--- 1182,1204 ----
        r.varobj = *varp;
  
        r.type_changed = type_changed;
  
!       if (r.type_changed)
  	VEC_safe_push (varobj_update_result, result, &r);
  
!       if (install_new_value ((*varp), new, type_changed))
! 	{
! 	  /* If type_changed is 1, install_new_value will never return
! 	     non-zero, so we'll never report the same variable twice.  */
! 	  gdb_assert (!type_changed);
! 	  VEC_safe_push (varobj_update_result, result, &r);
! 	}
!       
!       if (new == NULL)
! 	{
! 	  r.status = VAROBJ_NOT_IN_SCOPE;
! 	  return result;
! 	}
      }
  
    VEC_safe_push (varobj_p, stack, *varp);

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

[Marc Khouzam]

 

> -----Original Message-----
> From: Nick Roberts [mailto:nickrob@snap.net.nz] 
> Sent: Saturday, May 02, 2009 7:09 PM
> To: Marc Khouzam
> Cc: gdb-patches@sourceware.org
> Subject: RE: [Patch] [MI] Out-of-scope varObjects no longer 
> trigger a var-update change
> 
> Marc Khouzam writes:
>  > Below is the session.  The testcase is part of my Eclipse
>  > regression testsuite and basically looks for the content
>  > of a variable name the same thing as a previous variable,
>  > which is part of a method named the same thing as where the
>  > previous variable was.  You'll understand better from the code
>  > below :-)
> 
> OK, I see this now.  The failure occurs because `public' is considered
> an unchangeable field by GDB.
> 
>  > Note that the below passes after I applied my proposed patch.
> 
> As a general principle, if a regression occurs I try to 
> remove some of the
> added logic, rather than add to it, as I find the latter tends to make
> the logic more convoluted.
> 
> The procedure, varobj_update, used to return a scalar that 
> corresponded to the
> status field of the structure. varobj_update_result.  I don't 
> know what the
> advantage of returning a vector of structures is but, in any 
> case, the changed
> field of varobj_update_result appears not to be used outside 
> varobj_update.  I
> would suggest a change something like below.  A full patch 
> would remove the
> changed field altogether.
>

I tried to apply the patch below but it didn't fix my problem.
I believe that in my case, type_changed is 0
and
install_new_value returns 0.
So, even with your patch, nothing gets pushed on the result
vector, no?

> -- 
> Nick                                           
> http://www.inet.net.nz/~nickrob
> 
> 
> *** varobj.c	25 Apr 2009 13:04:27 +1200	1.127
> --- varobj.c	03 May 2009 03:27:17 +1200	
> *************** VEC(varobj_update_result) *varobj_update
> *** 1182,1198 ****
>         r.varobj = *varp;
>   
>         r.type_changed = type_changed;
> -       if (install_new_value ((*varp), new, type_changed))
> - 	r.changed = 1;
> -       
> -       if (new == NULL)
> - 	r.status = VAROBJ_NOT_IN_SCOPE;
>   
> !       if (r.type_changed || r.changed)
>   	VEC_safe_push (varobj_update_result, result, &r);
>   
> !       if (r.status == VAROBJ_NOT_IN_SCOPE)
> ! 	return result;
>       }
>   
>     VEC_safe_push (varobj_p, stack, *varp);
> --- 1182,1204 ----
>         r.varobj = *varp;
>   
>         r.type_changed = type_changed;
>   
> !       if (r.type_changed)
>   	VEC_safe_push (varobj_update_result, result, &r);
>   
> !       if (install_new_value ((*varp), new, type_changed))
> ! 	{
> ! 	  /* If type_changed is 1, install_new_value will never return
> ! 	     non-zero, so we'll never report the same variable 
> twice.  */
> ! 	  gdb_assert (!type_changed);
> ! 	  VEC_safe_push (varobj_update_result, result, &r);
> ! 	}
> !       
> !       if (new == NULL)
> ! 	{
> ! 	  r.status = VAROBJ_NOT_IN_SCOPE;
> ! 	  return result;
> ! 	}
>       }
>   
>     VEC_safe_push (varobj_p, stack, *varp);
> 

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

[Vladimir Prus]

Nick Roberts wrote:

> Marc Khouzam writes:
>  > Below is the session.  The testcase is part of my Eclipse
>  > regression testsuite and basically looks for the content
>  > of a variable name the same thing as a previous variable,
>  > which is part of a method named the same thing as where the
>  > previous variable was.  You'll understand better from the code
>  > below :-)
> 
> OK, I see this now.  The failure occurs because `public' is considered
> an unchangeable field by GDB.
> 
>  > Note that the below passes after I applied my proposed patch.
> 
> As a general principle, if a regression occurs I try to remove some of the
> added logic, rather than add to it, as I find the latter tends to make
> the logic more convoluted.
> 
> The procedure, varobj_update, used to return a scalar that corresponded to the
> status field of the structure. varobj_update_result.  I don't know what the
> advantage of returning a vector of structures is but, in any case, the changed
> field of varobj_update_result appears not to be used outside varobj_update.  I
> would suggest a change something like below.  A full patch would remove the
> changed field altogether.

Can you post this as unified diff? I can't read context diffs at all :-(

- Volodya


 

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

[Marc Khouzam]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Vladimir Prus
> Sent: Monday, May 04, 2009 1:54 PM
> To: gdb-patches@sources.redhat.com
> Subject: RE: [Patch] [MI] Out-of-scope varObjects no longer 
> trigger a var-update change
> 
> Nick Roberts wrote:
> 
> > Marc Khouzam writes:
> >  > Below is the session.  The testcase is part of my Eclipse
> >  > regression testsuite and basically looks for the content
> >  > of a variable name the same thing as a previous variable,
> >  > which is part of a method named the same thing as where the
> >  > previous variable was.  You'll understand better from the code
> >  > below :-)
> > 
> > OK, I see this now.  The failure occurs because `public' is 
> considered
> > an unchangeable field by GDB.
> > 
> >  > Note that the below passes after I applied my proposed patch.
> > 
> > As a general principle, if a regression occurs I try to 
> remove some of the
> > added logic, rather than add to it, as I find the latter 
> tends to make
> > the logic more convoluted.
> > 
> > The procedure, varobj_update, used to return a scalar that 
> corresponded to the
> > status field of the structure. varobj_update_result.  I 
> don't know what the
> > advantage of returning a vector of structures is but, in 
> any case, the changed
> > field of varobj_update_result appears not to be used 
> outside varobj_update.  I
> > would suggest a change something like below.  A full patch 
> would remove the
> > changed field altogether.
> 
> Can you post this as unified diff? I can't read context diffs 
> at all :-(

It would make it easier on me as well (for next time).

Thanks in advance. 

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

[Nick Roberts]

 > I tried to apply the patch below but it didn't fix my problem.
 > I believe that in my case, type_changed is 0
 > and
 > install_new_value returns 0.
 > So, even with your patch, nothing gets pushed on the result
 > vector, no?

Hmm, looks like you are right.  It looks incomplete and I would have to revert
changes to varobj_update_one in mi-cmd-var.c also.

Perhaps your change is simplest after all.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

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

[Marc Khouzam]

Ping?

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Marc Khouzam
> Sent: Tuesday, April 28, 2009 4:03 PM
> To: gdb-patches@sourceware.org
> Subject: [Patch] [MI] Out-of-scope varObjects no longer 
> trigger a var-update change
> 
> 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);
>  
>        if (r.status == VAROBJ_NOT_IN_SCOPE)
> 

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

[Vladimir Prus]

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

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

[Marc Khouzam]

 

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Vladimir Prus
> Sent: May-17-09 3:14 AM
> To: gdb-patches@sources.redhat.com
> Subject: Re: [Patch] [MI] Out-of-scope varObjects no longer 
> trigger a var-update change
> 
> 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.

I never noticed this.  I guess it never came up because as soon as 
we see an out-of-scope update, we delete that object.

I wonder if it is safe to change this behavior.  It somehow feels safer
that if a frontend updates an out-of-scope object again, GDB would
remind 
the frontend that an object is out-of-scope, just in case.
But I have nothing to back this claim :-)  And the new behavior works
well with DSF-GDB and is more elegant, so I like it.

> 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 was not able to reproduce this behavior with GDB 6.8.
When stepping back into a method, I saw var-update report
that the object was in-scope again.

But your patch also does that, so it is good for me.

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

Yes, my Junit tests now pass.

Thanks!

Marc