MI and modifying register value

MI and modifying register value

[Jan Vrany]

Hi, 

while investigating MI client issue when setting a register value
from debugger I realized two things: 

1) There's an MI command to set register value: -data-write-register-values
   This command is not documented and looking at the code, seems to be somewhat
   half-implemented - does not use <format> parameter its comment mentions and
   only works for frame 0 (but using --frame 1 does not result in an error,
   it just silently changes value in frame 0). 

   Is this command some leftover that should not be used (but kept for some
   old clients)? Or is it something to fix? 

2) Another way to change register value is CLI command `set r4 = 0x4`. There's
   no mechanism the MI client gets notified about the change - there's no =register-changed
   async event. 

   Is this by purpose? I'd expect some kind of notification, just like there's =memory-changed
   event. Also, in Python API, there's memory_ckanged event which get triggered when
   using `set` to modify register. 

Best, Jan

   

Re: MI and modifying register value

[Tom Tromey]

>>>>> "Jan" == Jan Vrany via Gdb <gdb@sourceware.org> writes:

Jan> 1) There's an MI command to set register value: -data-write-register-values
Jan>    This command is not documented and looking at the code, seems to be somewhat
Jan>    half-implemented - does not use <format> parameter its comment mentions and
Jan>    only works for frame 0 (but using --frame 1 does not result in an error,
Jan>    it just silently changes value in frame 0). 

Jan>    Is this command some leftover that should not be used (but kept for some
Jan>    old clients)? Or is it something to fix? 

It's very old and seems to have landed without tests or documentation.

I think the addition of 'format' here must be some kind of copy-paste
bug.  I can't imagine how it would be useful.  However, in the spirit of
API conservatism I think it should just be left untouched -- we can just
document that it's a mistake and clients can send whatever they like for
this argument.

Fixing the frame bug seems completely fine to me.

Jan> 2) Another way to change register value is CLI command `set r4 = 0x4`. There's
Jan>    no mechanism the MI client gets notified about the change - there's no =register-changed
Jan>    async event. 

Surely it must be "set $r4 = 0x4" -- the "$" should be needed I think.

Jan>    Is this by purpose? I'd expect some kind of notification, just like there's =memory-changed
Jan>    event. Also, in Python API, there's memory_ckanged event which get triggered when
Jan>    using `set` to modify register. 

There's the weird "-data-list-changed-registers" command.  It doesn't
really documented "changed since what", but it seems to be "since last
time this was called".  There's a static local variable in the
implementation :-(

I do see there is a "register_changed" observable, so implementing the
notification seems relatively easy.  This observable seems to only track
changes made by the user, though.

Tom

Re: MI and modifying register value

[Jan Vrany]

On Fri, 2023-09-08 at 10:53 -0600, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany via Gdb <gdb@sourceware.org> writes:
> 
> Jan> 1) There's an MI command to set register value: -data-write-register-values
> Jan>    This command is not documented and looking at the code, seems to be somewhat
> Jan>    half-implemented - does not use <format> parameter its comment mentions and
> Jan>    only works for frame 0 (but using --frame 1 does not result in an error,
> Jan>    it just silently changes value in frame 0).
> 
> Jan>    Is this command some leftover that should not be used (but kept for some
> Jan>    old clients)? Or is it something to fix?
> 
> It's very old and seems to have landed without tests or documentation.
> 
> I think the addition of 'format' here must be some kind of copy-paste
> bug.  I can't imagine how it would be useful.  However, in the spirit of
> API conservatism I think it should just be left untouched -- we can just
> document that it's a mistake and clients can send whatever they like for
> this argument.
> 
> Fixing the frame bug seems completely fine to me.

Yes, seems like an old and largely abandoned command. 

> 
> Jan> 2) Another way to change register value is CLI command `set r4 = 0x4`. There's
> Jan>    no mechanism the MI client gets notified about the change - there's no =register-changed
> Jan>    async event. 
> 
> Surely it must be "set $r4 = 0x4" -- the "$" should be needed I think.

You're right, "$" is missing in my example. A "typo" on my side. 

> 
> Jan>    Is this by purpose? I'd expect some kind of notification, just like there's =memory-changed
> Jan>    event. Also, in Python API, there's memory_ckanged event which get triggered when
> Jan>    using `set` to modify register.
> 
> There's the weird "-data-list-changed-registers" command.  It doesn't
> really documented "changed since what", but it seems to be "since last
> time this was called".  There's a static local variable in the
> implementation :-(

Yes, but besides the issue of "changed since what?" (which would be nice to fix), it
does not really address the problem - imagine a UI that shows a list of registers
and then user from CLI modifies it with `set` command. With -data-list-changed-registers
I'd have to resort to issue this command each time an UI is redrawn / each time use enters 
a command in CLI which seems excessive.

> 
> I do see there is a "register_changed" observable, so implementing the
> notification seems relatively easy.  This observable seems to only track
> changes made by the user, though.

Yes. Tracking changes made by user is fine (for my use case). Anyways, I just
submitted a patch that allows me to implement such notification in Python [1]
without introducing new async event to all clients. The documentations says clients
should be prepared for responses having more data but says nothing about handling
new async events so this seemes "safer" :-)

Jan

[1] https://inbox.sourceware.org/gdb-patches/20230908210504.89194-2-jan.vrany@labware.com/T/#u