[PATCH] Fix -var-update for registers in frames 1 and up

[PATCH] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

This patch fixes a problem with using the MI -var-update command
to access the values of registers in frames other than the current
frame.  The patch includes a test that demonstrates the problem:

* run so there are several frames on the stack
* create a varobj for $pc in each frame, #'s 1 and above
* step one instruction, to modify the value of $pc
* call -var-update for each of the previously created varobjs
  to verify that they are not reported as having changed.

Without the patch, the -var-update command reported that $pc for all
frames 1 and above had changed to the value of $pc in frame 0.

The -var-create command takes a '--frame' argument and uses that
to select the frame for retrieving the register value.  But -var-update
has no such argument, and previously didn't do anything to select the
frame, so for frames other than frame 0 it returned the value of the
register for frame 0, instead of reporting the value as unchanged.

The solution implemented here is for varobj.c:varobj_update to
handle register varobjs by calling select_frame using the frame
associated with the varobj's value.  If the frame isn't found
varobj_update throws an error.

Thanks
--Don

gdb/testsuite/ChangeLog:
2016-06-07  Don Breazeal  <donb@codesourcery.com>

	* gdb.mi/mi-frame-regs.exp: New test.

gdb/ChangeLog:
2016-06-07  Don Breazeal  <donb@codesourcery.com>

	* varobj.c (varobj_update): If the varobj represents a register,
	select the frame associated with the varobj's value.

---
 gdb/testsuite/gdb.mi/mi-frame-regs.exp | 92 ++++++++++++++++++++++++++++++++++
 gdb/varobj.c                           | 13 +++++
 2 files changed, 105 insertions(+)

diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
new file mode 100644
index 0000000..9ae0fb0
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
@@ -0,0 +1,92 @@
+# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
+
+# Test essential Machine interface (MI) operations
+#
+# Verify that the pc register values for stack frames 1 and above are
+# not reported as changed by -var-update after the current pc changes.
+#
+
+global gdb_prompt
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if {[mi_gdb_start]} then {
+    continue
+}
+
+standard_testfile basics.c
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+		 executable {debug}] != "" } then {
+     untested mi-frame-regs.exp
+     return -1
+}
+
+# Create a varobj for the pc register in each of the frames other
+# than frame 0.
+
+proc var_create_regs {nframes} {
+    global hex
+
+    for {set i 1} {$i < $nframes} {incr i} {
+	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
+		    "\\^done,.*value=\"$hex.*" \
+	            "create varobj for pc in frame $i"
+    }
+}
+
+# Check that the value of the pc register for each of the 
+# frames 1 and above is not reported to have changed.
+
+proc var_update_regs {nframes} {
+
+    for {set i 1} {$i < $nframes} {incr i} {
+	mi_gdb_test "-var-update 1 var$i" \
+	            "\\^done,(changelist=\\\[\\\])" \
+	            "pc unchanged in -var-update for frame $i"
+    }
+}
+
+mi_run_to_main
+
+# Run to the function 'callee3'.
+set line_callee3_head  [gdb_get_line_number "callee3 ("]
+set line_callee3_body  [expr $line_callee3_head + 2]
+mi_create_breakpoint "basics.c:callee3" \
+		     "insert breakpoint at basics.c:callee3" \
+		     -number 2 -func callee3 -file ".*basics.c" \
+		     -line $line_callee3_body
+
+mi_execute_to "exec-continue" "breakpoint-hit" "callee3" ".*" ".*${srcfile}" "$line_callee3_body" \
+	      { "" "disp=\"keep\"" } "breakpoint hit"
+
+# At the breakpoint in callee3 there are 4 frames.  Create a varobj
+# for the pc in each of frames 1 and above.
+set nframes 4
+var_create_regs $nframes
+
+# Step one instruction to change the program counter.
+mi_execute_to "exec-next-instruction" "end-stepping-range" \
+	      "callee3" ".*" ".*${srcfile}" ".*" "" \
+	      "next instruction in callee3"
+
+# Make sure all is well with -var-update.
+var_update_regs $nframes
+
+mi_gdb_exit
+return 0
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 6f56cba..e8f2e04 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1610,6 +1610,19 @@ varobj_update (struct varobj **varp, int is_explicit)
       r.varobj = *varp;
       r.status = VAROBJ_IN_SCOPE;
 
+      /* If updating a register varobj, select the correct frame.  */
+      if ((*varp)->root->exp != NULL
+	  && (*varp)->root->exp->elts[0].opcode == OP_REGISTER)
+	{
+	  struct frame_info *fi;
+
+	  fi = frame_find_by_id (VALUE_FRAME_ID ((*varp)->value));
+	  if (fi != NULL)
+	    select_frame (fi);
+	  else
+	    error (_("Failed to find the frame for the specified register"));
+	}
+
       /* Update the root variable.  value_of_root can return NULL
 	 if the variable is no longer around, i.e. we stepped out of
 	 the frame in which a local existed.  We are letting the 
-- 
2.8.1

Re: [PATCH] Fix -var-update for registers in frames 1 and up

[Andrew Burgess]

* Don Breazeal <donb@codesourcery.com> [2016-06-07 14:36:57 -0700]:

> This patch fixes a problem with using the MI -var-update command
> to access the values of registers in frames other than the current
> frame.  The patch includes a test that demonstrates the problem:
> 
> * run so there are several frames on the stack
> * create a varobj for $pc in each frame, #'s 1 and above
> * step one instruction, to modify the value of $pc
> * call -var-update for each of the previously created varobjs
>   to verify that they are not reported as having changed.
> 
> Without the patch, the -var-update command reported that $pc for all
> frames 1 and above had changed to the value of $pc in frame 0.
> 
> The -var-create command takes a '--frame' argument and uses that
> to select the frame for retrieving the register value.  But -var-update
> has no such argument, and previously didn't do anything to select the
> frame, so for frames other than frame 0 it returned the value of the
> register for frame 0, instead of reporting the value as unchanged.

This shouldn't need special handling for register varobj values, if I
create a varobj watching value 'foo' in frame 1, but have a local
'foo' in frame 0, a change in frame 0 'foo' will not trigger a change
for frame 1's 'foo' varobj.

The magic is actually in varobj.c:check_scope, which makes use of the
varobj->root->frame to select the right frame based on the type of the
varobj, this is setup at varobj creation time.

The problem, is that for register expressions the varobj->root->frame
is not set correctly.  This frame tracking is done using the global
'innermost_block' which is set in the various parser files (for
example c-exp.y), however, this is not set for register expressions.
I think that we probably should be doing this in
write_dollar_variable.

> 
> The solution implemented here is for varobj.c:varobj_update to
> handle register varobjs by calling select_frame using the frame
> associated with the varobj's value.  If the frame isn't found
> varobj_update throws an error.
>

<snip>

> 
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 6f56cba..e8f2e04 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -1610,6 +1610,19 @@ varobj_update (struct varobj **varp, int is_explicit)
>        r.varobj = *varp;
>        r.status = VAROBJ_IN_SCOPE;
>  
> +      /* If updating a register varobj, select the correct frame.  */
> +      if ((*varp)->root->exp != NULL
> +	  && (*varp)->root->exp->elts[0].opcode == OP_REGISTER)
> +	{
> +	  struct frame_info *fi;
> +
> +	  fi = frame_find_by_id (VALUE_FRAME_ID ((*varp)->value));
> +	  if (fi != NULL)
> +	    select_frame (fi);
> +	  else
> +	    error (_("Failed to find the frame for the specified register"));
> +	}
> +
>        /* Update the root variable.  value_of_root can return NULL
>  	 if the variable is no longer around, i.e. we stepped out of
>  	 the frame in which a local existed.  We are letting the 

This will break things if I create a floating varobj tracking (say)
$pc.  In that case I _do_ expect to pick up the $pc in the outermost
frame. Something like:

   -var-create --thread 1 --frame 1 - @ $pc
   next
   -var-update 1 *

Should show my variable as having changed I think.

Thanks,
Andrew

Re: [PATCH] Fix -var-update for registers in frames 1 and up

[Andrew Burgess]

* Andrew Burgess <andrew.burgess@embecosm.com> [2016-06-08 14:00:51 +0100]:

> * Don Breazeal <donb@codesourcery.com> [2016-06-07 14:36:57 -0700]:
> 
> > This patch fixes a problem with using the MI -var-update command
> > to access the values of registers in frames other than the current
> > frame.  The patch includes a test that demonstrates the problem:
> > 
> > * run so there are several frames on the stack
> > * create a varobj for $pc in each frame, #'s 1 and above
> > * step one instruction, to modify the value of $pc
> > * call -var-update for each of the previously created varobjs
> >   to verify that they are not reported as having changed.
> > 
> > Without the patch, the -var-update command reported that $pc for all
> > frames 1 and above had changed to the value of $pc in frame 0.
> > 
> > The -var-create command takes a '--frame' argument and uses that
> > to select the frame for retrieving the register value.  But -var-update
> > has no such argument, and previously didn't do anything to select the
> > frame, so for frames other than frame 0 it returned the value of the
> > register for frame 0, instead of reporting the value as unchanged.
> 
> This shouldn't need special handling for register varobj values, if I
> create a varobj watching value 'foo' in frame 1, but have a local
> 'foo' in frame 0, a change in frame 0 'foo' will not trigger a change
> for frame 1's 'foo' varobj.
> 
> The magic is actually in varobj.c:check_scope, which makes use of the
> varobj->root->frame to select the right frame based on the type of the
> varobj, this is setup at varobj creation time.
> 
> The problem, is that for register expressions the varobj->root->frame
> is not set correctly.  This frame tracking is done using the global
> 'innermost_block' which is set in the various parser files (for
> example c-exp.y), however, this is not set for register expressions.
> I think that we probably should be doing this in
> write_dollar_variable.

Something like the following (untested):

diff --git a/gdb/parse.c b/gdb/parse.c
index 2b00708..224c022 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -705,6 +705,10 @@ handle_register:
   str.ptr++;
   write_exp_string (ps, str);
   write_exp_elt_opcode (ps, OP_REGISTER);
+  if (innermost_block == NULL
+      || contained_in (expression_context_block,
+		       innermost_block))
+    innermost_block = expression_context_block;
   return;
 }

Thanks,
Andrew

Re: [PATCH] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

On 6/8/2016 6:08 AM, Andrew Burgess wrote:
> * Andrew Burgess <andrew.burgess@embecosm.com> [2016-06-08 14:00:51 +0100]:
> 
>> * Don Breazeal <donb@codesourcery.com> [2016-06-07 14:36:57 -0700]:
>>
>>> This patch fixes a problem with using the MI -var-update command
>>> to access the values of registers in frames other than the current
>>> frame.  The patch includes a test that demonstrates the problem:
>>>
>>> * run so there are several frames on the stack
>>> * create a varobj for $pc in each frame, #'s 1 and above
>>> * step one instruction, to modify the value of $pc
>>> * call -var-update for each of the previously created varobjs
>>>   to verify that they are not reported as having changed.
>>>
>>> Without the patch, the -var-update command reported that $pc for all
>>> frames 1 and above had changed to the value of $pc in frame 0.
>>>
>>> The -var-create command takes a '--frame' argument and uses that
>>> to select the frame for retrieving the register value.  But -var-update
>>> has no such argument, and previously didn't do anything to select the
>>> frame, so for frames other than frame 0 it returned the value of the
>>> register for frame 0, instead of reporting the value as unchanged.
>>
>> This shouldn't need special handling for register varobj values, if I
>> create a varobj watching value 'foo' in frame 1, but have a local
>> 'foo' in frame 0, a change in frame 0 'foo' will not trigger a change
>> for frame 1's 'foo' varobj.
>>
>> The magic is actually in varobj.c:check_scope, which makes use of the
>> varobj->root->frame to select the right frame based on the type of the
>> varobj, this is setup at varobj creation time.
>>
>> The problem, is that for register expressions the varobj->root->frame
>> is not set correctly.  This frame tracking is done using the global
>> 'innermost_block' which is set in the various parser files (for
>> example c-exp.y), however, this is not set for register expressions.
>> I think that we probably should be doing this in
>> write_dollar_variable.

Andrew,
Thanks for explaining.  I had followed innermost_block quite a bit
in my debugging, but somehow convinced myself that wasn't the solution.

> 
> Something like the following (untested):
> 
> diff --git a/gdb/parse.c b/gdb/parse.c
> index 2b00708..224c022 100644
> --- a/gdb/parse.c
> +++ b/gdb/parse.c
> @@ -705,6 +705,10 @@ handle_register:
>    str.ptr++;
>    write_exp_string (ps, str);
>    write_exp_elt_opcode (ps, OP_REGISTER);
> +  if (innermost_block == NULL
> +      || contained_in (expression_context_block,
> +		       innermost_block))
> +    innermost_block = expression_context_block;
>    return;
>  }

Your solution works for both the fixed a floating varobj cases.
I've extended my new test to cover the floating case.  Unfortunately
in the full test suite I've run into some unexpected side-effects
that need further investigation.

Hopefully an updated patch will be forthcoming soon.
--Don

Re: [PATCH] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

On 6/8/2016 5:48 PM, Breazeal, Don wrote:
> On 6/8/2016 6:08 AM, Andrew Burgess wrote:
>> * Andrew Burgess <andrew.burgess@embecosm.com> [2016-06-08 14:00:51 +0100]:
>>
>>> * Don Breazeal <donb@codesourcery.com> [2016-06-07 14:36:57 -0700]:
>>>
>>>> This patch fixes a problem with using the MI -var-update command
>>>> to access the values of registers in frames other than the current
>>>> frame.  The patch includes a test that demonstrates the problem:
>>>>
>>>> * run so there are several frames on the stack
>>>> * create a varobj for $pc in each frame, #'s 1 and above
>>>> * step one instruction, to modify the value of $pc
>>>> * call -var-update for each of the previously created varobjs
>>>>   to verify that they are not reported as having changed.
>>>>
>>>> Without the patch, the -var-update command reported that $pc for all
>>>> frames 1 and above had changed to the value of $pc in frame 0.
>>>>
>>>> The -var-create command takes a '--frame' argument and uses that
>>>> to select the frame for retrieving the register value.  But -var-update
>>>> has no such argument, and previously didn't do anything to select the
>>>> frame, so for frames other than frame 0 it returned the value of the
>>>> register for frame 0, instead of reporting the value as unchanged.
>>>
>>> This shouldn't need special handling for register varobj values, if I
>>> create a varobj watching value 'foo' in frame 1, but have a local
>>> 'foo' in frame 0, a change in frame 0 'foo' will not trigger a change
>>> for frame 1's 'foo' varobj.
>>>
>>> The magic is actually in varobj.c:check_scope, which makes use of the
>>> varobj->root->frame to select the right frame based on the type of the
>>> varobj, this is setup at varobj creation time.
>>>
>>> The problem, is that for register expressions the varobj->root->frame
>>> is not set correctly.  This frame tracking is done using the global
>>> 'innermost_block' which is set in the various parser files (for
>>> example c-exp.y), however, this is not set for register expressions.
>>> I think that we probably should be doing this in
>>> write_dollar_variable.
> 
> Andrew,
> Thanks for explaining.  I had followed innermost_block quite a bit
> in my debugging, but somehow convinced myself that wasn't the solution.
> 
>>
>> Something like the following (untested):
>>
>> diff --git a/gdb/parse.c b/gdb/parse.c
>> index 2b00708..224c022 100644
>> --- a/gdb/parse.c
>> +++ b/gdb/parse.c
>> @@ -705,6 +705,10 @@ handle_register:
>>    str.ptr++;
>>    write_exp_string (ps, str);
>>    write_exp_elt_opcode (ps, OP_REGISTER);
>> +  if (innermost_block == NULL
>> +      || contained_in (expression_context_block,
>> +		       innermost_block))
>> +    innermost_block = expression_context_block;
>>    return;
>>  }
> 
> Your solution works for both the fixed a floating varobj cases.
> I've extended my new test to cover the floating case.  Unfortunately
> in the full test suite I've run into some unexpected side-effects
> that need further investigation.

I haven't gone through all the failures, but I have found the cause of
one of the side-effects of this change.  In the CLI, the 'display'
command depends on parsing a register expression to *not* set
innermost_block.  If the expression has a block, it has to be in-scope
or display will skip it rather than evaluating the expression.  See
printcmd.c:do_one_display, the handling of d->block.  It basically does:

         d->exp = parse_expression (d->exp_string);
         d->block = innermost_block;
---snip---
   if (d->block)
    {
      if (d->pspace == current_program_space)
        within_current_scope = contained_in (get_selected_block (0),
d->block);
---snip---
    }
  else
    within_current_scope = 1;
  if (!within_current_scope)
    return;

It seems like we will need a special case someplace.  In do_one_display
we could NULL out d->block for register expressions, or in varobj_update
we could expand on the original patch to properly handle floating
varobjs.  Perhaps since the problem under consideration is only in MI,
making the MI-specific change in varobj_update would minimize
side-effects in the CLI.
WDYT?

I will continue investigating the failures to see if there is anything
else there.
thanks
--Don

Re: [PATCH] Fix -var-update for registers in frames 1 and up

[Andrew Burgess]

* Don Breazeal <donb@codesourcery.com> [2016-06-09 11:23:09 -0700]:

> On 6/8/2016 5:48 PM, Breazeal, Don wrote:
> > On 6/8/2016 6:08 AM, Andrew Burgess wrote:
> >> * Andrew Burgess <andrew.burgess@embecosm.com> [2016-06-08 14:00:51 +0100]:
> >>
> >>> * Don Breazeal <donb@codesourcery.com> [2016-06-07 14:36:57 -0700]:
> >>>
> >>>> This patch fixes a problem with using the MI -var-update command
> >>>> to access the values of registers in frames other than the current
> >>>> frame.  The patch includes a test that demonstrates the problem:
> >>>>
> >>>> * run so there are several frames on the stack
> >>>> * create a varobj for $pc in each frame, #'s 1 and above
> >>>> * step one instruction, to modify the value of $pc
> >>>> * call -var-update for each of the previously created varobjs
> >>>>   to verify that they are not reported as having changed.
> >>>>
> >>>> Without the patch, the -var-update command reported that $pc for all
> >>>> frames 1 and above had changed to the value of $pc in frame 0.
> >>>>
> >>>> The -var-create command takes a '--frame' argument and uses that
> >>>> to select the frame for retrieving the register value.  But -var-update
> >>>> has no such argument, and previously didn't do anything to select the
> >>>> frame, so for frames other than frame 0 it returned the value of the
> >>>> register for frame 0, instead of reporting the value as unchanged.
> >>>
> >>> This shouldn't need special handling for register varobj values, if I
> >>> create a varobj watching value 'foo' in frame 1, but have a local
> >>> 'foo' in frame 0, a change in frame 0 'foo' will not trigger a change
> >>> for frame 1's 'foo' varobj.
> >>>
> >>> The magic is actually in varobj.c:check_scope, which makes use of the
> >>> varobj->root->frame to select the right frame based on the type of the
> >>> varobj, this is setup at varobj creation time.
> >>>
> >>> The problem, is that for register expressions the varobj->root->frame
> >>> is not set correctly.  This frame tracking is done using the global
> >>> 'innermost_block' which is set in the various parser files (for
> >>> example c-exp.y), however, this is not set for register expressions.
> >>> I think that we probably should be doing this in
> >>> write_dollar_variable.
> > 
> > Andrew,
> > Thanks for explaining.  I had followed innermost_block quite a bit
> > in my debugging, but somehow convinced myself that wasn't the solution.
> > 
> >>
> >> Something like the following (untested):
> >>
> >> diff --git a/gdb/parse.c b/gdb/parse.c
> >> index 2b00708..224c022 100644
> >> --- a/gdb/parse.c
> >> +++ b/gdb/parse.c
> >> @@ -705,6 +705,10 @@ handle_register:
> >>    str.ptr++;
> >>    write_exp_string (ps, str);
> >>    write_exp_elt_opcode (ps, OP_REGISTER);
> >> +  if (innermost_block == NULL
> >> +      || contained_in (expression_context_block,
> >> +		       innermost_block))
> >> +    innermost_block = expression_context_block;
> >>    return;
> >>  }
> > 
> > Your solution works for both the fixed a floating varobj cases.
> > I've extended my new test to cover the floating case.  Unfortunately
> > in the full test suite I've run into some unexpected side-effects
> > that need further investigation.
> 
> I haven't gone through all the failures, but I have found the cause of
> one of the side-effects of this change.  In the CLI, the 'display'
> command depends on parsing a register expression to *not* set
> innermost_block.  If the expression has a block, it has to be in-scope
> or display will skip it rather than evaluating the expression.

OK, so dollar variables shouldn't force any kind of scope
requirement.

>                                                                 See
> printcmd.c:do_one_display, the handling of d->block.  It basically does:
> 
>          d->exp = parse_expression (d->exp_string);
>          d->block = innermost_block;
> ---snip---
>    if (d->block)
>     {
>       if (d->pspace == current_program_space)
>         within_current_scope = contained_in (get_selected_block (0),
> d->block);
> ---snip---
>     }
>   else
>     within_current_scope = 1;
>   if (!within_current_scope)
>     return;
> 
> It seems like we will need a special case someplace.  In do_one_display
> we could NULL out d->block for register expressions, or in varobj_update
> we could expand on the original patch to properly handle floating
> varobjs.

I see it slightly different.  We have a mechanism that works fine, so
long as the var->root->frame is set up correctly on the varobj.  As a
minimum, any special case handling should be moved into varobj_create
and should focus on fixing var->root->frame.

But, I think we can do better.

Before expression parsing we set 'innermost_block = NULL;' then we
parse the expression and 'innermost_block' might have changed to
reflect a need for a "more inner" block.  The initial setting to NULL
as far as I can tell is basically saying, start with no scoping
requirement, and let the expression tell us what it needs.

But, what if instead of starting at NULL, we started at a block/frame
based on the type of varobj being created.   The expression parsing
can still force a "more inner" block requirement, but the default
would no longer be NULL.  It turns out we already have a variable
'block' inside varobj_create that is initialised based on the frame
that is, in turn, initialised based on the type of varobj being
created.  Here's a new patch:

diff --git a/gdb/varobj.c b/gdb/varobj.c
index 6f56cba..f89ae80 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -323,7 +323,7 @@ varobj_create (char *objname,
 	}
 
       p = expression;
-      innermost_block = NULL;
+      innermost_block = block;
       /* Wrap the call to parse expression, so we can 
          return a sensible error.  */
       TRY
@@ -2103,6 +2103,8 @@ new_root_variable (void)
   var->root->floating = 0;
   var->root->rootvar = NULL;
   var->root->is_valid = 1;
