Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] register_modified_event
@ 2002-08-19 15:17 Keith Seitz
  2002-08-19 15:49 ` Andrew Cagney
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Seitz @ 2002-08-19 15:17 UTC (permalink / raw)
  To: gdb-patches

Hi,

Here's another event fallout from Apple's MI work. This defines a new 
event (and adds two notifications for it), register_modified_event, which 
is sent whenever a register is modified (usually by a user in the command 
line, but could be via MI or varobj, etc).

As expected, this causes no regressions in the testsuite. I have tests for 
this, but I can't commit any of them until all my MI stuff is complete, 
since event notifications in MI are quite new (and still in approval 
process).

Keith

PS. FYI, I would have named it register_changed_event, just like the hook, 
but there is a name conflict with register_changed in regcache.c.

ChangeLog
2002-08-18  Keith Seitz  <keiths@redhat.com>

        * gdb-events.sh: Add selected_frame_level_changed event.
        * gdb-events.c: Regenerated.
        * gdb-events.c: Regenerated.
        * valops.c (value_assign): Add register_update event notification
        for lval_register and lval_reg_frame_relative.

Patch
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	19 Aug 2002 22:11:13 -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:register_modified:int regno:regno
  #*:void:annotate_starting_hook:void
  #*:void:annotate_stopped_hook:void
  #*:void:annotate_signalled_hook:void
*************** f:void:architecture_changed:void
*** 87,93 ****
  #*: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
--- 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	19 Aug 2002 22:11:41 -0000
*************** value_assign (struct value *toval, struc
*** 678,683 ****
--- 678,686 ----
  			      VALUE_CONTENTS (fromval), TYPE_LENGTH (type));
  #endif
  	}
+ 
+       register_modified_event (VALUE_REGNO (toval));
+ 
        /* 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 ****
--- 768,774 ----
  
  	if (register_changed_hook)
  	  register_changed_hook (-1);
+ 	register_modified_event (regno);
        }
        break;
  


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFA] register_modified_event
  2002-08-19 15:17 [RFA] register_modified_event Keith Seitz
@ 2002-08-19 15:49 ` Andrew Cagney
  2002-08-20  9:36   ` Keith Seitz
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Cagney @ 2002-08-19 15:49 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

> Hi,
> 
> Here's another event fallout from Apple's MI work. This defines a new 
> event (and adds two notifications for it), register_modified_event, which 
> is sent whenever a register is modified (usually by a user in the command 
> line, but could be via MI or varobj, etc).
> 
> As expected, this causes no regressions in the testsuite. I have tests for 
> this, but I can't commit any of them until all my MI stuff is complete, 
> since event notifications in MI are quite new (and still in approval 
> process).
> 
> Keith
> 
> PS. FYI, I would have named it register_changed_event, just like the hook, 
> but there is a name conflict with register_changed in regcache.c.

Keith,

I don't the register_modified (the changelog called it register_update) 
is correct.  Trying to supply a register number, for instance, is just 
misleading.

See:
http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=469

There is also a number of postings to gdb@ about this (I think it was 
JimI last major posting).

enjoy,
Andrew



> ChangeLog
> 2002-08-18  Keith Seitz  <keiths@redhat.com>
> 
>         * gdb-events.sh: Add selected_frame_level_changed event.
>         * gdb-events.c: Regenerated.
>         * gdb-events.c: Regenerated.
>         * valops.c (value_assign): Add register_update event notification
>         for lval_register and lval_reg_frame_relative.
> 
> Patch
> 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	19 Aug 2002 22:11:13 -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:register_modified:int regno:regno
>   #*:void:annotate_starting_hook:void
>   #*:void:annotate_stopped_hook:void
>   #*:void:annotate_signalled_hook:void
> *************** f:void:architecture_changed:void
> *** 87,93 ****
>   #*: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
> --- 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	19 Aug 2002 22:11:41 -0000
> *************** value_assign (struct value *toval, struc
> *** 678,683 ****
> --- 678,686 ----
>   			      VALUE_CONTENTS (fromval), TYPE_LENGTH (type));
>   #endif
>   	}
> + 
> +       register_modified_event (VALUE_REGNO (toval));
> + 
>         /* 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 ****
> --- 768,774 ----
>   
>   	if (register_changed_hook)
>   	  register_changed_hook (-1);
> + 	register_modified_event (regno);
>         }
>         break;
>   
> 
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFA] register_modified_event
  2002-08-19 15:49 ` Andrew Cagney
@ 2002-08-20  9:36   ` Keith Seitz
  2002-08-20 13:46     ` Andrew Cagney
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Seitz @ 2002-08-20  9:36 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Mon, 19 Aug 2002, Andrew Cagney wrote:

> I don't the register_modified (the changelog called it register_update) 
> is correct.  Trying to supply a register number, for instance, is just 
> misleading.

(ChangeLog is wrong, of course. It should be register_modified, not 
register_update.) As for supplying register numbers, I think you'd 
better take another look at MI's command set. Everything (except 
data-list-register-names) returns register numbers. In fact, there is no 
way to find out what the numbers mean at all with the current MI command 
set!

