2011-05-26 Pedro Alves <pedro@codesourcery.com>

gdb/
	* infcmd.c (finish_backward): Set a step-resume breakpoint at the
	function's entry point instead of a manually managed momentary
	breakpoint, and only ever issue one proceed call.
	* infrun.c (handle_inferior_event) <BPSTAT_WHAT_STEP_RESUME>: If
	doing a reverse-finish, switch to stepi mode, to do another step.
	(insert_step_resume_breakpoint_at_sal): Make public.
	(normal_stop): No need to save function value return registers if
	going reverse.
	* inferior.h (insert_step_resume_breakpoint_at_sal): Declare.

	gdb/testsuite/
	* gdb.reverse/finish-reverse-bkpt.exp: New test.

gdb/ChangeLog

@@ -1,3 +1,15 @@
+2011-05-26  Pedro Alves  <pedro@codesourcery.com>
+
+	* infcmd.c (finish_backward): Set a step-resume breakpoint at the
+	function's entry point instead of a manually managed momentary
+	breakpoint, and only ever issue one proceed call.
+	* infrun.c (handle_inferior_event) <BPSTAT_WHAT_STEP_RESUME>: If
+	doing a reverse-finish, switch to stepi mode, to do another step.
+	(insert_step_resume_breakpoint_at_sal): Make public.
+	(normal_stop): No need to save function value return registers if
+	going reverse.
+	* inferior.h (insert_step_resume_breakpoint_at_sal): Declare.
+
 2011-05-26  Pedro Alves  <pedro@codesourcery.com>
 
 	* breakpoint.h (enum bptype) <bp_hp_step_resume>: New.

gdb/infcmd.c

@@ -1528,11 +1528,8 @@ finish_backward (struct symbol *function)
 {
   struct symtab_and_line sal;
   struct thread_info *tp = inferior_thread ();
-  struct breakpoint *breakpoint;
-  struct cleanup *old_chain;
   CORE_ADDR pc;
   CORE_ADDR func_addr;
-  int back_up;
 
   pc = get_frame_pc (get_current_frame ());
 
@@ -1542,8 +1539,7 @@ finish_backward (struct symbol *function)
 
   sal = find_pc_line (func_addr, 0);
 
-  /* We don't need a return value.  */
-  tp->control.proceed_to_finish = 0;
+  tp->control.proceed_to_finish = 1;
   /* Special case: if we're sitting at the function entry point,
      then all we need to do is take a reverse singlestep.  We
      don't need to set a breakpoint, and indeed it would do us
@@ -1557,33 +1553,25 @@ finish_backward (struct symbol *function)
     {
       struct frame_info *frame = get_selected_frame (NULL);
       struct gdbarch *gdbarch = get_frame_arch (frame);
+      struct symtab_and_line sr_sal;
+
+      /* Set a step-resume at the function's entry point.  Once that's
+	 hit, we'll do one more step backwards.  */
+      init_sal (&sr_sal);
+      sr_sal.pc = sal.pc;
+      sr_sal.pspace = get_frame_program_space (frame);
+      insert_step_resume_breakpoint_at_sal (gdbarch,
+					    sr_sal, null_frame_id);
 
-      /* Set breakpoint and continue.  */
-      breakpoint =
-	set_momentary_breakpoint (gdbarch, sal,
-				  get_stack_frame_id (frame),
-				  bp_breakpoint);
-      /* Tell the breakpoint to keep quiet.  We won't be done
-         until we've done another reverse single-step.  */
-      breakpoint_set_silent (breakpoint, 1);
-      old_chain = make_cleanup_delete_breakpoint (breakpoint);
       proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
-      /* We will be stopped when proceed returns.  */
-      back_up = (bpstat_find_breakpoint (tp->control.stop_bpstat, breakpoint)
-		 != NULL);
-      do_cleanups (old_chain);
     }
   else
-    back_up = 1;
-  if (back_up)
     {
-      /* If in fact we hit the step-resume breakpoint (and not
-	 some other breakpoint), then we're almost there --
-	 we just need to back up by one more single-step.  */
+      /* We're almost there -- we just need to back up by one more
+	 single-step.  */
       tp->control.step_range_start = tp->control.step_range_end = 1;
       proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 1);
     }
-  return;
 }
 
 /* finish_forward -- helper function for finish_command.  */

gdb/inferior.h

@@ -190,6 +190,10 @@ extern void resume (int, enum target_signal);
 
 extern ptid_t user_visible_resume_ptid (int step);
 