+  var->root->thread_id = 0;
+  var->root->next = NULL;
 
   return var;
 }

I've actually tested this one, and I see 7 failures.  I've not looked
at them all yet, but the ones I have looked at all relate to the
output of various varobj commands now including a thread-id field when
they didn't before.  This is a knock on from the varobj having a frame
when it previously didn't.

The thing is, if we ask for a particular expression in a particular
frame, doesn't that imply a particular thread?  We probably need to
review the regressions, but I'm tempted, initially, to say that the
new thread-id is actually the right behaviour, and the test results
should be updated.


Thanks,
Andrew

Re: [PATCH] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

On 6/10/2016 3:32 AM, Andrew Burgess wrote:
> * Don Breazeal <donb@codesourcery.com> [2016-06-09 11:23:09 -0700]:
> 
>> On 6/8/2016 5:48 PM, Breazeal, Don wrote:
>>> On 6/8/2016 6:08 AM, Andrew Burgess wrote:
>>>> * Andrew Burgess <andrew.burgess@embecosm.com> [2016-06-08 14:00:51 +0100]:
>>>>
>>>>> * Don Breazeal <donb@codesourcery.com> [2016-06-07 14:36:57 -0700]:
>>>>>
>>>>>> This patch fixes a problem with using the MI -var-update command
>>>>>> to access the values of registers in frames other than the current
>>>>>> frame.  The patch includes a test that demonstrates the problem:
>>>>>>
>>>>>> * run so there are several frames on the stack
>>>>>> * create a varobj for $pc in each frame, #'s 1 and above
>>>>>> * step one instruction, to modify the value of $pc
>>>>>> * call -var-update for each of the previously created varobjs
>>>>>>   to verify that they are not reported as having changed.
>>>>>>
>>>>>> Without the patch, the -var-update command reported that $pc for all
>>>>>> frames 1 and above had changed to the value of $pc in frame 0.
>>>>>>
>>>>>> The -var-create command takes a '--frame' argument and uses that
>>>>>> to select the frame for retrieving the register value.  But -var-update
>>>>>> has no such argument, and previously didn't do anything to select the
>>>>>> frame, so for frames other than frame 0 it returned the value of the
>>>>>> register for frame 0, instead of reporting the value as unchanged.
>>>>>
>>>>> This shouldn't need special handling for register varobj values, if I
>>>>> create a varobj watching value 'foo' in frame 1, but have a local
>>>>> 'foo' in frame 0, a change in frame 0 'foo' will not trigger a change
>>>>> for frame 1's 'foo' varobj.
>>>>>
>>>>> The magic is actually in varobj.c:check_scope, which makes use of the
>>>>> varobj->root->frame to select the right frame based on the type of the
>>>>> varobj, this is setup at varobj creation time.
>>>>>
>>>>> The problem, is that for register expressions the varobj->root->frame
>>>>> is not set correctly.  This frame tracking is done using the global
>>>>> 'innermost_block' which is set in the various parser files (for
>>>>> example c-exp.y), however, this is not set for register expressions.
>>>>> I think that we probably should be doing this in
>>>>> write_dollar_variable.
>>>
>>> Andrew,
>>> Thanks for explaining.  I had followed innermost_block quite a bit
>>> in my debugging, but somehow convinced myself that wasn't the solution.
>>>
>>>>
>>>> Something like the following (untested):
>>>>
>>>> diff --git a/gdb/parse.c b/gdb/parse.c
>>>> index 2b00708..224c022 100644
>>>> --- a/gdb/parse.c
>>>> +++ b/gdb/parse.c
>>>> @@ -705,6 +705,10 @@ handle_register:
>>>>    str.ptr++;
>>>>    write_exp_string (ps, str);
>>>>    write_exp_elt_opcode (ps, OP_REGISTER);
>>>> +  if (innermost_block == NULL
>>>> +      || contained_in (expression_context_block,
>>>> +		       innermost_block))
>>>> +    innermost_block = expression_context_block;
>>>>    return;
>>>>  }
>>>
>>> Your solution works for both the fixed a floating varobj cases.
>>> I've extended my new test to cover the floating case.  Unfortunately
>>> in the full test suite I've run into some unexpected side-effects
>>> that need further investigation.
>>
>> I haven't gone through all the failures, but I have found the cause of
>> one of the side-effects of this change.  In the CLI, the 'display'
>> command depends on parsing a register expression to *not* set
>> innermost_block.  If the expression has a block, it has to be in-scope
>> or display will skip it rather than evaluating the expression.
> 
> OK, so dollar variables shouldn't force any kind of scope
> requirement.
> 
>>                                                                 See
>> printcmd.c:do_one_display, the handling of d->block.  It basically does:
>>
>>          d->exp = parse_expression (d->exp_string);
>>          d->block = innermost_block;
>> ---snip---
>>    if (d->block)
>>     {
>>       if (d->pspace == current_program_space)
>>         within_current_scope = contained_in (get_selected_block (0),
>> d->block);
>> ---snip---
>>     }
>>   else
>>     within_current_scope = 1;
>>   if (!within_current_scope)
>>     return;
>>
>> It seems like we will need a special case someplace.  In do_one_display
>> we could NULL out d->block for register expressions, or in varobj_update
>> we could expand on the original patch to properly handle floating
>> varobjs.
> 
> I see it slightly different.  We have a mechanism that works fine, so
> long as the var->root->frame is set up correctly on the varobj.  As a
> minimum, any special case handling should be moved into varobj_create
> and should focus on fixing var->root->frame.
> 
> But, I think we can do better.
> 
> Before expression parsing we set 'innermost_block = NULL;' then we
> parse the expression and 'innermost_block' might have changed to
> reflect a need for a "more inner" block.  The initial setting to NULL
> as far as I can tell is basically saying, start with no scoping
> requirement, and let the expression tell us what it needs.
> 
> But, what if instead of starting at NULL, we started at a block/frame
> based on the type of varobj being created.   The expression parsing
> can still force a "more inner" block requirement, but the default
> would no longer be NULL.  It turns out we already have a variable
> 'block' inside varobj_create that is initialised based on the frame
> that is, in turn, initialised based on the type of varobj being
> created.  Here's a new patch:
> 
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 6f56cba..f89ae80 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -323,7 +323,7 @@ varobj_create (char *objname,
>  	}
>  
>        p = expression;
> -      innermost_block = NULL;
> +      innermost_block = block;
>        /* Wrap the call to parse expression, so we can 
>           return a sensible error.  */
>        TRY
> @@ -2103,6 +2103,8 @@ new_root_variable (void)
>    var->root->floating = 0;
>    var->root->rootvar = NULL;
>    var->root->is_valid = 1;
> +  var->root->thread_id = 0;
> +  var->root->next = NULL;
>  
>    return var;
>  }

Hi Andrew,
Thanks for following up on this.  I like the fact that this patch is
MI-specific.

> 
> I've actually tested this one, and I see 7 failures.  I've not looked
> at them all yet, but the ones I have looked at all relate to the
> output of various varobj commands now including a thread-id field when
> they didn't before.  This is a knock on from the varobj having a frame
> when it previously didn't.
> 
> The thing is, if we ask for a particular expression in a particular
> frame, doesn't that imply a particular thread?  We probably need to
> review the regressions, but I'm tempted, initially, to say that the
> new thread-id is actually the right behaviour, and the test results
> should be updated.

I had seen the same FAIL with your previous patch in
gdb.mi/mi-var-create-rtti.exp, and reached the same conclusion.

I ran the test suite with your new patch and saw one failure that was an
actual failure:

FAIL: gdb.mi/mi-var-invalidate.exp: global_simple still alive

where -var-update expected an empty changelist but didn't get one.

Long story short, I was able to get rid of that failure by initializing
innermost_block to the global block instead of the innermost block of
the current frame, as in the patch below.  With that change the patch
passes regression testing and my new test.

The problem occurred in value_of_root_1, where var->root->valid_block
was no longer NULL:

 /* Determine whether the variable is still around. */
 if (var->root->valid_block == NULL || var->root->floating)
   within_scope = 1;

If we initialize using the global scope, then varobj_create initializes
var->root->frame for register varobjs, and value_of_root_1 can still
determine that global variables are in-scope.

Here's the updated patch:

diff --git a/gdb/varobj.c b/gdb/varobj.c
index 6f56cba..1e3f192 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -323,7 +323,7 @@ varobj_create (char *objname,
 	}

       p = expression;
-      innermost_block = NULL;
+      innermost_block = block_global_block (block);
       /* Wrap the call to parse expression, so we can
          return a sensible error.  */
       TRY
@@ -2103,6 +2103,8 @@ new_root_variable (void)
   var->root->floating = 0;
   var->root->rootvar = NULL;
   var->root->is_valid = 1;
+  var->root->thread_id = 0;
+  var->root->next = NULL;

   return var;
 }
@@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle)
   back_to = make_cleanup_restore_current_thread ();

   /* Determine whether the variable is still around.  */
-  if (var->root->valid_block == NULL || var->root->floating)
+  if (var->root->valid_block == NULL
+      || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL
+      || var->root->floating)
     within_scope = 1;
   else if (var->root->thread_id == 0)
     {

Maybe "var->root->valid_block == NULL" is unnecessary above...

If you think this is a reasonable solution, I'll go ahead and submit a
new version of the patch including the new test, test updates, etc.

thanks
--Don

Re: [PATCH] Fix -var-update for registers in frames 1 and up

[Andrew Burgess]

* Don Breazeal <donb@codesourcery.com> [2016-06-10 14:25:23 -0700]:

> On 6/10/2016 3:32 AM, Andrew Burgess wrote:
> > * Don Breazeal <donb@codesourcery.com> [2016-06-09 11:23:09 -0700]:
> > 
> >> On 6/8/2016 5:48 PM, Breazeal, Don wrote:
> >>> On 6/8/2016 6:08 AM, Andrew Burgess wrote:
> >>>> * Andrew Burgess <andrew.burgess@embecosm.com> [2016-06-08 14:00:51 +0100]:
> >>>>
> >>>>> * Don Breazeal <donb@codesourcery.com> [2016-06-07 14:36:57 -0700]:
> >>>>>
> >>>>>> This patch fixes a problem with using the MI -var-update command
> >>>>>> to access the values of registers in frames other than the current
> >>>>>> frame.  The patch includes a test that demonstrates the problem:
> >>>>>>
> >>>>>> * run so there are several frames on the stack
> >>>>>> * create a varobj for $pc in each frame, #'s 1 and above
> >>>>>> * step one instruction, to modify the value of $pc
> >>>>>> * call -var-update for each of the previously created varobjs
> >>>>>>   to verify that they are not reported as having changed.
> >>>>>>
> >>>>>> Without the patch, the -var-update command reported that $pc for all
> >>>>>> frames 1 and above had changed to the value of $pc in frame 0.
> >>>>>>
> >>>>>> The -var-create command takes a '--frame' argument and uses that
> >>>>>> to select the frame for retrieving the register value.  But -var-update
> >>>>>> has no such argument, and previously didn't do anything to select the
> >>>>>> frame, so for frames other than frame 0 it returned the value of the
> >>>>>> register for frame 0, instead of reporting the value as unchanged.
> >>>>>
> >>>>> This shouldn't need special handling for register varobj values, if I
> >>>>> create a varobj watching value 'foo' in frame 1, but have a local
> >>>>> 'foo' in frame 0, a change in frame 0 'foo' will not trigger a change
> >>>>> for frame 1's 'foo' varobj.
> >>>>>
> >>>>> The magic is actually in varobj.c:check_scope, which makes use of the
> >>>>> varobj->root->frame to select the right frame based on the type of the
> >>>>> varobj, this is setup at varobj creation time.
> >>>>>
> >>>>> The problem, is that for register expressions the varobj->root->frame
> >>>>> is not set correctly.  This frame tracking is done using the global
> >>>>> 'innermost_block' which is set in the various parser files (for
> >>>>> example c-exp.y), however, this is not set for register expressions.
> >>>>> I think that we probably should be doing this in
> >>>>> write_dollar_variable.
> >>>
> >>> Andrew,
> >>> Thanks for explaining.  I had followed innermost_block quite a bit
> >>> in my debugging, but somehow convinced myself that wasn't the solution.
> >>>
> >>>>
> >>>> Something like the following (untested):
> >>>>
> >>>> diff --git a/gdb/parse.c b/gdb/parse.c
> >>>> index 2b00708..224c022 100644
> >>>> --- a/gdb/parse.c
> >>>> +++ b/gdb/parse.c
> >>>> @@ -705,6 +705,10 @@ handle_register:
> >>>>    str.ptr++;
> >>>>    write_exp_string (ps, str);
> >>>>    write_exp_elt_opcode (ps, OP_REGISTER);
> >>>> +  if (innermost_block == NULL
> >>>> +      || contained_in (expression_context_block,
> >>>> +		       innermost_block))
> >>>> +    innermost_block = expression_context_block;
> >>>>    return;
> >>>>  }
> >>>
> >>> Your solution works for both the fixed a floating varobj cases.
> >>> I've extended my new test to cover the floating case.  Unfortunately
> >>> in the full test suite I've run into some unexpected side-effects
> >>> that need further investigation.
> >>
> >> I haven't gone through all the failures, but I have found the cause of
> >> one of the side-effects of this change.  In the CLI, the 'display'
> >> command depends on parsing a register expression to *not* set
> >> innermost_block.  If the expression has a block, it has to be in-scope
> >> or display will skip it rather than evaluating the expression.
> > 
> > OK, so dollar variables shouldn't force any kind of scope
> > requirement.
> > 
> >>                                                                 See
> >> printcmd.c:do_one_display, the handling of d->block.  It basically does:
> >>
> >>          d->exp = parse_expression (d->exp_string);
> >>          d->block = innermost_block;
> >> ---snip---
> >>    if (d->block)
> >>     {
> >>       if (d->pspace == current_program_space)
> >>         within_current_scope = contained_in (get_selected_block (0),
> >> d->block);
> >> ---snip---
> >>     }
> >>   else
> >>     within_current_scope = 1;
> >>   if (!within_current_scope)
> >>     return;
> >>
> >> It seems like we will need a special case someplace.  In do_one_display
> >> we could NULL out d->block for register expressions, or in varobj_update
> >> we could expand on the original patch to properly handle floating
> >> varobjs.
> > 
> > I see it slightly different.  We have a mechanism that works fine, so
> > long as the var->root->frame is set up correctly on the varobj.  As a
> > minimum, any special case handling should be moved into varobj_create
> > and should focus on fixing var->root->frame.
> > 
> > But, I think we can do better.
> > 
> > Before expression parsing we set 'innermost_block = NULL;' then we
> > parse the expression and 'innermost_block' might have changed to
> > reflect a need for a "more inner" block.  The initial setting to NULL
> > as far as I can tell is basically saying, start with no scoping
> > requirement, and let the expression tell us what it needs.
> > 
> > But, what if instead of starting at NULL, we started at a block/frame
> > based on the type of varobj being created.   The expression parsing
> > can still force a "more inner" block requirement, but the default
> > would no longer be NULL.  It turns out we already have a variable
> > 'block' inside varobj_create that is initialised based on the frame
> > that is, in turn, initialised based on the type of varobj being
> > created.  Here's a new patch:
> > 
> > diff --git a/gdb/varobj.c b/gdb/varobj.c
> > index 6f56cba..f89ae80 100644
> > --- a/gdb/varobj.c
> > +++ b/gdb/varobj.c
> > @@ -323,7 +323,7 @@ varobj_create (char *objname,
> >  	}
> >  
> >        p = expression;
> > -      innermost_block = NULL;
> > +      innermost_block = block;
> >        /* Wrap the call to parse expression, so we can 
> >           return a sensible error.  */
> >        TRY
> > @@ -2103,6 +2103,8 @@ new_root_variable (void)
> >    var->root->floating = 0;
> >    var->root->rootvar = NULL;
> >    var->root->is_valid = 1;
> > +  var->root->thread_id = 0;
> > +  var->root->next = NULL;
> >  
> >    return var;
> >  }
> 
> Hi Andrew,
> Thanks for following up on this.  I like the fact that this patch is
> MI-specific.
> 
> > 
> > I've actually tested this one, and I see 7 failures.  I've not looked
> > at them all yet, but the ones I have looked at all relate to the
> > output of various varobj commands now including a thread-id field when
> > they didn't before.  This is a knock on from the varobj having a frame
> > when it previously didn't.
> > 
> > The thing is, if we ask for a particular expression in a particular
> > frame, doesn't that imply a particular thread?  We probably need to
> > review the regressions, but I'm tempted, initially, to say that the
> > new thread-id is actually the right behaviour, and the test results
> > should be updated.
> 
> I had seen the same FAIL with your previous patch in
> gdb.mi/mi-var-create-rtti.exp, and reached the same conclusion.
> 
> I ran the test suite with your new patch and saw one failure that was an
> actual failure:
> 
> FAIL: gdb.mi/mi-var-invalidate.exp: global_simple still alive
> 
> where -var-update expected an empty changelist but didn't get one.
> 
> Long story short, I was able to get rid of that failure by initializing
> innermost_block to the global block instead of the innermost block of
> the current frame, as in the patch below.  With that change the patch
> passes regression testing and my new test.
> 
> The problem occurred in value_of_root_1, where var->root->valid_block
> was no longer NULL:
> 
>  /* Determine whether the variable is still around. */
>  if (var->root->valid_block == NULL || var->root->floating)
>    within_scope = 1;
> 
> If we initialize using the global scope, then varobj_create initializes
> var->root->frame for register varobjs, and value_of_root_1 can still
> determine that global variables are in-scope.
> 
> Here's the updated patch:
> 
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 6f56cba..1e3f192 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -323,7 +323,7 @@ varobj_create (char *objname,
>  	}
> 
>        p = expression;
> -      innermost_block = NULL;
> +      innermost_block = block_global_block (block);
>        /* Wrap the call to parse expression, so we can
>           return a sensible error.  */
>        TRY
> @@ -2103,6 +2103,8 @@ new_root_variable (void)
>    var->root->floating = 0;
>    var->root->rootvar = NULL;
>    var->root->is_valid = 1;
> +  var->root->thread_id = 0;
> +  var->root->next = NULL;
> 
>    return var;
>  }
> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle)
>    back_to = make_cleanup_restore_current_thread ();
> 
>    /* Determine whether the variable is still around.  */
> -  if (var->root->valid_block == NULL || var->root->floating)
> +  if (var->root->valid_block == NULL
> +      || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL
> +      || var->root->floating)
>      within_scope = 1;
>    else if (var->root->thread_id == 0)
>      {
> 
> Maybe "var->root->valid_block == NULL" is unnecessary above...
> 
> If you think this is a reasonable solution, I'll go ahead and submit a
> new version of the patch including the new test, test updates, etc.

I think this sounds good.

Thanks,
Andrew

[PATCH v2] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

This is an updated version of a patch to fix access to registers
for frames other than the current frame via var-update.  The piece
of the patch affecting GDB proper has been completely re-written.
It changes the initialization of varobj->root->frame in varobj_create
so that existing mechanisms (with minimal changes) select the correct
frame and provide correct register values from -var-update.

In addition, the new test for this functionality has been expanded
to cover an analogous case using floating variable objects, and
several tests have been updated to account for the fact that with
the new initialization, -var-create and -var-list-children now
provide a thread-id field where previously they didn't.

Thanks
--Don

------------
This patch fixes a problem with using the MI -var-update command
to access the values of registers in frames other than the current
frame.  The patch includes a test that demonstrates the problem:

* run so there are several frames on the stack
* create a fixed varobj for $pc in each frame, #'s 1 and above
* step one instruction, to modify the value of $pc
* call -var-update for each of the previously created varobjs
  to verify that they are not reported as having changed.

Without the patch, the -var-update command reported that $pc for all
frames 1 and above had changed to the value of $pc in frame 0.

When a varobj is created by -var-create, varobj->root->frame should
be set to specified frame.  Then for a subsequent -var-update,
varobj.c:check_scope can use varobj->root->frame to select the
right frame based on the type of varobj.

The problem is that for register expressions varobj->root->frame is not
set correctly.  This frame tracking is done using the global
'innermost_block' which is set in the various parser files (for example
c-exp.y).  However, this is not set for register expressions.

The fix is to change the initialization of innermost_block in
varobj_create from NULL to the global block, as returned by
block_global_block.  Then varobj_create sets varobj->root->frame
for register varobjs, and value_of_root_1 can check for the
global block when determining whether the variable is in-scope
and select the frame appropriately.

A side-effect of this change is that for register varobjs and some
global variable varobjs, the output of -var-create now includes the
thread-id field.  The rationale for this is as follow: if we ask for a
particular expression in a particular frame, that implies a particular
thread.  Thus including the thread-id is correct behavior, and the
test results have been updated accordingly.

In addition there is a new test, gdb.mi/mi-frame-regs.exp, which
performs the test described in the bullet list above as well as a
similar test using a floating variable object to represent $pc.

We attempted a couple alternative solutions:
* a special case in varobj_update to select the frame was not ideal
  because it didn't use the existing mechanisms to select the frame.
* setting innermost_block in write_dollar_variable had side-effects
  in the CLI that would have required special case code.

Tested on x86_64 Linux native, no regressions.

gdb/testsuite/ChangeLog:
2016-06-13  Don Breazeal  <dbreazea@my.domain.org>

	* gdb.ada/mi_interface.exp: Add thread-id field to expected
	output of -var-create and -var-list-children.
	* gdb.ada/mi_var_array.exp: Add thread-id field to expected
	output of -var-list-children.
	* gdb.mi/mi-break.exp (test_error): Add thread-id field to
	expected output of -var-create.
	* gdb.mi/mi-frame-regs.exp: New test script.
	* gdb.mi/mi-var-cmd.exp: Add thread-id field to expected
	output of -var-create.
	* gdb.mi/mi-var-create-rtti.exp: Likewise.

gdb/ChangeLog:
2016-06-13  Don Breazeal  <donb@codesourcery.com>
	    Andrew Burgess <andrew.burgess@embecosm.com>

	* varobj.c (varobj_create): Initialize innermost_block to
	the global block instead of NULL.
	(new_root_variable): Initialize the thread_id and next
	fields.
	(value_of_root_1): Set within_scope if the variable's
	valid_block field is the global block.

---
 gdb/testsuite/gdb.ada/mi_interface.exp      |   4 +-
 gdb/testsuite/gdb.ada/mi_var_array.exp      |   2 +-
 gdb/testsuite/gdb.mi/mi-break.exp           |   2 +-
 gdb/testsuite/gdb.mi/mi-frame-regs.exp      | 183 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-var-cmd.exp         |   4 +-
 gdb/testsuite/gdb.mi/mi-var-create-rtti.exp |   2 +-
 gdb/varobj.c                                |   8 +-
 7 files changed, 196 insertions(+), 9 deletions(-)

diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp
index 6000ec8..b948cd5 100644
--- a/gdb/testsuite/gdb.ada/mi_interface.exp
+++ b/gdb/testsuite/gdb.ada/mi_interface.exp
@@ -44,9 +44,9 @@ mi_continue_to_line \
     "stop at start of main Ada procedure"
 
 mi_gdb_test "-var-create ggg1 * ggg1" \
-    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \
+    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \
     "Create ggg1 varobj"
 
 mi_gdb_test "-var-list-children 1 ggg1" \
-    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \
+    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \
     "list ggg1's children"
diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp
index c648e7e..c02d4c9 100644
--- a/gdb/testsuite/gdb.ada/mi_var_array.exp
+++ b/gdb/testsuite/gdb.ada/mi_var_array.exp
@@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \
     "Create bt varobj"
 
 mi_gdb_test "-var-list-children vta" \
-    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \
+    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \
     "list vta's children"
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index 85f328d..bdd43e0 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -213,7 +213,7 @@ proc test_error {} {
     # containing function call, the internal breakpoint created to handle
     # function call would be reported, messing up MI output.
     mi_gdb_test "-var-create V * return_1()" \
-        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \
+        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \
         "create varobj for function call"
 
     mi_gdb_test "-var-update *" \
diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
new file mode 100644
index 0000000..45f81d6
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
@@ -0,0 +1,183 @@
+# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
+
+# Test essential Machine interface (MI) operations
+#
+# Verify that -var-update will provide the correct values for floating
+# and fixed varobjs that represent the pc register.
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile basics.c
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+		 executable {debug}] != "" } then {
+     untested mi-frame-regs.exp
+     return -1
+}
+
+# Return the address of the specified breakpoint.
+
+proc breakpoint_address {bpnum} {
+    global hex
+    global expect_out
+    global mi_gdb_prompt
+
+    send_gdb "info breakpoint $bpnum\n"
+    gdb_expect {
+	-re ".*($hex).*$mi_gdb_prompt$" {
+	    return $expect_out(1,string)
+	}
+	-re ".*$mi_gdb_prompt$" {
+	    return ""
+	}
+	timeout {
+	    return ""
+	}
+    }
+}
+
+# Test that a floating varobj representing $pc will provide the
+# correct value via -var-update as the program stops at
+# breakpoints in different functions.
+
+proc do_floating_varobj_test {} {
+    global srcfile
+    global hex
+    global expect_out
+
+    gdb_exit
+    if {[mi_gdb_start]} then {
+	continue
+    }
+
+    with_test_prefix "floating" {
+	mi_run_to_main
+
+	# Create a floating varobj for $pc.
+	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
+		    "\\^done,.*value=\"$hex.*" \
+	            "create varobj for pc in frame 0"
+
+	set nframes 4
+	for {set i 1} {$i < $nframes} {incr i} {
+
+	    # Run to a breakpoint in each callee function in succession.
+	    # Note that we can't use mi_runto because we need the
+	    # breakpoint to be persistent, so we can use its address.
+	    set bpnum [expr $i + 1]
+	    mi_create_breakpoint \
+	        "basics.c:callee$i" \
+		"insert breakpoint at basics.c:callee$i" \
+		-number $bpnum -func callee$i -file ".*basics.c"
+
+	    mi_execute_to "exec-continue" "breakpoint-hit" \
+			  "callee$i" ".*" ".*${srcfile}" ".*" \
+			  { "" "disp=\"keep\"" } "breakpoint hit"
+
+	    # Get the value of $pc from the floating varobj.
+	    mi_gdb_test "-var-update 1 var1" \
+			"\\^done,.*value=\"($hex) .*" \
+			"-var-update for frame $i"
+	    set pcval $expect_out(3,string)
+
+	    # Get the address of the current breakpoint.
+	    set bpaddr [breakpoint_address $bpnum]
+	    if {$bpaddr == ""} then {
+		unresolved "get address of breakpoint $bpnum"
+		return
+	    }
+
+	    # Check that the addresses are the same.
+	    if {[expr $bpaddr != $pcval]} then {
+	        fail "\$pc does not equal address of breakpoint"
+	    } else {
+	        pass "\$pc equals address of breakpoint"
+	    }
+	}
+    }
+}
+
+# Create a varobj for the pc register in each of the frames other
+# than frame 0.
+
+proc var_create_regs {nframes} {
+    global hex
+
+    for {set i 1} {$i < $nframes} {incr i} {
+	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
+		    "\\^done,.*value=\"$hex.*" \
+	            "create varobj for pc in frame $i"
+    }
+}
+
+# Check that -var-update reports that the value of the pc register
+# for each of the frames 1 and above is reported as unchanged.
+
+proc var_update_regs {nframes} {
+
+    for {set i 1} {$i < $nframes} {incr i} {
+	mi_gdb_test "-var-update 1 var$i" \
+	            "\\^done,(changelist=\\\[\\\])" \
+	            "pc unchanged in -var-update for frame $i"
+    }
+}
+
+# Test that fixed varobjs representing $pc in different stack frames
+# will provide the correct value via -var-update after the program
+# counter changes (without substantially changing the stack).
+
+proc do_fixed_varobj_test {} {
+    global srcfile
+
+    gdb_exit
+    if {[mi_gdb_start]} then {
+	continue
+    }
+
+    with_test_prefix "fixed" {
+	mi_run_to_main
+
+	# Run to the function 'callee3' so we have several frames.
+	mi_create_breakpoint "basics.c:callee3" \
+			     "insert breakpoint at basics.c:callee3" \
+			     -number 2 -func callee3 -file ".*basics.c"
+
+	mi_execute_to "exec-continue" "breakpoint-hit" \
+	              "callee3" ".*" ".*${srcfile}" ".*" \
+		      { "" "disp=\"keep\"" } "breakpoint hit"
+
+	# At the breakpoint in callee3 there are 4 frames.  Create a
+	# varobj for the pc in each of frames 1 and above.
+	set nframes 4
+	var_create_regs $nframes
+
+	# Step one instruction to change the program counter.
+	mi_execute_to "exec-next-instruction" "end-stepping-range" \
+		      "callee3" ".*" ".*${srcfile}" ".*" "" \
+		      "next instruction in callee3"
+
+	# Check that -var-update reports that the values are unchanged.
+	var_update_regs $nframes
+    }
+}
+
+do_fixed_varobj_test
+do_floating_varobj_test
+
+mi_gdb_exit
+return 0
diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
index 558cd6c..68e3cf8 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
@@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \
 	"assign same value to func (update)"
 
 mi_gdb_test "-var-create array_ptr * array_ptr" \
