MI: Another -var-update bug?

MI: Another -var-update bug?

[Nick Roberts]

Variable objects appear to test scope on a frame basis (in c_value_of root).
If we create a variable object for j in the inner block of the program below
then doing -var-update on line 10 doesn't report it as being out of scope.  But
struct varobj_root has a member struct block *valid_block.  Presumably the
addresses in this structure can be used to test if the variable is still in
scope or not

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



1   main ()
2   {
3     int i;
4     i = 1;
5     {
6       int j;
7       j = 2;
8       printf ("j = %d\n", j);
9     }
10    i = 3;
11  }

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

[Nick Roberts]

 > Variable objects appear to test scope on a frame basis (in c_value_of root).
 > If we create a variable object for j in the inner block of the program below
 > then doing -var-update on line 10 doesn't report it as being out of scope.
 > But struct varobj_root has a member struct block *valid_block.  Presumably
 > the addresses in this structure can be used to test if the variable is still
 > in scope or not

Something like below?

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


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

	* varobj.c: Include block.h.
	(c_value_of_root): Check scope within nested statements.


*** varobj.c	09 Dec 2006 10:59:12 +1300	1.65
--- varobj.c	13 Dec 2006 19:17:36 +1300	
***************
*** 26,31 ****
--- 26,32 ----
  #include "language.h"
  #include "wrapper.h"
  #include "gdbcmd.h"
+ #include "block.h"
  
  #include "gdb_assert.h"
  #include "gdb_string.h"
*************** c_value_of_root (struct varobj **var_han
*** 1958,1965 ****
        fi = frame_find_by_id (var->root->frame);
        within_scope = fi != NULL;
        /* FIXME: select_frame could fail */
!       if (within_scope)
! 	select_frame (fi);
      }
  
    if (within_scope)
--- 1959,1972 ----
        fi = frame_find_by_id (var->root->frame);
        within_scope = fi != NULL;
        /* FIXME: select_frame could fail */
!       if (fi)
! 	{
! 	  CORE_ADDR pc = get_frame_pc (fi);
! 	  if (pc <  BLOCK_START (var->root->valid_block) ||
! 	      pc >= BLOCK_END (var->root->valid_block))
! 	    within_scope = 0;
! 	  select_frame (fi);
! 	}	  
      }
  
    if (within_scope)

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

[Daniel Jacobowitz]

On Wed, Dec 13, 2006 at 07:28:05PM +1300, Nick Roberts wrote:
>  > Variable objects appear to test scope on a frame basis (in c_value_of root).
>  > If we create a variable object for j in the inner block of the program below
>  > then doing -var-update on line 10 doesn't report it as being out of scope.
>  > But struct varobj_root has a member struct block *valid_block.  Presumably
>  > the addresses in this structure can be used to test if the variable is still
>  > in scope or not
> 
> Something like below?

Yeah.  My only concern about this is a variable going out of scope
which might then come back into scope.  That can't happen if the frame
is gone (unless something has changed), but it can happen with blocks
once we support optimized blocks better (I posted a patch for this
once).

Here's an example.  Suppose we have this code:

  int func()
  {
    int i = foo();
    {
       int j = bar();
       j = j * j;
       i += j;
    }
    baz();
    return i;
  }

Since baz can't see i or j, it's legitimate for the compiler to move
the call to baz up to right after the call to bar.  Then we'll appear
to "leave" the block and "re-enter" it after another step.

Will the front end delete the varobj if it sees in_scope="false"?

-- 
Daniel Jacobowitz
CodeSourcery

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

[Nick Roberts]

 > Yeah.  My only concern about this is a variable going out of scope
 > which might then come back into scope.  That can't happen if the frame
 > is gone (unless something has changed), 

Currently variable objects consider a variable to be back in scope if a
frame is re-entered.

 >                                         but it can happen with blocks
 > once we support optimized blocks better (I posted a patch for this
 > once).
 > 
 > Here's an example.  Suppose we have this code:
 > 
 >   int func()
 >   {
 >     int i = foo();
 >     {
 >        int j = bar();
 >        j = j * j;
 >        i += j;
 >     }
 >     baz();
 >     return i;
 >   }
 > 
 > Since baz can't see i or j, it's legitimate for the compiler to move
 > the call to baz up to right after the call to bar.

That sounds like some kind of optimisation.  Does this happen with -O0?

 >                                                     Then we'll appear
 > to "leave" the block and "re-enter" it after another step.

