Re: [PATCH 1/2] fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp

[Bruno Larsen <blarsen@redhat.com>]

On 11/01/2023 19:27, Carl Love via Gdb-patches wrote:
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 181d961d80d..8ed538ea9ec 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2748,8 +2748,6 @@ clear_proceed_status_thread (struct thread_info *tp)
>   
>     tp->control.stop_step = 0;
>   
> -  tp->control.proceed_to_finish = 0;
> -
>     tp->control.stepping_command = 0;
>   
>     /* Discard any remaining commands or status from previous stop.  */
> @@ -6737,31 +6735,28 @@ process_event_stop_test (struct execution_control_state *ecs)
>   
>       case BPSTAT_WHAT_STEP_RESUME:
Something else that I failed to notice. Since you removed both comments 
that mention that this case is here for reverse finishing, there is no 
good way to figure out what GDB wants to do when this part of the code 
is reached. Adding a comment here mentioning it would fix that.
>         infrun_debug_printf ("BPSTAT_WHAT_STEP_RESUME");
> -
>         delete_step_resume_breakpoint (ecs->event_thread);
> -      if (ecs->event_thread->control.proceed_to_finish
> -	  && execution_direction == EXEC_REVERSE)
> +      fill_in_stop_func (gdbarch, ecs);
> +
> +      if (execution_direction == EXEC_REVERSE
> +	  && ecs->event_thread->stop_pc () == ecs->stop_func_start)
>   	{
>   	  struct thread_info *tp = ecs->event_thread;
> +	  stop_pc_sal = find_pc_line (ecs->event_thread->stop_pc (), 0);
>   
> -	  /* 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;
> -	}
> -      fill_in_stop_func (gdbarch, ecs);
> -      if (ecs->event_thread->stop_pc () == ecs->stop_func_start
> -	  && execution_direction == EXEC_REVERSE)
> -	{
> -	  /* We are stepping over a function call in reverse, and just
> -	     hit the step-resume breakpoint at the start address of
> -	     the function.  Go back to single-stepping, which should
> -	     take us back to the function call.  */
> -	  ecs->event_thread->stepping_over_breakpoint = 1;
> +	  /* When setting a step range, need to call set_step_info
> +	     to setup the current_line/symtab fields as well.  */
> +	  set_step_info (tp, frame, stop_pc_sal);
> +
> +	  /* We are finishing a function in reverse or stepping over a function
> +	     call 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 to the function call.
> +
> +	     Return using a step range so we will keep stepping back to the
> +	     first instruction in the source code line.  */
> +	  tp->control.step_range_start = ecs->stop_func_start;
> +	  tp->control.step_range_end = ecs->stop_func_start;
>   	  keep_going (ecs);
>   	  return;
>   	}


-- 
Cheers,
Bruno