-	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \
+	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \
 	"create global variable array_ptr"
 
 mi_gdb_test "-var-assign array_ptr array2" \
@@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee"
 # A varobj we fail to read during -var-update should be considered
 # out of scope.
 mi_gdb_test "-var-create null_ptr * **0" \
-    {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \
+    {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \
     "create null_ptr"
 
 # Allow this to succeed, if address zero is readable, although it
diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
index 3bcb36c..a8cc76f 100644
--- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
@@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*"
 # We use a explicit cast to (void *) as that is the
 # type that caused the bug this testcase is testing for.
 mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \
-	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \
+	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \
 	    "-var-create sp1 * \$sp"
 gdb_exit
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 6f56cba..1e3f192 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -323,7 +323,7 @@ varobj_create (char *objname,
 	}
 
       p = expression;
-      innermost_block = NULL;
+      innermost_block = block_global_block (block);
       /* Wrap the call to parse expression, so we can 
          return a sensible error.  */
       TRY
@@ -2103,6 +2103,8 @@ new_root_variable (void)
   var->root->floating = 0;
   var->root->rootvar = NULL;
   var->root->is_valid = 1;
+  var->root->thread_id = 0;
+  var->root->next = NULL;
 
   return var;
 }
@@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle)
   back_to = make_cleanup_restore_current_thread ();
 
   /* Determine whether the variable is still around.  */