Entering another function doesn't take existing variables out of scope
does it?  Block addresses are measured against the PC of the frame that
the variable is defined in.

 > Will the front end delete the varobj if it sees in_scope="false"?

I can't speak for others but in Emacs I just use a grey font for variables that
go out of scope and leave it to the user to explicily delete them.  The worst
scenario, if there is a problem at all, is that the watch expression would be
inexplicably greyed for one step.


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

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

[Daniel Jacobowitz]

On Sun, Dec 31, 2006 at 11:38:46AM +1300, Nick Roberts wrote:
> Currently variable objects consider a variable to be back in scope if a
> frame is re-entered.

Hmm.  Interesting... not sure if that's the most useful behavior -
frame re-entrance is a matter of a great deal of luck.

>  > Since baz can't see i or j, it's legitimate for the compiler to move
>  > the call to baz up to right after the call to bar.
> 
> That sounds like some kind of optimisation.  Does this happen with -O0?

Probably not, but it would be up to the compiler.

>  >                                                     Then we'll appear
>  > to "leave" the block and "re-enter" it after another step.
> 
> Entering another function doesn't take existing variables out of scope
> does it?  Block addresses are measured against the PC of the frame that
> the variable is defined in.

That's not what I meant by leaving the block.  We'll appear to execute
a line inside the block, a line outside the block (from the same
function), and then a line inside the block again.  But I guess this is
not much different from calling a function, right?

> I can't speak for others but in Emacs I just use a grey font for variables that
> go out of scope and leave it to the user to explicily delete them.  The worst
> scenario, if there is a problem at all, is that the watch expression would be
> inexplicably greyed for one step.

Good, that's not bad at all.  If Vlad doesn't think this is likely to
break kdevelop or Eclipse, the patch is OK.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Daniel Jacobowitz]

On Sat, Dec 30, 2006 at 09:19:47PM -0500, Daniel Jacobowitz wrote:
> > I can't speak for others but in Emacs I just use a grey font for variables that
> > go out of scope and leave it to the user to explicily delete them.  The worst
> > scenario, if there is a problem at all, is that the watch expression would be
> > inexplicably greyed for one step.
> 
> Good, that's not bad at all.  If Vlad doesn't think this is likely to
> break kdevelop or Eclipse, the patch is OK.

So, given that, why did you commit it?  Maybe Vlad agreed with the
patch in another thread that I didn't read?

-- 
Daniel Jacobowitz
CodeSourcery

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

[Nick Roberts]

 > > Good, that's not bad at all.  If Vlad doesn't think this is likely to
 > > break kdevelop or Eclipse, the patch is OK.
 > 
 > So, given that, why did you commit it?  Maybe Vlad agreed with the
 > patch in another thread that I didn't read?

He was part of the original thread and had posted messages subsequent to yours.
I presumed that if he thought it would break anything he would have said so.

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

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

[Vladimir Prus]

Nick Roberts wrote:

>  > > Good, that's not bad at all.  If Vlad doesn't think this is likely to
>  > > break kdevelop or Eclipse, the patch is OK.
>  > 
>  > So, given that, why did you commit it?  Maybe Vlad agreed with the
>  > patch in another thread that I didn't read?
> 
> He was part of the original thread and had posted messages subsequent to
> yours. I presumed that if he thought it would break anything he would have
> said so.

Had I immediately knew that this patch will break something, I'd say so.
But in practice, while I'm sure KDevelop won't care, I was not able to
make definite statement about Eclipse without looking at the code, and
I had no time for such look back then.

Given that this is already checked in, I'll let you know if I find any
problems with Eclipse.

- Volodya

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

[Nick Roberts]

 > Had I immediately knew that this patch will break something, I'd say so.
 > But in practice, while I'm sure KDevelop won't care, I was not able to
 > make definite statement about Eclipse without looking at the code, and
 > I had no time for such look back then.
 > 
 > Given that this is already checked in, I'll let you know if I find any
 > problems with Eclipse.

Generally, now that the release has been made, once it is agreed that a patch
is likely to do the right thing, I'd like see it checked in.  That way its
likely to get more testing while the next release is almost six months away.

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

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

[Vladimir Prus]

Nick Roberts wrote:

> Something like below?

This patch introduces this for me:

        FAIL: gdb.mi/mi-var-cp.exp: update RX (3)

Do you get this failure to? If yes, can you please fix it?
If no, let me know and I'll investigate.

