Re: [patch] Fix another stale frame_info *

[Pedro Alves <palves@redhat.com>]

On 02/23/2012 08:37 AM, Jan Kratochvil wrote:

> +  if (frame_id_p (frame_caller_id))
> +    frame_caller_pc = frame_unwind_caller_pc (frame);
> +

Yeah.  If we swap the order we do things, we can avoid needing this
bit.  WDYT?  It seems slightly cleaner to me.  Tested on x86_64 Fedora 16.

2012-02-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	* breakpoint.c (until_break_command): Install breakpoints after
	all frame manipulations.
---

 gdb/breakpoint.c |   41 ++++++++++++++++++++++++-----------------
 1 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3740157..fa09238 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9853,6 +9853,8 @@ until_break_command (char *arg, int from_tty, int anywhere)
   struct symtabs_and_lines sals;
   struct symtab_and_line sal;
   struct frame_info *frame = get_selected_frame (NULL);
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  struct frame_id stack_frame_id = get_stack_frame_id (frame);
   struct breakpoint *breakpoint;
   struct breakpoint *breakpoint2 = NULL;
   struct cleanup *old_chain;
@@ -9883,32 +9885,25 @@ until_break_command (char *arg, int from_tty, int anywhere)

   resolve_sal_pc (&sal);

-  if (anywhere)
-    /* If the user told us to continue until a specified location,
-       we don't specify a frame at which we need to stop.  */
-    breakpoint = set_momentary_breakpoint (get_frame_arch (frame), sal,
-					   null_frame_id, bp_until);
-  else
-    /* Otherwise, specify the selected frame, because we want to stop
-       only at the very same frame.  */
-    breakpoint = set_momentary_breakpoint (get_frame_arch (frame), sal,
-					   get_stack_frame_id (frame),
-					   bp_until);
-
-  old_chain = make_cleanup_delete_breakpoint (breakpoint);
-
   tp = inferior_thread ();
   thread = tp->num;

+  old_chain = make_cleanup (null_cleanup, NULL);
+
+  /* Installing a breakpoint invalidates the frame chain (as it may
+     need to switch threads), so do any frame handling first.  */
+
   /* Keep within the current frame, or in frames called by the current
      one.  */

   if (frame_id_p (frame_unwind_caller_id (frame)))
     {
-      sal = find_pc_line (frame_unwind_caller_pc (frame), 0);
-      sal.pc = frame_unwind_caller_pc (frame);
+      struct symtab_and_line sal2;
+
+      sal2 = find_pc_line (frame_unwind_caller_pc (frame), 0);
+      sal2.pc = frame_unwind_caller_pc (frame);
       breakpoint2 = set_momentary_breakpoint (frame_unwind_caller_arch (frame),
-					      sal,
+					      sal2,
 					      frame_unwind_caller_id (frame),
 					      bp_until);
       make_cleanup_delete_breakpoint (breakpoint2);
@@ -9917,6 +9912,18 @@ until_break_command (char *arg, int from_tty, int anywhere)
       make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
     }

+  if (anywhere)
+    /* If the user told us to continue until a specified location,
+       we don't specify a frame at which we need to stop.  */
+    breakpoint = set_momentary_breakpoint (gdbarch, sal,
+					   null_frame_id, bp_until);
+  else
+    /* Otherwise, specify the selected frame, because we want to stop
+       only at the very same frame.  */
+    breakpoint = set_momentary_breakpoint (gdbarch, sal,
+					   stack_frame_id, bp_until);
+  make_cleanup_delete_breakpoint (breakpoint);
+
   proceed (-1, TARGET_SIGNAL_DEFAULT, 0);

   /* If we are running asynchronously, and proceed call above has