Re: [PATCH] gdb/reverse: Fix stepping over recursive functions

[Bruno Larsen <blarsen@redhat.com>]

Please disregard. This patch was sent without proper testing, and failed to fix the whole problem. A better fix should be coming later this week. Sorry for the noise :(

Cheers!
Bruno Larsen

On 5/23/22 10:46, Bruno Larsen wrote:
> Currently, when using GDB to do reverse debugging, if we try to use the
> command "reverse next" to skip a recursive function, instead of skipping
> all of the recursive calls and stopping in the previous line, we stop at
> the second to last recursive call, and need to manually step backwards
> until we leave the first call.  This is well documented in PR gdb/16678.
> 
> This bug happens because when GDB notices that a reverse step has
> entered into a function, GDB will add a step_resume_breakpoint at the
> start of the function, then single step out of the prologue once that
> breakpoint is hit.  Recursion poses a problem because that breakpoint will
> be hit many times before GDB should actually stop the inferior.  To fix
> this issue, when the step_resume_breakpoint is hit (and GDB is executing
> backwards), we analyze if the caller of the frame is the original frame
> where we started the "reverse next", and if it is, GDB will stop the
> inferior, otherwise GDB will just ignore the breakpoint.
> ---
>   gdb/infrun.c                               | 22 ++++++++++++++++++
>   gdb/testsuite/gdb.reverse/step-reverse.c   | 13 +++++++++++
>   gdb/testsuite/gdb.reverse/step-reverse.exp | 26 +++++++++++++++++++++-
>   3 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 02c98b50c8c..ee42f7dfa34 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -6233,6 +6233,28 @@ handle_signal_stop (struct execution_control_state *ecs)
>   	  infrun_debug_printf ("[%s] hit its single-step breakpoint",
>   			       ecs->ptid.to_string ().c_str ());
>   	}
> +
> +      /* If we are executing backwards, we are doing a "next" and the
> +	 current frame is not the same as where we started, the
> +	 step_resume_breakpoint we have just hit has been added to the start
> +	 of a function so we can skip the whle function.  However, if we are
> +	 skipping a recursive call, we only want to act as if we hit the
> +	 breakpoint only when the caller of the current frame is the original
> +	 frame we were single stepping from.  */
> +      if (execution_direction == EXEC_REVERSE
> +	  && ecs->event_thread->control.step_over_calls == STEP_OVER_ALL
> +	  && !frame_id_eq (get_stack_frame_id (get_current_frame ()),
> +			   ecs->event_thread->control.step_stack_frame_id))
> +	{
> +	  /* If the caller's ID is not the same as the starting frame, we
> +	     can ignore this breakpoint.  */
> +	  if (!frame_id_eq (frame_unwind_caller_id (get_current_frame ()),
> +			    ecs->event_thread->control.step_stack_frame_id))
> +	    {
> +	      keep_going (ecs);
> +	      return;
> +	    }
> +	}
>       }
>     delete_just_stopped_threads_single_step_breakpoints ();
>   
> diff --git a/gdb/testsuite/gdb.reverse/step-reverse.c b/gdb/testsuite/gdb.reverse/step-reverse.c
> index aea2a98541d..958d4b297b0 100644
> --- a/gdb/testsuite/gdb.reverse/step-reverse.c
> +++ b/gdb/testsuite/gdb.reverse/step-reverse.c
> @@ -26,6 +26,17 @@ int callee() {		/* ENTER CALLEE */
>     return myglob++;	/* ARRIVED IN CALLEE */
>   }			/* RETURN FROM CALLEE */
>   
> +/* We need to make this function take more than a single instruction
> +   to run, otherwise it could hide PR gdb/16678, as reverse execution can
> +   step over a single-instruction function.  */
> +int recursive_callee (){
> +    if (myglob == 0) return 0;
> +    myglob /= 2;
> +    if (myglob>1)
> +	myglob++;
> +    return recursive_callee ();	/* REVERSE NEXT FAIL */
> +}
> +
>   /* A structure which, we hope, will need to be passed using memcpy.  */
>   struct rhomboidal {
>     int rather_large[100];
> @@ -61,6 +72,8 @@ int main () {
>      a[5] = a[3] - a[4]; /* FINISH TEST */
>      callee();	/* STEPI TEST */
>      
> +   recursive_callee (); /* NEXT OVER THIS RECURSION */
> +
>      /* Test "nexti" */
>      callee();	/* NEXTI TEST */
>   
> diff --git a/gdb/testsuite/gdb.reverse/step-reverse.exp b/gdb/testsuite/gdb.reverse/step-reverse.exp
> index 997b62604d5..4238675431e 100644
> --- a/gdb/testsuite/gdb.reverse/step-reverse.exp
> +++ b/gdb/testsuite/gdb.reverse/step-reverse.exp
> @@ -118,7 +118,7 @@ gdb_test_multiple "stepi" "$test_message" {
>   
>   set test_message "stepi back from function call"
>   gdb_test_multiple "stepi" "$test_message" {
> -    -re "NEXTI TEST.*$gdb_prompt $" {
> +    -re -wrap "NEXT OVER THIS RECURSION.*" {
>   	pass "$test_message"
>       }
>       -re "ARRIVED IN CALLEE.*$gdb_prompt $" {
> @@ -138,6 +138,9 @@ gdb_test_multiple "stepi" "$test_message" {
>       }
>   }
>   
> +# next through a recursive function call
> +gdb_test "next" "NEXTI TEST.*" "next over recursion"
> +
>   ###
>   ###
>   ###
> @@ -145,6 +148,27 @@ gdb_test_multiple "stepi" "$test_message" {
>   # Set reverse execution direction
>   
>   gdb_test_no_output "set exec-dir reverse" "set reverse execution"
> +set step_out 0
> +gdb_test_multiple "next" "reverse next over recursion" {
> +    -re -wrap ".*NEXT OVER THIS RECURSION.*" {
> +	pass "reverse next over recursion"
> +    }
> +    -re -wrap ".*REVERSE NEXT FAIL.*" {
> +	fail "reverse next over recursion"
> +	set step_out 1
> +    }
> +}
> +if { "$step_out" == 1 } {
> +    gdb_test_multiple "next" "stepping out of recursion" {
> +	-re -wrap "NEXT OVER THIS RECURSION.*" {
> +	    set step_out 0
> +	}
> +	-re -wrap ".*" {
> +	    send_gdb "next\n"
> +	    exp_continue
> +	}
> +    }
> +}
>   
>   # stepi backward thru return and into a function
>