Fix reverse stepping multiple contiguous PC ranges over the line table.
Commit Message
GDB maintainers:
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.
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. The first scenario consists of multiple
assignment statements on the same line. A patch was proposed to address the
issue by Luis Machado and briefly discussed on the mailing list in Feb 2021.
https://sourceware.org/pipermail/gdb-patches/2021-February/175678.html
The discussion was revived by Carl Love with regards to fixing the same
issue on PowerPC.
https://sourceware.org/pipermail/gdb-patches/2022-March/186463.html
The patch was not completed and has been on Carl's to do list for some time.
Discussion of a patch to change how the reverse-step and reverse-next
commands submitted by Carl Love was started in thread:
https://sourceware.org/pipermail/gdb-patches/2023-January/195563.html
The patch was withdrawn as it was pointed out the proposed patch would
change the intended behavior of the commands as documented in the GDB
manual. However, it was pointed out by Pedro Alves <pedro@palves.net>
that the reverse-step and reverse-next commands do not work when there
are multiple function calls on the same line. This is a second scenario
where the commands do not work correctly.
The following patch is an extended version of the original patch by
Luis Machado to fix the issues in scenario 1 to also address the issues in
scenario 2.
--------------------------------------------------------
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.
This happens because we have this check in infrun.c:process_event_stop_test:
/* When stepping backward, stop at beginning of line range
(unless it's the function entry point, in which case
keep going back to the call point). */
CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
if (stop_pc == ecs->event_thread->control.step_range_start
&& stop_pc != ecs->stop_func_start
&& execution_direction == EXEC_REVERSE)
end_stepping_range (ecs);
else
keep_going (ecs);
Since we've reached ecs->event_thread->control.step_range_start, we stop
stepping backwards.
The right thing to do is to look 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.
Another solution I thought about is to merge the contiguous ranges when
we are reading the line tables. Though I'm not sure if we really want to
process that data as opposed to keeping it as the compiler created, and
then working around that.
The test case gdb.reverse/map-to-same-line.exp is added to test the fix
for the issues in scenario 1.
---------------------------------------------------------
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 test cases gdb.reverse/func-map-to-same-line-no-colum-info.exp and
gdb.reverse/func-map-to-same-line.exp were 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 ++
.../func-map-to-same-line-no-column-info.exp | 135 ++++++++++++++++
.../gdb.reverse/func-map-to-same-line.c | 36 +++++
.../gdb.reverse/func-map-to-same-line.exp | 123 ++++++++++++++
gdb/testsuite/gdb.reverse/map-to-same-line.c | 58 +++++++
.../gdb.reverse/map-to-same-line.exp | 153 ++++++++++++++++++
8 files changed, 627 insertions(+)
create mode 100644 gdb/testsuite/gdb.reverse/func-map-to-same-line-no-column-info.exp
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
Comments
On 27/04/2023 22:59, Carl Love wrote:
> GDB maintainers:
>
> 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.
>
> 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. The first scenario consists of multiple
> assignment statements on the same line. A patch was proposed to address the
> issue by Luis Machado and briefly discussed on the mailing list in Feb 2021.
>
> https://sourceware.org/pipermail/gdb-patches/2021-February/175678.html
>
> The discussion was revived by Carl Love with regards to fixing the same
> issue on PowerPC.
>
> https://sourceware.org/pipermail/gdb-patches/2022-March/186463.html
>
> The patch was not completed and has been on Carl's to do list for some time.
>
> Discussion of a patch to change how the reverse-step and reverse-next
> commands submitted by Carl Love was started in thread:
>
> https://sourceware.org/pipermail/gdb-patches/2023-January/195563.html
>
> The patch was withdrawn as it was pointed out the proposed patch would
> change the intended behavior of the commands as documented in the GDB
> manual. However, it was pointed out by Pedro Alves <pedro@palves.net>
> that the reverse-step and reverse-next commands do not work when there
> are multiple function calls on the same line. This is a second scenario
> where the commands do not work correctly.
>
> The following patch is an extended version of the original patch by
> Luis Machado to fix the issues in scenario 1 to also address the issues in
> scenario 2.
>
> --------------------------------------------------------
>
> 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.
>
> This happens because we have this check in infrun.c:process_event_stop_test:
>
> /* When stepping backward, stop at beginning of line range
> (unless it's the function entry point, in which case
> keep going back to the call point). */
> CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
> if (stop_pc == ecs->event_thread->control.step_range_start
> && stop_pc != ecs->stop_func_start
> && execution_direction == EXEC_REVERSE)
> end_stepping_range (ecs);
> else
> keep_going (ecs);
>
> Since we've reached ecs->event_thread->control.step_range_start, we stop
> stepping backwards.
>
> The right thing to do is to look 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.
>
> Another solution I thought about is to merge the contiguous ranges when
> we are reading the line tables. Though I'm not sure if we really want to
> process that data as opposed to keeping it as the compiler created, and
> then working around that.
>
> The test case gdb.reverse/map-to-same-line.exp is added to test the fix
> for the issues in scenario 1.
>
> ---------------------------------------------------------
>
> 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 test cases gdb.reverse/func-map-to-same-line-no-colum-info.exp and
> gdb.reverse/func-map-to-same-line.exp were 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>
Hey Carl,
Thanks for working on this. I'm wondering which parts will be part of
the final commit messages and which is just context for the mailing
list, so some clarity would be nice, but that is not a huge deal.
I wanted to test this change, but it doesn't apply anymore on master,
and `git apply --3way` can't figure out how to do it. Which commit did
you use as base (or alternatively, can you rebase it)?
Bruno:
On Tue, 2023-05-02 at 16:15 +0200, Bruno Larsen wrote:
I had intended everything from here down as part of the commit message.
It is admittedly a lot. It could be cut down my making the first part
could be moved to the mailing list context. The commit message would
then start with the two scenarios the patch fixes.
>
> > ---------------------------------------------------------------
> > 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. The first scenario consists of
> > multiple
> > assignment statements on the same line. A patch was proposed to
> > address the
> > issue by Luis Machado and briefly discussed on the mailing list in
> > Feb 2021.
> >
> > https://sourceware.org/pipermail/gdb-patches/2021-February/175678.html
> >
> > The discussion was revived by Carl Love with regards to fixing the
> > same
> > issue on PowerPC.
> >
> > https://sourceware.org/pipermail/gdb-patches/2022-March/186463.html
> >
> > The patch was not completed and has been on Carl's to do list for
> > some time.
> >
> > Discussion of a patch to change how the reverse-step and reverse-
> > next
> > commands submitted by Carl Love was started in thread:
> >
> > https://sourceware.org/pipermail/gdb-patches/2023-January/195563.html
> >
> > The patch was withdrawn as it was pointed out the proposed patch
> > would
> > change the intended behavior of the commands as documented in the
> > GDB
> > manual. However, it was pointed out by Pedro Alves <
> > pedro@palves.net>
> > that the reverse-step and reverse-next commands do not work when
> > there
> > are multiple function calls on the same line. This is a second
> > scenario
> > where the commands do not work correctly.
> >
> > The following patch is an extended version of the original patch by
> > Luis Machado to fix the issues in scenario 1 to also address the
> > issues in
> > scenario 2.
> >
Move the above to the mailing list
Start here with the commit message
> > --------------------------------------------------------
> >
> > 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.
> >
> > This happens because we have this check in
> > infrun.c:process_event_stop_test:
> >
> > /* When stepping backward, stop at beginning of line range
> > (unless it's the function entry point, in which case
> > keep going back to the call point). */
> > CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
> > if (stop_pc == ecs->event_thread->control.step_range_start
> > && stop_pc != ecs->stop_func_start
> > && execution_direction == EXEC_REVERSE)
> > end_stepping_range (ecs);
> > else
> > keep_going (ecs);
> >
> > Since we've reached ecs->event_thread->control.step_range_start, we
> > stop
> > stepping backwards.
> >
> > The right thing to do is to look 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.
> >
> > Another solution I thought about is to merge the contiguous ranges
> > when
> > we are reading the line tables. Though I'm not sure if we really
> > want to
> > process that data as opposed to keeping it as the compiler created,
> > and
> > then working around that.
> >
> > The test case gdb.reverse/map-to-same-line.exp is added to test the
> > fix
> > for the issues in scenario 1.
> >
> > ---------------------------------------------------------
> >
> > 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 test cases gdb.reverse/func-map-to-same-line-no-colum-info.exp
> > and
> > gdb.reverse/func-map-to-same-line.exp were 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>
End of commit message
>
> Hey Carl,
>
> Thanks for working on this. I'm wondering which parts will be part
> of
> the final commit messages and which is just context for the mailing
> list, so some clarity would be nice, but that is not a huge deal.
>
> I wanted to test this change, but it doesn't apply anymore on
> master,
> and `git apply --3way` can't figure out how to do it. Which commit
> did
> you use as base (or alternatively, can you rebase it)?
OK, let me rebase it and repost. Let me know if you think the
suggested commit message is still too much. Thanks.
Carl
>
On 02/05/2023 17:40, Carl Love wrote:
> Bruno:
>
> On Tue, 2023-05-02 at 16:15 +0200, Bruno Larsen wrote:
>
> I had intended everything from here down as part of the commit message.
> It is admittedly a lot. It could be cut down my making the first part
> could be moved to the mailing list context. The commit message would
> then start with the two scenarios the patch fixes.
>>> ---------------------------------------------------------------
>>> 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. The first scenario consists of
>>> multiple
>>> assignment statements on the same line. A patch was proposed to
>>> address the
>>> issue by Luis Machado and briefly discussed on the mailing list in
>>> Feb 2021.
>>>
>>> https://sourceware.org/pipermail/gdb-patches/2021-February/175678.html
>>>
>>> The discussion was revived by Carl Love with regards to fixing the
>>> same
>>> issue on PowerPC.
>>>
>>> https://sourceware.org/pipermail/gdb-patches/2022-March/186463.html
>>>
>>> The patch was not completed and has been on Carl's to do list for
>>> some time.
>>>
>>> Discussion of a patch to change how the reverse-step and reverse-
>>> next
>>> commands submitted by Carl Love was started in thread:
>>>
>>> https://sourceware.org/pipermail/gdb-patches/2023-January/195563.html
>>>
>>> The patch was withdrawn as it was pointed out the proposed patch
>>> would
>>> change the intended behavior of the commands as documented in the
>>> GDB
>>> manual. However, it was pointed out by Pedro Alves <
>>> pedro@palves.net>
>>> that the reverse-step and reverse-next commands do not work when
>>> there
>>> are multiple function calls on the same line. This is a second
>>> scenario
>>> where the commands do not work correctly.
>>>
>>> The following patch is an extended version of the original patch by
>>> Luis Machado to fix the issues in scenario 1 to also address the
>>> issues in
>>> scenario 2.
>>>
> Move the above to the mailing list
>
> Start here with the commit message
>
>>> --------------------------------------------------------
>>>
>>> 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.
>>>
>>> This happens because we have this check in
>>> infrun.c:process_event_stop_test:
>>>
>>> /* When stepping backward, stop at beginning of line range
>>> (unless it's the function entry point, in which case
>>> keep going back to the call point). */
>>> CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
>>> if (stop_pc == ecs->event_thread->control.step_range_start
>>> && stop_pc != ecs->stop_func_start
>>> && execution_direction == EXEC_REVERSE)
>>> end_stepping_range (ecs);
>>> else
>>> keep_going (ecs);
>>>
>>> Since we've reached ecs->event_thread->control.step_range_start, we
>>> stop
>>> stepping backwards.
>>>
>>> The right thing to do is to look 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.
>>>
>>> Another solution I thought about is to merge the contiguous ranges
>>> when
>>> we are reading the line tables. Though I'm not sure if we really
>>> want to
>>> process that data as opposed to keeping it as the compiler created,
>>> and
>>> then working around that.
>>>
>>> The test case gdb.reverse/map-to-same-line.exp is added to test the
>>> fix
>>> for the issues in scenario 1.
>>>
>>> ---------------------------------------------------------
>>>
>>> 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 test cases gdb.reverse/func-map-to-same-line-no-colum-info.exp
>>> and
>>> gdb.reverse/func-map-to-same-line.exp were 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>
> End of commit message
>> Hey Carl,
>>
>> Thanks for working on this. I'm wondering which parts will be part
>> of
>> the final commit messages and which is just context for the mailing
>> list, so some clarity would be nice, but that is not a huge deal.
>>
>> I wanted to test this change, but it doesn't apply anymore on
>> master,
>> and `git apply --3way` can't figure out how to do it. Which commit
>> did
>> you use as base (or alternatively, can you rebase it)?
> OK, let me rebase it and repost. Let me know if you think the
> suggested commit message is still too much. Thanks.
Hey sorry, this part was my bad, no need to rebase. I'll look at the
commit message in a bit
On 27/04/2023 22:59, Carl Love wrote:
> GDB maintainers:
>
> 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.
>
> 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. The first scenario consists of multiple
> assignment statements on the same line. A patch was proposed to address the
> issue by Luis Machado and briefly discussed on the mailing list in Feb 2021.
>
> https://sourceware.org/pipermail/gdb-patches/2021-February/175678.html
>
> The discussion was revived by Carl Love with regards to fixing the same
> issue on PowerPC.
>
> https://sourceware.org/pipermail/gdb-patches/2022-March/186463.html
>
> The patch was not completed and has been on Carl's to do list for some time.
>
> Discussion of a patch to change how the reverse-step and reverse-next
> commands submitted by Carl Love was started in thread:
>
> https://sourceware.org/pipermail/gdb-patches/2023-January/195563.html
>
> The patch was withdrawn as it was pointed out the proposed patch would
> change the intended behavior of the commands as documented in the GDB
> manual. However, it was pointed out by Pedro Alves <pedro@palves.net>
> that the reverse-step and reverse-next commands do not work when there
> are multiple function calls on the same line. This is a second scenario
> where the commands do not work correctly.
>
> The following patch is an extended version of the original patch by
> Luis Machado to fix the issues in scenario 1 to also address the issues in
> scenario 2.
>
> --------------------------------------------------------
Hi Carl, thanks for clarifying the intended commit message. I'm reacting
to it here because I also have some thoughts on the code, now that I
managed to apply it locally.
Starting on the commit message, it would be nice to have a 1-line
description of the problem before describing the scenarios in depth.
Taking the first line of the previous block is enough IMO.
> 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.
>
> This happens because we have this check in infrun.c:process_event_stop_test:
>
> /* When stepping backward, stop at beginning of line range
> (unless it's the function entry point, in which case
> keep going back to the call point). */
> CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
> if (stop_pc == ecs->event_thread->control.step_range_start
> && stop_pc != ecs->stop_func_start
> && execution_direction == EXEC_REVERSE)
> end_stepping_range (ecs);
> else
> keep_going (ecs);
>
> Since we've reached ecs->event_thread->control.step_range_start, we stop
> stepping backwards.
I think these last 3 paragraphs should be moved. I like to finish
commits with a description of the solution, rather than having it in the
middle of the text. Also, I think we like to avoid mentioning explicit
code in the commit text (though I might be mistaken).
> The right thing to do is to look 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.
>
> Another solution I thought about is to merge the contiguous ranges when
> we are reading the line tables. Though I'm not sure if we really want to
> process that data as opposed to keeping it as the compiler created, and
> then working around that.
This paragraph doesn't need to be here in the final commit message IMO.
It was nice context for the mailing list but is not necessary for future
reference, I don't think.
>
> The test case gdb.reverse/map-to-same-line.exp is added to test the fix
> for the issues in scenario 1.
>
> ---------------------------------------------------------
>
> 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 test cases gdb.reverse/func-map-to-same-line-no-colum-info.exp and
> gdb.reverse/func-map-to-same-line.exp were 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 ++
> .../func-map-to-same-line-no-column-info.exp | 135 ++++++++++++++++
> .../gdb.reverse/func-map-to-same-line.c | 36 +++++
> .../gdb.reverse/func-map-to-same-line.exp | 123 ++++++++++++++
> gdb/testsuite/gdb.reverse/map-to-same-line.c | 58 +++++++
> .../gdb.reverse/map-to-same-line.exp | 153 ++++++++++++++++++
> 8 files changed, 627 insertions(+)
> create mode 100644 gdb/testsuite/gdb.reverse/func-map-to-same-line-no-column-info.exp
> 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 2f1c6cd694b..59374a05471 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -113,6 +113,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
> @@ -6768,6 +6771,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
> + 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).
> @@ -7569,6 +7591,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
> @@ -7631,6 +7675,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-no-column-info.exp b/gdb/testsuite/gdb.reverse/func-map-to-same-line-no-column-info.exp
> new file mode 100644
> index 00000000000..20529c90fc2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line-no-column-info.exp
> @@ -0,0 +1,135 @@
> +# 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.
> +
> +if ![supports_reverse] {
> + return
> +}
Nowadays you should use require instead of the if clause, like in
gdb.reverse/break-reverse.exp
> +
> +# This test uses the gcc no-column-info command.
Correct me if I'm wrong, but it seems to me that the other test is a
more generic version of this one, so this test could check for a gcc
recent enough to support this feature, instead of just generically gcc.
That said, gcc added it on version
7(https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=0029b929c9719a), is it
old enough that we don't care?
> +if ![is_c_compiler_gcc] {
> + unsupported "gcc is required for this test"
> + return 0
> +}
> +
> +set srcfile func-map-to-same-line.c
> +set executable func-map-to-same-line
> +
> +set options [list debug additional_flags=-gno-column-info]
> +
> +if {[build_executable "failed to prepare" $executable $srcfile $options] == -1}\
> + {
> + return -1
> +}
> +
> +clean_restart $executable
> +
> +runto_main
> +set target_remote [gdb_is_target_remote]
> +
> +if [supports_process_record] {
> + # Activate process record/replay.
> + gdb_test_no_output "record" "turn on process record for test1"
> +}
> +
> +# 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 \
> + "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 \\(\\);.*" \
> + "reverse-next to line with two functions"
> +
> +# 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-step" ".*b = 2;.*" \
> + "reverse-step to previous line b = 2"
The point of this test is to confirm that we are at the very first
instruction of the line, right? So I would think it is better to do a
reverse-stepi here, to make sure that even walking a single instruction
we reach a different line. Either that or doing what Pedro did in his
email: save the PC before executing the line, then do and undo the line
and confirm that PCs match exactly.
> +
> +
> +# Setup for test 2
> +# Go back to the start of the function
> +gdb_test "reverse-continue" "a = 1;" "At start of main, setup for test 2"
> +
> +# Turn off record to clear logs and turn on again
> +gdb_test "record stop" "Process record is stopped.*" \
> + "turn off process record for test1"
> +gdb_test_no_output "record" "turn on process record for test2"
Since you don't require process record for this test, you can't assume
these to work. I think its better to clean restart and record if the
process supports recording, this way you're sure to reset history no
matter the inferior.
> +
> +# Delete all breakpoints and catchpoints.
> +delete_breakpoints
> +
> +
> +# 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 \
> + "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" ".*}.*" \
> + "reverse-step into func2 "
> +
> +# The second reverse step should take us into func1 ().
> +gdb_test "reverse-step" ".*}.*" \
> + "reverse-step into func1 "
> +
> +# The third reverse step should take us call of func1 ().
> +gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
> + "reverse-step to line func1(); func2(), at call for func1 "
> +
> +# The fourth reverse step should take us to b = 2 ().
> +gdb_test "reverse-step" ".*b = 2;.*" \
> + "reverse-step to line b = 2 "
Ditto from the other test like this. Also, I feel that, while the test
name for the last 2 gdb_test are different, they don't meaningfully
communicate which part of the test is failing. I think it would be
better if you differentiated them by adding "for step test" or "for test
2" at the end of the name would make it easier to understand where
things went wrong when looking at the sum file.
> 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..e9787ef9ff5
> --- /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 ()
> +{
> +}
> +
> +void
> +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..b632a236bbe
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp
> @@ -0,0 +1,123 @@
> +# 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.
> +
> +if ![supports_reverse] {
> + return
> +}
I'm not sure it's worth separating these 2 tests into separate files.
You could instead just have most of the test defined as a proc, and call
it twice, once after compiling the inferior with column info, the other
compiling without if gcc is used. This way it's less likely that the
tests will diverge over time.
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> + return -1
> +}
> +
> +runto_main
> +set target_remote [gdb_is_target_remote]
> +
> +if [supports_process_record] {
> + # Activate process record/replay.
> + gdb_test_no_output "record" "turn on process record for test1"
> +}
> +
> +# 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 ();
> +# The assumption for this test is the dwarf info contain the column
> +# 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 \
> + "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 \\(\\);.*" \
> + "reverse-next to line with two functions"
> +
> +# 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-step" ".*b = 2;.*" \
> + "reverse-step to previous line b = 2"
> +
> +
> +# Setup for test 2
> +# Go back to the start of the function
> +gdb_test "reverse-continue" "a = 1;" "At start of main, setup for test 2"
> +
> +# Turn off record to clear logs and turn on again
> +gdb_test "record stop" "Process record is stopped.*" \
> + "turn off process record for test1"
> +gdb_test_no_output "record" "turn on process record for test2"
> +
> +# Delete all breakpoints and catchpoints.
> +delete_breakpoints
> +
> +
> +# 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 \
> + "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" ".*}.*" \
> + "reverse-step into func2 "
> +
> +# The second reverse step should take us into func1 ().
> +gdb_test "reverse-step" ".*}.*" \
> + "reverse-step into func1 "
> +
> +# The third reverse step should take us call of func1 ().
> +gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
> + "reverse-step to line func1(); func2(), at call for func1 "
> +
> +# The fourth reverse step should take us to b = 2 ().
> +gdb_test "reverse-step" ".*b = 2;.*" \
> + "reverse-step to line b = 2 "
> 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..a01579c2a8d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/map-to-same-line.exp
> @@ -0,0 +1,153 @@
> +# 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.
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if {![dwarf2_support]} {
> + unsupported "dwarf2 support required for this test"
> + return 0
> +}
Again, the new way to check for these is "required". And IIUC, you can
add multiple requirements into a singe require call.
> +
> +if [get_compiler_info] {
> + return -1
> +}
> +
> +# The DWARF assembler requires the gcc compiler.
> +if ![is_c_compiler_gcc] {
> + unsupported "gcc is required for this test"
> + return 0
> +}
> +
> +# This test suitable only for process record-replay
> +if ![supports_process_record] {
> + return
> +}
> +
> +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
> +}
> +
> +if ![runto_main] {
> + return -1
> +}
runto_main already errors out and leaves, I think, so no need for the if
clause.
> +
> +# 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 "" {
> + }
> +}
> +
> +# Activate process record/replay
> +gdb_test_no_output "record" "turn on process record"
> +
> +gdb_test "tbreak main_return" "Temporary breakpoint .*" "breakpoint at return"
you can set a temporary breakpoint using gdb_breakpoint "..." temporary,
no need to manually call tbreak.
> +gdb_test "continue" "Temporary breakpoint .*" "run to end of main"
gdb_continue_to_breakpoint can handle temporary breakpoints as well.
> +gdb_test "display \$pc" ".*pc =.*" "display pc"
> +
> +# 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"
> +}
I'm not sure if it is needed, but I don't think it would hurt to also
test reverse-next in a separate foreach right after this one.
Bruno:
On Wed, 2023-05-03 at 11:53 +0200, Bruno Larsen wrote:
> On 27/04/2023 22:59, Carl Love wrote:
<snip>
>
> Hi Carl, thanks for clarifying the intended commit message. I'm
> reacting
> to it here because I also have some thoughts on the code, now that I
> managed to apply it locally.
>
> Starting on the commit message, it would be nice to have a 1-line
> description of the problem before describing the scenarios in depth.
> Taking the first line of the previous block is enough IMO.
Yes, agreed. Kept the first line before the discussion of the
different failure scenarios.
>
> > 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.
> >
> > This happens because we have this check in
> > infrun.c:process_event_stop_test:
> >
> > /* When stepping backward, stop at beginning of line range
> > (unless it's the function entry point, in which case
> > keep going back to the call point). */
> > CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
> > if (stop_pc == ecs->event_thread->control.step_range_start
> > && stop_pc != ecs->stop_func_start
> > && execution_direction == EXEC_REVERSE)
> > end_stepping_range (ecs);
> > else
> > keep_going (ecs);
> >
> > Since we've reached ecs->event_thread->control.step_range_start, we
> > stop
> > stepping backwards.
> I think these last 3 paragraphs should be moved. I like to finish
> commits with a description of the solution, rather than having it in
> the
> middle of the text. Also, I think we like to avoid mentioning
> explicit
> code in the commit text (though I might be mistaken).
OK, I moved the fix discussion to the end. I also dropped the explicit
reference to infrun.c:process_event_stop_test.
> > The right thing to do is to look 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.
> >
> > Another solution I thought about is to merge the contiguous ranges
> > when
> > we are reading the line tables. Though I'm not sure if we really
> > want to
> > process that data as opposed to keeping it as the compiler created,
> > and
> > then working around that.
> This paragraph doesn't need to be here in the final commit message
> IMO.
> It was nice context for the mailing list but is not necessary for
> future
OK, removed it from the commit log and need to update the mailing list
message with this.
> reference, I don't think.
> > The test case gdb.reverse/map-to-same-line.exp is added to test the
> > fix
> > for the issues in scenario 1.
> >
> > ---------------------------------------------------------
> >
> > 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 test cases gdb.reverse/func-map-to-same-line-no-colum-info.exp
> > and
> > gdb.reverse/func-map-to-same-line.exp were 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 ++
> > .../func-map-to-same-line-no-column-info.exp | 135
> > ++++++++++++++++
> > .../gdb.reverse/func-map-to-same-line.c | 36 +++++
> > .../gdb.reverse/func-map-to-same-line.exp | 123
> > ++++++++++++++
> > gdb/testsuite/gdb.reverse/map-to-same-line.c | 58 +++++++
> > .../gdb.reverse/map-to-same-line.exp | 153
> > ++++++++++++++++++
> > 8 files changed, 627 insertions(+)
> > create mode 100644 gdb/testsuite/gdb.reverse/func-map-to-same-
> > line-no-column-info.exp
> > 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 2f1c6cd694b..59374a05471 100644
> > --- a/gdb/infrun.c
> > +++ b/gdb/infrun.c
> > @@ -113,6 +113,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
> > @@ -6768,6 +6771,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
> > + 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).
> > @@ -7569,6 +7591,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
> > @@ -7631,6 +7675,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-no-
> > column-info.exp b/gdb/testsuite/gdb.reverse/func-map-to-same-line-
> > no-column-info.exp
> > new file mode 100644
> > index 00000000000..20529c90fc2
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line-no-column-
> > info.exp
> > @@ -0,0 +1,135 @@
> > +# 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.
> > +
> > +if ![supports_reverse] {
> > + return
> > +}
> Nowadays you should use require instead of the if clause, like in
> gdb.reverse/break-reverse.exp
OK, changed that
> > +
> > +# This test uses the gcc no-column-info command.
> Correct me if I'm wrong, but it seems to me that the other test is a
> more generic version of this one, so this test could check for a gcc
> recent enough to support this feature, instead of just generically
> gcc.
> That said, gcc added it on version
> 7(
> https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=0029b929c9719a
> ), is it
> old enough that we don't care?
GCC supports line tables, I don't know that clang or other compilers
do. So all we really need is to check for gcc. The line table stuff
was added a long time ago so not sure that we really need to check for
version 7 at this point. So just checked that we are using gcc. The
"other test" func-map-to-same-line.exp expects the line table so it
should probably also be checking that we are using gcc.
> > +if ![is_c_compiler_gcc] {
> > + unsupported "gcc is required for this test"
> > + return 0
> > +}
> > +
> > +set srcfile func-map-to-same-line.c
> > +set executable func-map-to-same-line
> > +
> > +set options [list debug additional_flags=-gno-column-info]
> > +
> > +if {[build_executable "failed to prepare" $executable $srcfile
> > $options] == -1}\
> > + {
> > + return -1
> > +}
> > +
> > +clean_restart $executable
> > +
> > +runto_main
> > +set target_remote [gdb_is_target_remote]
> > +
> > +if [supports_process_record] {
> > + # Activate process record/replay.
> > + gdb_test_no_output "record" "turn on process record for test1"
> > +}
> > +
> > +# 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 \
> > + "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 \\(\\);.*" \
> > + "reverse-next to line with two functions"
> > +
> > +# 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-step" ".*b = 2;.*" \
> > + "reverse-step to previous line b = 2"
> The point of this test is to confirm that we are at the very first
> instruction of the line, right? So I would think it is better to do
> a
> reverse-stepi here, to make sure that even walking a single
> instruction
> we reach a different line.
Yes, we should be on the first instruction of the line so stepi would
be a better test to prove we are really on the first instruction.
Changed the reverse-step to reverse-stepi.
> Either that or doing what Pedro did in his
> email: save the PC before executing the line, then do and undo the
> line
> and confirm that PCs match exactly.
> > +
> > +
> > +# Setup for test 2
> > +# Go back to the start of the function
> > +gdb_test "reverse-continue" "a = 1;" "At start of main, setup for
> > test 2"
> > +
> > +# Turn off record to clear logs and turn on again
> > +gdb_test "record stop" "Process record is stopped.*" \
> > + "turn off process record for test1"
> > +gdb_test_no_output "record" "turn on process record for test2"
> Since you don't require process record for this test, you can't
> assume
> these to work. I think its better to clean restart and record if the
> process supports recording, this way you're sure to reset history no
> matter the inferior.
No, the test requires process record. If record is not supported, we
can't do reverse execution. That said, doing a "clean restart" and
then record would be another way, probably better way, of clearing the
history. Put in a clean restart rather than turning off/on the
recording to clear the log file.
> > +
> > +# Delete all breakpoints and catchpoints.
> > +delete_breakpoints
> > +
> > +
> > +# 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 \
> > + "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" ".*}.*" \
> > + "reverse-step into func2 "
> > +
> > +# The second reverse step should take us into func1 ().
> > +gdb_test "reverse-step" ".*}.*" \
> > + "reverse-step into func1 "
> > +
> > +# The third reverse step should take us call of func1 ().
> > +gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
> > + "reverse-step to line func1(); func2(), at call for func1 "
> > +
> > +# The fourth reverse step should take us to b = 2 ().
> > +gdb_test "reverse-step" ".*b = 2;.*" \
> > + "reverse-step to line b = 2 "
> Ditto from the other test like this.
Yes, stepi is a better test to prove you were on the first instruction
in the line. Changed.
> Also, I feel that, while the test
> name for the last 2 gdb_test are different, they don't meaningfully
> communicate which part of the test is failing. I think it would be
> better if you differentiated them by adding "for step test" or "for
> test
> 2" at the end of the name would make it easier to understand where
> things went wrong when looking at the sum file.
OK, agreed. For the above tests I added "test#," where # is either 1
or 2 to the test names. For example:
+gdb_test "reverse-step" ".*b = 2;.*" \
+ "reverse-step to line b = 2 "
was changed to
+gdb_test "reverse-stepi" ".*b = 2;.*" \
+ "test2, reverse-step to line b = 2 "
I also updated the test to identify the closing } for func1 and func2
to make it clearer in the test which function we just steped back into.
> > 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..e9787ef9ff5
> > --- /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 ()
> > +{
> > +}
> > +
> > +void
> > +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..b632a236bbe
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp
> > @@ -0,0 +1,123 @@
> > +# 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.
> > +
> > +if ![supports_reverse] {
> > + return
> > +}
>
> I'm not sure it's worth separating these 2 tests into separate
> files.
> You could instead just have most of the test defined as a proc, and
> call
> it twice, once after compiling the inferior with column info, the
> other
> compiling without if gcc is used. This way it's less likely that the
> tests will diverge over time.
OK, good point. I created a test proc which is then called with the
binary compiled with and without line information. The extra exp test
file was removed.
>
> > +
> > +standard_testfile
> > +
> > +if { [prepare_for_testing "failed to prepare" $testfile $srcfile]
> > } {
> > + return -1
> > +}
> > +
> > +runto_main
> > +set target_remote [gdb_is_target_remote]
> > +
> > +if [supports_process_record] {
> > + # Activate process record/replay.
> > + gdb_test_no_output "record" "turn on process record for test1"
> > +}
> > +
> > +# 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
> > ();
> > +# The assumption for this test is the dwarf info contain the
> > column
> > +# 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 \
> > + "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 \\(\\);.*" \
> > + "reverse-next to line with two functions"
> > +
> > +# 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-step" ".*b = 2;.*" \
> > + "reverse-step to previous line b = 2"
> > +
> > +
> > +# Setup for test 2
> > +# Go back to the start of the function
> > +gdb_test "reverse-continue" "a = 1;" "At start of main, setup for
> > test 2"
> > +
> > +# Turn off record to clear logs and turn on again
> > +gdb_test "record stop" "Process record is stopped.*" \
> > + "turn off process record for test1"
> > +gdb_test_no_output "record" "turn on process record for test2"
> > +
> > +# Delete all breakpoints and catchpoints.
> > +delete_breakpoints
> > +
> > +
> > +# 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 \
> > + "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" ".*}.*" \
> > + "reverse-step into func2 "
> > +
> > +# The second reverse step should take us into func1 ().
> > +gdb_test "reverse-step" ".*}.*" \
> > + "reverse-step into func1 "
> > +
> > +# The third reverse step should take us call of func1 ().
> > +gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
> > + "reverse-step to line func1(); func2(), at call for func1 "
> > +
> > +# The fourth reverse step should take us to b = 2 ().
> > +gdb_test "reverse-step" ".*b = 2;.*" \
> > + "reverse-step to line b = 2 "
> > 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..a01579c2a8d
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.reverse/map-to-same-line.exp
> > @@ -0,0 +1,153 @@
> > +# 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.
> > +
> > +load_lib dwarf.exp
> > +
> > +# This test can only be run on targets which support DWARF-2 and
> > use gas.
> > +if {![dwarf2_support]} {
> > + unsupported "dwarf2 support required for this test"
> > + return 0
> > +}
> Again, the new way to check for these is "required". And IIUC, you
> can
> add multiple requirements into a singe require call.
OK, updated.
> > +
> > +if [get_compiler_info] {
> > + return -1
> > +}
> > +
> > +# The DWARF assembler requires the gcc compiler.
> > +if ![is_c_compiler_gcc] {
> > + unsupported "gcc is required for this test"
> > + return 0
> > +}
> > +
> > +# This test suitable only for process record-replay
> > +if ![supports_process_record] {
> > + return
> > +}
> > +
> > +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
> > +}
> > +
> > +if ![runto_main] {
> > + return -1
> > +}
> runto_main already errors out and leaves, I think, so no need for the
> if
> clause.
OK, updated.
> > +
> > +# 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 "" {
> > + }
> > +}
> > +
> > +# Activate process record/replay
> > +gdb_test_no_output "record" "turn on process record"
> > +
> > +gdb_test "tbreak main_return" "Temporary breakpoint .*"
> > "breakpoint at return"
> you can set a temporary breakpoint using gdb_breakpoint "..."
> temporary,
> no need to manually call tbreak.
> > +gdb_test "continue" "Temporary breakpoint .*" "run to end of main"
> gdb_continue_to_breakpoint can handle temporary breakpoints as well.
OK, updated the break and continue statements.
> > +gdb_test "display \$pc" ".*pc =.*" "display pc"
> > +
> > +# 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"
> > +}
>
> I'm not sure if it is needed, but I don't think it would hurt to
> also
> test reverse-next in a separate foreach right after this one.
OK, added a clean restart, run to the end of main then do reverse next
using a foreach line.
Carl
On 04/05/2023 04:55, Carl Love wrote:
> Bruno:
>
> On Wed, 2023-05-03 at 11:53 +0200, Bruno Larsen wrote:
>> On 27/04/2023 22:59, Carl Love wrote:
> <snip>
<snip>
>>> +
>>> +# This test uses the gcc no-column-info command.
>> Correct me if I'm wrong, but it seems to me that the other test is a
>> more generic version of this one, so this test could check for a gcc
>> recent enough to support this feature, instead of just generically
>> gcc.
>> That said, gcc added it on version
>> 7(
>> https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=0029b929c9719a
>> ), is it
>> old enough that we don't care?
> GCC supports line tables, I don't know that clang or other compilers
> do. So all we really need is to check for gcc. The line table stuff
> was added a long time ago so not sure that we really need to check for
> version 7 at this point. So just checked that we are using gcc. The
> "other test" func-map-to-same-line.exp expects the line table so it
> should probably also be checking that we are using gcc.
GCC 7.1 (first gcc 7 release) was on May 2nd 2017, almost exactly 6
years ago, and there was a gcc 6.8 release in october 2018. I don't know
if 5 years is long enough to assume that everyone has abandoned the old
version (especially seeing as we sometimes test for gcc 4 or 3, but that
might just be old cruft). That said, it's not a blocker for me, so /shrug
Also, clang will have line tables - otherwise almost nothing on our test
suite would work. It doesn't have column info, though, which is why I'm
fine with it being ignored in the test that uses -gno-column-info.
>
>>> +if ![is_c_compiler_gcc] {
>>> + unsupported "gcc is required for this test"
>>> + return 0
>>> +}
>>> +
>>> +set srcfile func-map-to-same-line.c
>>> +set executable func-map-to-same-line
>>> +
>>> +set options [list debug additional_flags=-gno-column-info]
>>> +
>>> +if {[build_executable "failed to prepare" $executable $srcfile
>>> $options] == -1}\
>>> + {
>>> + return -1
>>> +}
>>> +
>>> +clean_restart $executable
>>> +
>>> +runto_main
>>> +set target_remote [gdb_is_target_remote]
>>> +
>>> +if [supports_process_record] {
>>> + # Activate process record/replay.
>>> + gdb_test_no_output "record" "turn on process record for test1"
>>> +}
>>> +
>>> +# 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 \
>>> + "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 \\(\\);.*" \
>>> + "reverse-next to line with two functions"
>>> +
>>> +# 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-step" ".*b = 2;.*" \
>>> + "reverse-step to previous line b = 2"
>> The point of this test is to confirm that we are at the very first
>> instruction of the line, right? So I would think it is better to do
>> a
>> reverse-stepi here, to make sure that even walking a single
>> instruction
>> we reach a different line.
> Yes, we should be on the first instruction of the line so stepi would
> be a better test to prove we are really on the first instruction.
> Changed the reverse-step to reverse-stepi.
>
>> Either that or doing what Pedro did in his
>> email: save the PC before executing the line, then do and undo the
>> line
>> and confirm that PCs match exactly.
>>> +
>>> +
>>> +# Setup for test 2
>>> +# Go back to the start of the function
>>> +gdb_test "reverse-continue" "a = 1;" "At start of main, setup for
>>> test 2"
>>> +
>>> +# Turn off record to clear logs and turn on again
>>> +gdb_test "record stop" "Process record is stopped.*" \
>>> + "turn off process record for test1"
>>> +gdb_test_no_output "record" "turn on process record for test2"
>> Since you don't require process record for this test, you can't
>> assume
>> these to work. I think its better to clean restart and record if the
>> process supports recording, this way you're sure to reset history no
>> matter the inferior.
> No, the test requires process record. If record is not supported, we
> can't do reverse execution. That said, doing a "clean restart" and
> then record would be another way, probably better way, of clearing the
> history. Put in a clean restart rather than turning off/on the
> recording to clear the log file.
From my understanding, you could have architectures or inferiors that
support reverse execution without needing to record, that's why
"supports_reverse" and "supports_process_record" are different.
However, if you want to restrict this to record-only, that's fine, I
just think it should be a requirement at the top of the test, not in the
middle of the execution.
Bruno:
On Thu, 2023-05-04 at 11:24 +0200, Bruno Larsen wrote:
> On 04/05/2023 04:55, Carl Love wrote:
> > Bruno:
> >
> > On Wed, 2023-05-03 at 11:53 +0200, Bruno Larsen wrote:
> > > On 27/04/2023 22:59, Carl Love wrote:
> > <snip>
> <snip>
> > > > +
> > > > +# This test uses the gcc no-column-info command.
> > > Correct me if I'm wrong, but it seems to me that the other test
> > > is a
> > > more generic version of this one, so this test could check for a
> > > gcc
> > > recent enough to support this feature, instead of just
> > > generically
> > > gcc.
> > > That said, gcc added it on version
> > > 7(
> > > https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=0029b929c9719a
> > > ), is it
> > > old enough that we don't care?
> > GCC supports line tables, I don't know that clang or other
> > compilers
> > do. So all we really need is to check for gcc. The line table
> > stuff
> > was added a long time ago so not sure that we really need to check
> > for
> > version 7 at this point. So just checked that we are using
> > gcc. The
> > "other test" func-map-to-same-line.exp expects the line table so it
> > should probably also be checking that we are using gcc.
>
> GCC 7.1 (first gcc 7 release) was on May 2nd 2017, almost exactly 6
> years ago, and there was a gcc 6.8 release in october 2018. I don't
> know
> if 5 years is long enough to assume that everyone has abandoned the
> old
> version (especially seeing as we sometimes test for gcc 4 or 3, but
> that
> might just be old cruft). That said, it's not a blocker for me, so
> /shrug
OK, I would think that it has been there long enough. But it does
sound like there is the occasional test on an old gcc. So, OK lets
restrict it to GCC 7 and later.
>
> Also, clang will have line tables - otherwise almost nothing on our
> test
> suite would work. It doesn't have column info, though, which is why
> I'm
> fine with it being ignored in the test that uses -gno-column-info.
>
> > > > +if ![is_c_compiler_gcc] {
> > > > + unsupported "gcc is required for this test"
> > > > + return 0
> > > > +}
> > > > +
> > > >
<snip>
> From my understanding, you could have architectures or inferiors
> that
> support reverse execution without needing to record, that's why
> "supports_reverse" and "supports_process_record" are different.
>
> However, if you want to restrict this to record-only, that's fine, I
> just think it should be a requirement at the top of the test, not in
> the
> middle of the execution.
I can't imagine how you can do remote execution without recording but
if they can do that OK. The test really should be at the top, missed
that part. So, I will change this to "supports_reverse" and get it at
the top where it belongs.
Thanks for the clarification and help with this.
Carl
> I wanted to test this change, but it doesn't apply anymore on master,
> and `git apply --3way` can't figure out how to do it. Which commit did
> you use as base (or alternatively, can you rebase it)?
To help with this, you can consider setting the `format.useAutoBase`
configuration option in git (documented in the git-format-patch man
page).
Simon
@@ -113,6 +113,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
@@ -6768,6 +6771,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
+ 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).
@@ -7569,6 +7591,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
@@ -7631,6 +7675,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);
}
@@ -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 *
@@ -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);
new file mode 100644
@@ -0,0 +1,135 @@
+# 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.
+
+if ![supports_reverse] {
+ return
+}
+
+# This test uses the gcc no-column-info command.
+if ![is_c_compiler_gcc] {
+ unsupported "gcc is required for this test"
+ return 0
+}
+
+set srcfile func-map-to-same-line.c
+set executable func-map-to-same-line
+
+set options [list debug additional_flags=-gno-column-info]
+
+if {[build_executable "failed to prepare" $executable $srcfile $options] == -1}\
+ {
+ return -1
+}
+
+clean_restart $executable
+
+runto_main
+set target_remote [gdb_is_target_remote]
+
+if [supports_process_record] {
+ # Activate process record/replay.
+ gdb_test_no_output "record" "turn on process record for test1"
+}
+
+# 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 \
+ "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 \\(\\);.*" \
+ "reverse-next to line with two functions"
+
+# 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-step" ".*b = 2;.*" \
+ "reverse-step to previous line b = 2"
+
+
+# Setup for test 2
+# Go back to the start of the function
+gdb_test "reverse-continue" "a = 1;" "At start of main, setup for test 2"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop" "Process record is stopped.*" \
+ "turn off process record for test1"
+gdb_test_no_output "record" "turn on process record for test2"
+
+# Delete all breakpoints and catchpoints.
+delete_breakpoints
+
+
+# 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 \
+ "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" ".*}.*" \
+ "reverse-step into func2 "
+
+# The second reverse step should take us into func1 ().
+gdb_test "reverse-step" ".*}.*" \
+ "reverse-step into func1 "
+
+# The third reverse step should take us call of func1 ().
+gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
+ "reverse-step to line func1(); func2(), at call for func1 "
+
+# The fourth reverse step should take us to b = 2 ().
+gdb_test "reverse-step" ".*b = 2;.*" \
+ "reverse-step to line b = 2 "
new file mode 100644
@@ -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 ()
+{
+}
+
+void
+func2 ()
+{
+}
+
+int main ()
+{
+ int a, b;
+ a = 1;
+ b = 2;
+ func1 (); func2 ();
+ a = a + b; // START REVERSE TEST
+}
new file mode 100644
@@ -0,0 +1,123 @@
+# 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.
+
+if ![supports_reverse] {
+ return
+}
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+ return -1
+}
+
+runto_main
+set target_remote [gdb_is_target_remote]
+
+if [supports_process_record] {
+ # Activate process record/replay.
+ gdb_test_no_output "record" "turn on process record for test1"
+}
+
+# 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 ();
+# The assumption for this test is the dwarf info contain the column
+# 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 \
+ "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 \\(\\);.*" \
+ "reverse-next to line with two functions"
+
+# 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-step" ".*b = 2;.*" \
+ "reverse-step to previous line b = 2"
+
+
+# Setup for test 2
+# Go back to the start of the function
+gdb_test "reverse-continue" "a = 1;" "At start of main, setup for test 2"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop" "Process record is stopped.*" \
+ "turn off process record for test1"
+gdb_test_no_output "record" "turn on process record for test2"
+
+# Delete all breakpoints and catchpoints.
+delete_breakpoints
+
+
+# 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 \
+ "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" ".*}.*" \
+ "reverse-step into func2 "
+
+# The second reverse step should take us into func1 ().
+gdb_test "reverse-step" ".*}.*" \
+ "reverse-step into func1 "
+
+# The third reverse step should take us call of func1 ().
+gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
+ "reverse-step to line func1(); func2(), at call for func1 "
+
+# The fourth reverse step should take us to b = 2 ().
+gdb_test "reverse-step" ".*b = 2;.*" \
+ "reverse-step to line b = 2 "
new file mode 100644
@@ -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 */
+}
new file mode 100644
@@ -0,0 +1,153 @@
+# 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.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+ unsupported "dwarf2 support required for this test"
+ return 0
+}
+
+if [get_compiler_info] {
+ return -1
+}
+
+# The DWARF assembler requires the gcc compiler.
+if ![is_c_compiler_gcc] {
+ unsupported "gcc is required for this test"
+ return 0
+}
+
+# This test suitable only for process record-replay
+if ![supports_process_record] {
+ return
+}
+
+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
+}
+
+if ![runto_main] {
+ return -1
+}
+
+# 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 "" {
+ }
+}
+
+# Activate process record/replay
+gdb_test_no_output "record" "turn on process record"
+
+gdb_test "tbreak main_return" "Temporary breakpoint .*" "breakpoint at return"
+gdb_test "continue" "Temporary breakpoint .*" "run to end of main"
+gdb_test "display \$pc" ".*pc =.*" "display pc"
+
+# 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"
+}