+extern void insert_step_resume_breakpoint_at_sal (struct gdbarch *,
+						  struct symtab_and_line ,
+						  struct frame_id);
+
 /* From misc files */
 
 extern void default_print_registers_info (struct gdbarch *gdbarch,

gdb/infrun.c

@@ -103,10 +103,6 @@ static void insert_hp_step_resume_breakpoint_at_frame (struct frame_info *);
 
 static void insert_step_resume_breakpoint_at_caller (struct frame_info *);
 
-static void insert_step_resume_breakpoint_at_sal (struct gdbarch *,
-						  struct symtab_and_line ,
-						  struct frame_id);
-
 static void insert_longjmp_resume_breakpoint (struct gdbarch *, CORE_ADDR);
 
 /* When set, stop the 'step' command if we enter a function which has
@@ -4354,6 +4350,20 @@ process_event_stop_test:
 	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STEP_RESUME\n");
 
 	delete_step_resume_breakpoint (ecs->event_thread);
+	if (ecs->event_thread->control.proceed_to_finish
+	    && execution_direction == EXEC_REVERSE)
+	  {
+	    struct thread_info *tp = ecs->event_thread;
+
+	    /* We are finishing a function in reverse, and just hit
+	       the step-resume breakpoint at the start address of the
+	       function, and we're almost there -- just need to back
+	       up by one more single-step, which should take us back
+	       to the function call.  */
+	    tp->control.step_range_start = tp->control.step_range_end = 1;
+	    keep_going (ecs);
+	    return;
+	  }
 	if (stop_pc == ecs->stop_func_start
 	    && execution_direction == EXEC_REVERSE)
 	  {
@@ -5235,7 +5245,7 @@ insert_step_resume_breakpoint_at_sal_1 (struct gdbarch *gdbarch,
     = set_momentary_breakpoint (gdbarch, sr_sal, sr_id, sr_type);
 }
 
-static void
+void
 insert_step_resume_breakpoint_at_sal (struct gdbarch *gdbarch,
 				      struct symtab_and_line sr_sal,
 				      struct frame_id sr_id)
@@ -5859,7 +5869,8 @@ normal_stop (void)
 
   /* Save the function value return registers, if we care.
      We might be about to restore their previous contents.  */
-  if (inferior_thread ()->control.proceed_to_finish)
+  if (inferior_thread ()->control.proceed_to_finish
+      && execution_direction != EXEC_REVERSE)
     {
       /* This should not be necessary.  */
       if (stop_registers)

gdb/testsuite/ChangeLog

@@ -1,3 +1,7 @@
+2011-05-26  Pedro Alves  <pedro@codesourcery.com>
+
+	* gdb.reverse/finish-reverse-bkpt.exp: New test.
+
 2011-05-26  Pedro Alves  <pedro@codesourcery.com>
 
 	* gdb.reverse/next-reverse-bkpt-over-sr.exp: New test.

gdb/testsuite/gdb.reverse/finish-reverse-bkpt.exp

@@ -0,0 +1,57 @@
+# Copyright 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.
+#
+# 'reverse-finish' used to have a bug where user breakpoints set at
+# the functions entry would be ignored.  Make sure the bug doesn't
+# reappear.
+
+if ![target_info exists gdb,can_reverse] {
+    return
+}
+
+set testfile "finish-reverse-bkpt"
+set srcfile finish-reverse.c
+
+if { [prepare_for_testing $testfile.exp "$testfile" $srcfile] } {
+    return -1
+}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+if [target_info exists gdb,use_precord] {
+    # Activate process record/replay
+    gdb_test_no_output "record" "Turn on process record"
+}
+
+set breakloc [gdb_get_line_number "VOID FUNC" "$srcfile"]
+gdb_test "break void_func" \
+    "Breakpoint $decimal at .*/$srcfile, line $breakloc\." \
+    "set breakpoint on void_func"
+gdb_continue_to_breakpoint "void_func" ".*/$srcfile:$breakloc.*"
+
+gdb_test "break \*void_func" \
+    "Breakpoint $decimal at .*" \
+    "set breakpoint at void_func's entry"
+
+gdb_test "reverse-finish" \
+    ".*Breakpoint .*, void_func.*" \
+    "reverse-finish from void_func trips breakpoint at entry"
+
+gdb_test "frame" "#0  void_func.*" "no spurious proceed after breakpoint stop"