Re: [PATCH v3] Fix reverse stepping multiple contiguous PC ranges over the line table.

[Luis Machado <luis.machado@arm.com>]

On 5/4/23 16:59, Carl Love wrote:
> 
> Bruno, GDB maintainers:
f>
> Version 3, added the gcc version check as discussed further from
> version 2 of the patch.  Also updated the tests to check for supporting
> reverse execution rather than requiring recording.  I also noticed
> there were a couple more instances of a requirement check, i.e. if []
> which I changed to "require" per the current style for checking on the
> test requirements.
> 
> 
> The following patch fixes issues on PowerPC with the reverse-step and
> reverse-next instructions when there are multiple assignment statements
> on the same line and when there are multiple function calls on the same
> line. The commit log below discusses these issues in further depth.
> The discussion included what the correct operation should be for these
> commands based on the GDB documentation.  The proposed patch at that
> time changed how the commands worked on other platforms such as X86 in
> a way they no longer matched the documentation.
> 
> The issue is the line table contains multiple entries for the same
> source line.  The patch adds a function to search the line table to
> find the address of the first instruction of a line.  When setup up the
> reverse stepping range, the function is called to make sure the start
> of the range corresponds to the address of the first instruction for
> the line.  This approach was used.  When Luis initially developed the
> patch, he considered merging the contiguous ranges in the line table
> when reading the line tables. He decided it was better to work with the
> data directly in the line table rather than creating and using a
> modified version of the line table.
> 
> The following patch fixes the execution of the reveres-step and
> reverse-next commands for both senarios of multiple statements on the
> same line for PowerPC and aarch64-linux.  Unlike the previous patch, it
> does not change the operation of the commands on other platforms, i.e.
> X86.  The patch adds new test cases for both scenarios to verify they
> work correctly.
> 
> The patch has been tested on PowerPC, Intel X86 and aarch64-linux with
> no new regression failures.
> 
> Please let me know if the patch is acceptable for mainline.  Thanks.
> 
>                     Carl
> 
> ---------------------------------------------------------------
> Fix reverse stepping multiple contiguous PC ranges over the line table.
> 
> There are a couple of scenarios where the GDB reverse-step and reverse-next
> commands do not work correctly.
> 
> Scenario 1 issue description by Luis Machado:
> 
> When running GDB's testsuite on aarch64-linux/Ubuntu 20.04 (also spotted on
> the ppc backend), I noticed some failures in gdb.reverse/solib-precsave.exp
> and gdb.reverse/solib-reverse.exp.
> 
> The failure happens around the following code:
> 
> 38  b[1] = shr2(17);          /* middle part two */
> 40  b[0] = 6;   b[1] = 9;     /* generic statement, end part two */
> 42  shr1 ("message 1\n");     /* shr1 one */
> 
> Normal execution:
> 
> - step from line 38 will land on line 40.
> - step from line 40 will land on line 42.
> 
> Reverse execution:
> - step from line 42 will land on line 40.
> - step from line 40 will land on line 40.
> - step from line 40 will land on line 38.
> 
> The problem here is that line 40 contains two contiguous but distinct
> PC ranges in the line table, like so:
> 
> Line 40 - [0x7ec ~ 0x7f4]
> Line 40 - [0x7f4 ~ 0x7fc]
> 
> The two distinct ranges are generated because GCC started outputting source
> column information, which GDB doesn't take into account at the moment.
> 
> When stepping forward from line 40, we skip both of these ranges and land on
> line 42. When stepping backward from line 42, we stop at the start PC of the
> second (or first, going backwards) range of line 40.
> 
> Since we've reached ecs->event_thread->control.step_range_start, we stop
> stepping backwards.
> 
> ---------------------------------------------------------
> 
> Scenario 2 issue described by Pedro Alves:
> 
> The following explanation of the issue was taken from the gdb mailing list
> discussion of the withdrawn patch to change the behavior of the reverse-step
> and reverse-next commands.  Specifically, message from Pedro Alves
> <pedro@palves.net> where he demonstrates the issue where you have multiple
> function calls on the same source code line:
> 
> https://sourceware.org/pipermail/gdb-patches/2023-January/196110.html
> 
> The source line looks like:
> 
>     func1 ();  func2 ();
> 
> so stepping backwards over that line should always stop at the first
> instruction of the line, not in the middle.  Let's simplify this.
> 
> Here's the full source code of my example:
> 
> (gdb) list 1
> 1       void func1 ()
> 2       {
> 3       }
> 4
> 5       void func2 ()
> 6       {
> 7       }
> 8
> 9       int main ()
> 10      {
> 11        func1 (); func2 ();
> 12      }
> 
> Compiled with:
> 
>   $ gcc reverse.c -o reverse -g3 -O0
>   $ gcc -v
>   ...
>   gcc version 11.3.0 (Ubuntu 11.3.0-1ubuntu1~22.04)
> 
> Now let's debug it with target record, using current gdb git master (f3d8ae90b236),
> without your patch:
> 
>   $ gdb ~/reverse
>   GNU gdb (GDB) 14.0.50.20230124-git
>   ...
>   Reading symbols from /home/pedro/reverse...
>   (gdb) start
>   Temporary breakpoint 1 at 0x1147: file reverse.c, line 11.
>   Starting program: /home/pedro/reverse
>   [Thread debugging using libthread_db enabled]
>   Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> 
>   Temporary breakpoint 1, main () at reverse.c:11
>   11        func1 (); func2 ();
>   (gdb) record
> 
>   (gdb) disassemble /s
>   Dump of assembler code for function main:
>   reverse.c:
>   10      {
>      0x000055555555513f <+0>:     endbr64
>      0x0000555555555143 <+4>:     push   %rbp
>      0x0000555555555144 <+5>:     mov    %rsp,%rbp
> 
>   11        func1 (); func2 ();
>   => 0x0000555555555147 <+8>:     mov    $0x0,%eax
>      0x000055555555514c <+13>:    call   0x555555555129 <func1>
>      0x0000555555555151 <+18>:    mov    $0x0,%eax
>      0x0000555555555156 <+23>:    call   0x555555555134 <func2>
>      0x000055555555515b <+28>:    mov    $0x0,%eax
> 
>   12      }
>      0x0000555555555160 <+33>:    pop    %rbp
>      0x0000555555555161 <+34>:    ret
>   End of assembler dump.
> 
>   (gdb) n
>   12      }
> 
> So far so good, a "next" stepped over the whole of line 11 and stopped at line 12.
> 
> Let's confirm where we are now:
> 
>   (gdb) disassemble /s
>   Dump of assembler code for function main:
>   reverse.c:
>   10      {
>      0x000055555555513f <+0>:     endbr64
>      0x0000555555555143 <+4>:     push   %rbp
>      0x0000555555555144 <+5>:     mov    %rsp,%rbp
> 
>   11        func1 (); func2 ();
>      0x0000555555555147 <+8>:     mov    $0x0,%eax
>      0x000055555555514c <+13>:    call   0x555555555129 <func1>
>      0x0000555555555151 <+18>:    mov    $0x0,%eax
>      0x0000555555555156 <+23>:    call   0x555555555134 <func2>
>      0x000055555555515b <+28>:    mov    $0x0,%eax
> 
>   12      }
>   => 0x0000555555555160 <+33>:    pop    %rbp
>      0x0000555555555161 <+34>:    ret
>   End of assembler dump.
> 
> Good, we're at the first instruction of line 12.
> 
> Now let's undo the "next", with "reverse-next":
> 
>   (gdb) reverse-next
>   11        func1 (); func2 ();
> 
> Seemingly stopped at line 11.  Let's see exactly where:
> 
>   (gdb) disassemble /s
>   Dump of assembler code for function main:
>   reverse.c:
>   10      {
>      0x000055555555513f <+0>:     endbr64
>      0x0000555555555143 <+4>:     push   %rbp
>      0x0000555555555144 <+5>:     mov    %rsp,%rbp
> 
>   11        func1 (); func2 ();
>      0x0000555555555147 <+8>:     mov    $0x0,%eax
>      0x000055555555514c <+13>:    call   0x555555555129 <func1>
>   => 0x0000555555555151 <+18>:    mov    $0x0,%eax
>      0x0000555555555156 <+23>:    call   0x555555555134 <func2>
>      0x000055555555515b <+28>:    mov    $0x0,%eax
> 
>   12      }
>      0x0000555555555160 <+33>:    pop    %rbp
>      0x0000555555555161 <+34>:    ret
>   End of assembler dump.
>   (gdb)
> 
> And lo, we stopped in the middle of line 11!  That is a bug, we should have
> stepped back all the way to the beginning of the line.  The "reverse-next"
> should have fully undone the prior "next" command.
> 
> The above issues were fixed by introducing a new function that looks for
> adjacent PC ranges for the same line, until we notice a line change. Then
> we take that as the start PC of the range.  The new start PC for the range
> is used for the control.step_range_start when setting up a step range.
> 
> The test case gdb.reverse/map-to-same-line.exp is added to test the fix
> for the issues in scenario 1.
> 
> The test case gdb.reverse/func-map-to-same-line.exp was added to test the
> fix for scenario 2 when the binary was compiled with and without line
> table information.
> 
> bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28426
> 
> Co-authored-by: Luis Machado <luis.machado@arm.com>
> Co-authored-by: Carl Love <cel@us.ibm.com>
> ---
>   gdb/infrun.c                                  |  57 +++++++
>   gdb/symtab.c                                  |  49 ++++++
>   gdb/symtab.h                                  |  16 ++
>   .../gdb.reverse/func-map-to-same-line.c       |  36 ++++
>   .../gdb.reverse/func-map-to-same-line.exp     | 146 ++++++++++++++++
>   gdb/testsuite/gdb.reverse/map-to-same-line.c  |  58 +++++++
>   .../gdb.reverse/map-to-same-line.exp          | 156 ++++++++++++++++++
>   7 files changed, 518 insertions(+)
>   create mode 100644 gdb/testsuite/gdb.reverse/func-map-to-same-line.c
>   create mode 100644 gdb/testsuite/gdb.reverse/func-map-to-same-line.exp
>   create mode 100644 gdb/testsuite/gdb.reverse/map-to-same-line.c
>   create mode 100644 gdb/testsuite/gdb.reverse/map-to-same-line.exp
> 
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index efe2c00c489..8555e3c979f 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -114,6 +114,9 @@ static struct async_event_handler *infrun_async_inferior_event_token;
>      Starts off as -1, indicating "never enabled/disabled".  */
>   static int infrun_is_async = -1;
>   
> +static CORE_ADDR update_line_range_start (CORE_ADDR pc,
> +					  struct execution_control_state *ecs);
> +
>   /* See infrun.h.  */
>   
>   void
> @@ -6769,6 +6772,25 @@ handle_signal_stop (struct execution_control_state *ecs)
>     process_event_stop_test (ecs);
>   }
>   
> +CORE_ADDR
> +update_line_range_start (CORE_ADDR pc, struct execution_control_state *ecs)
> +{
> +  /* The line table may have multiple entries for the same source code line.
> +     Given the PC, check the line table and return the PC that corresponds
> +     to the line table entry for the source line that PC is in.  */
> +  CORE_ADDR start_line_pc = ecs->event_thread->control.step_range_start;
> +  gdb::optional<CORE_ADDR> real_range_start;
> +
> +  /* Call find_line_range_start to get smallest address in the

s/smallest/the smallest

> +     linetable for multiple Line X entries in the line table.  */
> +  real_range_start = find_line_range_start (pc);
> +
> +  if (real_range_start.has_value ())
> +    start_line_pc = *real_range_start;
> +
> +  return start_line_pc;
> +}
> +
>   /* Come here when we've got some debug event / signal we can explain
>      (IOW, not a random signal), and test whether it should cause a
>      stop, or whether we should resume the inferior (transparently).
> @@ -7570,6 +7592,28 @@ process_event_stop_test (struct execution_control_state *ecs)
>   
>         if (stop_pc_sal.is_stmt)
>   	{
> +	  if (execution_direction == EXEC_REVERSE)
> +	    {
> +	      /* We are stepping backwards make sure we have reached the
> +		 beginning of the line.  */
> +	      CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
> +	      CORE_ADDR start_line_pc
> +		= update_line_range_start (stop_pc, ecs);
> +
> +	      if (stop_pc != start_line_pc)
> +		{
> +		  /* Have not reached the beginning of the source code line.
> +		     Set a step range.  Execution should stop in any function
> +		     calls we execute back into before reaching the beginning
> +		     of the line.  */
> +		  ecs->event_thread->control.step_range_start = start_line_pc;
> +		  ecs->event_thread->control.step_range_end = stop_pc;
> +		  set_step_info (ecs->event_thread, frame, stop_pc_sal);
> +		  keep_going (ecs);
> +		  return;
> +		}
> +	    }
> +
>   	  /* We are at the start of a statement.
>   
>   	     So stop.  Note that we don't stop if we step into the middle of a
> @@ -7632,6 +7676,19 @@ process_event_stop_test (struct execution_control_state *ecs)
>       set_step_info (ecs->event_thread, frame, stop_pc_sal);
>   
>     infrun_debug_printf ("keep going");
> +
> +  if (execution_direction == EXEC_REVERSE)
> +    {
> +      CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
> +
> +      /* Make sure the stop_pc is set to the beginning of the line.  */
> +      if (stop_pc != ecs->event_thread->control.step_range_start)
> +	{
> +	  stop_pc = update_line_range_start (stop_pc, ecs);
> +	  ecs->event_thread->control.step_range_start = stop_pc;
> +	}
> +    }
> +
>     keep_going (ecs);
>   }
>   
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 27611a34ec4..91d35616eb9 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -3282,6 +3282,55 @@ find_pc_line (CORE_ADDR pc, int notcurrent)
>     return sal;
>   }
>   
> +/* Compare two symtab_and_line entries.  Return true if both have
> +   the same line number and the same symtab pointer.  That means we
> +   are dealing with two entries from the same line and from the same
> +   source file.
> +
> +   Return false otherwise.  */
> +
> +static bool
> +sal_line_symtab_matches_p (const symtab_and_line &sal1,
> +			   const symtab_and_line &sal2)
> +{
> +  return (sal1.line == sal2.line && sal1.symtab == sal2.symtab);
> +}
> +
> +/* See symtah.h.  */
> +
> +gdb::optional<CORE_ADDR>
> +find_line_range_start (CORE_ADDR pc)
> +{
> +  struct symtab_and_line current_sal = find_pc_line (pc, 0);
> +
> +  if (current_sal.line == 0)
> +    return {};
> +
> +  struct symtab_and_line prev_sal = find_pc_line (current_sal.pc - 1, 0);
> +
> +  /* If the previous entry is for a different line, that means we are already
> +     at the entry with the start PC for this line.  */
> +  if (!sal_line_symtab_matches_p (prev_sal, current_sal))
> +    return current_sal.pc;
> +
> +  /* Otherwise, keep looking for entries for the same line but with
> +     smaller PC's.  */
> +  bool done = false;
> +  CORE_ADDR prev_pc;
> +  while (!done)
> +    {
> +      prev_pc = prev_sal.pc;
> +
> +      prev_sal = find_pc_line (prev_pc - 1, 0);
> +
> +      /* Did we notice a line change?  If so, we are done with the search.  */
> +      if (!sal_line_symtab_matches_p (prev_sal, current_sal))
> +	done = true;
> +    }
> +
> +  return prev_pc;
> +}
> +
>   /* See symtab.h.  */
>   
>   struct symtab *
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 404d0ab30a8..f54305636da 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -2346,6 +2346,22 @@ extern struct symtab_and_line find_pc_line (CORE_ADDR, int);
>   extern struct symtab_and_line find_pc_sect_line (CORE_ADDR,
>   						 struct obj_section *, int);
>   
> +/* Given PC, and assuming it is part of a range of addresses that is part of a
> +   line, go back through the linetable and find the starting PC of that
> +   line.
> +
> +   For example, suppose we have 3 PC ranges for line X:
> +
> +   Line X - [0x0 - 0x8]
> +   Line X - [0x8 - 0x10]
> +   Line X - [0x10 - 0x18]
> +
> +   If we call the function with PC == 0x14, we want to return 0x0, as that is
> +   the starting PC of line X, and the ranges are contiguous.
> +*/
> +
> +extern gdb::optional<CORE_ADDR> find_line_range_start (CORE_ADDR pc);
> +
>   /* Wrapper around find_pc_line to just return the symtab.  */
>   
>   extern struct symtab *find_pc_line_symtab (CORE_ADDR);
> diff --git a/gdb/testsuite/gdb.reverse/func-map-to-same-line.c b/gdb/testsuite/gdb.reverse/func-map-to-same-line.c
> new file mode 100644
> index 00000000000..412ab180943
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.c
> @@ -0,0 +1,36 @@
> +/* 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 test is used to test the reverse-step and reverse-next instruction
> +   execution for a source line that contains multiple function calls.  */
> +
> +void
> +func1 ()
> +{
> +} // END FUNC1
> +
> +void
> +func2 ()
> +{
> +} // END FUNC2
> +
> +int main ()
> +{
> +  int a, b;
> +  a = 1;
> +  b = 2;
> +  func1 (); func2 ();
> +  a = a + b;     // START REVERSE TEST
> +}
> diff --git a/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp
> new file mode 100644
> index 00000000000..4eae042a6bf
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp
> @@ -0,0 +1,146 @@
> +# 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".
> +
> +# This test checks to make sure there is no regression failures for
> +# the reverse-next command when stepping back over two functions in
> +# the same line.
> +
> +require supports_reverse
> +
> +# This test uses the gcc no-column-info command which was added in gcc 7.1.
> +require get_compiler_info  "gcc-7-*"
> +
> +proc run_tests {msg} {
> +    global srcfile
> +    global executable
> +
> +    runto_main
> +    set target_remote [gdb_is_target_remote]
> +
> +    gdb_test_no_output "record" "turn on process record"
> +
> +    # This regression test verifies the reverse-step and reverse-next commands
> +    # work properly when executing backwards thru a source line containing
> +    # two function calls on the same source line, i.e. func1 (); func2 ();
> +    # This test is compiled so the dwarf info not contain the line table
> +    # information.
> +
> +    # Test 1, reverse-next command
> +    # Set breakpoint at the line after the function calls.
> +    set bp_start_reverse_test [gdb_get_line_number "START REVERSE TEST" \
> +				   $srcfile]
> +    gdb_breakpoint $srcfile:$bp_start_reverse_test temporary
> +
> +    # Continue to break point for reverse-next test.
> +    # Command definition:  reverse-next [count]
> +    #   Run backward to the beginning of the previous line executed in the
> +    #   current (innermost) stack frame. If the line contains function calls,
> +    #   they will be “un-executed” without stopping. Starting from the first
> +    #   line of a function, reverse-next will take you back to the caller of
> +    #   that function, before the function was called, just as the normal next
> +    #   command would take you from the last line of a function back to its
> +    #   return to its caller 2 .
> +    gdb_continue_to_breakpoint \
> +	"$msg: test1: stopped at command reverse-next test start location" \
> +	".*$srcfile:$bp_start_reverse_test\r\n.*"
> +
> +    # The reverse-next should step all the way back to the beginning of the
> +    # line, i.e. at the beginning of the func1 call.
> +    gdb_test "reverse-next" ".*func1 \\(\\); func2 \\(\\);.*" \
> +	"$msg: test1: reverse-next to line with two functions"
> +
> +    # We should be stopped at the first instruction of the line. A reverse-step
> +    # should step back and stop at the beginning of the previous line b = 2,
> +    # i.e. not in func1 ().
> +    gdb_test "reverse-stepi" ".*b = 2;.*" \
> +	"$msg: test1: reverse-stepi to previous line b = 2"
> +
> +
> +    # Setup for test 2
> +    clean_restart $executable
> +    runto_main
> +
> +    gdb_test_no_output "record" "turn on process record"
> +
> +    # Test 2, reverse-step command
> +    # Set breakpoint at the line after the function calls.
> +    gdb_breakpoint $srcfile:$bp_start_reverse_test temporary
> +
> +    #  Continue to the start of the reverse-step test.
> +    #  Command definition:  reverse-step [count]
> +    #    Run the program backward until control reaches the start of a
> +    #    different source line; then stop it, and return control to gdb.
> +    #    Like the step command, reverse-step will only stop at the beginning
> +    #    of a source line. It “un-executes” the previously executed source
> +    #    line. If the previous source line included calls to debuggable
> +    #    functions, reverse-step will step (backward) into the called function,
> +    #    stopping at the beginning of the last statement in the called
> +    #    function (typically a return statement).  Also, as with the step
> +    #    command, if non-debuggable functions are called, reverse-step will
> +    #    run thru them backward without stopping.
> +
> +    gdb_continue_to_breakpoint \
> +	"$msg: test2: stopped at command reverse-step test start location" \
> +	".*$srcfile:$bp_start_reverse_test\r\n.*"
> +
> +    # The first reverse step should take us call of func2 ().
> +    gdb_test "reverse-step" ".*END FUNC2.*" \
> +	"$msg: test2: reverse-step into func2 "
> +
> +    # The second reverse step should take us into func1 ().
> +    gdb_test "reverse-step" ".*END FUNC1.*" \
> +	"$msg: test2: reverse-step into func1 "
> +
> +    # The third reverse step should take us call of func1 ().
> +    gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
> +	"$msg: test2: reverse-step to line func1(); func2(), at call for func1 "
> +
> +    # We should be stopped at the first instruction of the line. A reverse
> +    # stepi should take us to b = 2 ().
> +    gdb_test "reverse-stepi" ".*b = 2;.*" \
> +	"$msg: test2: reverse-stepi to line b = 2 "
> +}
> +
> +set srcfile  func-map-to-same-line.c
> +set executable func-map-to-same-line
> +
> +# test with gcc column info enabled
> +set options [list debug additional_flags=]
> +
> +if {[build_executable "failed to prepare" $executable $srcfile $options] == -1}\
> + {
> +    return -1
> +}
> +
> +clean_restart $executable
> +
> +run_tests {"with-column-info"}
> +
> +
> +#test with gcc column info disabled
> +set options [list debug additional_flags=-gno-column-info]
> +
> +if {[build_executable "failed to prepare" $executable $srcfile $options] == -1}\
> + {
> +    return -1
> +}
> +
> +set $executable executable_without_column_info
> +clean_restart $executable
> +
> +run_tests {"no-column-info"}
> diff --git a/gdb/testsuite/gdb.reverse/map-to-same-line.c b/gdb/testsuite/gdb.reverse/map-to-same-line.c
> new file mode 100644
> index 00000000000..f20d778f40e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/map-to-same-line.c
> @@ -0,0 +1,58 @@
> +/* 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/ >.  */
> +
> +/* The purpose of this test is to create a DWARF line table that contains two
> +   or more entries for the same line.  When stepping (forwards or backwards),
> +   GDB should step over the entire line and not just a particular entry in the
> +   line table.  */
> +
> +int
> +main ()
> +{     /* TAG: main prologue */
> +  asm ("main_label: .globl main_label");
> +  int i = 1, j = 2, k;
> +  float f1 = 2.0, f2 = 4.1, f3;
> +  const char *str_1 = "foo", *str_2 = "bar", *str_3;
> +
> +  asm ("line1: .globl line1");
> +  k = i; f3 = f1; str_3 = str_1;    /* TAG: line 1 */
> +
> +  asm ("line2: .globl line2");
> +  k = j; f3 = f2; str_3 = str_2;    /* TAG: line 2 */
> +
> +  asm ("line3: .globl line3");
> +  k = i; f3 = f1; str_3 = str_1;    /* TAG: line 3 */
> +
> +  asm ("line4: .globl line4");
> +  k = j; f3 = f2; str_3 = str_2;    /* TAG: line 4 */
> +
> +  asm ("line5: .globl line5");
> +  k = i; f3 = f1; str_3 = str_1;    /* TAG: line 5 */
> +
> +  asm ("line6: .globl line6");
> +  k = j; f3 = f2; str_3 = str_2;    /* TAG: line 6 */
> +
> +  asm ("line7: .globl line7");
> +  k = i; f3 = f1; str_3 = str_1;    /* TAG: line 7 */
> +
> +  asm ("line8: .globl line8");
> +  k = j; f3 = f2; str_3 = str_2;    /* TAG: line 8 */
> +
> +  asm ("main_return: .globl main_return");
> +  k = j; f3 = f2; str_3 = str_2;    /* TAG: main return */
> +
> +  asm ("end_of_sequence: .globl end_of_sequence");
> +  return 0; /* TAG: main return */
> +}
> diff --git a/gdb/testsuite/gdb.reverse/map-to-same-line.exp b/gdb/testsuite/gdb.reverse/map-to-same-line.exp
> new file mode 100644
> index 00000000000..02f3f4d8c9c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/map-to-same-line.exp
> @@ -0,0 +1,156 @@
> +# 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/ >.
> +
> +# When stepping (forwards or backwards), GDB should step over the entire line
> +# and not just a particular entry in the line table. This test was added to
> +# verify the find_line_range_start function properly sets the step range for a
> +# line that consists of multiple statements, i.e. multiple entries in the line
> +# table.  This test creates a DWARF line table that contains two entries for
> +# the same line to do the needed testing.
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +require dwarf2_support
> +load_lib dwarf.exp
> +
> +# The DWARF assembler requires the gcc compiler.
> +require is_c_compiler_gcc
> +
> +# This test suitable only for process that can do reverse execution
> +requires supports_reverse
> +
> +standard_testfile .c .S
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
> +    return -1
> +}
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    global srcdir subdir srcfile
> +    declare_labels integer_label L
> +
> +    # Find start address and length of program
> +    lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
> +	main_start main_len
> +    set main_end "$main_start + $main_len"
> +
> +    cu {} {
> +	compile_unit {
> +	    {language @DW_LANG_C}
> +	    {name map-to-same-line.c}
> +	    {stmt_list $L DW_FORM_sec_offset}
> +	    {low_pc 0 addr}
> +	} {
> +	    subprogram {
> +		{external 1 flag}
> +		{name main}
> +		{low_pc $main_start addr}
> +		{high_pc $main_len DW_FORM_data4}
> +	    }
> +	}
> +    }
> +
> +    lines {version 2 default_is_stmt 1} L {
> +	include_dir "${srcdir}/${subdir}"
> +	file_name "$srcfile" 1
> +
> +	# Generate the line table program with distinct source lines being
> +	# mapped to the same line entry. Line 1, 5 and 8 contain 1 statement
> +	# each.  Line 2 contains 2 statements.  Line 3 contains 3 statements.
> +	program {
> +	    DW_LNE_set_address $main_start
> +	    line [gdb_get_line_number "TAG: main prologue"]
> +	    DW_LNS_copy
> +	    DW_LNE_set_address line1
> +	    line [gdb_get_line_number "TAG: line 1" ]
> +	    DW_LNS_copy
> +	    DW_LNE_set_address line2
> +	    line [gdb_get_line_number "TAG: line 2" ]
> +	    DW_LNS_copy
> +	    DW_LNE_set_address line3
> +	    line [gdb_get_line_number "TAG: line 2" ]
> +	    DW_LNS_copy
> +	    DW_LNE_set_address line4
> +	    line [gdb_get_line_number "TAG: line 3" ]
> +	    DW_LNS_copy
> +	    DW_LNE_set_address line5
> +	    line [gdb_get_line_number "TAG: line 3" ]
> +	    DW_LNS_copy
> +	    DW_LNE_set_address line6
> +	    line [gdb_get_line_number "TAG: line 3" ]
> +	    DW_LNS_copy
> +	    DW_LNE_set_address line7
> +	    line [gdb_get_line_number "TAG: line 5" ]
> +	    DW_LNS_copy
> +	    DW_LNE_set_address line8
> +	    line [gdb_get_line_number "TAG: line 8" ]
> +	    DW_LNS_copy
> +	    DW_LNE_set_address main_return
> +	    line [gdb_get_line_number "TAG: main return"]
> +	    DW_LNS_copy
> +	    DW_LNE_set_address end_of_sequence
> +	    DW_LNE_end_sequence
> +	}
> +    }
> +}
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	[list $srcfile $asm_file] {nodebug} ] } {
> +    return -1
> +}
> +
> +runto_main
> +
> +# Print the line table
> +gdb_test_multiple "maint info line-table ${testfile}" "" {
> +    -re "\r\n$decimal\[ \t\]+$decimal\[ \t\]+($hex)\[ \t\]+Y\[^\r\n\]*" {
> +	lappend is_stmt $expect_out(1,string)
> +	exp_continue
> +    }
> +    -re -wrap "" {
> +    }
> +}
> +
> +# Do the reverse-step test
> +gdb_test_no_output "record" "turn on process record"
> +
> +set bp_main_return [gdb_get_line_number "TAG: main return" $srcfile]
> +gdb_breakpoint $srcfile:$bp_main_return
> +gdb_continue_to_breakpoint  "run to end of main, reverse-step test" ".*$srcfile:$bp_main_return.*"
> +gdb_test "display \$pc" ".*pc =.*" "display pc, reverse-step test"
> +
> +# At this point, GDB has already recorded the execution up until the return
> +# statement.  Reverse-step and test if GDB transitions between lines in the
> +# expected order.  It should reverse-step across lines 8, 5, 3, 2 and 1.
> +foreach line {8 5 3 2 1} {
> +    gdb_test "reverse-step" ".*TAG: line $line.*" "reverse step to line $line"
> +}
> +
> +## Clean restart, test reverse-next command
> +clean_restart ${testfile}
> +runto_main
> +gdb_test_no_output "record" "turn on process record, reverst-next test"
> +
> +set bp_main_return [gdb_get_line_number "TAG: main return" $srcfile]
> +gdb_breakpoint $srcfile:$bp_main_return
> +gdb_continue_to_breakpoint  "run to end of main, reverse-next test" ".*$srcfile:$bp_main_return.*"
> +gdb_test "display \$pc" ".*pc =.*" "display pc, reverse-next test"
> +
> +# At this point, GDB has already recorded the execution up until the return
> +# statement.  Reverse-next and test if GDB transitions between lines in the
> +# expected order.  It should reverse-next across lines 8, 5, 3, 2 and 1.
> +foreach line {8 5 3 2 1} {
> +    gdb_test "reverse-next" ".*TAG: line $line.*" "reverse next to line $line"
> +}

Other than the nit, this LGTM. Thanks for picking this one up and improving it to fix other issues.

Reviewed-by: Luis Machado <luis.machado@arm.com>