Re: [rfc] Don't thwack frame->pc in infun.c (DECR_PC_AFTER_BREAK)

[Andrew Cagney <ac131313 at redhat dot com>]

> Hello,
>
> The file "infrun.c" currently contains the `cute' trick:
>
>    /* Make sure that the current_frame's pc is correct.  This
>       is a correction for setting up the frame info before doing
>       DECR_PC_AFTER_BREAK */
> -  if (target_has_execution && get_current_frame ())
> -    (get_current_frame ())->pc = read_pc ();
>
> Come the revolution, and when all DECR_PC_AFTER_BREAK has been slain, this hack will just go away.
>
> In the mean time, I'm trying to figure out a cleaner way of doing it ...
>
> First thing to note is that the get_current_frame() [!= NULL] part of the `if' conditional is redundant.  If there isn't a current frame, get_current_frame() will throw an error.  Consequently the test can be dropped (with the side effect of forcing a frame create).
>
> Second thing to note is that by changing the frame's PC the code has invalidated anything cached in that frame.  Consequently, the code should flush the cache, rather than just tweak the PC.  An unnecessary flush can be avoided by checking to see if the PC changed.
>
> The attached patch implements both of these changes.  It only flushes the cache when the frame's PC has, some how, become invalid.
>
> Unfortunatly, for the i386 (and alpha), this has consequences.  It guarentees the sequence:
>
> - create frame (parse prologue)
> - flush frames
> - create frame (parse prologue)
>
> Where parse prologue involves target reads and, hence, is considered expensive.   (I don't feel too guilty since the i386 has its [er, deprecated] codestream which should lessen the damange).
>
> So anyway, the above needs to be changed.  An alternative might be to:
>
>     if (target_has_execution)
>       deprecated_resync_current_frame_pc_hack (read_pc ());
>
> thoughts?
I went with option B and committed the attached.

Andrew

2002-12-13  Andrew Cagney  <ac131313@redhat.com>

	* frame.c (deprecated_update_current_frame_pc_hack): New
	function.
	* frame.h (deprecated_update_current_frame_pc_hack): Declare.
	* infrun.c (normal_stop): Use said function instead of directly
	modifying the frame's PC.
	
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.45
diff -u -r1.45 frame.c
--- frame.c	13 Dec 2002 16:40:25 -0000	1.45
+++ frame.c	13 Dec 2002 21:52:24 -0000
@@ -1291,6 +1291,17 @@
 }
 
 void
+deprecated_update_current_frame_pc_hack (CORE_ADDR pc)
+{
+  /* FIXME: cagney/2002-12-06: Has the PC in the current frame
+     changed?  "infrun.c", Thanks to DECR_PC_AFTER_BREAK, can change
+     the PC after the initial frame create.  This puts things back in
+     sync.  */
+  if (current_frame != NULL)
+    current_frame->pc = pc;
+}
+
+void
 _initialize_frame (void)
 {
   obstack_init (&frame_cache_obstack);
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.50
diff -u -r1.50 frame.h
--- frame.h	13 Dec 2002 20:21:32 -0000	1.50
+++ frame.h	13 Dec 2002 21:52:25 -0000
@@ -654,4 +654,9 @@
 							 long size);
 extern struct frame_extra_info *get_frame_extra_info (struct frame_info *fi);
 
+/* FIXME: cagney/2002-12-06: Has the PC in the current frame changed?
+   "infrun.c", Thanks to DECR_PC_AFTER_BREAK, can change the PC after
+   the initial frame create.  This puts things back in sync.  */
+extern void deprecated_update_current_frame_pc_hack (CORE_ADDR pc);
+
 #endif /* !defined (FRAME_H)  */
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.87
diff -u -r1.87 infrun.c
--- infrun.c	12 Dec 2002 22:41:21 -0000	1.87
+++ infrun.c	13 Dec 2002 21:52:40 -0000
@@ -3066,8 +3066,12 @@
   /* Make sure that the current_frame's pc is correct.  This
      is a correction for setting up the frame info before doing
      DECR_PC_AFTER_BREAK */
-  if (target_has_execution && get_current_frame ())
-    (get_current_frame ())->pc = read_pc ();
+  if (target_has_execution)
+    /* FIXME: cagney/2002-12-06: Has the PC changed?  Thanks to
+       DECR_PC_AFTER_BREAK, the program counter can change.  Ask the
+       frame code to check for this and sort out any resultant mess.
+       DECR_PC_AFTER_BREAK needs to just go away.  */
+    deprecated_update_current_frame_pc_hack (read_pc ());
 
   if (target_has_execution && breakpoints_inserted)
     {