> /* FIXME: select_frame could fail */
> !       if (fi)
> !       {
> !         CORE_ADDR pc = get_frame_pc (fi);
> !         if (pc <  BLOCK_START (var->root->valid_block) ||
> !             pc >= BLOCK_END (var->root->valid_block))
> !           within_scope = 0;
> !         select_frame (fi);
> !       }

The code later in this function is only executed if "within_scope" is
true. Would it be better to call select_frame only if within_scope is
set to true, like this:

         if (pc <  BLOCK_START (var->root->valid_block) ||
             pc >= BLOCK_END (var->root->valid_block))
          within_scope = 0;
        else
          select_frame (fi);

?

- Volodya

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

[Nick Roberts]

 > This patch introduces this for me:
 > 
 >         FAIL: gdb.mi/mi-var-cp.exp: update RX (3)
 > 
 > Do you get this failure to? If yes, can you please fix it?
 > If no, let me know and I'll investigate.

I don't get this failure.

 > > /* FIXME: select_frame could fail */
 > > !       if (fi)
 > > !       {
 > > !         CORE_ADDR pc = get_frame_pc (fi);
 > > !         if (pc <  BLOCK_START (var->root->valid_block) ||
 > > !             pc >= BLOCK_END (var->root->valid_block))
 > > !           within_scope = 0;
 > > !         select_frame (fi);
 > > !       }
 > 
 > The code later in this function is only executed if "within_scope" is
 > true. Would it be better to call select_frame only if within_scope is
 > set to true, like this:
 > 
 >          if (pc <  BLOCK_START (var->root->valid_block) ||
 >              pc >= BLOCK_END (var->root->valid_block))
 >           within_scope = 0;
 >         else
 >           select_frame (fi);

Yes, it looks like the call to select_frame is not needed when the variable is
out of scope.  Daniel also suggested removing the call to reinit_frame_cache.

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

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

[Vladimir Prus]

On Friday 05 January 2007 02:59, Nick Roberts wrote:
>  > This patch introduces this for me:
>  > 
>  >         FAIL: gdb.mi/mi-var-cp.exp: update RX (3)
>  > 
>  > Do you get this failure to? If yes, can you please fix it?
>  > If no, let me know and I'll investigate.
> 
> I don't get this failure.

I suppose you've got some different version of gcc. I'll take a look.

>  > > /* FIXME: select_frame could fail */
>  > > !       if (fi)
>  > > !       {
>  > > !         CORE_ADDR pc = get_frame_pc (fi);
>  > > !         if (pc <  BLOCK_START (var->root->valid_block) ||
>  > > !             pc >= BLOCK_END (var->root->valid_block))
>  > > !           within_scope = 0;
>  > > !         select_frame (fi);
>  > > !       }
>  > 
>  > The code later in this function is only executed if "within_scope" is
>  > true. Would it be better to call select_frame only if within_scope is
>  > set to true, like this:
>  > 
>  >          if (pc <  BLOCK_START (var->root->valid_block) ||
>  >              pc >= BLOCK_END (var->root->valid_block))
>  >           within_scope = 0;
>  >         else
>  >           select_frame (fi);
> 
> Yes, it looks like the call to select_frame is not needed when the variable is
> out of scope.  

Good. Do we have a rule in place that small fixes to one's most recent commit that 
seem obvious to you can be just committed? If yes, can you change this?

> Daniel also suggested removing the call to reinit_frame_cache. 

I think that would be great as a separate patch.

- Volodya

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

[Daniel Jacobowitz]

On Fri, Jan 05, 2007 at 12:10:09PM +0300, Vladimir Prus wrote:
> On Friday 05 January 2007 02:59, Nick Roberts wrote:
> >  > This patch introduces this for me:
> >  > 
> >  >         FAIL: gdb.mi/mi-var-cp.exp: update RX (3)
> >  > 
> >  > Do you get this failure to? If yes, can you please fix it?
> >  > If no, let me know and I'll investigate.
> > 
> > I don't get this failure.
> 
> I suppose you've got some different version of gcc. I'll take a look.

I don't see the failure either.

> > Yes, it looks like the call to select_frame is not needed when the variable is
> > out of scope.  
> 
> Good. Do we have a rule in place that small fixes to one's most recent commit that 
> seem obvious to you can be just committed? If yes, can you change this?

We don't have such a rule - but this would be an obvious change anyway,
in my opinion.

> > Daniel also suggested removing the call to reinit_frame_cache. 
> 
> I think that would be great as a separate patch.

Yes, I've been meaning to do it.  Wasn't it part of a patch of mine you
reposted recently?

-- 
Daniel Jacobowitz
CodeSourcery