RE: [PATCH 2/2 v5] Fix reverse stepping multiple contiguous PC ranges over the line table.

[Carl Love <cel@us.ibm.com>]

Sorry, ignore this message.  Accidentally hit send before I was done
with the message.

            Carl 
--------------------------------------------------


On Thu, 2023-06-22 at 13:38 -0700, Carl Love wrote:
> Simon:
> 
> On Mon, 2023-06-19 at 13:58 -0400, Simon Marchi wrote:
> <snip>
> 
> 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);
> > > +
> > 
> > This forward-declaration is not needed.
> 
> I tried removing the forward-declaration and the compile fails with
> the
> message:
> 
>    ../../binutils-gdb-reverse-multiple-
> contiguous/gdb/infrun.c:6773:1:
>    error: no previous declaration for ‘CORE_ADDR
>    update_line_range_start(CORE_ADDR, execution_control_state*)’ [-
>    Werror=missing-declarations]
>     6773 | update_line_range_start (CORE_ADDR pc, struct
>    execution_control_state *ecs)
>          | ^~~~~~~~~~~~~~~~~~~~~~~
>    cc1plus: all warnings being treated as errors
>    make[2]: *** [Makefile:1922: infrun.o] Error 1
>    make[2]: Leaving directory '/home/carll/GDB/build-reverse-
> multiple-
>    contiguous/gdb'
>    make[1]: *** [Makefile:13569: all-gdb] Error 2
>    make[1]: Leaving directory '/home/carll/GDB/build-reverse-
> multiple-
>    contiguous'
>    make: *** [Makefile:1005: all] Error 2
> 
> Leaving the forward declaration in the code.
> 
>    > 
> > >  /* 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)
> > 
> > Please add a comment for the function.
> 
> Done.
> 
> > > +{
> > > +  /* 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 the smallest address in
> > > the
> > > +     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;
> > 
> > When I read this, I wonder: why was control.step_range_start not
> > set
> > to
> > the "real" range start in the first place (not only in the context
> > of
> > reverse execution, every time it is set)?  It would seem more
> > robust
> > than patching it afterwards in some very specific spots.
> > 
> > I could see some benefits for range-stepping uses cases too
> > (relevant
> > when debugging remotely).  Using your example here:
> > 
> >    Line X - [0x0 - 0x8]
> >    Line X - [0x8 - 0x10]
> >    Line X - [0x10 - 0x18]
> > 
> > Imagine we are stopped at 0x14, and we type "next", and 0x14 is a
> > conditional jump to 0x5.  It seems like current GDB would send a
> > "range
> > step" request to GDBserver, to step in the [0x10, 0x18[
> > range.  When
> > reaching 0x5, execution would stop, and GDB would resume it again
> > with
> > the [0x0,0x8[ range.  When reaching 0x8, it would stop again, GDB
> > would
> > resume it with [0x8,0x10[, and so on.  If GDB could send a "range
> > step"
> > request with the [0x0,0x18[ range, it would avoid those unnecessary
> > intermediary stop.
> > 
> > > +}
> > > +
> > >  /* 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);
> > 
> > Unnecessary parenthesis.
> 
> Removed unnecessary parenthesis.
> 
> > > +}
> > > +
> > > +/* 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;
> > 
> > Algorithmic complexity question: given that line tables are sorted
> > by
> > address, would it work to start at the current line table item, and
> > go
> > look at the previous ones until we find one that is no longer
> > contiguous and same line?  find_pc_line is somewhat heavy, so if we
> > don't need to do it repeatedly...
> > 
> > > +}
> > > +
> > >  /* 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.
> > 
> > I think that putting this example in the comment is great.  It
> > makes
> > it
> > much more obvious what the function specifically does.
> > 
> > > +*/
> > > +
> > > +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..da944874e86
> > > --- /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
> > main (void)
> > 
> 
> Fixed.
> 
> > > +{
> > > +  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..89e226b0f84
> > > --- /dev/null
> > > +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp
> > > @@ -0,0 +1,140 @@
> > > +# 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.
> > > +
> > > +proc run_tests {} {
> > > +    global srcfile
> > > +    global executable
> > > +
> > > +    runto_main
> > 
> > We typically check for runto_main's success:
> 
> Fixed two instances of runto_main in this test case and two in the
> other test case.
> 
> >   if { ![runto_main] } {
> >       return
> >   }
> > 
> > runto_main logs a FAIL on failure.  There are a few runto_mains in
> > the
> > patch.
> > 
> > > +    set target_remote [gdb_is_target_remote]
> > 
> > target_remote seems unused
> 
> Removed.
> 
> > > +
> > > +    with_test_prefix "test1" {
> > > +	gdb_test_no_output "record" "turn on process record"
> > > +    }
> > 
> > with_test_prefix with a single test in it is really just the same
> > as:
> > 
> >   gdb_test_no_output "record" "test1: turn on process record"
> > 
> > In fact, you have some other tests with the "test1:" or "test2:"
> > prefix,
> > I think they should be moved to the with_test_prefix.  And maybe
> > use
> > "next" and "step" instead of "test1" and "test2".
> 
> Yup, cleaner to have the with_test_prefix cover the whole test. 
> Changed test1 to next-test and test2 to step-next.
> 
> > > +
> > > +    # 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 \
> > > +	"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 \\(\\);.*" \
> > > +	"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;.*" \
> > > +	"test1: reverse-stepi to previous line b = 2"
> > > +
> > > +
> > > +    # Setup for test 2
> > > +    clean_restart $executable
> > > +    runto_main
> > > +
> > > +    with_test_prefix "test2" {
> > > +	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 \
> > > +	"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.*" \
> > > +	"test2: reverse-step into func2 "
> > > +
> > > +    # The second reverse step should take us into func1 ().
> > > +    gdb_test "reverse-step" ".*END FUNC1.*" \
> > > +	"test2: reverse-step into func1 "
> > > +
> > > +    # The third reverse step should take us call of func1 ().
> > > +    gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
> > > +	"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;.*" \
> > > +	"test2: reverse-stepi to line b = 2 "
> > > +}
> > > +
> > > +set srcfile  func-map-to-same-line.c
> > > +set executable func-map-to-same-line
> > 
> > Wondering if this test should use standard_testfile (like almost
> > every
> > other tests) to set these.
> 
> OK, changed to use the standard_testfile.
> 
> > > +
> > > +# test with and without gcc column info enabled
> > > +foreach_with_prefix with_column_info {yes no} {
> > > +    if {$with_column_info == "yes"} {
> > > +	set options [list debug column-info]
> > > +    } else {
> > > +	set options [list debug no-column-info]
> > > +    }
> > 
> > I didn't think of this when proposing the foreach_with_prefix, but
> > you
> > could perhaps use:
> > 
> >   foreach_with_prefix column_info_flag {column-info no-column-info}
> > 
> > ... to avoid this boilerplate.  You can then use $column_info_flag
> > directly when setting options.
> > 
> OK, that cleans things up a bit.  Changed.
> +
> +    if {[build_executable "failed to prepare" $executable $srcfile
> \
> +	     $options] == -1} {
> +	return -1
> +    }
> +
> +    clean_restart $executable
> 
> clean_restart can go in run_tests.
> 
> 
> +    run_tests
> +}
> 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.
> 
> Just wondering if the copyright years are right.
> 
> New files so yea, should just start with 2023.
> +
> +   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 ()
> 
> void in the parenthesis
> 
> Fixed in both test files.
> 
> +{     /* 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..16a359d90ec
> --- /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.
> +load_lib dwarf.exp
> +require dwarf2_support
> +
> +# The DWARF assembler requires the gcc compiler.
> +require is_c_compiler_gcc
> +
> +# This test suitable only for process that can do reverse
> execution
> +require 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"
> +}
> 
> It seems like the step and next tests are identical, so I guess it
> could
> be factored out using:
> 
>   foreach_with_prefix method {step next} {
>       ...
>   }
> 
> ?
> 
> Yup, redid the code using the foreach_with_prefix.
> 
> Simon
> > yR