[PATCH] MI: -var-update bug

[PATCH] MI: -var-update bug

[Nick Roberts]

This patch fixes a bug where -var-update doesn't restore the selected frame
when there is an out of scope variable object.

Here is one way see this bug (it might need an OS that randomizes the virtual
address space of a process but there are other ways to make it work in this
case):

gdb -i=mi yourprog
...
(gdb)
start
...
(gdb)
-var-create - * anyvariable
(gdb)
b somesubroutine
...
(gdb)
r
...
(gdb)
up
^done,frame={level="1",....
             ^^^^^^^^^
(gdb)
-var-update *
^done,changelist=[{name="var1",in_scope="false"}]
(gdb)
frame
&"frame\n"
^done,frame={level="0",...
             ^^^^^^^^^

A conservative change would be to additionally restore the selected frame when
GDB returns from varobj_update with a value of -1 because new == NULL but I
think the frame only gets changed by value_of_root -> c_value_of_root ->
reinit_frame_cache, so it's safe to just move the frame restoration forward.

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


2006-12-09  Nick Roberts  <nickrob@snap.net.nz>

	* varobj.c (varobj_update): Ensure frame is restored when
	variable object is out of scope.


*** varobj.c	09 Dec 2006 01:44:28 +1300	1.64
--- varobj.c	09 Dec 2006 01:47:18 +1300	
*************** varobj_update (struct varobj **varp, str
*** 1052,1057 ****
--- 1052,1063 ----
       has changed. */
    type_changed = 1;
    new = value_of_root (varp, &type_changed);
+ 
+   /* Restore selected frame */
+   fi = frame_find_by_id (old_fid);
+   if (fi)
+     select_frame (fi);
+ 
    if (new == NULL)
      {
        (*varp)->error = 1;
*************** varobj_update (struct varobj **varp, str
*** 1147,1157 ****
        *(*changelist + changed) = NULL;
      }
  
-   /* Restore selected frame */
-   fi = frame_find_by_id (old_fid);
-   if (fi)
-     select_frame (fi);
- 
    if (type_changed)
      return -2;
    else

Re: [PATCH] MI: -var-update bug

[Daniel Jacobowitz]

On Sat, Dec 09, 2006 at 08:34:09AM +1300, Nick Roberts wrote:
> 2006-12-09  Nick Roberts  <nickrob@snap.net.nz>
> 
> 	* varobj.c (varobj_update): Ensure frame is restored when
> 	variable object is out of scope.

Thanks, this is OK.

That reinit_frame_cache call is silly.  I've been meaning to remove it
for ages.  Do you see any reason not to?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] MI: -var-update bug

[Nick Roberts]

 > That reinit_frame_cache call is silly.  I've been meaning to remove it
 > for ages.  Do you see any reason not to?

I don't understand frame.c well enough to judge yet (likewise with eval.c and
your patch) but if you think its silly lets remove it.  Without randomisation,
I think some of this code (in c_value_of_root) would put the variable object
back into scope because it would find the frame id.

More generally, should we make GDB delete all variable objects if we restart
execution or are there OSes (without randomisation) where it is still useful?


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

Re: [PATCH] MI: -var-update bug

[Daniel Jacobowitz]

On Sat, Dec 09, 2006 at 08:51:35AM +1300, Nick Roberts wrote:
>  > That reinit_frame_cache call is silly.  I've been meaning to remove it
>  > for ages.  Do you see any reason not to?
> 
> I don't understand frame.c well enough to judge yet (likewise with eval.c and
> your patch) but if you think its silly lets remove it.  Without randomisation,
> I think some of this code (in c_value_of_root) would put the variable object
> back into scope because it would find the frame id.
> 
> More generally, should we make GDB delete all variable objects if we restart
> execution or are there OSes (without randomisation) where it is still useful?

Randomisation isn't even the issue - I think that what you've got now
is simply an accident, and varobjs associated with a particular frame
should not become valid if a similar looking frame reappears later.

Right now we never delete varobjs automatically.  We could preserve
that, but set a flag on the varobjs indicating they're permanently out
of scope?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] MI: -var-update bug

[Nick Roberts]

 > Randomisation isn't even the issue - I think that what you've got now
 > is simply an accident, and varobjs associated with a particular frame
 > should not become valid if a similar looking frame reappears later.

OK that shows I've misunderstood.  I thought it was looking for a frame
to associate with it.
 
 > Right now we never delete varobjs automatically.  We could preserve
 > that, but set a flag on the varobjs indicating they're permanently out
 > of scope?

What value is a variable object that is permanently out of scope?


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

Re: [PATCH] MI: -var-update bug

[Daniel Jacobowitz]

On Sat, Dec 09, 2006 at 09:05:11AM +1300, Nick Roberts wrote:
>  > Randomisation isn't even the issue - I think that what you've got now
>  > is simply an accident, and varobjs associated with a particular frame
>  > should not become valid if a similar looking frame reappears later.
> 
> OK that shows I've misunderstood.  I thought it was looking for a frame
> to associate with it.

If a varobj is associated with a particular frame, and that frame
leaves the stack, I think we should report in_scope="false".  I'm
thinking that we should always report in_scope="false" after that
point... even if another frame that happens to have the same frame
ID appears later.

There seem to be a bunch of different ways a varobj can associate
with a frame; I guess we don't need to stop varobjs that have
use_current_frame or no valid_block?

>  > Right now we never delete varobjs automatically.  We could preserve
>  > that, but set a flag on the varobjs indicating they're permanently out
>  > of scope?
> 
> What value is a variable object that is permanently out of scope?

Just in_scope="false".

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] MI: -var-update bug

[Nick Roberts]

 > >  > Randomisation isn't even the issue - I think that what you've got now
 > >  > is simply an accident, and varobjs associated with a particular frame
 > >  > should not become valid if a similar looking frame reappears later.
 > > 
 > > OK that shows I've misunderstood.  I thought it was looking for a frame
 > > to associate with it.
 > 
 > If a varobj is associated with a particular frame, and that frame
 > leaves the stack, I think we should report in_scope="false".  I'm
 > thinking that we should always report in_scope="false" after that
 > point... even if another frame that happens to have the same frame
 > ID appears later.

That's why I thought randomisation was an issue: without it presumably when
GDB stops at the same point (after restarting) for the second time, the frame
looks the same as it did the first time.  In this situation, the user might
want to keep the variable object.

Where the user creates a variable object, comes out of the frame and goes
into the next, it might be best for the variable object to be out of scope.
Does GDB check the procedure name is different in this case? (if it's the
same procedure, the variable object could presumably stay in scope).

 > There seem to be a bunch of different ways a varobj can associate
 > with a frame; I guess we don't need to stop varobjs that have
 > use_current_frame or no valid_block?

Do you mean use_selected_frame?

-var-create - * fred  (USE_CURRENT_FRAME)    Usual case.  Documented.
-var-create - @ fred  (USE_SELECTED_FRAME)   Undocumented.

I think the latter is evaluated in the frame where execution stops.

 > >  > Right now we never delete varobjs automatically.  We could preserve
 > >  > that, but set a flag on the varobjs indicating they're permanently out
 > >  > of scope?
 > > 
 > > What value is a variable object that is permanently out of scope?
 > 
 > Just in_scope="false".

I mean "what worth/benefit". :-)

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