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

[Don Breazeal <donb@codesourcery.com>]

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