-  if (var->root->valid_block == NULL || var->root->floating)
+  if (var->root->valid_block == NULL
+      || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL
+      || var->root->floating)
     within_scope = 1;
   else if (var->root->thread_id == 0)
     {
-- 
2.8.1

[PING] [PATCH v2] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

On 6/13/2016 2:54 PM, Don Breazeal wrote:
> This is an updated version of a patch to fix access to registers
> for frames other than the current frame via var-update.  The piece
> of the patch affecting GDB proper has been completely re-written.
> It changes the initialization of varobj->root->frame in varobj_create
> so that existing mechanisms (with minimal changes) select the correct
> frame and provide correct register values from -var-update.
> 
> In addition, the new test for this functionality has been expanded
> to cover an analogous case using floating variable objects, and
> several tests have been updated to account for the fact that with
> the new initialization, -var-create and -var-list-children now
> provide a thread-id field where previously they didn't.
> 
> Thanks
> --Don
> 
> ------------
> This patch fixes a problem with using the MI -var-update command
> to access the values of registers in frames other than the current
> frame.  The patch includes a test that demonstrates the problem:
> 
> * run so there are several frames on the stack
> * create a fixed varobj for $pc in each frame, #'s 1 and above
> * step one instruction, to modify the value of $pc
> * call -var-update for each of the previously created varobjs
>   to verify that they are not reported as having changed.
> 
> Without the patch, the -var-update command reported that $pc for all
> frames 1 and above had changed to the value of $pc in frame 0.
> 
> When a varobj is created by -var-create, varobj->root->frame should
> be set to specified frame.  Then for a subsequent -var-update,
> varobj.c:check_scope can use varobj->root->frame to select the
> right frame based on the type of varobj.
> 
> The problem is that for register expressions varobj->root->frame is not
> set correctly.  This frame tracking is done using the global
> 'innermost_block' which is set in the various parser files (for example
> c-exp.y).  However, this is not set for register expressions.
> 
> The fix is to change the initialization of innermost_block in
> varobj_create from NULL to the global block, as returned by
> block_global_block.  Then varobj_create sets varobj->root->frame
> for register varobjs, and value_of_root_1 can check for the
> global block when determining whether the variable is in-scope
> and select the frame appropriately.
> 
> A side-effect of this change is that for register varobjs and some
> global variable varobjs, the output of -var-create now includes the
> thread-id field.  The rationale for this is as follow: if we ask for a
> particular expression in a particular frame, that implies a particular
> thread.  Thus including the thread-id is correct behavior, and the
> test results have been updated accordingly.
> 
> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which
> performs the test described in the bullet list above as well as a
> similar test using a floating variable object to represent $pc.
> 
> We attempted a couple alternative solutions:
> * a special case in varobj_update to select the frame was not ideal
>   because it didn't use the existing mechanisms to select the frame.
> * setting innermost_block in write_dollar_variable had side-effects
>   in the CLI that would have required special case code.
> 
> Tested on x86_64 Linux native, no regressions.
> 
> gdb/testsuite/ChangeLog:
> 2016-06-13  Don Breazeal  <dbreazea@my.domain.org>
> 
> 	* gdb.ada/mi_interface.exp: Add thread-id field to expected
> 	output of -var-create and -var-list-children.
> 	* gdb.ada/mi_var_array.exp: Add thread-id field to expected
> 	output of -var-list-children.
> 	* gdb.mi/mi-break.exp (test_error): Add thread-id field to
> 	expected output of -var-create.
> 	* gdb.mi/mi-frame-regs.exp: New test script.
> 	* gdb.mi/mi-var-cmd.exp: Add thread-id field to expected
> 	output of -var-create.
> 	* gdb.mi/mi-var-create-rtti.exp: Likewise.
> 
> gdb/ChangeLog:
> 2016-06-13  Don Breazeal  <donb@codesourcery.com>
> 	    Andrew Burgess <andrew.burgess@embecosm.com>
> 
> 	* varobj.c (varobj_create): Initialize innermost_block to
> 	the global block instead of NULL.
> 	(new_root_variable): Initialize the thread_id and next
> 	fields.
> 	(value_of_root_1): Set within_scope if the variable's
> 	valid_block field is the global block.
> 
> ---
>  gdb/testsuite/gdb.ada/mi_interface.exp      |   4 +-
>  gdb/testsuite/gdb.ada/mi_var_array.exp      |   2 +-
>  gdb/testsuite/gdb.mi/mi-break.exp           |   2 +-
>  gdb/testsuite/gdb.mi/mi-frame-regs.exp      | 183 ++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.mi/mi-var-cmd.exp         |   4 +-
>  gdb/testsuite/gdb.mi/mi-var-create-rtti.exp |   2 +-
>  gdb/varobj.c                                |   8 +-
>  7 files changed, 196 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp
> index 6000ec8..b948cd5 100644
> --- a/gdb/testsuite/gdb.ada/mi_interface.exp
> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp
> @@ -44,9 +44,9 @@ mi_continue_to_line \
>      "stop at start of main Ada procedure"
>  
>  mi_gdb_test "-var-create ggg1 * ggg1" \
> -    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \
> +    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \
>      "Create ggg1 varobj"
>  
>  mi_gdb_test "-var-list-children 1 ggg1" \
> -    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \
> +    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \
>      "list ggg1's children"
> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp
> index c648e7e..c02d4c9 100644
> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp
> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp
> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \
>      "Create bt varobj"
>  
>  mi_gdb_test "-var-list-children vta" \
> -    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \
> +    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \
>      "list vta's children"
> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
> index 85f328d..bdd43e0 100644
> --- a/gdb/testsuite/gdb.mi/mi-break.exp
> +++ b/gdb/testsuite/gdb.mi/mi-break.exp
> @@ -213,7 +213,7 @@ proc test_error {} {
>      # containing function call, the internal breakpoint created to handle
>      # function call would be reported, messing up MI output.
>      mi_gdb_test "-var-create V * return_1()" \
> -        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \
> +        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \
>          "create varobj for function call"
>  
>      mi_gdb_test "-var-update *" \
> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
> new file mode 100644
> index 0000000..45f81d6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
> @@ -0,0 +1,183 @@
> +# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
> +
> +# Test essential Machine interface (MI) operations
> +#
> +# Verify that -var-update will provide the correct values for floating
> +# and fixed varobjs that represent the pc register.
> +#
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +standard_testfile basics.c
> +
> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> +		 executable {debug}] != "" } then {
> +     untested mi-frame-regs.exp
> +     return -1
> +}
> +
> +# Return the address of the specified breakpoint.
> +
> +proc breakpoint_address {bpnum} {
> +    global hex
> +    global expect_out
> +    global mi_gdb_prompt
> +
> +    send_gdb "info breakpoint $bpnum\n"
> +    gdb_expect {
> +	-re ".*($hex).*$mi_gdb_prompt$" {
> +	    return $expect_out(1,string)
> +	}
> +	-re ".*$mi_gdb_prompt$" {
> +	    return ""
> +	}
> +	timeout {
> +	    return ""
> +	}
> +    }
> +}
> +
> +# Test that a floating varobj representing $pc will provide the
> +# correct value via -var-update as the program stops at
> +# breakpoints in different functions.
> +
> +proc do_floating_varobj_test {} {
> +    global srcfile
> +    global hex
> +    global expect_out
> +
> +    gdb_exit
> +    if {[mi_gdb_start]} then {
> +	continue
> +    }
> +
> +    with_test_prefix "floating" {
> +	mi_run_to_main
> +
> +	# Create a floating varobj for $pc.
> +	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
> +		    "\\^done,.*value=\"$hex.*" \
> +	            "create varobj for pc in frame 0"
> +
> +	set nframes 4
> +	for {set i 1} {$i < $nframes} {incr i} {
> +
> +	    # Run to a breakpoint in each callee function in succession.
> +	    # Note that we can't use mi_runto because we need the
> +	    # breakpoint to be persistent, so we can use its address.
> +	    set bpnum [expr $i + 1]
> +	    mi_create_breakpoint \
> +	        "basics.c:callee$i" \
> +		"insert breakpoint at basics.c:callee$i" \
> +		-number $bpnum -func callee$i -file ".*basics.c"
> +
> +	    mi_execute_to "exec-continue" "breakpoint-hit" \
> +			  "callee$i" ".*" ".*${srcfile}" ".*" \
> +			  { "" "disp=\"keep\"" } "breakpoint hit"
> +
> +	    # Get the value of $pc from the floating varobj.
> +	    mi_gdb_test "-var-update 1 var1" \
> +			"\\^done,.*value=\"($hex) .*" \
> +			"-var-update for frame $i"
> +	    set pcval $expect_out(3,string)
> +
> +	    # Get the address of the current breakpoint.
> +	    set bpaddr [breakpoint_address $bpnum]
> +	    if {$bpaddr == ""} then {
> +		unresolved "get address of breakpoint $bpnum"
> +		return
> +	    }
> +
> +	    # Check that the addresses are the same.
> +	    if {[expr $bpaddr != $pcval]} then {
> +	        fail "\$pc does not equal address of breakpoint"
> +	    } else {
> +	        pass "\$pc equals address of breakpoint"
> +	    }
> +	}
> +    }
> +}
> +
> +# Create a varobj for the pc register in each of the frames other
> +# than frame 0.
> +
> +proc var_create_regs {nframes} {
> +    global hex
> +
> +    for {set i 1} {$i < $nframes} {incr i} {
> +	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
> +		    "\\^done,.*value=\"$hex.*" \
> +	            "create varobj for pc in frame $i"
> +    }
> +}
> +
> +# Check that -var-update reports that the value of the pc register
> +# for each of the frames 1 and above is reported as unchanged.
> +
> +proc var_update_regs {nframes} {
> +
> +    for {set i 1} {$i < $nframes} {incr i} {
> +	mi_gdb_test "-var-update 1 var$i" \
> +	            "\\^done,(changelist=\\\[\\\])" \
> +	            "pc unchanged in -var-update for frame $i"
> +    }
> +}
> +
> +# Test that fixed varobjs representing $pc in different stack frames
> +# will provide the correct value via -var-update after the program
> +# counter changes (without substantially changing the stack).
> +
> +proc do_fixed_varobj_test {} {
> +    global srcfile
> +
> +    gdb_exit
> +    if {[mi_gdb_start]} then {
> +	continue
> +    }
> +
> +    with_test_prefix "fixed" {
> +	mi_run_to_main
> +
> +	# Run to the function 'callee3' so we have several frames.
> +	mi_create_breakpoint "basics.c:callee3" \
> +			     "insert breakpoint at basics.c:callee3" \
> +			     -number 2 -func callee3 -file ".*basics.c"
> +
> +	mi_execute_to "exec-continue" "breakpoint-hit" \
> +	              "callee3" ".*" ".*${srcfile}" ".*" \
> +		      { "" "disp=\"keep\"" } "breakpoint hit"
> +
> +	# At the breakpoint in callee3 there are 4 frames.  Create a
> +	# varobj for the pc in each of frames 1 and above.
> +	set nframes 4
> +	var_create_regs $nframes
> +
> +	# Step one instruction to change the program counter.
> +	mi_execute_to "exec-next-instruction" "end-stepping-range" \
> +		      "callee3" ".*" ".*${srcfile}" ".*" "" \
> +		      "next instruction in callee3"
> +
> +	# Check that -var-update reports that the values are unchanged.
> +	var_update_regs $nframes
> +    }
> +}
> +
> +do_fixed_varobj_test
> +do_floating_varobj_test
> +
> +mi_gdb_exit
> +return 0
> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
> index 558cd6c..68e3cf8 100644
> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp
> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \
>  	"assign same value to func (update)"
>  
>  mi_gdb_test "-var-create array_ptr * array_ptr" \
> -	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \
> +	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \
>  	"create global variable array_ptr"
>  
>  mi_gdb_test "-var-assign array_ptr array2" \
> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee"
>  # A varobj we fail to read during -var-update should be considered
>  # out of scope.
>  mi_gdb_test "-var-create null_ptr * **0" \
> -    {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \
> +    {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \
>      "create null_ptr"
>  
>  # Allow this to succeed, if address zero is readable, although it
> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
> index 3bcb36c..a8cc76f 100644
> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*"
>  # We use a explicit cast to (void *) as that is the
>  # type that caused the bug this testcase is testing for.
>  mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \
> -	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \
> +	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \
>  	    "-var-create sp1 * \$sp"
>  gdb_exit
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 6f56cba..1e3f192 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -323,7 +323,7 @@ varobj_create (char *objname,
>  	}
>  
>        p = expression;
> -      innermost_block = NULL;
> +      innermost_block = block_global_block (block);
>        /* Wrap the call to parse expression, so we can 
>           return a sensible error.  */
>        TRY
> @@ -2103,6 +2103,8 @@ new_root_variable (void)
>    var->root->floating = 0;
>    var->root->rootvar = NULL;
>    var->root->is_valid = 1;
> +  var->root->thread_id = 0;
> +  var->root->next = NULL;
>  
>    return var;
>  }
> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle)
>    back_to = make_cleanup_restore_current_thread ();
>  
>    /* Determine whether the variable is still around.  */
> -  if (var->root->valid_block == NULL || var->root->floating)
> +  if (var->root->valid_block == NULL
> +      || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL
> +      || var->root->floating)
>      within_scope = 1;
>    else if (var->root->thread_id == 0)
>      {
> 

Re: [PING] [PATCH v2] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

ping

On 6/20/2016 8:31 AM, Don Breazeal wrote:
> On 6/13/2016 2:54 PM, Don Breazeal wrote:
>> This is an updated version of a patch to fix access to registers
>> for frames other than the current frame via var-update.  The piece
>> of the patch affecting GDB proper has been completely re-written.
>> It changes the initialization of varobj->root->frame in varobj_create
>> so that existing mechanisms (with minimal changes) select the correct
>> frame and provide correct register values from -var-update.
>>
>> In addition, the new test for this functionality has been expanded
>> to cover an analogous case using floating variable objects, and
>> several tests have been updated to account for the fact that with
>> the new initialization, -var-create and -var-list-children now
>> provide a thread-id field where previously they didn't.
>>
>> Thanks
>> --Don
>>
>> ------------
>> This patch fixes a problem with using the MI -var-update command
>> to access the values of registers in frames other than the current
>> frame.  The patch includes a test that demonstrates the problem:
>>
>> * run so there are several frames on the stack
>> * create a fixed varobj for $pc in each frame, #'s 1 and above
>> * step one instruction, to modify the value of $pc
>> * call -var-update for each of the previously created varobjs
>>   to verify that they are not reported as having changed.
>>
>> Without the patch, the -var-update command reported that $pc for all
>> frames 1 and above had changed to the value of $pc in frame 0.
>>
>> When a varobj is created by -var-create, varobj->root->frame should
>> be set to specified frame.  Then for a subsequent -var-update,
>> varobj.c:check_scope can use varobj->root->frame to select the
>> right frame based on the type of varobj.
>>
>> The problem is that for register expressions varobj->root->frame is not
>> set correctly.  This frame tracking is done using the global
>> 'innermost_block' which is set in the various parser files (for example
>> c-exp.y).  However, this is not set for register expressions.
>>
>> The fix is to change the initialization of innermost_block in
>> varobj_create from NULL to the global block, as returned by
>> block_global_block.  Then varobj_create sets varobj->root->frame
>> for register varobjs, and value_of_root_1 can check for the
>> global block when determining whether the variable is in-scope
>> and select the frame appropriately.
>>
>> A side-effect of this change is that for register varobjs and some
>> global variable varobjs, the output of -var-create now includes the
>> thread-id field.  The rationale for this is as follow: if we ask for a
>> particular expression in a particular frame, that implies a particular
>> thread.  Thus including the thread-id is correct behavior, and the
>> test results have been updated accordingly.
>>
>> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which
>> performs the test described in the bullet list above as well as a
>> similar test using a floating variable object to represent $pc.
>>
>> We attempted a couple alternative solutions:
>> * a special case in varobj_update to select the frame was not ideal
>>   because it didn't use the existing mechanisms to select the frame.
>> * setting innermost_block in write_dollar_variable had side-effects
>>   in the CLI that would have required special case code.
>>
>> Tested on x86_64 Linux native, no regressions.
>>
>> gdb/testsuite/ChangeLog:
>> 2016-06-13  Don Breazeal  <dbreazea@my.domain.org>
>>
>> 	* gdb.ada/mi_interface.exp: Add thread-id field to expected
>> 	output of -var-create and -var-list-children.
>> 	* gdb.ada/mi_var_array.exp: Add thread-id field to expected
>> 	output of -var-list-children.
>> 	* gdb.mi/mi-break.exp (test_error): Add thread-id field to
>> 	expected output of -var-create.
>> 	* gdb.mi/mi-frame-regs.exp: New test script.
>> 	* gdb.mi/mi-var-cmd.exp: Add thread-id field to expected
>> 	output of -var-create.
>> 	* gdb.mi/mi-var-create-rtti.exp: Likewise.
>>
>> gdb/ChangeLog:
>> 2016-06-13  Don Breazeal  <donb@codesourcery.com>
>> 	    Andrew Burgess <andrew.burgess@embecosm.com>
>>
>> 	* varobj.c (varobj_create): Initialize innermost_block to
>> 	the global block instead of NULL.
>> 	(new_root_variable): Initialize the thread_id and next
>> 	fields.
>> 	(value_of_root_1): Set within_scope if the variable's
>> 	valid_block field is the global block.
>>
>> ---
>>  gdb/testsuite/gdb.ada/mi_interface.exp      |   4 +-
>>  gdb/testsuite/gdb.ada/mi_var_array.exp      |   2 +-
>>  gdb/testsuite/gdb.mi/mi-break.exp           |   2 +-
>>  gdb/testsuite/gdb.mi/mi-frame-regs.exp      | 183 ++++++++++++++++++++++++++++
>>  gdb/testsuite/gdb.mi/mi-var-cmd.exp         |   4 +-
>>  gdb/testsuite/gdb.mi/mi-var-create-rtti.exp |   2 +-
>>  gdb/varobj.c                                |   8 +-
>>  7 files changed, 196 insertions(+), 9 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp
>> index 6000ec8..b948cd5 100644
>> --- a/gdb/testsuite/gdb.ada/mi_interface.exp
>> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp
>> @@ -44,9 +44,9 @@ mi_continue_to_line \
>>      "stop at start of main Ada procedure"
>>  
>>  mi_gdb_test "-var-create ggg1 * ggg1" \
>> -    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \
>> +    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \
>>      "Create ggg1 varobj"
>>  
>>  mi_gdb_test "-var-list-children 1 ggg1" \
>> -    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \
>> +    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \
>>      "list ggg1's children"
>> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp
>> index c648e7e..c02d4c9 100644
>> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp
>> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp
>> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \
>>      "Create bt varobj"
>>  
>>  mi_gdb_test "-var-list-children vta" \
>> -    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \
>> +    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \
>>      "list vta's children"
>> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
>> index 85f328d..bdd43e0 100644
>> --- a/gdb/testsuite/gdb.mi/mi-break.exp
>> +++ b/gdb/testsuite/gdb.mi/mi-break.exp
>> @@ -213,7 +213,7 @@ proc test_error {} {
>>      # containing function call, the internal breakpoint created to handle
>>      # function call would be reported, messing up MI output.
>>      mi_gdb_test "-var-create V * return_1()" \
>> -        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \
>> +        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \
>>          "create varobj for function call"
>>  
>>      mi_gdb_test "-var-update *" \
>> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
>> new file mode 100644
>> index 0000000..45f81d6
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
>> @@ -0,0 +1,183 @@
>> +# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
>> +
>> +# Test essential Machine interface (MI) operations
>> +#
>> +# Verify that -var-update will provide the correct values for floating
>> +# and fixed varobjs that represent the pc register.
>> +#
>> +
>> +load_lib mi-support.exp
>> +set MIFLAGS "-i=mi"
>> +
>> +standard_testfile basics.c
>> +
>> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
>> +		 executable {debug}] != "" } then {
>> +     untested mi-frame-regs.exp
>> +     return -1
>> +}
>> +
>> +# Return the address of the specified breakpoint.
>> +
>> +proc breakpoint_address {bpnum} {
>> +    global hex
>> +    global expect_out
>> +    global mi_gdb_prompt
>> +
>> +    send_gdb "info breakpoint $bpnum\n"
>> +    gdb_expect {
>> +	-re ".*($hex).*$mi_gdb_prompt$" {
>> +	    return $expect_out(1,string)
>> +	}
>> +	-re ".*$mi_gdb_prompt$" {
>> +	    return ""
>> +	}
>> +	timeout {
>> +	    return ""
>> +	}
>> +    }
>> +}
>> +
>> +# Test that a floating varobj representing $pc will provide the
>> +# correct value via -var-update as the program stops at
>> +# breakpoints in different functions.
>> +
>> +proc do_floating_varobj_test {} {
>> +    global srcfile
>> +    global hex
>> +    global expect_out
>> +
>> +    gdb_exit
>> +    if {[mi_gdb_start]} then {
>> +	continue
>> +    }
>> +
>> +    with_test_prefix "floating" {
>> +	mi_run_to_main
>> +
>> +	# Create a floating varobj for $pc.
>> +	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
>> +		    "\\^done,.*value=\"$hex.*" \
>> +	            "create varobj for pc in frame 0"
>> +
>> +	set nframes 4
>> +	for {set i 1} {$i < $nframes} {incr i} {
>> +
>> +	    # Run to a breakpoint in each callee function in succession.
>> +	    # Note that we can't use mi_runto because we need the
>> +	    # breakpoint to be persistent, so we can use its address.
>> +	    set bpnum [expr $i + 1]
>> +	    mi_create_breakpoint \
>> +	        "basics.c:callee$i" \
>> +		"insert breakpoint at basics.c:callee$i" \
>> +		-number $bpnum -func callee$i -file ".*basics.c"
>> +
>> +	    mi_execute_to "exec-continue" "breakpoint-hit" \
>> +			  "callee$i" ".*" ".*${srcfile}" ".*" \
>> +			  { "" "disp=\"keep\"" } "breakpoint hit"
>> +
>> +	    # Get the value of $pc from the floating varobj.
>> +	    mi_gdb_test "-var-update 1 var1" \
>> +			"\\^done,.*value=\"($hex) .*" \
>> +			"-var-update for frame $i"
>> +	    set pcval $expect_out(3,string)
>> +
>> +	    # Get the address of the current breakpoint.
>> +	    set bpaddr [breakpoint_address $bpnum]
>> +	    if {$bpaddr == ""} then {
>> +		unresolved "get address of breakpoint $bpnum"
>> +		return
>> +	    }
>> +
>> +	    # Check that the addresses are the same.
>> +	    if {[expr $bpaddr != $pcval]} then {
>> +	        fail "\$pc does not equal address of breakpoint"
>> +	    } else {
>> +	        pass "\$pc equals address of breakpoint"
>> +	    }
>> +	}
>> +    }
>> +}
>> +
>> +# Create a varobj for the pc register in each of the frames other
>> +# than frame 0.
>> +
>> +proc var_create_regs {nframes} {
>> +    global hex
>> +
>> +    for {set i 1} {$i < $nframes} {incr i} {
>> +	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
>> +		    "\\^done,.*value=\"$hex.*" \
>> +	            "create varobj for pc in frame $i"
>> +    }
>> +}
>> +
>> +# Check that -var-update reports that the value of the pc register
>> +# for each of the frames 1 and above is reported as unchanged.
>> +
>> +proc var_update_regs {nframes} {
>> +
>> +    for {set i 1} {$i < $nframes} {incr i} {
>> +	mi_gdb_test "-var-update 1 var$i" \
>> +	            "\\^done,(changelist=\\\[\\\])" \
>> +	            "pc unchanged in -var-update for frame $i"
>> +    }
>> +}
>> +
>> +# Test that fixed varobjs representing $pc in different stack frames
>> +# will provide the correct value via -var-update after the program
>> +# counter changes (without substantially changing the stack).
>> +
>> +proc do_fixed_varobj_test {} {
>> +    global srcfile
>> +
>> +    gdb_exit
>> +    if {[mi_gdb_start]} then {
>> +	continue
>> +    }
>> +
>> +    with_test_prefix "fixed" {
>> +	mi_run_to_main
>> +
>> +	# Run to the function 'callee3' so we have several frames.
>> +	mi_create_breakpoint "basics.c:callee3" \
>> +			     "insert breakpoint at basics.c:callee3" \
>> +			     -number 2 -func callee3 -file ".*basics.c"
>> +
>> +	mi_execute_to "exec-continue" "breakpoint-hit" \
>> +	              "callee3" ".*" ".*${srcfile}" ".*" \
>> +		      { "" "disp=\"keep\"" } "breakpoint hit"
>> +
>> +	# At the breakpoint in callee3 there are 4 frames.  Create a
>> +	# varobj for the pc in each of frames 1 and above.
>> +	set nframes 4
>> +	var_create_regs $nframes
>> +
>> +	# Step one instruction to change the program counter.
>> +	mi_execute_to "exec-next-instruction" "end-stepping-range" \
>> +		      "callee3" ".*" ".*${srcfile}" ".*" "" \
>> +		      "next instruction in callee3"
>> +
>> +	# Check that -var-update reports that the values are unchanged.
>> +	var_update_regs $nframes
>> +    }
>> +}
>> +
>> +do_fixed_varobj_test
>> +do_floating_varobj_test
>> +
>> +mi_gdb_exit
>> +return 0
>> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>> index 558cd6c..68e3cf8 100644
>> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \
>>  	"assign same value to func (update)"
>>  
>>  mi_gdb_test "-var-create array_ptr * array_ptr" \
>> -	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \
>> +	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \
>>  	"create global variable array_ptr"
>>  
>>  mi_gdb_test "-var-assign array_ptr array2" \
>> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee"
>>  # A varobj we fail to read during -var-update should be considered
>>  # out of scope.
>>  mi_gdb_test "-var-create null_ptr * **0" \
>> -    {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \
>> +    {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \
>>      "create null_ptr"
>>  
>>  # Allow this to succeed, if address zero is readable, although it
>> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>> index 3bcb36c..a8cc76f 100644
>> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*"
>>  # We use a explicit cast to (void *) as that is the
>>  # type that caused the bug this testcase is testing for.
>>  mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \
>> -	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \
>> +	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \
>>  	    "-var-create sp1 * \$sp"
>>  gdb_exit
>> diff --git a/gdb/varobj.c b/gdb/varobj.c
>> index 6f56cba..1e3f192 100644
>> --- a/gdb/varobj.c
>> +++ b/gdb/varobj.c
>> @@ -323,7 +323,7 @@ varobj_create (char *objname,
>>  	}
>>  
>>        p = expression;
>> -      innermost_block = NULL;
>> +      innermost_block = block_global_block (block);
>>        /* Wrap the call to parse expression, so we can 
>>           return a sensible error.  */
>>        TRY
>> @@ -2103,6 +2103,8 @@ new_root_variable (void)
>>    var->root->floating = 0;
>>    var->root->rootvar = NULL;
>>    var->root->is_valid = 1;
>> +  var->root->thread_id = 0;
>> +  var->root->next = NULL;
>>  
>>    return var;
>>  }
>> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle)
>>    back_to = make_cleanup_restore_current_thread ();
>>  
>>    /* Determine whether the variable is still around.  */
>> -  if (var->root->valid_block == NULL || var->root->floating)
>> +  if (var->root->valid_block == NULL
>> +      || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL
>> +      || var->root->floating)
>>      within_scope = 1;
>>    else if (var->root->thread_id == 0)
>>      {
>>
> 

[PING] [PATCH v2] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

ping

On 7/11/2016 7:48 AM, Don Breazeal wrote:
> 
> ping
> 
> On 6/20/2016 8:31 AM, Don Breazeal wrote:
>> On 6/13/2016 2:54 PM, Don Breazeal wrote:
>>> This is an updated version of a patch to fix access to registers
>>> for frames other than the current frame via var-update.  The piece
>>> of the patch affecting GDB proper has been completely re-written.
>>> It changes the initialization of varobj->root->frame in varobj_create
>>> so that existing mechanisms (with minimal changes) select the correct
>>> frame and provide correct register values from -var-update.
>>>
>>> In addition, the new test for this functionality has been expanded
>>> to cover an analogous case using floating variable objects, and
>>> several tests have been updated to account for the fact that with
>>> the new initialization, -var-create and -var-list-children now
>>> provide a thread-id field where previously they didn't.
>>>
>>> Thanks
>>> --Don
>>>
>>> ------------
>>> This patch fixes a problem with using the MI -var-update command
>>> to access the values of registers in frames other than the current
>>> frame.  The patch includes a test that demonstrates the problem:
>>>
>>> * run so there are several frames on the stack
>>> * create a fixed varobj for $pc in each frame, #'s 1 and above
>>> * step one instruction, to modify the value of $pc
>>> * call -var-update for each of the previously created varobjs
>>>   to verify that they are not reported as having changed.
>>>
>>> Without the patch, the -var-update command reported that $pc for all
>>> frames 1 and above had changed to the value of $pc in frame 0.
>>>
>>> When a varobj is created by -var-create, varobj->root->frame should
>>> be set to specified frame.  Then for a subsequent -var-update,
>>> varobj.c:check_scope can use varobj->root->frame to select the
>>> right frame based on the type of varobj.
>>>
>>> The problem is that for register expressions varobj->root->frame is not
>>> set correctly.  This frame tracking is done using the global
>>> 'innermost_block' which is set in the various parser files (for example
>>> c-exp.y).  However, this is not set for register expressions.
>>>
>>> The fix is to change the initialization of innermost_block in
>>> varobj_create from NULL to the global block, as returned by
>>> block_global_block.  Then varobj_create sets varobj->root->frame
>>> for register varobjs, and value_of_root_1 can check for the
>>> global block when determining whether the variable is in-scope
>>> and select the frame appropriately.
>>>
>>> A side-effect of this change is that for register varobjs and some
>>> global variable varobjs, the output of -var-create now includes the
>>> thread-id field.  The rationale for this is as follow: if we ask for a
>>> particular expression in a particular frame, that implies a particular
>>> thread.  Thus including the thread-id is correct behavior, and the
>>> test results have been updated accordingly.
>>>
>>> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which
>>> performs the test described in the bullet list above as well as a
>>> similar test using a floating variable object to represent $pc.
>>>
>>> We attempted a couple alternative solutions:
>>> * a special case in varobj_update to select the frame was not ideal
>>>   because it didn't use the existing mechanisms to select the frame.
>>> * setting innermost_block in write_dollar_variable had side-effects
>>>   in the CLI that would have required special case code.
>>>
>>> Tested on x86_64 Linux native, no regressions.
>>>
>>> gdb/testsuite/ChangeLog:
>>> 2016-06-13  Don Breazeal  <dbreazea@my.domain.org>
>>>
>>> 	* gdb.ada/mi_interface.exp: Add thread-id field to expected
>>> 	output of -var-create and -var-list-children.
>>> 	* gdb.ada/mi_var_array.exp: Add thread-id field to expected
>>> 	output of -var-list-children.
>>> 	* gdb.mi/mi-break.exp (test_error): Add thread-id field to
>>> 	expected output of -var-create.
>>> 	* gdb.mi/mi-frame-regs.exp: New test script.
>>> 	* gdb.mi/mi-var-cmd.exp: Add thread-id field to expected
>>> 	output of -var-create.
>>> 	* gdb.mi/mi-var-create-rtti.exp: Likewise.
>>>
>>> gdb/ChangeLog:
>>> 2016-06-13  Don Breazeal  <donb@codesourcery.com>
>>> 	    Andrew Burgess <andrew.burgess@embecosm.com>
>>>
>>> 	* varobj.c (varobj_create): Initialize innermost_block to
>>> 	the global block instead of NULL.
>>> 	(new_root_variable): Initialize the thread_id and next
>>> 	fields.
>>> 	(value_of_root_1): Set within_scope if the variable's
>>> 	valid_block field is the global block.
>>>
>>> ---
>>>  gdb/testsuite/gdb.ada/mi_interface.exp      |   4 +-
>>>  gdb/testsuite/gdb.ada/mi_var_array.exp      |   2 +-
>>>  gdb/testsuite/gdb.mi/mi-break.exp           |   2 +-
>>>  gdb/testsuite/gdb.mi/mi-frame-regs.exp      | 183 ++++++++++++++++++++++++++++
>>>  gdb/testsuite/gdb.mi/mi-var-cmd.exp         |   4 +-
>>>  gdb/testsuite/gdb.mi/mi-var-create-rtti.exp |   2 +-
>>>  gdb/varobj.c                                |   8 +-
>>>  7 files changed, 196 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp
>>> index 6000ec8..b948cd5 100644
>>> --- a/gdb/testsuite/gdb.ada/mi_interface.exp
>>> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp
>>> @@ -44,9 +44,9 @@ mi_continue_to_line \
>>>      "stop at start of main Ada procedure"
>>>  
>>>  mi_gdb_test "-var-create ggg1 * ggg1" \
>>> -    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \
>>> +    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \
>>>      "Create ggg1 varobj"
>>>  
>>>  mi_gdb_test "-var-list-children 1 ggg1" \
>>> -    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \
>>> +    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \
>>>      "list ggg1's children"
>>> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp
>>> index c648e7e..c02d4c9 100644
>>> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp
>>> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp
>>> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \
>>>      "Create bt varobj"
>>>  
>>>  mi_gdb_test "-var-list-children vta" \
>>> -    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \
>>> +    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \
>>>      "list vta's children"
>>> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
>>> index 85f328d..bdd43e0 100644
>>> --- a/gdb/testsuite/gdb.mi/mi-break.exp
>>> +++ b/gdb/testsuite/gdb.mi/mi-break.exp
>>> @@ -213,7 +213,7 @@ proc test_error {} {
>>>      # containing function call, the internal breakpoint created to handle
>>>      # function call would be reported, messing up MI output.
>>>      mi_gdb_test "-var-create V * return_1()" \
>>> -        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \
>>> +        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \
>>>          "create varobj for function call"
>>>  
>>>      mi_gdb_test "-var-update *" \
>>> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
>>> new file mode 100644
>>> index 0000000..45f81d6
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
>>> @@ -0,0 +1,183 @@
>>> +# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
>>> +
>>> +# Test essential Machine interface (MI) operations
>>> +#
>>> +# Verify that -var-update will provide the correct values for floating
>>> +# and fixed varobjs that represent the pc register.
>>> +#
>>> +
>>> +load_lib mi-support.exp
>>> +set MIFLAGS "-i=mi"
>>> +
>>> +standard_testfile basics.c
>>> +
>>> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
>>> +		 executable {debug}] != "" } then {
>>> +     untested mi-frame-regs.exp
>>> +     return -1
>>> +}
>>> +
>>> +# Return the address of the specified breakpoint.
>>> +
>>> +proc breakpoint_address {bpnum} {
>>> +    global hex
>>> +    global expect_out
>>> +    global mi_gdb_prompt
>>> +
>>> +    send_gdb "info breakpoint $bpnum\n"
>>> +    gdb_expect {
>>> +	-re ".*($hex).*$mi_gdb_prompt$" {
>>> +	    return $expect_out(1,string)
>>> +	}
>>> +	-re ".*$mi_gdb_prompt$" {
>>> +	    return ""
>>> +	}
>>> +	timeout {
>>> +	    return ""
>>> +	}
>>> +    }
>>> +}
>>> +
>>> +# Test that a floating varobj representing $pc will provide the
>>> +# correct value via -var-update as the program stops at
>>> +# breakpoints in different functions.
>>> +
>>> +proc do_floating_varobj_test {} {
>>> +    global srcfile
>>> +    global hex
>>> +    global expect_out
>>> +
>>> +    gdb_exit
>>> +    if {[mi_gdb_start]} then {
>>> +	continue
>>> +    }
>>> +
>>> +    with_test_prefix "floating" {
>>> +	mi_run_to_main
>>> +
>>> +	# Create a floating varobj for $pc.
>>> +	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
>>> +		    "\\^done,.*value=\"$hex.*" \
>>> +	            "create varobj for pc in frame 0"
>>> +
>>> +	set nframes 4
>>> +	for {set i 1} {$i < $nframes} {incr i} {
>>> +
>>> +	    # Run to a breakpoint in each callee function in succession.
>>> +	    # Note that we can't use mi_runto because we need the
>>> +	    # breakpoint to be persistent, so we can use its address.
>>> +	    set bpnum [expr $i + 1]
>>> +	    mi_create_breakpoint \
>>> +	        "basics.c:callee$i" \
>>> +		"insert breakpoint at basics.c:callee$i" \
>>> +		-number $bpnum -func callee$i -file ".*basics.c"
>>> +
>>> +	    mi_execute_to "exec-continue" "breakpoint-hit" \
>>> +			  "callee$i" ".*" ".*${srcfile}" ".*" \
>>> +			  { "" "disp=\"keep\"" } "breakpoint hit"
>>> +
>>> +	    # Get the value of $pc from the floating varobj.
>>> +	    mi_gdb_test "-var-update 1 var1" \
>>> +			"\\^done,.*value=\"($hex) .*" \
>>> +			"-var-update for frame $i"
>>> +	    set pcval $expect_out(3,string)
>>> +
>>> +	    # Get the address of the current breakpoint.
>>> +	    set bpaddr [breakpoint_address $bpnum]
>>> +	    if {$bpaddr == ""} then {
>>> +		unresolved "get address of breakpoint $bpnum"
>>> +		return
>>> +	    }
>>> +
>>> +	    # Check that the addresses are the same.
>>> +	    if {[expr $bpaddr != $pcval]} then {
>>> +	        fail "\$pc does not equal address of breakpoint"
>>> +	    } else {
>>> +	        pass "\$pc equals address of breakpoint"
>>> +	    }
>>> +	}
>>> +    }
>>> +}
>>> +
>>> +# Create a varobj for the pc register in each of the frames other
>>> +# than frame 0.
>>> +
>>> +proc var_create_regs {nframes} {
>>> +    global hex
>>> +
>>> +    for {set i 1} {$i < $nframes} {incr i} {
>>> +	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
>>> +		    "\\^done,.*value=\"$hex.*" \
>>> +	            "create varobj for pc in frame $i"
>>> +    }
>>> +}
>>> +
>>> +# Check that -var-update reports that the value of the pc register
>>> +# for each of the frames 1 and above is reported as unchanged.
>>> +
>>> +proc var_update_regs {nframes} {
>>> +
>>> +    for {set i 1} {$i < $nframes} {incr i} {
>>> +	mi_gdb_test "-var-update 1 var$i" \
>>> +	            "\\^done,(changelist=\\\[\\\])" \
>>> +	            "pc unchanged in -var-update for frame $i"
>>> +    }
>>> +}
>>> +
>>> +# Test that fixed varobjs representing $pc in different stack frames
>>> +# will provide the correct value via -var-update after the program
>>> +# counter changes (without substantially changing the stack).
>>> +
>>> +proc do_fixed_varobj_test {} {
>>> +    global srcfile
>>> +
>>> +    gdb_exit
>>> +    if {[mi_gdb_start]} then {
>>> +	continue
>>> +    }
>>> +
>>> +    with_test_prefix "fixed" {
>>> +	mi_run_to_main
>>> +
>>> +	# Run to the function 'callee3' so we have several frames.
>>> +	mi_create_breakpoint "basics.c:callee3" \
>>> +			     "insert breakpoint at basics.c:callee3" \
>>> +			     -number 2 -func callee3 -file ".*basics.c"
>>> +
>>> +	mi_execute_to "exec-continue" "breakpoint-hit" \
>>> +	              "callee3" ".*" ".*${srcfile}" ".*" \
>>> +		      { "" "disp=\"keep\"" } "breakpoint hit"
>>> +
>>> +	# At the breakpoint in callee3 there are 4 frames.  Create a
>>> +	# varobj for the pc in each of frames 1 and above.
>>> +	set nframes 4
>>> +	var_create_regs $nframes
>>> +
>>> +	# Step one instruction to change the program counter.
>>> +	mi_execute_to "exec-next-instruction" "end-stepping-range" \
>>> +		      "callee3" ".*" ".*${srcfile}" ".*" "" \
>>> +		      "next instruction in callee3"
>>> +
>>> +	# Check that -var-update reports that the values are unchanged.
>>> +	var_update_regs $nframes
>>> +    }
>>> +}
>>> +
>>> +do_fixed_varobj_test
>>> +do_floating_varobj_test
>>> +
>>> +mi_gdb_exit
>>> +return 0
>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>> index 558cd6c..68e3cf8 100644
>>> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \
>>>  	"assign same value to func (update)"
>>>  
>>>  mi_gdb_test "-var-create array_ptr * array_ptr" \
>>> -	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \
>>> +	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \
>>>  	"create global variable array_ptr"
>>>  
>>>  mi_gdb_test "-var-assign array_ptr array2" \
>>> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee"
>>>  # A varobj we fail to read during -var-update should be considered
>>>  # out of scope.
>>>  mi_gdb_test "-var-create null_ptr * **0" \
>>> -    {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \
>>> +    {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \
>>>      "create null_ptr"
>>>  
>>>  # Allow this to succeed, if address zero is readable, although it
>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>> index 3bcb36c..a8cc76f 100644
>>> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*"
>>>  # We use a explicit cast to (void *) as that is the
>>>  # type that caused the bug this testcase is testing for.
>>>  mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \
>>> -	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \
>>> +	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \
>>>  	    "-var-create sp1 * \$sp"
>>>  gdb_exit
>>> diff --git a/gdb/varobj.c b/gdb/varobj.c
>>> index 6f56cba..1e3f192 100644
>>> --- a/gdb/varobj.c
>>> +++ b/gdb/varobj.c
>>> @@ -323,7 +323,7 @@ varobj_create (char *objname,
>>>  	}
>>>  
>>>        p = expression;
>>> -      innermost_block = NULL;
>>> +      innermost_block = block_global_block (block);
>>>        /* Wrap the call to parse expression, so we can 
>>>           return a sensible error.  */
>>>        TRY
>>> @@ -2103,6 +2103,8 @@ new_root_variable (void)
>>>    var->root->floating = 0;
>>>    var->root->rootvar = NULL;
>>>    var->root->is_valid = 1;
>>> +  var->root->thread_id = 0;
>>> +  var->root->next = NULL;
>>>  
>>>    return var;
>>>  }
>>> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle)
>>>    back_to = make_cleanup_restore_current_thread ();
>>>  
>>>    /* Determine whether the variable is still around.  */
>>> -  if (var->root->valid_block == NULL || var->root->floating)
>>> +  if (var->root->valid_block == NULL
>>> +      || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL
>>> +      || var->root->floating)
>>>      within_scope = 1;
>>>    else if (var->root->thread_id == 0)
>>>      {
>>>
>>
> 

Re: [PING] [PATCH v2] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

Hi, see below,

On 7/20/2016 2:06 PM, Don Breazeal wrote:
> ping
> 
> On 7/11/2016 7:48 AM, Don Breazeal wrote:
>>
>> ping
>>
>> On 6/20/2016 8:31 AM, Don Breazeal wrote:
>>> On 6/13/2016 2:54 PM, Don Breazeal wrote:
>>>> This is an updated version of a patch to fix access to registers
>>>> for frames other than the current frame via var-update.  The piece
>>>> of the patch affecting GDB proper has been completely re-written.
>>>> It changes the initialization of varobj->root->frame in varobj_create
>>>> so that existing mechanisms (with minimal changes) select the correct
>>>> frame and provide correct register values from -var-update.
>>>>
>>>> In addition, the new test for this functionality has been expanded
>>>> to cover an analogous case using floating variable objects, and
>>>> several tests have been updated to account for the fact that with
>>>> the new initialization, -var-create and -var-list-children now
>>>> provide a thread-id field where previously they didn't.
>>>>
>>>> Thanks
>>>> --Don
>>>>
>>>> ------------
>>>> This patch fixes a problem with using the MI -var-update command
>>>> to access the values of registers in frames other than the current
>>>> frame.  The patch includes a test that demonstrates the problem:
>>>>
>>>> * run so there are several frames on the stack
>>>> * create a fixed varobj for $pc in each frame, #'s 1 and above
>>>> * step one instruction, to modify the value of $pc
>>>> * call -var-update for each of the previously created varobjs
>>>>   to verify that they are not reported as having changed.
>>>>
>>>> Without the patch, the -var-update command reported that $pc for all
>>>> frames 1 and above had changed to the value of $pc in frame 0.
>>>>
>>>> When a varobj is created by -var-create, varobj->root->frame should
>>>> be set to specified frame.  Then for a subsequent -var-update,
>>>> varobj.c:check_scope can use varobj->root->frame to select the
>>>> right frame based on the type of varobj.
>>>>
>>>> The problem is that for register expressions varobj->root->frame is not
>>>> set correctly.  This frame tracking is done using the global
>>>> 'innermost_block' which is set in the various parser files (for example
>>>> c-exp.y).  However, this is not set for register expressions.
>>>>
>>>> The fix is to change the initialization of innermost_block in
>>>> varobj_create from NULL to the global block, as returned by
>>>> block_global_block.  Then varobj_create sets varobj->root->frame
>>>> for register varobjs, and value_of_root_1 can check for the
>>>> global block when determining whether the variable is in-scope
>>>> and select the frame appropriately.
>>>>
>>>> A side-effect of this change is that for register varobjs and some
>>>> global variable varobjs, the output of -var-create now includes the
>>>> thread-id field.  The rationale for this is as follow: if we ask for a
>>>> particular expression in a particular frame, that implies a particular
>>>> thread.  Thus including the thread-id is correct behavior, and the
>>>> test results have been updated accordingly.
>>>>
>>>> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which
>>>> performs the test described in the bullet list above as well as a
>>>> similar test using a floating variable object to represent $pc.
>>>>
>>>> We attempted a couple alternative solutions:
>>>> * a special case in varobj_update to select the frame was not ideal
>>>>   because it didn't use the existing mechanisms to select the frame.
>>>> * setting innermost_block in write_dollar_variable had side-effects
>>>>   in the CLI that would have required special case code.
>>>>
>>>> Tested on x86_64 Linux native, no regressions.
>>>>
>>>> gdb/testsuite/ChangeLog:
>>>> 2016-06-13  Don Breazeal  <dbreazea@my.domain.org>
>>>>
>>>> 	* gdb.ada/mi_interface.exp: Add thread-id field to expected
>>>> 	output of -var-create and -var-list-children.
>>>> 	* gdb.ada/mi_var_array.exp: Add thread-id field to expected
>>>> 	output of -var-list-children.
>>>> 	* gdb.mi/mi-break.exp (test_error): Add thread-id field to
>>>> 	expected output of -var-create.
>>>> 	* gdb.mi/mi-frame-regs.exp: New test script.
>>>> 	* gdb.mi/mi-var-cmd.exp: Add thread-id field to expected
>>>> 	output of -var-create.
>>>> 	* gdb.mi/mi-var-create-rtti.exp: Likewise.
>>>>
>>>> gdb/ChangeLog:
>>>> 2016-06-13  Don Breazeal  <donb@codesourcery.com>
>>>> 	    Andrew Burgess <andrew.burgess@embecosm.com>
>>>>
>>>> 	* varobj.c (varobj_create): Initialize innermost_block to
>>>> 	the global block instead of NULL.
>>>> 	(new_root_variable): Initialize the thread_id and next
>>>> 	fields.
>>>> 	(value_of_root_1): Set within_scope if the variable's
>>>> 	valid_block field is the global block.
>>>>
>>>> ---
>>>>  gdb/testsuite/gdb.ada/mi_interface.exp      |   4 +-
>>>>  gdb/testsuite/gdb.ada/mi_var_array.exp      |   2 +-
>>>>  gdb/testsuite/gdb.mi/mi-break.exp           |   2 +-
>>>>  gdb/testsuite/gdb.mi/mi-frame-regs.exp      | 183 ++++++++++++++++++++++++++++
>>>>  gdb/testsuite/gdb.mi/mi-var-cmd.exp         |   4 +-
>>>>  gdb/testsuite/gdb.mi/mi-var-create-rtti.exp |   2 +-
>>>>  gdb/varobj.c                                |   8 +-
>>>>  7 files changed, 196 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp
>>>> index 6000ec8..b948cd5 100644
>>>> --- a/gdb/testsuite/gdb.ada/mi_interface.exp
>>>> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp
>>>> @@ -44,9 +44,9 @@ mi_continue_to_line \
>>>>      "stop at start of main Ada procedure"
>>>>  
>>>>  mi_gdb_test "-var-create ggg1 * ggg1" \
>>>> -    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \
>>>> +    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \
>>>>      "Create ggg1 varobj"
>>>>  
>>>>  mi_gdb_test "-var-list-children 1 ggg1" \
>>>> -    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \
>>>> +    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \
>>>>      "list ggg1's children"
>>>> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp
>>>> index c648e7e..c02d4c9 100644
>>>> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp
>>>> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp
>>>> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \
>>>>      "Create bt varobj"
>>>>  
>>>>  mi_gdb_test "-var-list-children vta" \
>>>> -    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \
>>>> +    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \
>>>>      "list vta's children"
>>>> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
>>>> index 85f328d..bdd43e0 100644
>>>> --- a/gdb/testsuite/gdb.mi/mi-break.exp
>>>> +++ b/gdb/testsuite/gdb.mi/mi-break.exp
>>>> @@ -213,7 +213,7 @@ proc test_error {} {
>>>>      # containing function call, the internal breakpoint created to handle
>>>>      # function call would be reported, messing up MI output.
>>>>      mi_gdb_test "-var-create V * return_1()" \
>>>> -        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \
>>>> +        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \
>>>>          "create varobj for function call"
>>>>  
>>>>      mi_gdb_test "-var-update *" \
>>>> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
>>>> new file mode 100644
>>>> index 0000000..45f81d6
>>>> --- /dev/null
>>>> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
>>>> @@ -0,0 +1,183 @@
>>>> +# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
>>>> +
>>>> +# Test essential Machine interface (MI) operations
>>>> +#
>>>> +# Verify that -var-update will provide the correct values for floating
>>>> +# and fixed varobjs that represent the pc register.
>>>> +#
>>>> +
>>>> +load_lib mi-support.exp
>>>> +set MIFLAGS "-i=mi"
>>>> +
>>>> +standard_testfile basics.c
>>>> +
>>>> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
>>>> +		 executable {debug}] != "" } then {
>>>> +     untested mi-frame-regs.exp
>>>> +     return -1
>>>> +}
>>>> +
>>>> +# Return the address of the specified breakpoint.
>>>> +
>>>> +proc breakpoint_address {bpnum} {
>>>> +    global hex
>>>> +    global expect_out
>>>> +    global mi_gdb_prompt
>>>> +
>>>> +    send_gdb "info breakpoint $bpnum\n"
>>>> +    gdb_expect {
>>>> +	-re ".*($hex).*$mi_gdb_prompt$" {
>>>> +	    return $expect_out(1,string)
>>>> +	}
>>>> +	-re ".*$mi_gdb_prompt$" {
>>>> +	    return ""
>>>> +	}
>>>> +	timeout {
>>>> +	    return ""
>>>> +	}
>>>> +    }
>>>> +}
>>>> +
>>>> +# Test that a floating varobj representing $pc will provide the
>>>> +# correct value via -var-update as the program stops at
>>>> +# breakpoints in different functions.
>>>> +
>>>> +proc do_floating_varobj_test {} {
>>>> +    global srcfile
>>>> +    global hex
>>>> +    global expect_out
>>>> +
>>>> +    gdb_exit
>>>> +    if {[mi_gdb_start]} then {
>>>> +	continue
>>>> +    }
>>>> +
>>>> +    with_test_prefix "floating" {
>>>> +	mi_run_to_main
>>>> +
>>>> +	# Create a floating varobj for $pc.
>>>> +	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
>>>> +		    "\\^done,.*value=\"$hex.*" \
>>>> +	            "create varobj for pc in frame 0"
>>>> +
>>>> +	set nframes 4
>>>> +	for {set i 1} {$i < $nframes} {incr i} {
>>>> +
>>>> +	    # Run to a breakpoint in each callee function in succession.
>>>> +	    # Note that we can't use mi_runto because we need the
>>>> +	    # breakpoint to be persistent, so we can use its address.
>>>> +	    set bpnum [expr $i + 1]
>>>> +	    mi_create_breakpoint \
>>>> +	        "basics.c:callee$i" \
>>>> +		"insert breakpoint at basics.c:callee$i" \
>>>> +		-number $bpnum -func callee$i -file ".*basics.c"
>>>> +
>>>> +	    mi_execute_to "exec-continue" "breakpoint-hit" \
>>>> +			  "callee$i" ".*" ".*${srcfile}" ".*" \
>>>> +			  { "" "disp=\"keep\"" } "breakpoint hit"
>>>> +
>>>> +	    # Get the value of $pc from the floating varobj.
>>>> +	    mi_gdb_test "-var-update 1 var1" \
>>>> +			"\\^done,.*value=\"($hex) .*" \
>>>> +			"-var-update for frame $i"
>>>> +	    set pcval $expect_out(3,string)
>>>> +
>>>> +	    # Get the address of the current breakpoint.
>>>> +	    set bpaddr [breakpoint_address $bpnum]
>>>> +	    if {$bpaddr == ""} then {
>>>> +		unresolved "get address of breakpoint $bpnum"
>>>> +		return
>>>> +	    }
>>>> +
>>>> +	    # Check that the addresses are the same.
>>>> +	    if {[expr $bpaddr != $pcval]} then {
>>>> +	        fail "\$pc does not equal address of breakpoint"
>>>> +	    } else {
>>>> +	        pass "\$pc equals address of breakpoint"
>>>> +	    }
>>>> +	}
>>>> +    }
>>>> +}
>>>> +
>>>> +# Create a varobj for the pc register in each of the frames other
>>>> +# than frame 0.
>>>> +
>>>> +proc var_create_regs {nframes} {
>>>> +    global hex
>>>> +
>>>> +    for {set i 1} {$i < $nframes} {incr i} {
>>>> +	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
>>>> +		    "\\^done,.*value=\"$hex.*" \
>>>> +	            "create varobj for pc in frame $i"
>>>> +    }
>>>> +}
>>>> +
>>>> +# Check that -var-update reports that the value of the pc register
>>>> +# for each of the frames 1 and above is reported as unchanged.
>>>> +
>>>> +proc var_update_regs {nframes} {
>>>> +
>>>> +    for {set i 1} {$i < $nframes} {incr i} {
>>>> +	mi_gdb_test "-var-update 1 var$i" \
>>>> +	            "\\^done,(changelist=\\\[\\\])" \
>>>> +	            "pc unchanged in -var-update for frame $i"
>>>> +    }
>>>> +}
>>>> +
>>>> +# Test that fixed varobjs representing $pc in different stack frames
>>>> +# will provide the correct value via -var-update after the program
>>>> +# counter changes (without substantially changing the stack).
>>>> +
>>>> +proc do_fixed_varobj_test {} {
>>>> +    global srcfile
>>>> +
>>>> +    gdb_exit
>>>> +    if {[mi_gdb_start]} then {
>>>> +	continue
>>>> +    }
>>>> +
>>>> +    with_test_prefix "fixed" {
>>>> +	mi_run_to_main
>>>> +
>>>> +	# Run to the function 'callee3' so we have several frames.
>>>> +	mi_create_breakpoint "basics.c:callee3" \
>>>> +			     "insert breakpoint at basics.c:callee3" \
>>>> +			     -number 2 -func callee3 -file ".*basics.c"
>>>> +
>>>> +	mi_execute_to "exec-continue" "breakpoint-hit" \
>>>> +	              "callee3" ".*" ".*${srcfile}" ".*" \
>>>> +		      { "" "disp=\"keep\"" } "breakpoint hit"
>>>> +
>>>> +	# At the breakpoint in callee3 there are 4 frames.  Create a
>>>> +	# varobj for the pc in each of frames 1 and above.
>>>> +	set nframes 4
>>>> +	var_create_regs $nframes
>>>> +
>>>> +	# Step one instruction to change the program counter.
>>>> +	mi_execute_to "exec-next-instruction" "end-stepping-range" \
>>>> +		      "callee3" ".*" ".*${srcfile}" ".*" "" \
>>>> +		      "next instruction in callee3"
>>>> +
>>>> +	# Check that -var-update reports that the values are unchanged.
>>>> +	var_update_regs $nframes
>>>> +    }
>>>> +}
>>>> +
>>>> +do_fixed_varobj_test
>>>> +do_floating_varobj_test
>>>> +
>>>> +mi_gdb_exit
>>>> +return 0
>>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>>> index 558cd6c..68e3cf8 100644
>>>> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>>> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>>> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \
>>>>  	"assign same value to func (update)"
>>>>  
>>>>  mi_gdb_test "-var-create array_ptr * array_ptr" \
>>>> -	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \
>>>> +	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \
>>>>  	"create global variable array_ptr"
>>>>  
>>>>  mi_gdb_test "-var-assign array_ptr array2" \
>>>> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee"
>>>>  # A varobj we fail to read during -var-update should be considered
>>>>  # out of scope.
>>>>  mi_gdb_test "-var-create null_ptr * **0" \
>>>> -    {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \
>>>> +    {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \
>>>>      "create null_ptr"
>>>>  
>>>>  # Allow this to succeed, if address zero is readable, although it
>>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>>> index 3bcb36c..a8cc76f 100644
>>>> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>>> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>>> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*"
>>>>  # We use a explicit cast to (void *) as that is the
>>>>  # type that caused the bug this testcase is testing for.
>>>>  mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \
>>>> -	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \
>>>> +	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \
>>>>  	    "-var-create sp1 * \$sp"
>>>>  gdb_exit
>>>> diff --git a/gdb/varobj.c b/gdb/varobj.c
>>>> index 6f56cba..1e3f192 100644
>>>> --- a/gdb/varobj.c
>>>> +++ b/gdb/varobj.c
>>>> @@ -323,7 +323,7 @@ varobj_create (char *objname,
>>>>  	}
>>>>  
>>>>        p = expression;
>>>> -      innermost_block = NULL;
>>>> +      innermost_block = block_global_block (block);
>>>>        /* Wrap the call to parse expression, so we can 
>>>>           return a sensible error.  */
>>>>        TRY
>>>> @@ -2103,6 +2103,8 @@ new_root_variable (void)
>>>>    var->root->floating = 0;
>>>>    var->root->rootvar = NULL;
>>>>    var->root->is_valid = 1;
>>>> +  var->root->thread_id = 0;
>>>> +  var->root->next = NULL;
>>>>  
>>>>    return var;
>>>>  }
>>>> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle)
>>>>    back_to = make_cleanup_restore_current_thread ();
>>>>  
>>>>    /* Determine whether the variable is still around.  */
>>>> -  if (var->root->valid_block == NULL || var->root->floating)
>>>> +  if (var->root->valid_block == NULL
>>>> +      || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL
>>>> +      || var->root->floating)
>>>>      within_scope = 1;
>>>>    else if (var->root->thread_id == 0)
>>>>      {
>>>>
>>>
>>
> 
I've created PR mi/20395 for this issue and marked it with the Target
Milestone 7.12.  Since it's an issue of returning incorrect data I'm
proposing that it's a blocker.
thanks
--Don

[PING^4] [PATCH v2] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

ping

On 7/21/2016 9:59 AM, Don Breazeal wrote:
> Hi, see below,
> 
> On 7/20/2016 2:06 PM, Don Breazeal wrote:
>> ping
>>
>> On 7/11/2016 7:48 AM, Don Breazeal wrote:
>>>
>>> ping
>>>
>>> On 6/20/2016 8:31 AM, Don Breazeal wrote:
>>>> On 6/13/2016 2:54 PM, Don Breazeal wrote:
>>>>> This is an updated version of a patch to fix access to registers
>>>>> for frames other than the current frame via var-update.  The piece
>>>>> of the patch affecting GDB proper has been completely re-written.
>>>>> It changes the initialization of varobj->root->frame in varobj_create
>>>>> so that existing mechanisms (with minimal changes) select the correct
>>>>> frame and provide correct register values from -var-update.
>>>>>
>>>>> In addition, the new test for this functionality has been expanded
>>>>> to cover an analogous case using floating variable objects, and
>>>>> several tests have been updated to account for the fact that with
>>>>> the new initialization, -var-create and -var-list-children now
>>>>> provide a thread-id field where previously they didn't.
>>>>>
>>>>> Thanks
>>>>> --Don
>>>>>
>>>>> ------------
>>>>> This patch fixes a problem with using the MI -var-update command
>>>>> to access the values of registers in frames other than the current
>>>>> frame.  The patch includes a test that demonstrates the problem:
>>>>>
>>>>> * run so there are several frames on the stack
>>>>> * create a fixed varobj for $pc in each frame, #'s 1 and above
>>>>> * step one instruction, to modify the value of $pc
>>>>> * call -var-update for each of the previously created varobjs
>>>>>   to verify that they are not reported as having changed.
>>>>>
>>>>> Without the patch, the -var-update command reported that $pc for all
>>>>> frames 1 and above had changed to the value of $pc in frame 0.
>>>>>
>>>>> When a varobj is created by -var-create, varobj->root->frame should
>>>>> be set to specified frame.  Then for a subsequent -var-update,
>>>>> varobj.c:check_scope can use varobj->root->frame to select the
>>>>> right frame based on the type of varobj.
>>>>>
>>>>> The problem is that for register expressions varobj->root->frame is not
>>>>> set correctly.  This frame tracking is done using the global
>>>>> 'innermost_block' which is set in the various parser files (for example
>>>>> c-exp.y).  However, this is not set for register expressions.
>>>>>
>>>>> The fix is to change the initialization of innermost_block in
>>>>> varobj_create from NULL to the global block, as returned by
>>>>> block_global_block.  Then varobj_create sets varobj->root->frame
>>>>> for register varobjs, and value_of_root_1 can check for the
>>>>> global block when determining whether the variable is in-scope
>>>>> and select the frame appropriately.
>>>>>
>>>>> A side-effect of this change is that for register varobjs and some
>>>>> global variable varobjs, the output of -var-create now includes the
>>>>> thread-id field.  The rationale for this is as follow: if we ask for a
>>>>> particular expression in a particular frame, that implies a particular
>>>>> thread.  Thus including the thread-id is correct behavior, and the
>>>>> test results have been updated accordingly.
>>>>>
>>>>> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which
>>>>> performs the test described in the bullet list above as well as a
>>>>> similar test using a floating variable object to represent $pc.
>>>>>
>>>>> We attempted a couple alternative solutions:
>>>>> * a special case in varobj_update to select the frame was not ideal
>>>>>   because it didn't use the existing mechanisms to select the frame.
>>>>> * setting innermost_block in write_dollar_variable had side-effects
>>>>>   in the CLI that would have required special case code.
>>>>>
>>>>> Tested on x86_64 Linux native, no regressions.
>>>>>
>>>>> gdb/testsuite/ChangeLog:
>>>>> 2016-06-13  Don Breazeal  <dbreazea@my.domain.org>
>>>>>
>>>>> 	* gdb.ada/mi_interface.exp: Add thread-id field to expected
>>>>> 	output of -var-create and -var-list-children.
>>>>> 	* gdb.ada/mi_var_array.exp: Add thread-id field to expected
>>>>> 	output of -var-list-children.
>>>>> 	* gdb.mi/mi-break.exp (test_error): Add thread-id field to
>>>>> 	expected output of -var-create.
>>>>> 	* gdb.mi/mi-frame-regs.exp: New test script.
>>>>> 	* gdb.mi/mi-var-cmd.exp: Add thread-id field to expected
>>>>> 	output of -var-create.
>>>>> 	* gdb.mi/mi-var-create-rtti.exp: Likewise.
>>>>>
>>>>> gdb/ChangeLog:
>>>>> 2016-06-13  Don Breazeal  <donb@codesourcery.com>
>>>>> 	    Andrew Burgess <andrew.burgess@embecosm.com>
>>>>>
>>>>> 	* varobj.c (varobj_create): Initialize innermost_block to
>>>>> 	the global block instead of NULL.
>>>>> 	(new_root_variable): Initialize the thread_id and next
>>>>> 	fields.
>>>>> 	(value_of_root_1): Set within_scope if the variable's
>>>>> 	valid_block field is the global block.
>>>>>
>>>>> ---
>>>>>  gdb/testsuite/gdb.ada/mi_interface.exp      |   4 +-
>>>>>  gdb/testsuite/gdb.ada/mi_var_array.exp      |   2 +-
>>>>>  gdb/testsuite/gdb.mi/mi-break.exp           |   2 +-
>>>>>  gdb/testsuite/gdb.mi/mi-frame-regs.exp      | 183 ++++++++++++++++++++++++++++
>>>>>  gdb/testsuite/gdb.mi/mi-var-cmd.exp         |   4 +-
>>>>>  gdb/testsuite/gdb.mi/mi-var-create-rtti.exp |   2 +-
>>>>>  gdb/varobj.c                                |   8 +-
>>>>>  7 files changed, 196 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp
>>>>> index 6000ec8..b948cd5 100644
>>>>> --- a/gdb/testsuite/gdb.ada/mi_interface.exp
>>>>> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp
>>>>> @@ -44,9 +44,9 @@ mi_continue_to_line \
>>>>>      "stop at start of main Ada procedure"
>>>>>  
>>>>>  mi_gdb_test "-var-create ggg1 * ggg1" \
>>>>> -    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \
>>>>> +    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \
>>>>>      "Create ggg1 varobj"
>>>>>  
>>>>>  mi_gdb_test "-var-list-children 1 ggg1" \
>>>>> -    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \
>>>>> +    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \
>>>>>      "list ggg1's children"
>>>>> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp
>>>>> index c648e7e..c02d4c9 100644
>>>>> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp
>>>>> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp
>>>>> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \
>>>>>      "Create bt varobj"
>>>>>  
>>>>>  mi_gdb_test "-var-list-children vta" \
>>>>> -    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \
>>>>> +    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \
>>>>>      "list vta's children"
>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
>>>>> index 85f328d..bdd43e0 100644
>>>>> --- a/gdb/testsuite/gdb.mi/mi-break.exp
>>>>> +++ b/gdb/testsuite/gdb.mi/mi-break.exp
>>>>> @@ -213,7 +213,7 @@ proc test_error {} {
>>>>>      # containing function call, the internal breakpoint created to handle
>>>>>      # function call would be reported, messing up MI output.
>>>>>      mi_gdb_test "-var-create V * return_1()" \
>>>>> -        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \
>>>>> +        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \
>>>>>          "create varobj for function call"
>>>>>  
>>>>>      mi_gdb_test "-var-update *" \
>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
>>>>> new file mode 100644
>>>>> index 0000000..45f81d6
>>>>> --- /dev/null
>>>>> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
>>>>> @@ -0,0 +1,183 @@
>>>>> +# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
>>>>> +
>>>>> +# Test essential Machine interface (MI) operations
>>>>> +#
>>>>> +# Verify that -var-update will provide the correct values for floating
>>>>> +# and fixed varobjs that represent the pc register.
>>>>> +#
>>>>> +
>>>>> +load_lib mi-support.exp
>>>>> +set MIFLAGS "-i=mi"
>>>>> +
>>>>> +standard_testfile basics.c
>>>>> +
>>>>> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
>>>>> +		 executable {debug}] != "" } then {
>>>>> +     untested mi-frame-regs.exp
>>>>> +     return -1
>>>>> +}
>>>>> +
>>>>> +# Return the address of the specified breakpoint.
>>>>> +
>>>>> +proc breakpoint_address {bpnum} {
>>>>> +    global hex
>>>>> +    global expect_out
>>>>> +    global mi_gdb_prompt
>>>>> +
>>>>> +    send_gdb "info breakpoint $bpnum\n"
>>>>> +    gdb_expect {
>>>>> +	-re ".*($hex).*$mi_gdb_prompt$" {
>>>>> +	    return $expect_out(1,string)
>>>>> +	}
>>>>> +	-re ".*$mi_gdb_prompt$" {
>>>>> +	    return ""
>>>>> +	}
>>>>> +	timeout {
>>>>> +	    return ""
>>>>> +	}
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +# Test that a floating varobj representing $pc will provide the
>>>>> +# correct value via -var-update as the program stops at
>>>>> +# breakpoints in different functions.
>>>>> +
>>>>> +proc do_floating_varobj_test {} {
>>>>> +    global srcfile
>>>>> +    global hex
>>>>> +    global expect_out
>>>>> +
>>>>> +    gdb_exit
>>>>> +    if {[mi_gdb_start]} then {
>>>>> +	continue
>>>>> +    }
>>>>> +
>>>>> +    with_test_prefix "floating" {
>>>>> +	mi_run_to_main
>>>>> +
>>>>> +	# Create a floating varobj for $pc.
>>>>> +	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
>>>>> +		    "\\^done,.*value=\"$hex.*" \
>>>>> +	            "create varobj for pc in frame 0"
>>>>> +
>>>>> +	set nframes 4
>>>>> +	for {set i 1} {$i < $nframes} {incr i} {
>>>>> +
>>>>> +	    # Run to a breakpoint in each callee function in succession.
>>>>> +	    # Note that we can't use mi_runto because we need the
>>>>> +	    # breakpoint to be persistent, so we can use its address.
>>>>> +	    set bpnum [expr $i + 1]
>>>>> +	    mi_create_breakpoint \
>>>>> +	        "basics.c:callee$i" \
>>>>> +		"insert breakpoint at basics.c:callee$i" \
>>>>> +		-number $bpnum -func callee$i -file ".*basics.c"
>>>>> +
>>>>> +	    mi_execute_to "exec-continue" "breakpoint-hit" \
>>>>> +			  "callee$i" ".*" ".*${srcfile}" ".*" \
>>>>> +			  { "" "disp=\"keep\"" } "breakpoint hit"
>>>>> +
>>>>> +	    # Get the value of $pc from the floating varobj.
>>>>> +	    mi_gdb_test "-var-update 1 var1" \
>>>>> +			"\\^done,.*value=\"($hex) .*" \
>>>>> +			"-var-update for frame $i"
>>>>> +	    set pcval $expect_out(3,string)
>>>>> +
>>>>> +	    # Get the address of the current breakpoint.
>>>>> +	    set bpaddr [breakpoint_address $bpnum]
>>>>> +	    if {$bpaddr == ""} then {
>>>>> +		unresolved "get address of breakpoint $bpnum"
>>>>> +		return
>>>>> +	    }
>>>>> +
>>>>> +	    # Check that the addresses are the same.
>>>>> +	    if {[expr $bpaddr != $pcval]} then {
>>>>> +	        fail "\$pc does not equal address of breakpoint"
>>>>> +	    } else {
>>>>> +	        pass "\$pc equals address of breakpoint"
>>>>> +	    }
>>>>> +	}
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +# Create a varobj for the pc register in each of the frames other
>>>>> +# than frame 0.
>>>>> +
>>>>> +proc var_create_regs {nframes} {
>>>>> +    global hex
>>>>> +
>>>>> +    for {set i 1} {$i < $nframes} {incr i} {
>>>>> +	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
>>>>> +		    "\\^done,.*value=\"$hex.*" \
>>>>> +	            "create varobj for pc in frame $i"
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +# Check that -var-update reports that the value of the pc register
>>>>> +# for each of the frames 1 and above is reported as unchanged.
>>>>> +
>>>>> +proc var_update_regs {nframes} {
>>>>> +
>>>>> +    for {set i 1} {$i < $nframes} {incr i} {
>>>>> +	mi_gdb_test "-var-update 1 var$i" \
>>>>> +	            "\\^done,(changelist=\\\[\\\])" \
>>>>> +	            "pc unchanged in -var-update for frame $i"
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +# Test that fixed varobjs representing $pc in different stack frames
>>>>> +# will provide the correct value via -var-update after the program
>>>>> +# counter changes (without substantially changing the stack).
>>>>> +
>>>>> +proc do_fixed_varobj_test {} {
>>>>> +    global srcfile
>>>>> +
>>>>> +    gdb_exit
>>>>> +    if {[mi_gdb_start]} then {
>>>>> +	continue
>>>>> +    }
>>>>> +
>>>>> +    with_test_prefix "fixed" {
>>>>> +	mi_run_to_main
>>>>> +
>>>>> +	# Run to the function 'callee3' so we have several frames.
>>>>> +	mi_create_breakpoint "basics.c:callee3" \
>>>>> +			     "insert breakpoint at basics.c:callee3" \
>>>>> +			     -number 2 -func callee3 -file ".*basics.c"
>>>>> +
>>>>> +	mi_execute_to "exec-continue" "breakpoint-hit" \
>>>>> +	              "callee3" ".*" ".*${srcfile}" ".*" \
>>>>> +		      { "" "disp=\"keep\"" } "breakpoint hit"
>>>>> +
>>>>> +	# At the breakpoint in callee3 there are 4 frames.  Create a
>>>>> +	# varobj for the pc in each of frames 1 and above.
>>>>> +	set nframes 4
>>>>> +	var_create_regs $nframes
>>>>> +
>>>>> +	# Step one instruction to change the program counter.
>>>>> +	mi_execute_to "exec-next-instruction" "end-stepping-range" \
>>>>> +		      "callee3" ".*" ".*${srcfile}" ".*" "" \
>>>>> +		      "next instruction in callee3"
>>>>> +
>>>>> +	# Check that -var-update reports that the values are unchanged.
>>>>> +	var_update_regs $nframes
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +do_fixed_varobj_test
>>>>> +do_floating_varobj_test
>>>>> +
>>>>> +mi_gdb_exit
>>>>> +return 0
>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>>>> index 558cd6c..68e3cf8 100644
>>>>> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>>>> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>>>> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \
>>>>>  	"assign same value to func (update)"
>>>>>  
>>>>>  mi_gdb_test "-var-create array_ptr * array_ptr" \
>>>>> -	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \
>>>>> +	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \
>>>>>  	"create global variable array_ptr"
>>>>>  
>>>>>  mi_gdb_test "-var-assign array_ptr array2" \
>>>>> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee"
>>>>>  # A varobj we fail to read during -var-update should be considered
>>>>>  # out of scope.
>>>>>  mi_gdb_test "-var-create null_ptr * **0" \
>>>>> -    {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \
>>>>> +    {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \
>>>>>      "create null_ptr"
>>>>>  
>>>>>  # Allow this to succeed, if address zero is readable, although it
>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>>>> index 3bcb36c..a8cc76f 100644
>>>>> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>>>> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>>>> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*"
>>>>>  # We use a explicit cast to (void *) as that is the
>>>>>  # type that caused the bug this testcase is testing for.
>>>>>  mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \
>>>>> -	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \
>>>>> +	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \
>>>>>  	    "-var-create sp1 * \$sp"
>>>>>  gdb_exit
>>>>> diff --git a/gdb/varobj.c b/gdb/varobj.c
>>>>> index 6f56cba..1e3f192 100644
>>>>> --- a/gdb/varobj.c
>>>>> +++ b/gdb/varobj.c
>>>>> @@ -323,7 +323,7 @@ varobj_create (char *objname,
>>>>>  	}
>>>>>  
>>>>>        p = expression;
>>>>> -      innermost_block = NULL;
>>>>> +      innermost_block = block_global_block (block);
>>>>>        /* Wrap the call to parse expression, so we can 
>>>>>           return a sensible error.  */
>>>>>        TRY
>>>>> @@ -2103,6 +2103,8 @@ new_root_variable (void)
>>>>>    var->root->floating = 0;
>>>>>    var->root->rootvar = NULL;
>>>>>    var->root->is_valid = 1;
>>>>> +  var->root->thread_id = 0;
>>>>> +  var->root->next = NULL;
>>>>>  
>>>>>    return var;
>>>>>  }
>>>>> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle)
>>>>>    back_to = make_cleanup_restore_current_thread ();
>>>>>  
>>>>>    /* Determine whether the variable is still around.  */
>>>>> -  if (var->root->valid_block == NULL || var->root->floating)
>>>>> +  if (var->root->valid_block == NULL
>>>>> +      || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL
>>>>> +      || var->root->floating)
>>>>>      within_scope = 1;
>>>>>    else if (var->root->thread_id == 0)
>>>>>      {
>>>>>
>>>>
>>>
>>
> I've created PR mi/20395 for this issue and marked it with the Target
> Milestone 7.12.  Since it's an issue of returning incorrect data I'm
> proposing that it's a blocker.
> thanks
> --Don
> 

[PING] [PATCH v2] Fix -var-update for registers in frames 1 and up

[Don Breazeal]

Hi.  This patch has been in the queue for quite a while.  ping.
Thanks!
--Don

>>>>
>>>> On 6/20/2016 8:31 AM, Don Breazeal wrote:
>>>>> On 6/13/2016 2:54 PM, Don Breazeal wrote:
>>>>>> This is an updated version of a patch to fix access to registers
>>>>>> for frames other than the current frame via var-update.  The piece
>>>>>> of the patch affecting GDB proper has been completely re-written.
>>>>>> It changes the initialization of varobj->root->frame in varobj_create
>>>>>> so that existing mechanisms (with minimal changes) select the correct
>>>>>> frame and provide correct register values from -var-update.
>>>>>>
>>>>>> In addition, the new test for this functionality has been expanded
>>>>>> to cover an analogous case using floating variable objects, and
>>>>>> several tests have been updated to account for the fact that with
>>>>>> the new initialization, -var-create and -var-list-children now
>>>>>> provide a thread-id field where previously they didn't.
>>>>>>
>>>>>> Thanks
>>>>>> --Don
>>>>>>
>>>>>> ------------
>>>>>> This patch fixes a problem with using the MI -var-update command
>>>>>> to access the values of registers in frames other than the current
>>>>>> frame.  The patch includes a test that demonstrates the problem:
>>>>>>
>>>>>> * run so there are several frames on the stack
>>>>>> * create a fixed varobj for $pc in each frame, #'s 1 and above
>>>>>> * step one instruction, to modify the value of $pc
>>>>>> * call -var-update for each of the previously created varobjs
>>>>>>   to verify that they are not reported as having changed.
>>>>>>
>>>>>> Without the patch, the -var-update command reported that $pc for all
>>>>>> frames 1 and above had changed to the value of $pc in frame 0.
>>>>>>
>>>>>> When a varobj is created by -var-create, varobj->root->frame should
>>>>>> be set to specified frame.  Then for a subsequent -var-update,
>>>>>> varobj.c:check_scope can use varobj->root->frame to select the
>>>>>> right frame based on the type of varobj.
>>>>>>
>>>>>> The problem is that for register expressions varobj->root->frame is not
>>>>>> set correctly.  This frame tracking is done using the global
>>>>>> 'innermost_block' which is set in the various parser files (for example
>>>>>> c-exp.y).  However, this is not set for register expressions.
>>>>>>
>>>>>> The fix is to change the initialization of innermost_block in
>>>>>> varobj_create from NULL to the global block, as returned by
>>>>>> block_global_block.  Then varobj_create sets varobj->root->frame
>>>>>> for register varobjs, and value_of_root_1 can check for the
>>>>>> global block when determining whether the variable is in-scope
>>>>>> and select the frame appropriately.
>>>>>>
>>>>>> A side-effect of this change is that for register varobjs and some
>>>>>> global variable varobjs, the output of -var-create now includes the
>>>>>> thread-id field.  The rationale for this is as follow: if we ask for a
>>>>>> particular expression in a particular frame, that implies a particular
>>>>>> thread.  Thus including the thread-id is correct behavior, and the
>>>>>> test results have been updated accordingly.
>>>>>>
>>>>>> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which
>>>>>> performs the test described in the bullet list above as well as a
>>>>>> similar test using a floating variable object to represent $pc.
>>>>>>
>>>>>> We attempted a couple alternative solutions:
>>>>>> * a special case in varobj_update to select the frame was not ideal
>>>>>>   because it didn't use the existing mechanisms to select the frame.
>>>>>> * setting innermost_block in write_dollar_variable had side-effects
>>>>>>   in the CLI that would have required special case code.
>>>>>>
>>>>>> Tested on x86_64 Linux native, no regressions.
>>>>>>
>>>>>> gdb/testsuite/ChangeLog:
>>>>>> 2016-06-13  Don Breazeal  <dbreazea@my.domain.org>
>>>>>>
>>>>>> 	* gdb.ada/mi_interface.exp: Add thread-id field to expected
>>>>>> 	output of -var-create and -var-list-children.
>>>>>> 	* gdb.ada/mi_var_array.exp: Add thread-id field to expected
>>>>>> 	output of -var-list-children.
>>>>>> 	* gdb.mi/mi-break.exp (test_error): Add thread-id field to
>>>>>> 	expected output of -var-create.
>>>>>> 	* gdb.mi/mi-frame-regs.exp: New test script.
>>>>>> 	* gdb.mi/mi-var-cmd.exp: Add thread-id field to expected
>>>>>> 	output of -var-create.
>>>>>> 	* gdb.mi/mi-var-create-rtti.exp: Likewise.
>>>>>>
>>>>>> gdb/ChangeLog:
>>>>>> 2016-06-13  Don Breazeal  <donb@codesourcery.com>
>>>>>> 	    Andrew Burgess <andrew.burgess@embecosm.com>
>>>>>>
>>>>>> 	* varobj.c (varobj_create): Initialize innermost_block to
>>>>>> 	the global block instead of NULL.
>>>>>> 	(new_root_variable): Initialize the thread_id and next
>>>>>> 	fields.
>>>>>> 	(value_of_root_1): Set within_scope if the variable's
>>>>>> 	valid_block field is the global block.
>>>>>>
>>>>>> ---
>>>>>>  gdb/testsuite/gdb.ada/mi_interface.exp      |   4 +-
>>>>>>  gdb/testsuite/gdb.ada/mi_var_array.exp      |   2 +-
>>>>>>  gdb/testsuite/gdb.mi/mi-break.exp           |   2 +-
>>>>>>  gdb/testsuite/gdb.mi/mi-frame-regs.exp      | 183 ++++++++++++++++++++++++++++
>>>>>>  gdb/testsuite/gdb.mi/mi-var-cmd.exp         |   4 +-
>>>>>>  gdb/testsuite/gdb.mi/mi-var-create-rtti.exp |   2 +-
>>>>>>  gdb/varobj.c                                |   8 +-
>>>>>>  7 files changed, 196 insertions(+), 9 deletions(-)
>>>>>>
>>>>>> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp
>>>>>> index 6000ec8..b948cd5 100644
>>>>>> --- a/gdb/testsuite/gdb.ada/mi_interface.exp
>>>>>> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp
>>>>>> @@ -44,9 +44,9 @@ mi_continue_to_line \
>>>>>>      "stop at start of main Ada procedure"
>>>>>>  
>>>>>>  mi_gdb_test "-var-create ggg1 * ggg1" \
>>>>>> -    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \
>>>>>> +    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \
>>>>>>      "Create ggg1 varobj"
>>>>>>  
>>>>>>  mi_gdb_test "-var-list-children 1 ggg1" \
>>>>>> -    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \
>>>>>> +    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \
>>>>>>      "list ggg1's children"
>>>>>> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp
>>>>>> index c648e7e..c02d4c9 100644
>>>>>> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp
>>>>>> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp
>>>>>> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \
>>>>>>      "Create bt varobj"
>>>>>>  
>>>>>>  mi_gdb_test "-var-list-children vta" \
>>>>>> -    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \
>>>>>> +    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \
>>>>>>      "list vta's children"
>>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
>>>>>> index 85f328d..bdd43e0 100644
>>>>>> --- a/gdb/testsuite/gdb.mi/mi-break.exp
>>>>>> +++ b/gdb/testsuite/gdb.mi/mi-break.exp
>>>>>> @@ -213,7 +213,7 @@ proc test_error {} {
>>>>>>      # containing function call, the internal breakpoint created to handle
>>>>>>      # function call would be reported, messing up MI output.
>>>>>>      mi_gdb_test "-var-create V * return_1()" \
>>>>>> -        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \
>>>>>> +        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \
>>>>>>          "create varobj for function call"
>>>>>>  
>>>>>>      mi_gdb_test "-var-update *" \
>>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
>>>>>> new file mode 100644
>>>>>> index 0000000..45f81d6
>>>>>> --- /dev/null
>>>>>> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
>>>>>> @@ -0,0 +1,183 @@
>>>>>> +# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
>>>>>> +
>>>>>> +# Test essential Machine interface (MI) operations
>>>>>> +#
>>>>>> +# Verify that -var-update will provide the correct values for floating
>>>>>> +# and fixed varobjs that represent the pc register.
>>>>>> +#
>>>>>> +
>>>>>> +load_lib mi-support.exp
>>>>>> +set MIFLAGS "-i=mi"
>>>>>> +
>>>>>> +standard_testfile basics.c
>>>>>> +
>>>>>> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
>>>>>> +		 executable {debug}] != "" } then {
>>>>>> +     untested mi-frame-regs.exp
>>>>>> +     return -1
>>>>>> +}
>>>>>> +
>>>>>> +# Return the address of the specified breakpoint.
>>>>>> +
>>>>>> +proc breakpoint_address {bpnum} {
>>>>>> +    global hex
>>>>>> +    global expect_out
>>>>>> +    global mi_gdb_prompt
>>>>>> +
>>>>>> +    send_gdb "info breakpoint $bpnum\n"
>>>>>> +    gdb_expect {
>>>>>> +	-re ".*($hex).*$mi_gdb_prompt$" {
>>>>>> +	    return $expect_out(1,string)
>>>>>> +	}
>>>>>> +	-re ".*$mi_gdb_prompt$" {
>>>>>> +	    return ""
>>>>>> +	}
>>>>>> +	timeout {
>>>>>> +	    return ""
>>>>>> +	}
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +# Test that a floating varobj representing $pc will provide the
>>>>>> +# correct value via -var-update as the program stops at
>>>>>> +# breakpoints in different functions.
>>>>>> +
>>>>>> +proc do_floating_varobj_test {} {
>>>>>> +    global srcfile
>>>>>> +    global hex
>>>>>> +    global expect_out
>>>>>> +
>>>>>> +    gdb_exit
>>>>>> +    if {[mi_gdb_start]} then {
>>>>>> +	continue
>>>>>> +    }
>>>>>> +
>>>>>> +    with_test_prefix "floating" {
>>>>>> +	mi_run_to_main
>>>>>> +
>>>>>> +	# Create a floating varobj for $pc.
>>>>>> +	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
>>>>>> +		    "\\^done,.*value=\"$hex.*" \
>>>>>> +	            "create varobj for pc in frame 0"
>>>>>> +
>>>>>> +	set nframes 4
>>>>>> +	for {set i 1} {$i < $nframes} {incr i} {
>>>>>> +
>>>>>> +	    # Run to a breakpoint in each callee function in succession.
>>>>>> +	    # Note that we can't use mi_runto because we need the
>>>>>> +	    # breakpoint to be persistent, so we can use its address.
>>>>>> +	    set bpnum [expr $i + 1]
>>>>>> +	    mi_create_breakpoint \
>>>>>> +	        "basics.c:callee$i" \
>>>>>> +		"insert breakpoint at basics.c:callee$i" \
>>>>>> +		-number $bpnum -func callee$i -file ".*basics.c"
>>>>>> +
>>>>>> +	    mi_execute_to "exec-continue" "breakpoint-hit" \
>>>>>> +			  "callee$i" ".*" ".*${srcfile}" ".*" \
>>>>>> +			  { "" "disp=\"keep\"" } "breakpoint hit"
>>>>>> +
>>>>>> +	    # Get the value of $pc from the floating varobj.
>>>>>> +	    mi_gdb_test "-var-update 1 var1" \
>>>>>> +			"\\^done,.*value=\"($hex) .*" \
>>>>>> +			"-var-update for frame $i"
>>>>>> +	    set pcval $expect_out(3,string)
>>>>>> +
>>>>>> +	    # Get the address of the current breakpoint.
>>>>>> +	    set bpaddr [breakpoint_address $bpnum]
>>>>>> +	    if {$bpaddr == ""} then {
>>>>>> +		unresolved "get address of breakpoint $bpnum"
>>>>>> +		return
>>>>>> +	    }
>>>>>> +
>>>>>> +	    # Check that the addresses are the same.
>>>>>> +	    if {[expr $bpaddr != $pcval]} then {
>>>>>> +	        fail "\$pc does not equal address of breakpoint"
>>>>>> +	    } else {
>>>>>> +	        pass "\$pc equals address of breakpoint"
>>>>>> +	    }
>>>>>> +	}
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +# Create a varobj for the pc register in each of the frames other
>>>>>> +# than frame 0.
>>>>>> +
>>>>>> +proc var_create_regs {nframes} {
>>>>>> +    global hex
>>>>>> +
>>>>>> +    for {set i 1} {$i < $nframes} {incr i} {
>>>>>> +	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
>>>>>> +		    "\\^done,.*value=\"$hex.*" \
>>>>>> +	            "create varobj for pc in frame $i"
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +# Check that -var-update reports that the value of the pc register
>>>>>> +# for each of the frames 1 and above is reported as unchanged.
>>>>>> +
>>>>>> +proc var_update_regs {nframes} {
>>>>>> +
>>>>>> +    for {set i 1} {$i < $nframes} {incr i} {
>>>>>> +	mi_gdb_test "-var-update 1 var$i" \
>>>>>> +	            "\\^done,(changelist=\\\[\\\])" \
>>>>>> +	            "pc unchanged in -var-update for frame $i"
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +# Test that fixed varobjs representing $pc in different stack frames
>>>>>> +# will provide the correct value via -var-update after the program
>>>>>> +# counter changes (without substantially changing the stack).
>>>>>> +
>>>>>> +proc do_fixed_varobj_test {} {
>>>>>> +    global srcfile
>>>>>> +
>>>>>> +    gdb_exit
>>>>>> +    if {[mi_gdb_start]} then {
>>>>>> +	continue
>>>>>> +    }
>>>>>> +
>>>>>> +    with_test_prefix "fixed" {
>>>>>> +	mi_run_to_main
>>>>>> +
>>>>>> +	# Run to the function 'callee3' so we have several frames.
>>>>>> +	mi_create_breakpoint "basics.c:callee3" \
>>>>>> +			     "insert breakpoint at basics.c:callee3" \
>>>>>> +			     -number 2 -func callee3 -file ".*basics.c"
>>>>>> +
>>>>>> +	mi_execute_to "exec-continue" "breakpoint-hit" \
>>>>>> +	              "callee3" ".*" ".*${srcfile}" ".*" \
>>>>>> +		      { "" "disp=\"keep\"" } "breakpoint hit"
>>>>>> +
>>>>>> +	# At the breakpoint in callee3 there are 4 frames.  Create a
>>>>>> +	# varobj for the pc in each of frames 1 and above.
>>>>>> +	set nframes 4
>>>>>> +	var_create_regs $nframes
>>>>>> +
>>>>>> +	# Step one instruction to change the program counter.
>>>>>> +	mi_execute_to "exec-next-instruction" "end-stepping-range" \
>>>>>> +		      "callee3" ".*" ".*${srcfile}" ".*" "" \
>>>>>> +		      "next instruction in callee3"
>>>>>> +
>>>>>> +	# Check that -var-update reports that the values are unchanged.
>>>>>> +	var_update_regs $nframes
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +do_fixed_varobj_test
>>>>>> +do_floating_varobj_test
>>>>>> +
>>>>>> +mi_gdb_exit
>>>>>> +return 0
>>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>>>>> index 558cd6c..68e3cf8 100644
>>>>>> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>>>>> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
>>>>>> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \
>>>>>>  	"assign same value to func (update)"
>>>>>>  
>>>>>>  mi_gdb_test "-var-create array_ptr * array_ptr" \
>>>>>> -	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \
>>>>>> +	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \
>>>>>>  	"create global variable array_ptr"
>>>>>>  
>>>>>>  mi_gdb_test "-var-assign array_ptr array2" \
>>>>>> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee"
>>>>>>  # A varobj we fail to read during -var-update should be considered
>>>>>>  # out of scope.
>>>>>>  mi_gdb_test "-var-create null_ptr * **0" \
>>>>>> -    {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \
>>>>>> +    {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \
>>>>>>      "create null_ptr"
>>>>>>  
>>>>>>  # Allow this to succeed, if address zero is readable, although it
>>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>>>>> index 3bcb36c..a8cc76f 100644
>>>>>> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>>>>> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
>>>>>> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*"
>>>>>>  # We use a explicit cast to (void *) as that is the
>>>>>>  # type that caused the bug this testcase is testing for.
>>>>>>  mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \
>>>>>> -	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \
>>>>>> +	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \
>>>>>>  	    "-var-create sp1 * \$sp"
>>>>>>  gdb_exit
>>>>>> diff --git a/gdb/varobj.c b/gdb/varobj.c
>>>>>> index 6f56cba..1e3f192 100644
>>>>>> --- a/gdb/varobj.c
>>>>>> +++ b/gdb/varobj.c
>>>>>> @@ -323,7 +323,7 @@ varobj_create (char *objname,
>>>>>>  	}
>>>>>>  
>>>>>>        p = expression;
>>>>>> -      innermost_block = NULL;
>>>>>> +      innermost_block = block_global_block (block);
>>>>>>        /* Wrap the call to parse expression, so we can 
>>>>>>           return a sensible error.  */
>>>>>>        TRY
>>>>>> @@ -2103,6 +2103,8 @@ new_root_variable (void)
>>>>>>    var->root->floating = 0;
>>>>>>    var->root->rootvar = NULL;
>>>>>>    var->root->is_valid = 1;
>>>>>> +  var->root->thread_id = 0;
>>>>>> +  var->root->next = NULL;
>>>>>>  
>>>>>>    return var;
>>>>>>  }
>>>>>> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle)
>>>>>>    back_to = make_cleanup_restore_current_thread ();
>>>>>>  
>>>>>>    /* Determine whether the variable is still around.  */
>>>>>> -  if (var->root->valid_block == NULL || var->root->floating)
>>>>>> +  if (var->root->valid_block == NULL
>>>>>> +      || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL
>>>>>> +      || var->root->floating)
>>>>>>      within_scope = 1;
>>>>>>    else if (var->root->thread_id == 0)
>>>>>>      {
>>>>>>
>>>>>
>>>>
>>>
>> I've created PR mi/20395 for this issue and marked it with the Target
>> Milestone 7.12.  Since it's an issue of returning incorrect data I'm
>> proposing that it's a blocker.
>> thanks
>> --Don
>>
> 

Re: [PATCH v2] Fix -var-update for registers in frames 1 and up

[Pedro Alves]

On 06/13/2016 10:54 PM, Don Breazeal wrote:

> The fix is to change the initialization of innermost_block in 
> varobj_create from NULL to the global block, as returned by 
> block_global_block.

Hmm, this sounds questionable to me.  innermost_block is an output
parameter, after all.  parse_exp_1 already takes an input block 
parameter.

> Then varobj_create sets varobj->root->frame for register varobjs, and
> value_of_root_1 can check for the global block when determining
> whether the variable is in-scope and select the frame appropriately.
> 
> A side-effect of this change is that for register varobjs and some 
> global variable varobjs, the output of -var-create now includes the 
> thread-id field.  The rationale for this is as follow: if we ask for
> a particular expression in a particular frame, that implies a
> particular thread.  Thus including the thread-id is correct behavior,
> and the test results have been updated accordingly.

Sounds OK for register varobjs, but it's not as clear for global
variable varobjs.

I see no way to tell -var-create to create a global variable fixed
varobj that is _not_ associated with a particular thread:

 -var-create {name | "-"} {frame-addr | "*" | "@"} expression

The docs say:

 If an expression specified when creating a fixed variable object
 refers to a local variable, the variable object becomes bound to
 the thread and frame in which the variable object is created. When such 
 variable object is updated, GDB makes sure that the thread/frame combination
 the variable object is bound to still exists, and re-evaluates the variable
 object in context of that thread/frame. 

So if the expression needed a frame, then it gets bound to
the frame/thread.  But if it didn't, then it won't, by my reading?

I worry about whether this might break frontends.

In principle, this sounds similar to watchpoints -- those also
need to check whether the original expression is still in scope,
for the case of watchpoints on local variables.
See update_watchpoint.

I'm still trying to wrap my head around all this, and I need to
read the varobj code to understand how this all works.

Thanks,
Pedro Alves

[PATCH v3] PR mi/20395 - Fix -var-update for registers in frames 1 and up

[Don Breazeal]

I have finally gotten back to this patch and would like to revive it.

Pedro, you had two concerns about the previous version of this patch.
1) In varobj.c:varobj_create, the patch changed innermost_block before
   the call to the expression parser.  You pointed out that
   innermost_block is an output parameter.

2) The patch had a side-effect of adding a thread-id field to the
   output of the -var-update command for global variables.  You were
   concerned that this wasn't consistent with the documentation and
   could potentially break front-ends.

Here is a new version of the patch.  It now assigns innermost_block
in a special case for registers after expression parsing is complete.
Because it does this just for registers, the output of -var-update
for globals is unaffected.

This does introduce some special case code, which Andrew had wanted
to avoid when he reviewed the original patch.  However, with the need
to distinguish between registers and globals wrt setting the block, I
don't see a way around it.

Updated patch follows.

BTW, is there something I should do to remove the Target Milestone
designation from the bug report, since this is not a regression?  I
don't see a way to do it.

Thanks!
--Don
-----------------------
This patch fixes a problem with using the MI -var-update command
to access the values of registers in frames other than the current
frame.  The patch includes a test that demonstrates the problem:

* run so there are several frames on the stack
* create a fixed varobj for $pc in each frame, #'s 1 and above
* step one instruction, to modify the value of $pc
* call -var-update for each of the previously created varobjs
  to verify that they are not reported as having changed.

Without the patch, the -var-update command reported that $pc for all
frames 1 and above had changed to the value of $pc in frame 0.

When a varobj is created by -var-create, varobj->root->frame should
be set to specified frame.  Then for a subsequent -var-update,
varobj.c:check_scope can use varobj->root->frame to select the
right frame based on the type of varobj.

The problem is that for register expressions varobj->root->frame is not
set correctly.  This frame tracking is done using the global
'innermost_block' which is set in the various parser files (for example
c-exp.y).  However, this is not set for register expressions.

The fix is to set innermost block to the global block when creating
a register varobj.  Then varobj_create sets varobj->root->frame for
register varobjs, and varobj_update will select the correct frame
for register varobjs.

We attempted several alternative solutions:
* a special case in varobj_update to select the frame was not ideal
  because it didn't use the existing mechanisms to select the frame.
* setting innermost_block in write_dollar_variable had side-effects
  in the CLI, breaking 'display' for registers.
* setting innermost_block in varobj_create prior to calling the
  expression parser.  This modified an output parameter on input
  and caused side-effects to -var-update output for globals.

Tested on x86_64 Linux native and native-gdbserver, no regressions.

gdb/testsuite/ChangeLog:
2016-10-04  Don Breazeal  <donb@codesourcery.com>

	* gdb.mi/mi-frame-regs.exp: New test.

gdb/ChangeLog:
2016-10-04  Don Breazeal  <donb@codesourcery.com>
	    Andrew Burgess <andrew.burgess@embecosm.com>

	PR mi/20395
	* varobj.c (varobj_create):  For registers, assign
	  innermost_block to the global block.

---
 gdb/testsuite/gdb.mi/mi-frame-regs.exp | 183 +++++++++++++++++++++++++++++++++
 gdb/varobj.c                           |   8 ++
 2 files changed, 191 insertions(+)

diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
new file mode 100644
index 0000000..45f81d6
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
@@ -0,0 +1,183 @@
+# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
+
+# Test essential Machine interface (MI) operations
+#
+# Verify that -var-update will provide the correct values for floating
+# and fixed varobjs that represent the pc register.
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile basics.c
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+		 executable {debug}] != "" } then {
+     untested mi-frame-regs.exp
+     return -1
+}
+
+# Return the address of the specified breakpoint.
+
+proc breakpoint_address {bpnum} {
+    global hex
+    global expect_out
+    global mi_gdb_prompt
+
+    send_gdb "info breakpoint $bpnum\n"
+    gdb_expect {
+	-re ".*($hex).*$mi_gdb_prompt$" {
+	    return $expect_out(1,string)
+	}
+	-re ".*$mi_gdb_prompt$" {
+	    return ""
+	}
+	timeout {
+	    return ""
+	}
+    }
+}
+
+# Test that a floating varobj representing $pc will provide the
+# correct value via -var-update as the program stops at
+# breakpoints in different functions.
+
+proc do_floating_varobj_test {} {
+    global srcfile
+    global hex
+    global expect_out
+
+    gdb_exit
+    if {[mi_gdb_start]} then {
+	continue
+    }
+
+    with_test_prefix "floating" {
+	mi_run_to_main
+
+	# Create a floating varobj for $pc.
+	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
+		    "\\^done,.*value=\"$hex.*" \
+	            "create varobj for pc in frame 0"
+
+	set nframes 4
+	for {set i 1} {$i < $nframes} {incr i} {
+
+	    # Run to a breakpoint in each callee function in succession.
+	    # Note that we can't use mi_runto because we need the
+	    # breakpoint to be persistent, so we can use its address.
+	    set bpnum [expr $i + 1]
+	    mi_create_breakpoint \
+	        "basics.c:callee$i" \
+		"insert breakpoint at basics.c:callee$i" \
+		-number $bpnum -func callee$i -file ".*basics.c"
+
+	    mi_execute_to "exec-continue" "breakpoint-hit" \
+			  "callee$i" ".*" ".*${srcfile}" ".*" \
+			  { "" "disp=\"keep\"" } "breakpoint hit"
+
+	    # Get the value of $pc from the floating varobj.
+	    mi_gdb_test "-var-update 1 var1" \
+			"\\^done,.*value=\"($hex) .*" \
+			"-var-update for frame $i"
+	    set pcval $expect_out(3,string)
+
+	    # Get the address of the current breakpoint.
+	    set bpaddr [breakpoint_address $bpnum]
+	    if {$bpaddr == ""} then {
+		unresolved "get address of breakpoint $bpnum"
+		return
+	    }
+
+	    # Check that the addresses are the same.
+	    if {[expr $bpaddr != $pcval]} then {
+	        fail "\$pc does not equal address of breakpoint"
+	    } else {
+	        pass "\$pc equals address of breakpoint"
+	    }
+	}
+    }
+}
+
+# Create a varobj for the pc register in each of the frames other
+# than frame 0.
+
+proc var_create_regs {nframes} {
+    global hex
+
+    for {set i 1} {$i < $nframes} {incr i} {
+	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
+		    "\\^done,.*value=\"$hex.*" \
+	            "create varobj for pc in frame $i"
+    }
+}
+
+# Check that -var-update reports that the value of the pc register
+# for each of the frames 1 and above is reported as unchanged.
+
+proc var_update_regs {nframes} {
+
+    for {set i 1} {$i < $nframes} {incr i} {
+	mi_gdb_test "-var-update 1 var$i" \
+	            "\\^done,(changelist=\\\[\\\])" \
+	            "pc unchanged in -var-update for frame $i"
+    }
+}
+
+# Test that fixed varobjs representing $pc in different stack frames
+# will provide the correct value via -var-update after the program
+# counter changes (without substantially changing the stack).
+
+proc do_fixed_varobj_test {} {
+    global srcfile
+
+    gdb_exit
+    if {[mi_gdb_start]} then {
+	continue
+    }
+
+    with_test_prefix "fixed" {
+	mi_run_to_main
+
+	# Run to the function 'callee3' so we have several frames.
+	mi_create_breakpoint "basics.c:callee3" \
+			     "insert breakpoint at basics.c:callee3" \
+			     -number 2 -func callee3 -file ".*basics.c"
+
+	mi_execute_to "exec-continue" "breakpoint-hit" \
+	              "callee3" ".*" ".*${srcfile}" ".*" \
+		      { "" "disp=\"keep\"" } "breakpoint hit"
+
+	# At the breakpoint in callee3 there are 4 frames.  Create a
+	# varobj for the pc in each of frames 1 and above.
+	set nframes 4
+	var_create_regs $nframes
+
+	# Step one instruction to change the program counter.
+	mi_execute_to "exec-next-instruction" "end-stepping-range" \
+		      "callee3" ".*" ".*${srcfile}" ".*" "" \
+		      "next instruction in callee3"
+
+	# Check that -var-update reports that the values are unchanged.
+	var_update_regs $nframes
+    }
+}
+
+do_fixed_varobj_test
+do_floating_varobj_test
+
+mi_gdb_exit
+return 0
diff --git a/gdb/varobj.c b/gdb/varobj.c
index fb1349a..80738af 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -348,6 +348,14 @@ varobj_create (char *objname,
 			      " as an expression.\n");
 	  return NULL;
 	}
+      else if (var->root->exp->elts[0].opcode == OP_REGISTER)
+	{
+	  /* Force the scope of a register varobj to be the global
+	     block.  This allows it to be associated with a frame
+	     and a thread.  */
+	  gdb_assert (innermost_block == NULL);
+	  innermost_block = block_global_block (block);
+	}
 
       var->format = variable_default_display (var);
       var->root->valid_block = innermost_block;
-- 
2.8.1

Re: [PATCH v3] PR mi/20395 - Fix -var-update for registers in frames 1 and up

[Pedro Alves]

On 10/04/2016 09:56 PM, Don Breazeal wrote:

> BTW, is there something I should do to remove the Target Milestone
> designation from the bug report, since this is not a regression?  I
> don't see a way to do it.

You should be able to remove it now.  Your account didn't have
"editbugs" privileges.

Thanks,
Pedro Alves

Re: [PATCH v3] PR mi/20395 - Fix -var-update for registers in frames 1 and up

[Andrew Burgess]

* Don Breazeal <donb@codesourcery.com> [2016-10-04 13:56:37 -0700]:

> +
> +	    # Check that the addresses are the same.
> +	    if {[expr $bpaddr != $pcval]} then {
> +	        fail "\$pc does not equal address of breakpoint"
> +	    } else {
> +	        pass "\$pc equals address of breakpoint"

This PASS is repeated multiple times and is not unique, which I think
test names are supposed to be.

Otherwise it looks fine to me, though I'm not a maintainer, so can't
approve the patch :)

Thanks,
Andrew

Re: [PATCH v3] PR mi/20395 - Fix -var-update for registers in frames 1 and up

[Don Breazeal]

On 10/6/2016 5:05 PM, Andrew Burgess wrote:
> * Don Breazeal <donb@codesourcery.com> [2016-10-04 13:56:37 -0700]:
> 
>> +
>> +	    # Check that the addresses are the same.
>> +	    if {[expr $bpaddr != $pcval]} then {
>> +	        fail "\$pc does not equal address of breakpoint"
>> +	    } else {
>> +	        pass "\$pc equals address of breakpoint"
> 
> This PASS is repeated multiple times and is not unique, which I think
> test names are supposed to be.
> 
> Otherwise it looks fine to me, though I'm not a maintainer, so can't
> approve the patch :)
> 
> Thanks,
> Andrew
> 

Thanks Andrew.  I've added a second "with_test_prefix" inside the
offending loop to address this.  The updated patch follows below,
as well as the comments from the previous email detailing the
changes prior to this.
--Don

On 10/4/2016 1:56 PM, Don Breazeal wrote:
> I have finally gotten back to this patch and would like to revive it.
> 
> Pedro, you had two concerns about the previous version of this patch.
> 1) In varobj.c:varobj_create, the patch changed innermost_block before
>    the call to the expression parser.  You pointed out that
>    innermost_block is an output parameter.
> 
> 2) The patch had a side-effect of adding a thread-id field to the
>    output of the -var-update command for global variables.  You were
>    concerned that this wasn't consistent with the documentation and
>    could potentially break front-ends.
> 
> Here is a new version of the patch.  It now assigns innermost_block
> in a special case for registers after expression parsing is complete.
> Because it does this just for registers, the output of -var-update
> for globals is unaffected.
> 
> This does introduce some special case code, which Andrew had wanted
> to avoid when he reviewed the original patch.  However, with the need
> to distinguish between registers and globals wrt setting the block, I
> don't see a way around it.
> 
> Updated patch follows.
>

This patch fixes a problem with using the MI -var-update command
to access the values of registers in frames other than the current
frame.  The patch includes a test that demonstrates the problem:

* run so there are several frames on the stack
* create a fixed varobj for $pc in each frame, #'s 1 and above
* step one instruction, to modify the value of $pc
* call -var-update for each of the previously created varobjs
  to verify that they are not reported as having changed.

Without the patch, the -var-update command reported that $pc for all
frames 1 and above had changed to the value of $pc in frame 0.

When a varobj is created by -var-create, varobj->root->frame should
be set to specified frame.  Then for a subsequent -var-update,
varobj.c:check_scope can use varobj->root->frame to select the
right frame based on the type of varobj.

The problem is that for register expressions varobj->root->frame is not
set correctly.  This frame tracking is done using the global
'innermost_block' which is set in the various parser files (for example
c-exp.y).  However, this is not set for register expressions.

The fix is to set innermost block to the global block when creating
a register varobj.  Then varobj_create sets varobj->root->frame for
register varobjs, and varobj_update will select the correct frame
for register varobjs.

We attempted several alternative solutions:
* a special case in varobj_update to select the frame was not ideal
  because it didn't use the existing mechanisms to select the frame.
* setting innermost_block in write_dollar_variable had side-effects
  in the CLI, breaking 'display' for registers.
* setting innermost_block in varobj_create prior to calling the
  expression parser.  This modified an output parameter on input
  and caused side-effects to -var-update output for globals.

Tested on x86_64 Linux native and native-gdbserver, no regressions.

gdb/testsuite/ChangeLog:
2016-10-14  Don Breazeal  <donb@codesourcery.com>

	* gdb.mi/mi-frame-regs.exp: New test.

gdb/ChangeLog:
2016-10-14  Don Breazeal  <donb@codesourcery.com>
	    Andrew Burgess <andrew.burgess@embecosm.com>

	PR mi/20395
	* varobj.c (varobj_create):  For registers, assign
	  innermost_block to the global block.

---
 gdb/testsuite/gdb.mi/mi-frame-regs.exp | 184
+++++++++++++++++++++++++++++++++
 gdb/varobj.c                           |   8 ++
 2 files changed, 192 insertions(+)

diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp
b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
new file mode 100644
index 0000000..829bc07
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
@@ -0,0 +1,184 @@
+# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
+
+# Test essential Machine interface (MI) operations
+#
+# Verify that -var-update will provide the correct values for floating
+# and fixed varobjs that represent the pc register.
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile basics.c
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+		 executable {debug}] != "" } then {
+     untested mi-frame-regs.exp
+     return -1
+}
+
+# Return the address of the specified breakpoint.
+
+proc breakpoint_address {bpnum} {
+    global hex
+    global expect_out
+    global mi_gdb_prompt
+
+    send_gdb "info breakpoint $bpnum\n"
+    gdb_expect {
+	-re ".*($hex).*$mi_gdb_prompt$" {
+	    return $expect_out(1,string)
+	}
+	-re ".*$mi_gdb_prompt$" {
+	    return ""
+	}
+	timeout {
+	    return ""
+	}
+    }
+}
+
+# Test that a floating varobj representing $pc will provide the
+# correct value via -var-update as the program stops at
+# breakpoints in different functions.
+
+proc do_floating_varobj_test {} {
+    global srcfile
+    global hex
+    global expect_out
+
+    gdb_exit
+    if {[mi_gdb_start]} then {
+	continue
+    }
+
+    with_test_prefix "floating" {
+	mi_run_to_main
+
+	# Create a floating varobj for $pc.
+	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
+		    "\\^done,.*value=\"$hex.*" \
+	            "create floating varobj for pc in frame 0"
+
+	set nframes 4
+	for {set i 1} {$i < $nframes} {incr i} {
+	    with_test_prefix "frame $i" {
+		# Run to a breakpoint in each callee function in succession.
+		# Note that we can't use mi_runto because we need the
+		# breakpoint to be persistent, so we can use its address.
+		set bpnum [expr $i + 1]
+		mi_create_breakpoint \
+		    "basics.c:callee$i" \
+		    "insert breakpoint at basics.c:callee$i" \
+		    -number $bpnum -func callee$i -file ".*basics.c"
+
+		mi_execute_to "exec-continue" "breakpoint-hit" \
+			      "callee$i" ".*" ".*${srcfile}" ".*" \
+			      { "" "disp=\"keep\"" } "breakpoint hit"
+
+		# Get the value of $pc from the floating varobj.
+		mi_gdb_test "-var-update 1 var1" \
+			    "\\^done,.*value=\"($hex) .*" \
+			    "-var-update for frame $i"
+		set pcval $expect_out(3,string)
+
+		# Get the address of the current breakpoint.
+		set bpaddr [breakpoint_address $bpnum]
+		if {$bpaddr == ""} then {
+		    unresolved "get address of breakpoint $bpnum"
+		    return
+		}
+
+		# Check that the addresses are the same.
+		if {[expr $bpaddr != $pcval]} then {
+		    fail "\$pc does not equal address of breakpoint"
+		} else {
+		    pass "\$pc equals address of breakpoint"
+		}
+	    }
+	}
+    }
+}
+
+# Create a varobj for the pc register in each of the frames other
+# than frame 0.
+
+proc var_create_regs {nframes} {
+    global hex
+
+    for {set i 1} {$i < $nframes} {incr i} {
+	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
+		    "\\^done,.*value=\"$hex.*" \
+	            "create varobj for pc in frame $i"
+    }
+}
+
+# Check that -var-update reports that the value of the pc register
+# for each of the frames 1 and above is reported as unchanged.
+
+proc var_update_regs {nframes} {
+
+    for {set i 1} {$i < $nframes} {incr i} {
+	mi_gdb_test "-var-update 1 var$i" \
+	            "\\^done,(changelist=\\\[\\\])" \
+	            "pc unchanged in -var-update for frame $i"
+    }
+}
+
+# Test that fixed varobjs representing $pc in different stack frames
+# will provide the correct value via -var-update after the program
+# counter changes (without substantially changing the stack).
+
+proc do_fixed_varobj_test {} {
+    global srcfile
+
+    gdb_exit
+    if {[mi_gdb_start]} then {
+	continue
+    }
+
+    with_test_prefix "fixed" {
+	mi_run_to_main
+
+	# Run to the function 'callee3' so we have several frames.
+	mi_create_breakpoint "basics.c:callee3" \
+			     "insert breakpoint at basics.c:callee3" \
+			     -number 2 -func callee3 -file ".*basics.c"
+
+	mi_execute_to "exec-continue" "breakpoint-hit" \
+	              "callee3" ".*" ".*${srcfile}" ".*" \
+		      { "" "disp=\"keep\"" } "breakpoint hit"
+
+	# At the breakpoint in callee3 there are 4 frames.  Create a
+	# varobj for the pc in each of frames 1 and above.
+	set nframes 4
+	var_create_regs $nframes
+
+	# Step one instruction to change the program counter.
+	mi_execute_to "exec-next-instruction" "end-stepping-range" \
+		      "callee3" ".*" ".*${srcfile}" ".*" "" \
+		      "next instruction in callee3"
+
+	# Check that -var-update reports that the values are unchanged.
+	var_update_regs $nframes
+    }
+}
+
+do_fixed_varobj_test
+do_floating_varobj_test
+
+mi_gdb_exit
+return 0
diff --git a/gdb/varobj.c b/gdb/varobj.c
index fb1349a..80738af 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -348,6 +348,14 @@ varobj_create (char *objname,
 			      " as an expression.\n");
 	  return NULL;
 	}
+      else if (var->root->exp->elts[0].opcode == OP_REGISTER)
+	{
+	  /* Force the scope of a register varobj to be the global
+	     block.  This allows it to be associated with a frame
+	     and a thread.  */
+	  gdb_assert (innermost_block == NULL);
+	  innermost_block = block_global_block (block);
+	}

       var->format = variable_default_display (var);
       var->root->valid_block = innermost_block;
-- 
2.8.1

[PING] Re: [PATCH v3] PR mi/20395 - Fix -var-update for registers in frames 1 and up

[Don Breazeal]

On 10/14/2016 9:59 AM, Don Breazeal wrote:
> On 10/6/2016 5:05 PM, Andrew Burgess wrote:
>> * Don Breazeal <donb@codesourcery.com> [2016-10-04 13:56:37 -0700]:
>>
>>> +
>>> +	    # Check that the addresses are the same.
>>> +	    if {[expr $bpaddr != $pcval]} then {
>>> +	        fail "\$pc does not equal address of breakpoint"
>>> +	    } else {
>>> +	        pass "\$pc equals address of breakpoint"
>>
>> This PASS is repeated multiple times and is not unique, which I think
>> test names are supposed to be.
>>
>> Otherwise it looks fine to me, though I'm not a maintainer, so can't
>> approve the patch :)
>>
>> Thanks,
>> Andrew
>>
> 
> Thanks Andrew.  I've added a second "with_test_prefix" inside the
> offending loop to address this.  The updated patch follows below,
> as well as the comments from the previous email detailing the
> changes prior to this.
> --Don
> 
> On 10/4/2016 1:56 PM, Don Breazeal wrote:
>> I have finally gotten back to this patch and would like to revive it.
>>
>> Pedro, you had two concerns about the previous version of this patch.
>> 1) In varobj.c:varobj_create, the patch changed innermost_block before
>>    the call to the expression parser.  You pointed out that
>>    innermost_block is an output parameter.
>>
>> 2) The patch had a side-effect of adding a thread-id field to the
>>    output of the -var-update command for global variables.  You were
>>    concerned that this wasn't consistent with the documentation and
>>    could potentially break front-ends.
>>
>> Here is a new version of the patch.  It now assigns innermost_block
>> in a special case for registers after expression parsing is complete.
>> Because it does this just for registers, the output of -var-update
>> for globals is unaffected.
>>
>> This does introduce some special case code, which Andrew had wanted
>> to avoid when he reviewed the original patch.  However, with the need
>> to distinguish between registers and globals wrt setting the block, I
>> don't see a way around it.
>>
>> Updated patch follows.
>>
> 
> This patch fixes a problem with using the MI -var-update command
> to access the values of registers in frames other than the current
> frame.  The patch includes a test that demonstrates the problem:
> 
> * run so there are several frames on the stack
> * create a fixed varobj for $pc in each frame, #'s 1 and above
> * step one instruction, to modify the value of $pc
> * call -var-update for each of the previously created varobjs
>   to verify that they are not reported as having changed.
> 
> Without the patch, the -var-update command reported that $pc for all
> frames 1 and above had changed to the value of $pc in frame 0.
> 
> When a varobj is created by -var-create, varobj->root->frame should
> be set to specified frame.  Then for a subsequent -var-update,
> varobj.c:check_scope can use varobj->root->frame to select the
> right frame based on the type of varobj.
> 
> The problem is that for register expressions varobj->root->frame is not
> set correctly.  This frame tracking is done using the global
> 'innermost_block' which is set in the various parser files (for example
> c-exp.y).  However, this is not set for register expressions.
> 
> The fix is to set innermost block to the global block when creating
> a register varobj.  Then varobj_create sets varobj->root->frame for
> register varobjs, and varobj_update will select the correct frame
> for register varobjs.
> 
> We attempted several alternative solutions:
> * a special case in varobj_update to select the frame was not ideal
>   because it didn't use the existing mechanisms to select the frame.
> * setting innermost_block in write_dollar_variable had side-effects
>   in the CLI, breaking 'display' for registers.
> * setting innermost_block in varobj_create prior to calling the
>   expression parser.  This modified an output parameter on input
>   and caused side-effects to -var-update output for globals.
> 
> Tested on x86_64 Linux native and native-gdbserver, no regressions.
> 
> gdb/testsuite/ChangeLog:
> 2016-10-14  Don Breazeal  <donb@codesourcery.com>
> 
> 	* gdb.mi/mi-frame-regs.exp: New test.
> 
> gdb/ChangeLog:
> 2016-10-14  Don Breazeal  <donb@codesourcery.com>
> 	    Andrew Burgess <andrew.burgess@embecosm.com>
> 
> 	PR mi/20395
> 	* varobj.c (varobj_create):  For registers, assign
> 	  innermost_block to the global block.
> 
> ---
>  gdb/testsuite/gdb.mi/mi-frame-regs.exp | 184
> +++++++++++++++++++++++++++++++++
>  gdb/varobj.c                           |   8 ++
>  2 files changed, 192 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp
> b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
> new file mode 100644
> index 0000000..829bc07
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
> @@ -0,0 +1,184 @@
> +# Copyright 1999-2016 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 3 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, see <http://www.gnu.org/licenses/>.
> +
> +# Test essential Machine interface (MI) operations
> +#
> +# Verify that -var-update will provide the correct values for floating
> +# and fixed varobjs that represent the pc register.
> +#
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +standard_testfile basics.c
> +
> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> +		 executable {debug}] != "" } then {
> +     untested mi-frame-regs.exp
> +     return -1
> +}
> +
> +# Return the address of the specified breakpoint.
> +
> +proc breakpoint_address {bpnum} {
> +    global hex
> +    global expect_out
> +    global mi_gdb_prompt
> +
> +    send_gdb "info breakpoint $bpnum\n"
> +    gdb_expect {
> +	-re ".*($hex).*$mi_gdb_prompt$" {
> +	    return $expect_out(1,string)
> +	}
> +	-re ".*$mi_gdb_prompt$" {
> +	    return ""
> +	}
> +	timeout {
> +	    return ""
> +	}
> +    }
> +}
> +
> +# Test that a floating varobj representing $pc will provide the
> +# correct value via -var-update as the program stops at
> +# breakpoints in different functions.
> +
> +proc do_floating_varobj_test {} {
> +    global srcfile
> +    global hex
> +    global expect_out
> +
> +    gdb_exit
> +    if {[mi_gdb_start]} then {
> +	continue
> +    }
> +
> +    with_test_prefix "floating" {
> +	mi_run_to_main
> +
> +	# Create a floating varobj for $pc.
> +	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
> +		    "\\^done,.*value=\"$hex.*" \
> +	            "create floating varobj for pc in frame 0"
> +
> +	set nframes 4
> +	for {set i 1} {$i < $nframes} {incr i} {
> +	    with_test_prefix "frame $i" {
> +		# Run to a breakpoint in each callee function in succession.
> +		# Note that we can't use mi_runto because we need the
> +		# breakpoint to be persistent, so we can use its address.
> +		set bpnum [expr $i + 1]
> +		mi_create_breakpoint \
> +		    "basics.c:callee$i" \
> +		    "insert breakpoint at basics.c:callee$i" \
> +		    -number $bpnum -func callee$i -file ".*basics.c"
> +
> +		mi_execute_to "exec-continue" "breakpoint-hit" \
> +			      "callee$i" ".*" ".*${srcfile}" ".*" \
> +			      { "" "disp=\"keep\"" } "breakpoint hit"
> +
> +		# Get the value of $pc from the floating varobj.
> +		mi_gdb_test "-var-update 1 var1" \
> +			    "\\^done,.*value=\"($hex) .*" \
> +			    "-var-update for frame $i"
> +		set pcval $expect_out(3,string)
> +
> +		# Get the address of the current breakpoint.
> +		set bpaddr [breakpoint_address $bpnum]
> +		if {$bpaddr == ""} then {
> +		    unresolved "get address of breakpoint $bpnum"
> +		    return
> +		}
> +
> +		# Check that the addresses are the same.
> +		if {[expr $bpaddr != $pcval]} then {
> +		    fail "\$pc does not equal address of breakpoint"
> +		} else {
> +		    pass "\$pc equals address of breakpoint"
> +		}
> +	    }
> +	}
> +    }
> +}
> +
> +# Create a varobj for the pc register in each of the frames other
> +# than frame 0.
> +
> +proc var_create_regs {nframes} {
> +    global hex
> +
> +    for {set i 1} {$i < $nframes} {incr i} {
> +	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
> +		    "\\^done,.*value=\"$hex.*" \
> +	            "create varobj for pc in frame $i"
> +    }
> +}
> +
> +# Check that -var-update reports that the value of the pc register
> +# for each of the frames 1 and above is reported as unchanged.
> +
> +proc var_update_regs {nframes} {
> +
> +    for {set i 1} {$i < $nframes} {incr i} {
> +	mi_gdb_test "-var-update 1 var$i" \
> +	            "\\^done,(changelist=\\\[\\\])" \
> +	            "pc unchanged in -var-update for frame $i"
> +    }
> +}
> +
> +# Test that fixed varobjs representing $pc in different stack frames
> +# will provide the correct value via -var-update after the program
> +# counter changes (without substantially changing the stack).
> +
> +proc do_fixed_varobj_test {} {
> +    global srcfile
> +
> +    gdb_exit
> +    if {[mi_gdb_start]} then {
> +	continue
> +    }
> +
> +    with_test_prefix "fixed" {
> +	mi_run_to_main
> +
> +	# Run to the function 'callee3' so we have several frames.
> +	mi_create_breakpoint "basics.c:callee3" \
> +			     "insert breakpoint at basics.c:callee3" \
> +			     -number 2 -func callee3 -file ".*basics.c"
> +
> +	mi_execute_to "exec-continue" "breakpoint-hit" \
> +	              "callee3" ".*" ".*${srcfile}" ".*" \
> +		      { "" "disp=\"keep\"" } "breakpoint hit"
> +
> +	# At the breakpoint in callee3 there are 4 frames.  Create a
> +	# varobj for the pc in each of frames 1 and above.
> +	set nframes 4
> +	var_create_regs $nframes
> +
> +	# Step one instruction to change the program counter.
> +	mi_execute_to "exec-next-instruction" "end-stepping-range" \
> +		      "callee3" ".*" ".*${srcfile}" ".*" "" \
> +		      "next instruction in callee3"
> +
> +	# Check that -var-update reports that the values are unchanged.
> +	var_update_regs $nframes
> +    }
> +}
> +
> +do_fixed_varobj_test
> +do_floating_varobj_test
> +
> +mi_gdb_exit
> +return 0
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index fb1349a..80738af 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -348,6 +348,14 @@ varobj_create (char *objname,
>  			      " as an expression.\n");
>  	  return NULL;
>  	}
> +      else if (var->root->exp->elts[0].opcode == OP_REGISTER)
> +	{
> +	  /* Force the scope of a register varobj to be the global
> +	     block.  This allows it to be associated with a frame
> +	     and a thread.  */
> +	  gdb_assert (innermost_block == NULL);
> +	  innermost_block = block_global_block (block);
> +	}
> 
>        var->format = variable_default_display (var);
>        var->root->valid_block = innermost_block;
> 
Ping.
thanks,
--Don