Re: [RFA] register_modified_event

[Keith Seitz <keiths@redhat.com>]

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.

> 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. Oh well, lesson learned. (Doh! And that is 
exactly what the PR in the database says. Fooey. I should have slept a 
little longer this morning. Sorry.)

> 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. (No, I'm not really trying to be confrontation, 
although I can certainly see why some might think so. "I'm not 
confrontational -- just slow! O:-)

> 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.)

Keith

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);
  \f
  /* 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;