[RFA] register_modified_event

[Andrew Cagney]

> On Tue, 20 Aug 2002, Andrew Cagney wrote:
> 
> 
>> (We're not talking about register numbers vs names.  You can figure out 
>> a name/number map from the command that returns the register names.  The 
>> current implementation is broken in that on some targets, it will leave 
>> gaps.  That can be fixed by either changing the interface or the code :-)
> 
> 
> Ok, I think I see what you're saying. I think we're actually on the same 
> page: there is no way to (currently) map numbers to names, 
> given that some targets have 'holes' in the register numbering, i.e., 
> register 21 doesn't exist on all CPUs of XXX architecture. I guess I was 
> presuming that this was already fixed (and it sorta is, on my interpreter 
> branch), and this was just a bad ordering of patches.
> 
> Obviously, we deal with this in Insight: we simply request a list of the 
> numbers and names together. I have a patch which adds a "-numbers" option 
> to "data-list-register-names". It will output register names and their 
> corresponding numbers. This is one easy way to fix this problem. There are 
> others. If you'd like, I could submit patches for this (including docs). 
> Otherwise, let me know how you'd prefer it be done, and I'll do it.

Yes.  The architecture (ARM) that MI was prototyped on didn't have holes 
in the register space so no one noticed ...

It can be fixed two ways:

- like you suggest, provide the name/number table (straight forward)

- have mi renumber things so that the holes are eliminated (Arrgtg....)


>> The problem here is that trying to include the ``modified register'' in 
>> the event is misleading to the point of being dangerous.  The best MI 
>> can offer the GUI is ``hey, target-changed!''.  Every time the target 
>> changes, the GUI needs to refresh everything via varobj.
> 
> 
> I apologize if I am a little hesitant to lose that nice granularity we 
> had for deciding what had changed, but that does make some sense. Hmm. In 
> fact, now that I think about it, I guess that's the safe way to do it,
> after all, with something bizarre like memory-mapped registers, the user 
> could be changing the FP.

Yep.  A single write can change everything, registers, variables, 
threads, the lot!

>> My understanding of JimI's comments (and I agree) is that nothing 
>> matters except how long it takes for the GUI to recover from a target 
>> changed (eg from inferior execution).
> 
> 
> Well, I wouldn't go so far as to say that _nothing_ else matters, but, 
> yes, I would agree with the two of you that recovering from a target 
> state change is paramount.

It's very pragmatic.  Making target-writes fast can never stop people 
complaining about single-step performance :-)

>> These change the target:
>> These do not change the target:
> 
> 
> Ok, that's pretty much what I presumed. 
> 
> Given my long-winded cluelessness, is this closer to what you had in mind? 
> (Note that I haven't made any attempts at replacing registers_changed or 
> flush_cached_frames. We can decide to do that in another set of patches. 
> I also haven't added registers_changed or flush_cached_frames to 
> target_changed, since the latter screws varoabj big time, and I'm not 
> ready to look at fixing that right now.)

Yes, but just call target_changed_event() directly.