$ ./gdb -nw -nx -i=mi
GNU gdb 2002-08-19-cvs
Copyright 2002 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i686-pc-linux-gnu".
(gdb)
-file-exec-and-symbols gdb
^done
(gdb)
-data-list-register-names
^done,register-names=["eax","ecx","edx","ebx","esp","ebp","esi","edi","eip","eflags","cs","ss","ds","es","fs","gs","st0","st1","st2","st3","st4","st5","st6","st7","fctrl","fstat","ftag","fiseg","fioff","foseg","fooff","fop","xmm0","xmm1","xmm2","xmm3","xmm4","xmm5","xmm6","xmm7","mxcsr","orig_eax","mm0","mm1","mm2","mm3","mm4","mm5","mm6","mm7"]
(gdb)
-break-insert main
^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0808c756",func="main",file="../../src/gdb/main.c",line="741",times="0"}
(gdb)
-exec-run
^running
(gdb)
*stopped,reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x0808c756",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbffff004"}],file="../../src/gdb/main.c",line="741"}
(gdb)
-data-list-register-values x 0
^done,register-values=[{number="0",register-values=[{number="0",value="0x8456784"}]
(gdb)
-data-list-changed-registers
^done,changed-registers=["0","3","4","5","6","7","8","9","10","11","12","13","16","18","20","21","23","24","26","27","28","29","30","31","40","41"]

I've already got another patch which modified data-list-register-names to 
get at the numbers, but I'm guessing that isn't quite what you had in 
mind. Am I correct in assuming that you want me to change the above to 
return names (easy to do) and to take anything which takes a number as 
input and change it to accepting a register name (which means a lot of 
looping)?

> See:
> http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=469
>
> There is also a number of postings to gdb@ about this (I think it was 
> JimI last major posting).

I presume that you mean this thread:

http://sources.redhat.com/ml/gdb/2002-03/msg00154.html

I guess Jim thought there were bigger fish to fry. So if a global
"target-changed" event is wanted, when is it to be sent?

When:
o register changed by user (either via command-line or other UI)
o memory changed by user (")
o inferior execution
o user changes threads (thread_command)
o user changes stack levels (frame_command, up_command, down_command)

I presume for the first two events, the third being implicit, and the last 
two getting events of their own. Am I anywhere near the mark?

Keith


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFA] register_modified_event
  2002-08-20  9:36   ` Keith Seitz
@ 2002-08-20 13:46     ` Andrew Cagney
  2002-08-20 15:36       ` Keith Seitz
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Cagney @ 2002-08-20 13:46 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

> On Mon, 19 Aug 2002, Andrew Cagney wrote:
> 
> 
>> I don't the register_modified (the changelog called it register_update) 
>> is correct.  Trying to supply a register number, for instance, is just 
>> misleading.
> 
> 
> (ChangeLog is wrong, of course. It should be register_modified, not 
> register_update.) As for supplying register numbers, I think you'd 
> better take another look at MI's command set. Everything (except 
> data-list-register-names) returns register numbers. In fact, there is no 
> way to find out what the numbers mean at all with the current MI command 
> set!

(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 :-)

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 presume that you mean this thread:
> 
> http://sources.redhat.com/ml/gdb/2002-03/msg00154.html
> 
> I guess Jim thought there were bigger fish to fry. So if a global
> "target-changed" event is wanted, when is it to be sent?

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

Given that single step refresh performance sets an upper bound on on 
memory/register write refresh performance, people will always notice a 
slow single-step long before they notice slow writes.

> When:

These change the target:

> o register changed by user (either via command-line or other UI)
> o memory changed by user (")
> o inferior execution

These do not change the target:

> o user changes threads (thread_command)
> o user changes stack levels (frame_command, up_command, down_command)

See also:
http://sources.redhat.com/gdb/papers/multi-arch/real-multi-arch/index.html#SEC33

enjoy,
Andrew



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFA] register_modified_event
  2002-08-20 13:46     ` Andrew Cagney
@ 2002-08-20 15:36       ` Keith Seitz
  2002-08-20 17:23         ` Andrew Cagney
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Seitz @ 2002-08-20 15:36 UTC (permalink / raw)
  To: gdb-patches

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;
  


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFA] register_modified_event
  2002-08-20 15:36       ` Keith Seitz
@ 2002-08-20 17:23         ` Andrew Cagney
  2002-08-21  8:34           ` Keith Seitz
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Cagney @ 2002-08-20 17:23 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

> 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);
>   \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;
>   
> 
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFA] register_modified_event
  2002-08-20 17:23         ` Andrew Cagney
@ 2002-08-21  8:34           ` Keith Seitz
  0 siblings, 0 replies; 7+ messages in thread
From: Keith Seitz @ 2002-08-21  8:34 UTC (permalink / raw)
  To: gdb-patches

On Tue, 20 Aug 2002, Andrew Cagney wrote:

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

I'll submit my patch to get the numbering, since I've done that already. 
We will see if anyone jumps out wanting to do it otherwise then.

> Yes, but just call target_changed_event() directly.

Done.

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

gdb/665

For the record, here is what I committed:

ChangeLog
2002-08-21  Keith Seitz  <keiths@redhat.com>

        * gdb-events.sh: Add target-changed event.
        * gdb-events.c: Regenerated.
        * gdb-events.c: Regenerated.
        * valops.c (value_assign): Add target-changed event notification
        to inlval_register, lval_memory, and lval_reg_frame_relative.

Patch
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	21 Aug 2002 15:33:39 -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.67
diff -p -r1.67 valops.c
*** valops.c	19 Aug 2002 23:19:53 -0000	1.67
--- valops.c	21 Aug 2002 15:33:40 -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_event ();
        }
        break;
  
*************** value_assign (struct value *toval, struc
*** 678,683 ****
--- 679,687 ----
  			      VALUE_CONTENTS (fromval), TYPE_LENGTH (type));
  #endif
  	}
+ 
+       target_changed_event ();
+ 
        /* 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_event ();
        }
        break;
  


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2002-08-21 15:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-08-19 15:17 [RFA] register_modified_event Keith Seitz
2002-08-19 15:49 ` Andrew Cagney
2002-08-20  9:36   ` Keith Seitz
2002-08-20 13:46     ` Andrew Cagney
2002-08-20 15:36       ` Keith Seitz
2002-08-20 17:23         ` Andrew Cagney
2002-08-21  8:34           ` Keith Seitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox