[PATCH] gdb/record: print frame information when exiting a recursive call

[PATCH] gdb/record: print frame information when exiting a recursive call

[Guinevere Larsen]

Currently,  when GDB is reverse stepping out of a function into the same
function due to a recursive call, it doesn't print frame information, as
reported by PR record/29178. This happens because when the inferior
leaves the current frame, GDB decides to refresh the step information,
clobbering the original step_frame_id, making it impossible to figure
out later on that the frame has been changed.

This commit changes GDB so that, if we notice we're in this exact
situation, we won't refresh the step information.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29178
---
 gdb/infrun.c                            | 18 +++++++++
 gdb/testsuite/gdb.reverse/recursion.c   | 38 +++++++++++++++++++
 gdb/testsuite/gdb.reverse/recursion.exp | 49 +++++++++++++++++++++++++
 3 files changed, 105 insertions(+)
 create mode 100644 gdb/testsuite/gdb.reverse/recursion.c
 create mode 100644 gdb/testsuite/gdb.reverse/recursion.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4730d290442..00e6215ebc8 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7679,6 +7679,11 @@ process_event_stop_test (struct execution_control_state *ecs)
     }
 
   bool refresh_step_info = true;
+
+  /* shorthand to make if statements smaller.  */
+  struct frame_id original_frame_id
+    = ecs->event_thread->control.step_frame_id;
+  struct frame_id curr_frame_id = get_frame_id (get_current_frame ());
   if ((ecs->event_thread->stop_pc () == stop_pc_sal.pc)
       && (ecs->event_thread->current_line != stop_pc_sal.line
 	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
@@ -7722,6 +7727,19 @@ process_event_stop_test (struct execution_control_state *ecs)
 			       "it's not the start of a statement");
 	}
     }
+  else if (execution_direction == EXEC_REVERSE
+	  && curr_frame_id != original_frame_id
+	  && original_frame_id.code_addr_p && curr_frame_id.code_addr_p
+	  && original_frame_id.code_addr == curr_frame_id.code_addr)
+    {
+      /* If we enter here, we're leaving a recursive function call.  In this
+	 situation, we shouldn't refresh the step information, because if we
+	 do, we'll lose the frame_id of when we started stepping, and this
+	 will make GDB not know we need to print frame information.  */
+      refresh_step_info = false;
+      infrun_debug_printf ("reverse stepping, left a recursive call, don't "
+			   "update step info so we remember we left a frame");
+    }
 
   /* We aren't done stepping.
 
diff --git a/gdb/testsuite/gdb.reverse/recursion.c b/gdb/testsuite/gdb.reverse/recursion.c
new file mode 100644
index 00000000000..747404ce22c
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/recursion.c
@@ -0,0 +1,38 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 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/>.  */
+
+/* Test GDB's ability to handle recursive functions when executing
+   in reverse.  */
+
+int
+foo (int x) {
+    if (x) return foo(x-1);
+    return 0;
+}
+
+int
+bar(int x){
+    int r = foo(x);
+    return 2*r;
+}
+
+int
+main() {
+    int i = bar(5);
+    int j = foo(5);
+    return 0;			/* END OF MAIN */
+}
diff --git a/gdb/testsuite/gdb.reverse/recursion.exp b/gdb/testsuite/gdb.reverse/recursion.exp
new file mode 100644
index 00000000000..331113bee0a
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/recursion.exp
@@ -0,0 +1,49 @@
+# Copyright 2008-2023 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.  It tests reverse stepping.
+# Lots of code borrowed from "step-test.exp".
+
+#
+# Test step and next in reverse
+#
+
+require supports_reverse
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+runto_main
+
+if [supports_process_record] {
+    # Activate process record/replay
+    gdb_test_no_output "record" "turn on process record"
+}
+
+set end_of_program [gdb_get_line_number "END OF MAIN" "$srcfile"]
+gdb_breakpoint $end_of_program
+gdb_continue_to_breakpoint ".*$srcfile/$end_of_program.*"
+
+## test if GDB can reverse over a recursive program
+gdb_test "reverse-next" ".*int j = foo.*" "Skipping recursion from outside"
+## setup and next over a recursion for inside a recursive call
+repeat_cmd_until "reverse-step" ".*" ".*foo .x=4.*"
+gdb_test "reverse-next" ".*return foo.*" "Skipping recursion from inside"
+gdb_test "reverse-next" ".*foo .x=5.*" "print frame when stepping out"
+gdb_test "reverse-next" ".*bar .x=5.*" "stepping into a different function"
+gdb_test "reverse-next" "main .. at .*" "stepping back to main"
-- 
2.41.0

Re: [PATCH] gdb/record: print frame information when exiting a recursive call

[Kevin Buettner]

Hi Guinevere,

Just a few nits.  See below...

On Sat, 23 Sep 2023 12:34:58 +0200
Guinevere Larsen via Gdb-patches <gdb-patches@sourceware.org> wrote:

> Currently,  when GDB is reverse stepping out of a function into the same
> function due to a recursive call, it doesn't print frame information, as
> reported by PR record/29178. This happens because when the inferior
> leaves the current frame, GDB decides to refresh the step information,
> clobbering the original step_frame_id, making it impossible to figure
> out later on that the frame has been changed.
> 
> This commit changes GDB so that, if we notice we're in this exact
> situation, we won't refresh the step information.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29178
> ---
>  gdb/infrun.c                            | 18 +++++++++
>  gdb/testsuite/gdb.reverse/recursion.c   | 38 +++++++++++++++++++
>  gdb/testsuite/gdb.reverse/recursion.exp | 49 +++++++++++++++++++++++++
>  3 files changed, 105 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.reverse/recursion.c
>  create mode 100644 gdb/testsuite/gdb.reverse/recursion.exp
> 
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 4730d290442..00e6215ebc8 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -7679,6 +7679,11 @@ process_event_stop_test (struct execution_control_state *ecs)
>      }
>  
>    bool refresh_step_info = true;
> +
> +  /* shorthand to make if statements smaller.  */

Capitalize "shorthand".

> +  struct frame_id original_frame_id
> +    = ecs->event_thread->control.step_frame_id;
> +  struct frame_id curr_frame_id = get_frame_id (get_current_frame ());

I think these could be used to simplify at least one other, already
existing, if-statement too.  Perhaps post another patch with that
change?  (Or make it a two-part series with the above addition w/
updates to existing code as part 1.)

>    if ((ecs->event_thread->stop_pc () == stop_pc_sal.pc)
>        && (ecs->event_thread->current_line != stop_pc_sal.line
>  	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
> @@ -7722,6 +7727,19 @@ process_event_stop_test (struct execution_control_state *ecs)
>  			       "it's not the start of a statement");
>  	}
>      }
> +  else if (execution_direction == EXEC_REVERSE
> +	  && curr_frame_id != original_frame_id
> +	  && original_frame_id.code_addr_p && curr_frame_id.code_addr_p
> +	  && original_frame_id.code_addr == curr_frame_id.code_addr)
> +    {
> +      /* If we enter here, we're leaving a recursive function call.  In this
> +	 situation, we shouldn't refresh the step information, because if we
> +	 do, we'll lose the frame_id of when we started stepping, and this
> +	 will make GDB not know we need to print frame information.  */
> +      refresh_step_info = false;
> +      infrun_debug_printf ("reverse stepping, left a recursive call, don't "
> +			   "update step info so we remember we left a frame");
> +    }
>  
>    /* We aren't done stepping.
>  
> diff --git a/gdb/testsuite/gdb.reverse/recursion.c b/gdb/testsuite/gdb.reverse/recursion.c
> new file mode 100644
> index 00000000000..747404ce22c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/recursion.c
> @@ -0,0 +1,38 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2023 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/>.  */
> +
> +/* Test GDB's ability to handle recursive functions when executing
> +   in reverse.  */
> +
> +int
> +foo (int x) {
> +    if (x) return foo(x-1);
> +    return 0;
> +}
> +
> +int
> +bar(int x){
> +    int r = foo(x);
> +    return 2*r;
> +}
> +
> +int
> +main() {
> +    int i = bar(5);
> +    int j = foo(5);
> +    return 0;			/* END OF MAIN */
> +}

Unless there's a good reason not to do so, I'd like to see the above
C code follow the GNU coding standard.

> diff --git a/gdb/testsuite/gdb.reverse/recursion.exp b/gdb/testsuite/gdb.reverse/recursion.exp
> new file mode 100644
> index 00000000000..331113bee0a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/recursion.exp
> @@ -0,0 +1,49 @@
> +# Copyright 2008-2023 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.  It tests reverse stepping.
> +# Lots of code borrowed from "step-test.exp".
> +
> +#
> +# Test step and next in reverse
> +#
> +
> +require supports_reverse
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> +    return -1
> +}
> +
> +runto_main
> +
> +if [supports_process_record] {
> +    # Activate process record/replay
> +    gdb_test_no_output "record" "turn on process record"
> +}
> +
> +set end_of_program [gdb_get_line_number "END OF MAIN" "$srcfile"]
> +gdb_breakpoint $end_of_program
> +gdb_continue_to_breakpoint ".*$srcfile/$end_of_program.*"
> +
> +## test if GDB can reverse over a recursive program
> +gdb_test "reverse-next" ".*int j = foo.*" "Skipping recursion from outside"
> +## setup and next over a recursion for inside a recursive call
> +repeat_cmd_until "reverse-step" ".*" ".*foo .x=4.*"
> +gdb_test "reverse-next" ".*return foo.*" "Skipping recursion from inside"
> +gdb_test "reverse-next" ".*foo .x=5.*" "print frame when stepping out"
> +gdb_test "reverse-next" ".*bar .x=5.*" "stepping into a different function"
> +gdb_test "reverse-next" "main .. at .*" "stepping back to main"
> -- 
> 2.41.0
> 

Re: [PATCH] gdb/record: print frame information when exiting a recursive call

[Guinevere Larsen]

On 23/09/2023 23:56, Kevin Buettner wrote:
> Hi Guinevere,
>
> Just a few nits.  See below...

Hi Kevin!

Thanks for taking a look, I'll fix all the nits for v2.

>
> On Sat, 23 Sep 2023 12:34:58 +0200
> Guinevere Larsen via Gdb-patches <gdb-patches@sourceware.org> wrote:
>
>> Currently,  when GDB is reverse stepping out of a function into the same
>> function due to a recursive call, it doesn't print frame information, as
>> reported by PR record/29178. This happens because when the inferior
>> leaves the current frame, GDB decides to refresh the step information,
>> clobbering the original step_frame_id, making it impossible to figure
>> out later on that the frame has been changed.
>>
>> This commit changes GDB so that, if we notice we're in this exact
>> situation, we won't refresh the step information.
>>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29178
>> ---
>>   gdb/infrun.c                            | 18 +++++++++
>>   gdb/testsuite/gdb.reverse/recursion.c   | 38 +++++++++++++++++++
>>   gdb/testsuite/gdb.reverse/recursion.exp | 49 +++++++++++++++++++++++++
>>   3 files changed, 105 insertions(+)
>>   create mode 100644 gdb/testsuite/gdb.reverse/recursion.c
>>   create mode 100644 gdb/testsuite/gdb.reverse/recursion.exp
>>
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index 4730d290442..00e6215ebc8 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -7679,6 +7679,11 @@ process_event_stop_test (struct execution_control_state *ecs)
>>       }
>>   
>>     bool refresh_step_info = true;
>> +
>> +  /* shorthand to make if statements smaller.  */
> Capitalize "shorthand".
>
>> +  struct frame_id original_frame_id
>> +    = ecs->event_thread->control.step_frame_id;
>> +  struct frame_id curr_frame_id = get_frame_id (get_current_frame ());
> I think these could be used to simplify at least one other, already
> existing, if-statement too.  Perhaps post another patch with that
> change?  (Or make it a two-part series with the above addition w/
> updates to existing code as part 1.)
Yeah, the if statement right above this one can be simplified quite a 
bit, but I was worried about ballooning too much. I'll send it 
separately for v2.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
>>     if ((ecs->event_thread->stop_pc () == stop_pc_sal.pc)
>>         && (ecs->event_thread->current_line != stop_pc_sal.line
>>   	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
>> @@ -7722,6 +7727,19 @@ process_event_stop_test (struct execution_control_state *ecs)
>>   			       "it's not the start of a statement");
>>   	}
>>       }
>> +  else if (execution_direction == EXEC_REVERSE
>> +	  && curr_frame_id != original_frame_id
>> +	  && original_frame_id.code_addr_p && curr_frame_id.code_addr_p
>> +	  && original_frame_id.code_addr == curr_frame_id.code_addr)
>> +    {
>> +      /* If we enter here, we're leaving a recursive function call.  In this
>> +	 situation, we shouldn't refresh the step information, because if we
>> +	 do, we'll lose the frame_id of when we started stepping, and this
>> +	 will make GDB not know we need to print frame information.  */
>> +      refresh_step_info = false;
>> +      infrun_debug_printf ("reverse stepping, left a recursive call, don't "
>> +			   "update step info so we remember we left a frame");
>> +    }
>>   
>>     /* We aren't done stepping.
>>   
>> diff --git a/gdb/testsuite/gdb.reverse/recursion.c b/gdb/testsuite/gdb.reverse/recursion.c
>> new file mode 100644
>> index 00000000000..747404ce22c
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.reverse/recursion.c
>> @@ -0,0 +1,38 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> +   Copyright 2023 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/>.  */
>> +
>> +/* Test GDB's ability to handle recursive functions when executing
>> +   in reverse.  */
>> +
>> +int
>> +foo (int x) {
>> +    if (x) return foo(x-1);
>> +    return 0;
>> +}
>> +
>> +int
>> +bar(int x){
>> +    int r = foo(x);
>> +    return 2*r;
>> +}
>> +
>> +int
>> +main() {
>> +    int i = bar(5);
>> +    int j = foo(5);
>> +    return 0;			/* END OF MAIN */
>> +}
> Unless there's a good reason not to do so, I'd like to see the above
> C code follow the GNU coding standard.
>
>> diff --git a/gdb/testsuite/gdb.reverse/recursion.exp b/gdb/testsuite/gdb.reverse/recursion.exp
>> new file mode 100644
>> index 00000000000..331113bee0a
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.reverse/recursion.exp
>> @@ -0,0 +1,49 @@
>> +# Copyright 2008-2023 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.  It tests reverse stepping.
>> +# Lots of code borrowed from "step-test.exp".
>> +
>> +#
>> +# Test step and next in reverse
>> +#
>> +
>> +require supports_reverse
>> +
>> +standard_testfile
>> +
>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
>> +    return -1
>> +}
>> +
>> +runto_main
>> +
>> +if [supports_process_record] {
>> +    # Activate process record/replay
>> +    gdb_test_no_output "record" "turn on process record"
>> +}
>> +
>> +set end_of_program [gdb_get_line_number "END OF MAIN" "$srcfile"]
>> +gdb_breakpoint $end_of_program
>> +gdb_continue_to_breakpoint ".*$srcfile/$end_of_program.*"
>> +
>> +## test if GDB can reverse over a recursive program
>> +gdb_test "reverse-next" ".*int j = foo.*" "Skipping recursion from outside"
>> +## setup and next over a recursion for inside a recursive call
>> +repeat_cmd_until "reverse-step" ".*" ".*foo .x=4.*"
>> +gdb_test "reverse-next" ".*return foo.*" "Skipping recursion from inside"
>> +gdb_test "reverse-next" ".*foo .x=5.*" "print frame when stepping out"
>> +gdb_test "reverse-next" ".*bar .x=5.*" "stepping into a different function"
>> +gdb_test "reverse-next" "main .. at .*" "stepping back to main"
>> -- 
>> 2.41.0
>>

[PATCH v2 0/2] Improving frame printing with recursive

[Guinevere Larsen]

This started with the first patch, just fixing PR record/29178. However,
Kevin pointed out that I could reuse some of the local variables to make
the function as a whole simpler, which I did on patch 2.

Guinevere Larsen (2):
  gdb/record: print frame information when exiting a recursive call
  gdb/infrun: simplify process_event_stop_test

 gdb/infrun.c                            | 36 ++++++++++++------
 gdb/testsuite/gdb.reverse/recursion.c   | 44 ++++++++++++++++++++++
 gdb/testsuite/gdb.reverse/recursion.exp | 49 +++++++++++++++++++++++++
 3 files changed, 117 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.reverse/recursion.c
 create mode 100644 gdb/testsuite/gdb.reverse/recursion.exp

-- 
2.41.0

[PATCH v2 1/2] gdb/record: print frame information when exiting a recursive call

[Guinevere Larsen]

Currently,  when GDB is reverse stepping out of a function into the same
function due to a recursive call, it doesn't print frame information, as
reported by PR record/29178. This happens because when the inferior
leaves the current frame, GDB decides to refresh the step information,
clobbering the original step_frame_id, making it impossible to figure
out later on that the frame has been changed.

This commit changes GDB so that, if we notice we're in this exact
situation, we won't refresh the step information.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29178
---
 gdb/infrun.c                            | 18 +++++++++
 gdb/testsuite/gdb.reverse/recursion.c   | 44 ++++++++++++++++++++++
 gdb/testsuite/gdb.reverse/recursion.exp | 49 +++++++++++++++++++++++++
 3 files changed, 111 insertions(+)
 create mode 100644 gdb/testsuite/gdb.reverse/recursion.c
 create mode 100644 gdb/testsuite/gdb.reverse/recursion.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4730d290442..3491327422d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6923,6 +6923,11 @@ process_event_stop_test (struct execution_control_state *ecs)
   frame = get_current_frame ();
   gdbarch = get_frame_arch (frame);
 
+  /* Shorthand to make if statements smaller.  */
+  struct frame_id original_frame_id
+    = ecs->event_thread->control.step_frame_id;
+  struct frame_id curr_frame_id = get_frame_id (get_current_frame ());
+
   switch (what.main_action)
     {
     case BPSTAT_WHAT_SET_LONGJMP_RESUME:
@@ -7722,6 +7727,19 @@ process_event_stop_test (struct execution_control_state *ecs)
 			       "it's not the start of a statement");
 	}
     }
+  else if (execution_direction == EXEC_REVERSE
+	  && curr_frame_id != original_frame_id
+	  && original_frame_id.code_addr_p && curr_frame_id.code_addr_p
+	  && original_frame_id.code_addr == curr_frame_id.code_addr)
+    {
+      /* If we enter here, we're leaving a recursive function call.  In this
+	 situation, we shouldn't refresh the step information, because if we
+	 do, we'll lose the frame_id of when we started stepping, and this
+	 will make GDB not know we need to print frame information.  */
+      refresh_step_info = false;
+      infrun_debug_printf ("reverse stepping, left a recursive call, don't "
+			   "update step info so we remember we left a frame");
+    }
 
   /* We aren't done stepping.
 
diff --git a/gdb/testsuite/gdb.reverse/recursion.c b/gdb/testsuite/gdb.reverse/recursion.c
new file mode 100644
index 00000000000..480362dd9ad
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/recursion.c
@@ -0,0 +1,44 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 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/>.  */
+
+/* Test GDB's ability to handle recursive functions when executing
+   in reverse.  */
+
+/* The recursive foo call must be the first line of the recursive
+   function, to test that we're not stepping too much and skipping
+   multiple calls when we should skip only one.  */
+int
+foo (int x)
+{
+  if (x) return foo(x-1);
+  return 0;
+}
+
+int
+bar (int x)
+{
+  int r = foo(x);
+  return 2*r;
+}
+
+int
+main ()
+{
+  int i = bar(5);
+  int j = foo(5);
+  return 0;			/* END OF MAIN */
+}
diff --git a/gdb/testsuite/gdb.reverse/recursion.exp b/gdb/testsuite/gdb.reverse/recursion.exp
new file mode 100644
index 00000000000..331113bee0a
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/recursion.exp
@@ -0,0 +1,49 @@
+# Copyright 2008-2023 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.  It tests reverse stepping.
+# Lots of code borrowed from "step-test.exp".
+
+#
+# Test step and next in reverse
+#
+
+require supports_reverse
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+runto_main
+
+if [supports_process_record] {
+    # Activate process record/replay
+    gdb_test_no_output "record" "turn on process record"
+}
+
+set end_of_program [gdb_get_line_number "END OF MAIN" "$srcfile"]
+gdb_breakpoint $end_of_program
+gdb_continue_to_breakpoint ".*$srcfile/$end_of_program.*"
+
+## test if GDB can reverse over a recursive program
+gdb_test "reverse-next" ".*int j = foo.*" "Skipping recursion from outside"
+## setup and next over a recursion for inside a recursive call
+repeat_cmd_until "reverse-step" ".*" ".*foo .x=4.*"
+gdb_test "reverse-next" ".*return foo.*" "Skipping recursion from inside"
+gdb_test "reverse-next" ".*foo .x=5.*" "print frame when stepping out"
+gdb_test "reverse-next" ".*bar .x=5.*" "stepping into a different function"
+gdb_test "reverse-next" "main .. at .*" "stepping back to main"
-- 
2.41.0

[PATCH v2 2/2] gdb/infrun: simplify process_event_stop_test

[Guinevere Larsen]

The previous commit introduced some local variables to make some if
statements simpler. This commit uses them more liberally throughout the
process_event_stop_test in order to simplify the function a little more.
No functional changes are expected.
---
 gdb/infrun.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3491327422d..71e52e230d5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7014,9 +7014,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 
 	if (init_frame)
 	  {
-	    struct frame_id current_id
-	      = get_frame_id (get_current_frame ());
-	    if (current_id == ecs->event_thread->initiating_frame)
+	    if (curr_frame_id == ecs->event_thread->initiating_frame)
 	      {
 		/* Case 2.  Fall through.  */
 	      }
@@ -7189,7 +7187,7 @@ process_event_stop_test (struct execution_control_state *ecs)
   if (pc_in_thread_step_range (ecs->event_thread->stop_pc (),
 			       ecs->event_thread)
       && (execution_direction != EXEC_REVERSE
-	  || get_frame_id (frame) == ecs->event_thread->control.step_frame_id))
+	  || curr_frame_id == original_frame_id))
     {
       infrun_debug_printf
 	("stepping inside range [%s-%s]",
@@ -7619,8 +7617,7 @@ process_event_stop_test (struct execution_control_state *ecs)
      frame machinery detected some skipped call sites, we have entered
      a new inline function.  */
 
-  if ((get_frame_id (get_current_frame ())
-       == ecs->event_thread->control.step_frame_id)
+  if ((curr_frame_id == original_frame_id)
       && inline_skipped_frames (ecs->event_thread))
     {
       infrun_debug_printf ("stepped into inlined function");
@@ -7668,10 +7665,8 @@ process_event_stop_test (struct execution_control_state *ecs)
      through a more inlined call beyond its call site.  */
 
   if (get_frame_type (get_current_frame ()) == INLINE_FRAME
-      && (get_frame_id (get_current_frame ())
-	  != ecs->event_thread->control.step_frame_id)
-      && stepped_in_from (get_current_frame (),
-			  ecs->event_thread->control.step_frame_id))
+      && (curr_frame_id != original_frame_id)
+      && stepped_in_from (get_current_frame (), original_frame_id))
     {
       infrun_debug_printf ("stepping through inlined function");
 
@@ -7701,8 +7696,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 	  end_stepping_range (ecs);
 	  return;
 	}
-      else if (get_frame_id (get_current_frame ())
-	       == ecs->event_thread->control.step_frame_id)
+      else if (curr_frame_id == original_frame_id)
 	{
 	  /* We are not at the start of a statement, and we have not changed
 	     frame.
-- 
2.41.0

Re: [PATCH v2 1/2] gdb/record: print frame information when exiting a recursive call

[Kevin Buettner]

On Sun, 24 Sep 2023 14:58:55 +0200
Guinevere Larsen <blarsen@redhat.com> wrote:

> +int
> +foo (int x)
> +{
> +  if (x) return foo(x-1);

Add space between 'foo' and left paren.

> +  return 0;
> +}
> +
> +int
> +bar (int x)
> +{
> +  int r = foo(x);

Likewise.

> +  return 2*r;
> +}
> +
> +int
> +main ()
> +{
> +  int i = bar(5);
> +  int j = foo(5);

Likewise for both bar and foo above.

> +  return 0;			/* END OF MAIN */
> +}