(I wonder if it would be better to add it to store_register() and 
target_write_memory()?  I suspect the problem will be that it will also 
trigger on breakpoints :-(.  Suggest filing a bug report about this.)

Andrew


> ChangeLog
> 2002-08-20  Keith Seitz  <keiths@redhat.com>
> 
> 	* target.h (target_changed): Add declaration.
> 	* target.c (target_changed): New function.
> 	* gdb-events.sh: Add target-changed event.
> 	* gdb-events.c: Regenerated.
> 	* gdb-events.c: Regenerated.
> 	* valops.c (value_assign): Call target_changed inlval_register,
> 	lval_memory, and lval_reg_frame_relative.
> 
> Patch
> Index: target.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.h,v
> retrieving revision 1.25
> diff -p -r1.25 target.h
> *** target.h	1 Aug 2002 21:20:14 -0000	1.25
> --- target.h	20 Aug 2002 22:32:12 -0000
> *************** target_resize_to_sections (struct target
> *** 1235,1240 ****
> --- 1235,1241 ----
>   
>   extern void remove_target_sections (bfd *abfd);
>   
> + extern void target_changed (void);
>   
>   /* Stuff that should be shared among the various remote targets.  */
>   
> Index: target.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.c,v
> retrieving revision 1.39
> diff -p -r1.39 target.c
> *** target.c	1 Aug 2002 21:20:14 -0000	1.39
> --- target.c	20 Aug 2002 22:32:12 -0000
> *************** do_monitor_command (char *cmd,
> *** 2463,2468 ****
> --- 2463,2483 ----
>     target_rcmd (cmd, gdb_stdtarg);
>   }
>   
> + /* This function should be called whenever the target's state is changed.
> + 
> +    For example, if the user changes a word in memory on a system with
> +    memory-mapped registers, he could be changing the PC or the FP. Instead
> +    of trying to figure out exactly what changed, just throw out everything
> +    we know about the target and rebuild our perception of its state.
> + 
> +    This will send an event to clients, informing them that the target's
> +    state has changed. */
> + void
> + target_changed (void)
> + {
> +   target_changed_event ();
> + }
> + 
>   void
>   initialize_targets (void)
>   {
> Index: gdb-events.sh
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdb-events.sh,v
> retrieving revision 1.13
> diff -p -r1.13 gdb-events.sh
> *** gdb-events.sh	16 Aug 2002 16:09:07 -0000	1.13
> --- gdb-events.sh	20 Aug 2002 22:32:12 -0000
> *************** f:void:tracepoint_create:int number:numb
> *** 65,70 ****
> --- 65,71 ----
>   f:void:tracepoint_delete:int number:number
>   f:void:tracepoint_modify:int number:number
>   f:void:architecture_changed:void
> + f:void:target_changed:void
>   #*:void:annotate_starting_hook:void
>   #*:void:annotate_stopped_hook:void
>   #*:void:annotate_signalled_hook:void
> *************** f:void:architecture_changed:void
> *** 87,94 ****
>   #*:void:readline_begin_hook:char *format, ...:format
>   #*:char *:readline_hook:char *prompt:prompt
>   #*:void:readline_end_hook:void
> - #*:void:register_changed_hook:int regno:regno
> - #*:void:memory_changed_hook:CORE_ADDR addr, int len:addr, len
>   #*:void:context_hook:int num:num
>   #*:int:target_wait_hook:int pid, struct target_waitstatus *status:pid, status
>   #*:void:call_command_hook:struct cmd_list_element *c, char *cmd, int from_tty:c, cmd, from_tty
> --- 88,93 ----
> Index: valops.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/valops.c,v
> retrieving revision 1.66
> diff -p -r1.66 valops.c
> *** valops.c	1 Aug 2002 17:18:33 -0000	1.66
> --- valops.c	20 Aug 2002 22:32:13 -0000
> *************** value_assign (struct value *toval, struc
> *** 633,638 ****
> --- 633,639 ----
>   	write_memory (changed_addr, dest_buffer, changed_len);
>   	if (memory_changed_hook)
>   	  memory_changed_hook (changed_addr, changed_len);
> + 	target_changed ();
>         }
>         break;
>   
> *************** value_assign (struct value *toval, struc
> *** 678,683 ****
> --- 679,687 ----
>   			      VALUE_CONTENTS (fromval), TYPE_LENGTH (type));
>   #endif
>   	}
> + 
> +       target_changed ();
> + 
>         /* Assigning to the stack pointer, frame pointer, and other
>            (architecture and calling convention specific) registers may
>            cause the frame cache to be out of date.  We just do this
> *************** value_assign (struct value *toval, struc
> *** 765,770 ****
> --- 769,775 ----
>   
>   	if (register_changed_hook)
>   	  register_changed_hook (-1);
> + 	target_changed ();
>         }
>         break;
>   
> 
>