* [PATCH 1/4] [gdb/testsuite] Simplify gdb.base/unwind-on-each-insn.exp
2023-01-19 10:46 [PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements Tom de Vries
@ 2023-01-19 10:46 ` Tom de Vries
2023-01-23 9:36 ` Tom de Vries
2023-01-19 10:46 ` [PATCH 2/4] [gdb/testsuite] Improve gdb.base/unwind-on-each-insn.exp Tom de Vries
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2023-01-19 10:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Bruno Larsen, Andrew Burgess, Luis Machado
In test-case gdb.base/unwind-on-each-insn.exp, we try to determine the last
insn in function foo.
This in it self is fragile, as demonstrated by commit 91836f41e20 ("Powerpc
fix for gdb.base/unwind-on-each-insn.exp").
But the purpose of finding the last insn is to stop stepping in foo when
arriving at that last insn.
There is however no guarantee that:
- the last insn is actually executed, nor
- that the last insn is executed last, nor
- that the last insn is executed once.
Fix this by simplying the test-case to continue stepping till stepping out of
foo.
Tested on x86_64-linux.
---
.../gdb.base/unwind-on-each-insn.exp | 62 ++++---------------
1 file changed, 11 insertions(+), 51 deletions(-)
diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
index c8748d5ae14..5e822effaf1 100644
--- a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
+++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
@@ -73,51 +73,6 @@ set main_fid [get_fid]
gdb_breakpoint "*foo"
gdb_continue_to_breakpoint "enter foo"
-# Figure out the range of addresses covered by this function.
-set last_addr_in_foo ""
-
-# The disassembly of foo on PowerPC looks like:
-# Dump of assembler code for function foo:
-# => 0x00000000100006dc <+0>: std r31,-8(r1)
-# 0x00000000100006e0 <+4>: stdu r1,-48(r1)
-# 0x00000000100006e4 <+8>: mr r31,r1
-# 0x00000000100006e8 <+12>: nop
-# 0x00000000100006ec <+16>: addi r1,r31,48
-# 0x00000000100006f0 <+20>: ld r31,-8(r1)
-# 0x00000000100006f4 <+24>: blr
-# 0x00000000100006f8 <+28>: .long 0x0
-# 0x00000000100006fc <+32>: .long 0x0
-# 0x0000000010000700 <+36>: .long 0x1000180
-# End of assembler dump.
-#
-# The last instruction in function foo is blr. Need to ignore the .long
-# entries following the blr instruction.
-
-gdb_test_multiple "disassemble foo" "" {
- -re "^disassemble foo\r\n" {
- exp_continue
- }
-
- -re "^Dump of assembler code for function foo:\r\n" {
- exp_continue
- }
-
- -re "^...($hex) \[<>+0-9:\s\t\]*\.long\[\s\t\]*\[^\r\n\]*\r\n" {
- exp_continue
- }
-
- -re "^...($hex) \[^\r\n\]+\r\n" {
- set last_addr_in_foo $expect_out(1,string)
- exp_continue
- }
-
- -wrap -re "^End of assembler dump\\." {
- gdb_assert { ![string equal $last_addr_in_foo ""] } \
- "found some addresses in foo"
- pass $gdb_test_name
- }
-}
-
# Record the current stack-pointer, and the frame base address.
lassign [get_sp_and_fba "in foo"] foo_sp foo_fba
set foo_fid [get_fid]
@@ -158,11 +113,6 @@ for { set i_count 1 } { true } { incr i_count } {
# Move back to the inner most frame.
gdb_test "frame 0" ".*"
- set pc [get_hexadecimal_valueof "\$pc" "*UNKNOWN*"]
- if { $pc == $last_addr_in_foo } {
- break
- }
-
if { $i_count > 100 } {
# We expect a handful of instructions, if we reach 100,
# something is going wrong. Avoid an infinite loop.
@@ -170,6 +120,16 @@ for { set i_count 1 } { true } { incr i_count } {
break
}
- gdb_test "stepi" ".*"
+ set in_foo 0
+ gdb_test_multiple "stepi" "" {
+ -re -wrap "$hex in foo \\(\\)" {
+ set in_foo 1
+ }
+ -re -wrap "" {}
+ }
+
+ if { ! $in_foo } {
+ break
+ }
}
}
--
2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] [gdb/testsuite] Simplify gdb.base/unwind-on-each-insn.exp
2023-01-19 10:46 ` [PATCH 1/4] [gdb/testsuite] Simplify gdb.base/unwind-on-each-insn.exp Tom de Vries
@ 2023-01-23 9:36 ` Tom de Vries
0 siblings, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2023-01-23 9:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Bruno Larsen, Andrew Burgess, Luis Machado
[-- Attachment #1: Type: text/plain, Size: 890 bytes --]
On 1/19/23 11:46, Tom de Vries via Gdb-patches wrote:
> In test-case gdb.base/unwind-on-each-insn.exp, we try to determine the last
> insn in function foo.
>
> This in it self is fragile, as demonstrated by commit 91836f41e20 ("Powerpc
> fix for gdb.base/unwind-on-each-insn.exp").
>
> But the purpose of finding the last insn is to stop stepping in foo when
> arriving at that last insn.
>
> There is however no guarantee that:
> - the last insn is actually executed, nor
> - that the last insn is executed last, nor
> - that the last insn is executed once.
>
> Fix this by simplying the test-case to continue stepping till stepping out of
> foo.
>
I re-read the commit log, found it not clear enough, and decided to make
the difference between:
- last disassembled insn, and
- last insn executed before returning to main.
a bit more explicit.
Committed as attached.
Thanks,
- Tom
[-- Attachment #2: 0001-gdb-testsuite-Simplify-gdb.base-unwind-on-each-insn..patch --]
[-- Type: text/x-patch, Size: 3659 bytes --]
From 767e15c09b474e0de3d9323918706cef6a5a8466 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Wed, 18 Jan 2023 14:12:41 +0100
Subject: [pushed] [gdb/testsuite] Simplify gdb.base/unwind-on-each-insn.exp
In test-case gdb.base/unwind-on-each-insn.exp, we try to determine the last
disassembled insn in function foo.
This in it self is fragile, as demonstrated by commit 91836f41e20 ("Powerpc
fix for gdb.base/unwind-on-each-insn.exp").
The use of the last disassembled insn in the test-case is to stop stepping in
foo once reaching it.
However, the intent is to stop stepping just before returning to main.
There is no guarantee that the last disassembled insn:
- is actually executed
- is executed just before returning to main
- is executed only once.
Fix this by simplying the test-case to continue stepping till stepping out of
foo.
Tested on x86_64-linux.
---
.../gdb.base/unwind-on-each-insn.exp | 62 ++++---------------
1 file changed, 11 insertions(+), 51 deletions(-)
diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
index c8748d5ae14..5e822effaf1 100644
--- a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
+++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
@@ -73,51 +73,6 @@ set main_fid [get_fid]
gdb_breakpoint "*foo"
gdb_continue_to_breakpoint "enter foo"
-# Figure out the range of addresses covered by this function.
-set last_addr_in_foo ""
-
-# The disassembly of foo on PowerPC looks like:
-# Dump of assembler code for function foo:
-# => 0x00000000100006dc <+0>: std r31,-8(r1)
-# 0x00000000100006e0 <+4>: stdu r1,-48(r1)
-# 0x00000000100006e4 <+8>: mr r31,r1
-# 0x00000000100006e8 <+12>: nop
-# 0x00000000100006ec <+16>: addi r1,r31,48
-# 0x00000000100006f0 <+20>: ld r31,-8(r1)
-# 0x00000000100006f4 <+24>: blr
-# 0x00000000100006f8 <+28>: .long 0x0
-# 0x00000000100006fc <+32>: .long 0x0
-# 0x0000000010000700 <+36>: .long 0x1000180
-# End of assembler dump.
-#
-# The last instruction in function foo is blr. Need to ignore the .long
-# entries following the blr instruction.
-
-gdb_test_multiple "disassemble foo" "" {
- -re "^disassemble foo\r\n" {
- exp_continue
- }
-
- -re "^Dump of assembler code for function foo:\r\n" {
- exp_continue
- }
-
- -re "^...($hex) \[<>+0-9:\s\t\]*\.long\[\s\t\]*\[^\r\n\]*\r\n" {
- exp_continue
- }
-
- -re "^...($hex) \[^\r\n\]+\r\n" {
- set last_addr_in_foo $expect_out(1,string)
- exp_continue
- }
-
- -wrap -re "^End of assembler dump\\." {
- gdb_assert { ![string equal $last_addr_in_foo ""] } \
- "found some addresses in foo"
- pass $gdb_test_name
- }
-}
-
# Record the current stack-pointer, and the frame base address.
lassign [get_sp_and_fba "in foo"] foo_sp foo_fba
set foo_fid [get_fid]
@@ -158,11 +113,6 @@ for { set i_count 1 } { true } { incr i_count } {
# Move back to the inner most frame.
gdb_test "frame 0" ".*"
- set pc [get_hexadecimal_valueof "\$pc" "*UNKNOWN*"]
- if { $pc == $last_addr_in_foo } {
- break
- }
-
if { $i_count > 100 } {
# We expect a handful of instructions, if we reach 100,
# something is going wrong. Avoid an infinite loop.
@@ -170,6 +120,16 @@ for { set i_count 1 } { true } { incr i_count } {
break
}
- gdb_test "stepi" ".*"
+ set in_foo 0
+ gdb_test_multiple "stepi" "" {
+ -re -wrap "$hex in foo \\(\\)" {
+ set in_foo 1
+ }
+ -re -wrap "" {}
+ }
+
+ if { ! $in_foo } {
+ break
+ }
}
}
base-commit: bc0c6793fb45b491c6324614298a2880a34d3966
--
2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] [gdb/testsuite] Improve gdb.base/unwind-on-each-insn.exp
2023-01-19 10:46 [PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements Tom de Vries
2023-01-19 10:46 ` [PATCH 1/4] [gdb/testsuite] Simplify gdb.base/unwind-on-each-insn.exp Tom de Vries
@ 2023-01-19 10:46 ` Tom de Vries
2023-01-23 9:55 ` Luis Machado
2023-01-19 10:46 ` [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function Tom de Vries
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2023-01-19 10:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Bruno Larsen, Andrew Burgess, Luis Machado
In test-case gdb.base/unwind-on-each-insn.exp, we stepi through function foo
to check various invariants at each insn, in particular hoping to excercise
insns that modify stack and frame pointers.
For x86_64-linux, we have a reasonably complex foo (and similar for -m32):
...
4004bc: 55 push %rbp
4004bd: 48 89 e5 mov %rsp,%rbp
4004c0: 90 nop
4004c1: 5d pop %rbp
4004c2: c3 ret
...
Both stack pointer (%rsp) and frame pointer (%rbp) are modified.
Also for s390x-linux (and similar for -m31):
...
0000000001000668 <foo>:
1000668: e3 b0 f0 58 00 24 stg %r11,88(%r15)
100066e: b9 04 00 bf lgr %r11,%r15
1000672: e3 b0 b0 58 00 04 lg %r11,88(%r11)
1000678: 07 fe br %r14
...
Frame pointer (%r11) is modified.
Likewise for powerpc64le-linux:
...
00000000100006c8 <foo>:
100006c8: f8 ff e1 fb std r31,-8(r1)
100006cc: d1 ff 21 f8 stdu r1,-48(r1)
100006d0: 78 0b 3f 7c mr r31,r1
100006d4: 00 00 00 60 nop
100006d8: 30 00 3f 38 addi r1,r31,48
100006dc: f8 ff e1 eb ld r31,-8(r1)
100006e0: 20 00 80 4e blr
...
Both stack pointer (r1) and frame pointer (r31) are modified.
But for aarch64-linux, we have:
...
00000000004005fc <foo>:
4005fc: d503201f nop
400600: d65f03c0 ret
...
There's no insn that modifies stack or frame pointer.
Fix this by making foo more complex, adding an (unused) argument:
...
0000000000400604 <foo>:
400604: d10043ff sub sp, sp, #0x10
400608: f90007e0 str x0, [sp, #8]
40060c: d503201f nop
400610: 910043ff add sp, sp, #0x10
400614: d65f03c0 ret
...
such that the stack pointer (sp) is modified.
[ Note btw that we're seeing the effects of -momit-leaf-frame-pointer, with
-mno-omit-leaf-frame-pointer we have instead:
...
0000000000400604 <foo>:
400604: a9be7bfd stp x29, x30, [sp, #-32]!
400608: 910003fd mov x29, sp
40060c: f9000fa0 str x0, [x29, #24]
400610: d503201f nop
400614: a8c27bfd ldp x29, x30, [sp], #32
400618: d65f03c0 ret
...
such that also the frame pointer (x29) is modified. ]
Having made foo more complex, we now run into the following fail on
x86_64/-m32:
...
FAIL: gdb.base/unwind-on-each-insn.exp: instruction 1: $sp_value == $main_sp
....
The problem is that the stack pointer has changed inbetween the sampling of
main_sp and *foo, in particular by the push insn:
...
804841a: 68 c0 84 04 08 push $0x80484c0
804841f: e8 10 00 00 00 call 8048434 <foo>
...
Fix this by updating main_sp.
On aarch64-linux, we run into PR tdep/30011:
...
FAIL: gdb.base/unwind-on-each-insn.exp: insn 5: $fba_value == $foo_fba
FAIL: gdb.base/unwind-on-each-insn.exp: insn 5: check frame-id matches
FAIL: gdb.base/unwind-on-each-insn.exp: insn 5: $sp_value == $main_sp
...
On powerpc64le-line, we run into PR tdep/30021:
...
FAIL: gdb.base/unwind-on-each-insn.exp: insn 7: $fba_value == $main_fba
FAIL: gdb.base/unwind-on-each-insn.exp: insn 7: [string equal $fid $main_fid]
...
Tested on:
- x86_64-linux (-m64 and -m32)
- s390x-linux (-m64 and -m31)
- powerpc64le-linux
- aarch64-linux
Also I've checked that the test-case still functions as regression test for PR
record/16678 on x86_64/-m32.
---
gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c | 2 +-
gdb/testsuite/gdb.base/unwind-on-each-insn.c | 4 ++--
gdb/testsuite/gdb.base/unwind-on-each-insn.exp | 7 ++++++-
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c b/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
index af9fc11ddab..635aa44135e 100644
--- a/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
+++ b/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
@@ -16,7 +16,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
void
-foo ()
+foo (const char *s)
{
/* Nothing. */
}
diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.c b/gdb/testsuite/gdb.base/unwind-on-each-insn.c
index 60bc83a6d4f..821df375115 100644
--- a/gdb/testsuite/gdb.base/unwind-on-each-insn.c
+++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.c
@@ -15,11 +15,11 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-extern void foo ();
+extern void foo (const char *);
int
main ()
{
- foo ();
+ foo ("foo");
return 0;
}
diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
index 5e822effaf1..c61ffe5d174 100644
--- a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
+++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
@@ -103,7 +103,12 @@ for { set i_count 1 } { true } { incr i_count } {
# Check we can unwind the stack-pointer and the frame base
# address correctly.
lassign [get_sp_and_fba "for main"] sp_value fba_value
- gdb_assert { $sp_value == $main_sp }
+ if { $i_count == 1 } {
+ # The stack-pointer may have changed while running to *foo.
+ set main_sp $sp_value
+ } else {
+ gdb_assert { $sp_value == $main_sp }
+ }
gdb_assert { $fba_value == $main_fba }
# Check we have a consistent value for main's frame-id.
--
2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] [gdb/testsuite] Improve gdb.base/unwind-on-each-insn.exp
2023-01-19 10:46 ` [PATCH 2/4] [gdb/testsuite] Improve gdb.base/unwind-on-each-insn.exp Tom de Vries
@ 2023-01-23 9:55 ` Luis Machado
0 siblings, 0 replies; 13+ messages in thread
From: Luis Machado @ 2023-01-23 9:55 UTC (permalink / raw)
To: Tom de Vries, gdb-patches; +Cc: Bruno Larsen, Andrew Burgess
Hi Tom,
On 1/19/23 10:46, Tom de Vries wrote:
> In test-case gdb.base/unwind-on-each-insn.exp, we stepi through function foo
> to check various invariants at each insn, in particular hoping to excercise
> insns that modify stack and frame pointers.
>
> For x86_64-linux, we have a reasonably complex foo (and similar for -m32):
> ...
> 4004bc: 55 push %rbp
> 4004bd: 48 89 e5 mov %rsp,%rbp
> 4004c0: 90 nop
> 4004c1: 5d pop %rbp
> 4004c2: c3 ret
> ...
> Both stack pointer (%rsp) and frame pointer (%rbp) are modified.
>
> Also for s390x-linux (and similar for -m31):
> ...
> 0000000001000668 <foo>:
> 1000668: e3 b0 f0 58 00 24 stg %r11,88(%r15)
> 100066e: b9 04 00 bf lgr %r11,%r15
> 1000672: e3 b0 b0 58 00 04 lg %r11,88(%r11)
> 1000678: 07 fe br %r14
> ...
> Frame pointer (%r11) is modified.
>
> Likewise for powerpc64le-linux:
> ...
> 00000000100006c8 <foo>:
> 100006c8: f8 ff e1 fb std r31,-8(r1)
> 100006cc: d1 ff 21 f8 stdu r1,-48(r1)
> 100006d0: 78 0b 3f 7c mr r31,r1
> 100006d4: 00 00 00 60 nop
> 100006d8: 30 00 3f 38 addi r1,r31,48
> 100006dc: f8 ff e1 eb ld r31,-8(r1)
> 100006e0: 20 00 80 4e blr
> ...
> Both stack pointer (r1) and frame pointer (r31) are modified.
>
> But for aarch64-linux, we have:
> ...
> 00000000004005fc <foo>:
> 4005fc: d503201f nop
> 400600: d65f03c0 ret
> ...
> There's no insn that modifies stack or frame pointer.
>
> Fix this by making foo more complex, adding an (unused) argument:
> ...
> 0000000000400604 <foo>:
> 400604: d10043ff sub sp, sp, #0x10
> 400608: f90007e0 str x0, [sp, #8]
> 40060c: d503201f nop
> 400610: 910043ff add sp, sp, #0x10
> 400614: d65f03c0 ret
> ...
> such that the stack pointer (sp) is modified.
>
> [ Note btw that we're seeing the effects of -momit-leaf-frame-pointer, with
> -mno-omit-leaf-frame-pointer we have instead:
> ...
> 0000000000400604 <foo>:
> 400604: a9be7bfd stp x29, x30, [sp, #-32]!
> 400608: 910003fd mov x29, sp
> 40060c: f9000fa0 str x0, [x29, #24]
> 400610: d503201f nop
> 400614: a8c27bfd ldp x29, x30, [sp], #32
> 400618: d65f03c0 ret
> ...
> such that also the frame pointer (x29) is modified. ]
>
> Having made foo more complex, we now run into the following fail on
> x86_64/-m32:
> ...
> FAIL: gdb.base/unwind-on-each-insn.exp: instruction 1: $sp_value == $main_sp
> ....
>
> The problem is that the stack pointer has changed inbetween the sampling of
> main_sp and *foo, in particular by the push insn:
> ...
> 804841a: 68 c0 84 04 08 push $0x80484c0
> 804841f: e8 10 00 00 00 call 8048434 <foo>
> ...
>
> Fix this by updating main_sp.
>
> On aarch64-linux, we run into PR tdep/30011:
> ...
> FAIL: gdb.base/unwind-on-each-insn.exp: insn 5: $fba_value == $foo_fba
> FAIL: gdb.base/unwind-on-each-insn.exp: insn 5: check frame-id matches
> FAIL: gdb.base/unwind-on-each-insn.exp: insn 5: $sp_value == $main_sp
> ...
>
> On powerpc64le-line, we run into PR tdep/30021:
> ...
> FAIL: gdb.base/unwind-on-each-insn.exp: insn 7: $fba_value == $main_fba
> FAIL: gdb.base/unwind-on-each-insn.exp: insn 7: [string equal $fid $main_fid]
> ...
>
> Tested on:
> - x86_64-linux (-m64 and -m32)
> - s390x-linux (-m64 and -m31)
> - powerpc64le-linux
> - aarch64-linux
>
> Also I've checked that the test-case still functions as regression test for PR
> record/16678 on x86_64/-m32.
> ---
> gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c | 2 +-
> gdb/testsuite/gdb.base/unwind-on-each-insn.c | 4 ++--
> gdb/testsuite/gdb.base/unwind-on-each-insn.exp | 7 ++++++-
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c b/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
> index af9fc11ddab..635aa44135e 100644
> --- a/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
> +++ b/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
> @@ -16,7 +16,7 @@
> along with this program. If not, see <http://www.gnu.org/licenses/>. */
>
> void
> -foo ()
> +foo (const char *s)
> {
> /* Nothing. */
> }
> diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.c b/gdb/testsuite/gdb.base/unwind-on-each-insn.c
> index 60bc83a6d4f..821df375115 100644
> --- a/gdb/testsuite/gdb.base/unwind-on-each-insn.c
> +++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.c
> @@ -15,11 +15,11 @@
> You should have received a copy of the GNU General Public License
> along with this program. If not, see <http://www.gnu.org/licenses/>. */
>
> -extern void foo ();
> +extern void foo (const char *);
>
> int
> main ()
> {
> - foo ();
> + foo ("foo");
> return 0;
> }
> diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
> index 5e822effaf1..c61ffe5d174 100644
> --- a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
> +++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
> @@ -103,7 +103,12 @@ for { set i_count 1 } { true } { incr i_count } {
> # Check we can unwind the stack-pointer and the frame base
> # address correctly.
> lassign [get_sp_and_fba "for main"] sp_value fba_value
> - gdb_assert { $sp_value == $main_sp }
> + if { $i_count == 1 } {
> + # The stack-pointer may have changed while running to *foo.
> + set main_sp $sp_value
> + } else {
> + gdb_assert { $sp_value == $main_sp }
> + }
> gdb_assert { $fba_value == $main_fba }
>
> # Check we have a consistent value for main's frame-id.
This is great. Thanks for improving it.
From aarch64's side, this LGTM.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function
2023-01-19 10:46 [PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements Tom de Vries
2023-01-19 10:46 ` [PATCH 1/4] [gdb/testsuite] Simplify gdb.base/unwind-on-each-insn.exp Tom de Vries
2023-01-19 10:46 ` [PATCH 2/4] [gdb/testsuite] Improve gdb.base/unwind-on-each-insn.exp Tom de Vries
@ 2023-01-19 10:46 ` Tom de Vries
2023-01-20 10:25 ` Tom de Vries
2023-01-23 10:07 ` Luis Machado
2023-01-19 10:46 ` [PATCH 4/4] [gdb/testsuite] Analyze non-leaf fn in gdb.base/unwind-on-each-insn.exp Tom de Vries
2023-01-25 12:32 ` [PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements Tom de Vries
4 siblings, 2 replies; 13+ messages in thread
From: Tom de Vries @ 2023-01-19 10:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Bruno Larsen, Andrew Burgess, Luis Machado
Consider the test-case test.c, compiled without debug info:
...
void
foo (const char *s)
{
}
int
main (void)
{
foo ("foo");
return 0;
}
...
Disassembly of foo:
...
0000000000400564 <foo>:
400564: d10043ff sub sp, sp, #0x10
400568: f90007e0 str x0, [sp, #8]
40056c: d503201f nop
400570: 910043ff add sp, sp, #0x10
400574: d65f03c0 ret
...
Now, let's do "info frame" at each insn in foo, as well as printing $sp
and $x29 (and strip the output of info frame to the first line, for brevity):
...
$ gdb -q a.out
Reading symbols from a.out...
(gdb) b *foo
Breakpoint 1 at 0x400564
(gdb) r
Starting program: a.out
Breakpoint 1, 0x0000000000400564 in foo ()
(gdb) display /x $sp
1: /x $sp = 0xfffffffff3a0
(gdb) display /x $x29
2: /x $x29 = 0xfffffffff3a0
(gdb) info frame
Stack level 0, frame at 0xfffffffff3a0:
(gdb) si
0x0000000000400568 in foo ()
1: /x $sp = 0xfffffffff390
2: /x $x29 = 0xfffffffff3a0
(gdb) info frame
Stack level 0, frame at 0xfffffffff3a0:
(gdb) si
0x000000000040056c in foo ()
1: /x $sp = 0xfffffffff390
2: /x $x29 = 0xfffffffff3a0
(gdb) info frame
Stack level 0, frame at 0xfffffffff3a0:
(gdb) si
0x0000000000400570 in foo ()
1: /x $sp = 0xfffffffff390
2: /x $x29 = 0xfffffffff3a0
(gdb) info frame
Stack level 0, frame at 0xfffffffff3a0:
(gdb) si
0x0000000000400574 in foo ()
1: /x $sp = 0xfffffffff3a0
2: /x $x29 = 0xfffffffff3a0
(gdb) info frame
Stack level 0, frame at 0xfffffffff3b0:
pc = 0x400574 in foo; saved pc = 0x40058c
(gdb) si
0x000000000040058c in main ()
1: /x $sp = 0xfffffffff3a0
2: /x $x29 = 0xfffffffff3a0
...
The "frame at" bit lists 0xfffffffff3a0 except at the last insn, where it
lists 0xfffffffff3b0.
The frame address is calculated here in aarch64_make_prologue_cache_1:
...
unwound_fp = get_frame_register_unsigned (this_frame, cache->framereg);
if (unwound_fp == 0)
return;
cache->prev_sp = unwound_fp + cache->framesize;
...
For insns after the prologue, we have cache->framereg == sp and
cache->framesize == 16, so unwound_fp + cache->framesize gives the wrong
answer once sp has been restored to entry value by the before-last insn.
Fix this by detecting the situation that the sp has been restored.
This fixes PR tdep/30011.
This also fixes the aarch64 FAILs in gdb.reverse/solib-precsave.exp and
gdb.reverse/solib-reverse.exp I reported in PR gdb/PR29721.
Tested on aarch64-linux.
PR tdep/30011
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30011
---
gdb/aarch64-tdep.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index b576d3b9d99..06349353716 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -996,7 +996,11 @@ aarch64_make_prologue_cache_1 (frame_info_ptr this_frame,
if (unwound_fp == 0)
return;
- cache->prev_sp = unwound_fp + cache->framesize;
+ if (cache->framereg == AARCH64_SP_REGNUM
+ && get_frame_register_unsigned (this_frame, AARCH64_FP_REGNUM) == unwound_fp)
+ cache->prev_sp = unwound_fp;
+ else
+ cache->prev_sp = unwound_fp + cache->framesize;
/* Calculate actual addresses of saved registers using offsets
determined by aarch64_analyze_prologue. */
--
2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function
2023-01-19 10:46 ` [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function Tom de Vries
@ 2023-01-20 10:25 ` Tom de Vries
2023-01-23 10:07 ` Luis Machado
1 sibling, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2023-01-20 10:25 UTC (permalink / raw)
To: gdb-patches; +Cc: Bruno Larsen, Andrew Burgess, Luis Machado
On 1/19/23 11:46, Tom de Vries via Gdb-patches wrote:
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index b576d3b9d99..06349353716 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -996,7 +996,11 @@ aarch64_make_prologue_cache_1 (frame_info_ptr this_frame,
> if (unwound_fp == 0)
> return;
>
> - cache->prev_sp = unwound_fp + cache->framesize;
> + if (cache->framereg == AARCH64_SP_REGNUM
> + && get_frame_register_unsigned (this_frame, AARCH64_FP_REGNUM) == unwound_fp)
> + cache->prev_sp = unwound_fp;
> + else
> + cache->prev_sp = unwound_fp + cache->framesize;
>
> /* Calculate actual addresses of saved registers using offsets
> determined by aarch64_analyze_prologue. */
I came across the aarch64 version of stack_frame_destroyed_p, and
realized I can do the fix like this:
...
@@ -999,7 +1001,10 @@ aarch64_make_prologue_cache_1 (frame_info_ptr
this_frame,
if (unwound_fp == 0)
return;
- cache->prev_sp = unwound_fp + cache->framesize;
+ cache->prev_sp = unwound_fp;
+ if (!aarch64_stack_frame_destroyed_p (get_frame_arch (this_frame),
+ cache->prev_pc))
+ cache->prev_sp += cache->framesize;
/* Calculate actual addresses of saved registers using offsets
determined by aarch64_analyze_prologue. */
...
This fixes both the leaf and non-leaf case.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function
2023-01-19 10:46 ` [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function Tom de Vries
2023-01-20 10:25 ` Tom de Vries
@ 2023-01-23 10:07 ` Luis Machado
2023-01-23 11:59 ` Tom de Vries
1 sibling, 1 reply; 13+ messages in thread
From: Luis Machado @ 2023-01-23 10:07 UTC (permalink / raw)
To: Tom de Vries, gdb-patches; +Cc: Bruno Larsen, Andrew Burgess
On 1/19/23 10:46, Tom de Vries wrote:
> Consider the test-case test.c, compiled without debug info:
> ...
> void
> foo (const char *s)
> {
> }
>
> int
> main (void)
> {
> foo ("foo");
> return 0;
> }
> ...
>
> Disassembly of foo:
> ...
> 0000000000400564 <foo>:
> 400564: d10043ff sub sp, sp, #0x10
> 400568: f90007e0 str x0, [sp, #8]
> 40056c: d503201f nop
> 400570: 910043ff add sp, sp, #0x10
> 400574: d65f03c0 ret
> ...
>
> Now, let's do "info frame" at each insn in foo, as well as printing $sp
> and $x29 (and strip the output of info frame to the first line, for brevity):
> ...
> $ gdb -q a.out
> Reading symbols from a.out...
> (gdb) b *foo
> Breakpoint 1 at 0x400564
> (gdb) r
> Starting program: a.out
>
> Breakpoint 1, 0x0000000000400564 in foo ()
> (gdb) display /x $sp
> 1: /x $sp = 0xfffffffff3a0
> (gdb) display /x $x29
> 2: /x $x29 = 0xfffffffff3a0
> (gdb) info frame
> Stack level 0, frame at 0xfffffffff3a0:
> (gdb) si
> 0x0000000000400568 in foo ()
> 1: /x $sp = 0xfffffffff390
> 2: /x $x29 = 0xfffffffff3a0
> (gdb) info frame
> Stack level 0, frame at 0xfffffffff3a0:
> (gdb) si
> 0x000000000040056c in foo ()
> 1: /x $sp = 0xfffffffff390
> 2: /x $x29 = 0xfffffffff3a0
> (gdb) info frame
> Stack level 0, frame at 0xfffffffff3a0:
> (gdb) si
> 0x0000000000400570 in foo ()
> 1: /x $sp = 0xfffffffff390
> 2: /x $x29 = 0xfffffffff3a0
> (gdb) info frame
> Stack level 0, frame at 0xfffffffff3a0:
> (gdb) si
> 0x0000000000400574 in foo ()
> 1: /x $sp = 0xfffffffff3a0
> 2: /x $x29 = 0xfffffffff3a0
> (gdb) info frame
> Stack level 0, frame at 0xfffffffff3b0:
> pc = 0x400574 in foo; saved pc = 0x40058c
> (gdb) si
> 0x000000000040058c in main ()
> 1: /x $sp = 0xfffffffff3a0
> 2: /x $x29 = 0xfffffffff3a0
> ...
>
> The "frame at" bit lists 0xfffffffff3a0 except at the last insn, where it
> lists 0xfffffffff3b0.
>
> The frame address is calculated here in aarch64_make_prologue_cache_1:
> ...
> unwound_fp = get_frame_register_unsigned (this_frame, cache->framereg);
> if (unwound_fp == 0)
> return;
>
> cache->prev_sp = unwound_fp + cache->framesize;
> ...
>
> For insns after the prologue, we have cache->framereg == sp and
> cache->framesize == 16, so unwound_fp + cache->framesize gives the wrong
> answer once sp has been restored to entry value by the before-last insn.
>
> Fix this by detecting the situation that the sp has been restored.
>
> This fixes PR tdep/30011.
>
> This also fixes the aarch64 FAILs in gdb.reverse/solib-precsave.exp and
> gdb.reverse/solib-reverse.exp I reported in PR gdb/PR29721.
I still see failures for gdb.reverse/solib-precsave.exp and gdb.reverse/solib-reverse.exp for both Ubuntu 22.04 and 20.04
on aarch64-linux.
Running /work/luimac01/work/builds/binutils-gdb-arm64-jammy/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.reverse/solib-prec
save.exp ...
FAIL: gdb.reverse/solib-precsave.exp: reverse-step into solib function one
FAIL: gdb.reverse/solib-precsave.exp: reverse-step within solib function one
FAIL: gdb.reverse/solib-precsave.exp: reverse-step back to main one
FAIL: gdb.reverse/solib-precsave.exp: reverse-step into solib function two
FAIL: gdb.reverse/solib-precsave.exp: reverse-step within solib function two
Running /work/luimac01/work/builds/binutils-gdb-arm64-jammy/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.reverse/solib-reve
rse.exp ...
FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function one
FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function one
FAIL: gdb.reverse/solib-reverse.exp: reverse-step back to main one
FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function two
FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function two
Maybe it addresses a different issue, but what I'm seeing is possibly something else (the linetable issue? I vaguely recall the situation for that).
>
> Tested on aarch64-linux.
> PR tdep/30011
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30011
> ---
> gdb/aarch64-tdep.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index b576d3b9d99..06349353716 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -996,7 +996,11 @@ aarch64_make_prologue_cache_1 (frame_info_ptr this_frame,
> if (unwound_fp == 0)
> return;
>
> - cache->prev_sp = unwound_fp + cache->framesize;
> + if (cache->framereg == AARCH64_SP_REGNUM
> + && get_frame_register_unsigned (this_frame, AARCH64_FP_REGNUM) == unwound_fp)
> + cache->prev_sp = unwound_fp;
> + else
> + cache->prev_sp = unwound_fp + cache->framesize;
>
> /* Calculate actual addresses of saved registers using offsets
> determined by aarch64_analyze_prologue. */
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function
2023-01-23 10:07 ` Luis Machado
@ 2023-01-23 11:59 ` Tom de Vries
2023-01-23 12:09 ` Luis Machado
0 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2023-01-23 11:59 UTC (permalink / raw)
To: Luis Machado, gdb-patches; +Cc: Bruno Larsen, Andrew Burgess
On 1/23/23 11:07, Luis Machado wrote:
> On 1/19/23 10:46, Tom de Vries wrote:
>> Consider the test-case test.c, compiled without debug info:
>> ...
>> void
>> foo (const char *s)
>> {
>> }
>>
>> int
>> main (void)
>> {
>> foo ("foo");
>> return 0;
>> }
>> ...
>>
>> Disassembly of foo:
>> ...
>> 0000000000400564 <foo>:
>> 400564: d10043ff sub sp, sp, #0x10
>> 400568: f90007e0 str x0, [sp, #8]
>> 40056c: d503201f nop
>> 400570: 910043ff add sp, sp, #0x10
>> 400574: d65f03c0 ret
>> ...
>>
>> Now, let's do "info frame" at each insn in foo, as well as printing $sp
>> and $x29 (and strip the output of info frame to the first line, for
>> brevity):
>> ...
>> $ gdb -q a.out
>> Reading symbols from a.out...
>> (gdb) b *foo
>> Breakpoint 1 at 0x400564
>> (gdb) r
>> Starting program: a.out
>>
>> Breakpoint 1, 0x0000000000400564 in foo ()
>> (gdb) display /x $sp
>> 1: /x $sp = 0xfffffffff3a0
>> (gdb) display /x $x29
>> 2: /x $x29 = 0xfffffffff3a0
>> (gdb) info frame
>> Stack level 0, frame at 0xfffffffff3a0:
>> (gdb) si
>> 0x0000000000400568 in foo ()
>> 1: /x $sp = 0xfffffffff390
>> 2: /x $x29 = 0xfffffffff3a0
>> (gdb) info frame
>> Stack level 0, frame at 0xfffffffff3a0:
>> (gdb) si
>> 0x000000000040056c in foo ()
>> 1: /x $sp = 0xfffffffff390
>> 2: /x $x29 = 0xfffffffff3a0
>> (gdb) info frame
>> Stack level 0, frame at 0xfffffffff3a0:
>> (gdb) si
>> 0x0000000000400570 in foo ()
>> 1: /x $sp = 0xfffffffff390
>> 2: /x $x29 = 0xfffffffff3a0
>> (gdb) info frame
>> Stack level 0, frame at 0xfffffffff3a0:
>> (gdb) si
>> 0x0000000000400574 in foo ()
>> 1: /x $sp = 0xfffffffff3a0
>> 2: /x $x29 = 0xfffffffff3a0
>> (gdb) info frame
>> Stack level 0, frame at 0xfffffffff3b0:
>> pc = 0x400574 in foo; saved pc = 0x40058c
>> (gdb) si
>> 0x000000000040058c in main ()
>> 1: /x $sp = 0xfffffffff3a0
>> 2: /x $x29 = 0xfffffffff3a0
>> ...
>>
>> The "frame at" bit lists 0xfffffffff3a0 except at the last insn, where it
>> lists 0xfffffffff3b0.
>>
>> The frame address is calculated here in aarch64_make_prologue_cache_1:
>> ...
>> unwound_fp = get_frame_register_unsigned (this_frame,
>> cache->framereg);
>> if (unwound_fp == 0)
>> return;
>>
>> cache->prev_sp = unwound_fp + cache->framesize;
>> ...
>>
>> For insns after the prologue, we have cache->framereg == sp and
>> cache->framesize == 16, so unwound_fp + cache->framesize gives the wrong
>> answer once sp has been restored to entry value by the before-last insn.
>>
>> Fix this by detecting the situation that the sp has been restored.
>>
>> This fixes PR tdep/30011.
>>
>> This also fixes the aarch64 FAILs in gdb.reverse/solib-precsave.exp and
>> gdb.reverse/solib-reverse.exp I reported in PR gdb/PR29721.
>
> I still see failures for gdb.reverse/solib-precsave.exp and
> gdb.reverse/solib-reverse.exp for both Ubuntu 22.04 and 20.04
> on aarch64-linux.
>
> Running
> /work/luimac01/work/builds/binutils-gdb-arm64-jammy/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.reverse/solib-prec
> save.exp ...
> FAIL: gdb.reverse/solib-precsave.exp: reverse-step into solib function one
> FAIL: gdb.reverse/solib-precsave.exp: reverse-step within solib function
> one
> FAIL: gdb.reverse/solib-precsave.exp: reverse-step back to main one
> FAIL: gdb.reverse/solib-precsave.exp: reverse-step into solib function two
> FAIL: gdb.reverse/solib-precsave.exp: reverse-step within solib function
> two
>
> Running
> /work/luimac01/work/builds/binutils-gdb-arm64-jammy/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.reverse/solib-reve
> rse.exp ...
> FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function one
> FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function one
> FAIL: gdb.reverse/solib-reverse.exp: reverse-step back to main one
> FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function two
> FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function two
>
> Maybe it addresses a different issue, but what I'm seeing is possibly
> something else (the linetable issue? I vaguely recall the situation for
> that).
>>
Hi,
that is very well possible. I'm not claiming to fix the test-case on
aarch64 in general, I'm very specifically claiming to fix the FAILs I
reported in a PR.
BTW the first FAIL in the PR is also different than the one you report
above, which is usually a hint that there may be a different root cause.
I'll commit (using an updated commit message claiming both PRs
tdep/30010 and tdep/30011) once I do another round of testing.
Thanks,
- Tom
>> Tested on aarch64-linux.
>> PR tdep/30011
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30011
>> ---
>> gdb/aarch64-tdep.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index b576d3b9d99..06349353716 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -996,7 +996,11 @@ aarch64_make_prologue_cache_1 (frame_info_ptr
>> this_frame,
>> if (unwound_fp == 0)
>> return;
>> - cache->prev_sp = unwound_fp + cache->framesize;
>> + if (cache->framereg == AARCH64_SP_REGNUM
>> + && get_frame_register_unsigned (this_frame, AARCH64_FP_REGNUM)
>> == unwound_fp)
>> + cache->prev_sp = unwound_fp;
>> + else
>> + cache->prev_sp = unwound_fp + cache->framesize;
>> /* Calculate actual addresses of saved registers using offsets
>> determined by aarch64_analyze_prologue. */
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function
2023-01-23 11:59 ` Tom de Vries
@ 2023-01-23 12:09 ` Luis Machado
0 siblings, 0 replies; 13+ messages in thread
From: Luis Machado @ 2023-01-23 12:09 UTC (permalink / raw)
To: Tom de Vries, gdb-patches; +Cc: Bruno Larsen, Andrew Burgess
On 1/23/23 11:59, Tom de Vries wrote:
> On 1/23/23 11:07, Luis Machado wrote:
>> On 1/19/23 10:46, Tom de Vries wrote:
>>> Consider the test-case test.c, compiled without debug info:
>>> ...
>>> void
>>> foo (const char *s)
>>> {
>>> }
>>>
>>> int
>>> main (void)
>>> {
>>> foo ("foo");
>>> return 0;
>>> }
>>> ...
>>>
>>> Disassembly of foo:
>>> ...
>>> 0000000000400564 <foo>:
>>> 400564: d10043ff sub sp, sp, #0x10
>>> 400568: f90007e0 str x0, [sp, #8]
>>> 40056c: d503201f nop
>>> 400570: 910043ff add sp, sp, #0x10
>>> 400574: d65f03c0 ret
>>> ...
>>>
>>> Now, let's do "info frame" at each insn in foo, as well as printing $sp
>>> and $x29 (and strip the output of info frame to the first line, for brevity):
>>> ...
>>> $ gdb -q a.out
>>> Reading symbols from a.out...
>>> (gdb) b *foo
>>> Breakpoint 1 at 0x400564
>>> (gdb) r
>>> Starting program: a.out
>>>
>>> Breakpoint 1, 0x0000000000400564 in foo ()
>>> (gdb) display /x $sp
>>> 1: /x $sp = 0xfffffffff3a0
>>> (gdb) display /x $x29
>>> 2: /x $x29 = 0xfffffffff3a0
>>> (gdb) info frame
>>> Stack level 0, frame at 0xfffffffff3a0:
>>> (gdb) si
>>> 0x0000000000400568 in foo ()
>>> 1: /x $sp = 0xfffffffff390
>>> 2: /x $x29 = 0xfffffffff3a0
>>> (gdb) info frame
>>> Stack level 0, frame at 0xfffffffff3a0:
>>> (gdb) si
>>> 0x000000000040056c in foo ()
>>> 1: /x $sp = 0xfffffffff390
>>> 2: /x $x29 = 0xfffffffff3a0
>>> (gdb) info frame
>>> Stack level 0, frame at 0xfffffffff3a0:
>>> (gdb) si
>>> 0x0000000000400570 in foo ()
>>> 1: /x $sp = 0xfffffffff390
>>> 2: /x $x29 = 0xfffffffff3a0
>>> (gdb) info frame
>>> Stack level 0, frame at 0xfffffffff3a0:
>>> (gdb) si
>>> 0x0000000000400574 in foo ()
>>> 1: /x $sp = 0xfffffffff3a0
>>> 2: /x $x29 = 0xfffffffff3a0
>>> (gdb) info frame
>>> Stack level 0, frame at 0xfffffffff3b0:
>>> pc = 0x400574 in foo; saved pc = 0x40058c
>>> (gdb) si
>>> 0x000000000040058c in main ()
>>> 1: /x $sp = 0xfffffffff3a0
>>> 2: /x $x29 = 0xfffffffff3a0
>>> ...
>>>
>>> The "frame at" bit lists 0xfffffffff3a0 except at the last insn, where it
>>> lists 0xfffffffff3b0.
>>>
>>> The frame address is calculated here in aarch64_make_prologue_cache_1:
>>> ...
>>> unwound_fp = get_frame_register_unsigned (this_frame, cache->framereg);
>>> if (unwound_fp == 0)
>>> return;
>>>
>>> cache->prev_sp = unwound_fp + cache->framesize;
>>> ...
>>>
>>> For insns after the prologue, we have cache->framereg == sp and
>>> cache->framesize == 16, so unwound_fp + cache->framesize gives the wrong
>>> answer once sp has been restored to entry value by the before-last insn.
>>>
>>> Fix this by detecting the situation that the sp has been restored.
>>>
>>> This fixes PR tdep/30011.
>>>
>>> This also fixes the aarch64 FAILs in gdb.reverse/solib-precsave.exp and
>>> gdb.reverse/solib-reverse.exp I reported in PR gdb/PR29721.
>>
>> I still see failures for gdb.reverse/solib-precsave.exp and gdb.reverse/solib-reverse.exp for both Ubuntu 22.04 and 20.04
>> on aarch64-linux.
>>
>> Running /work/luimac01/work/builds/binutils-gdb-arm64-jammy/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.reverse/solib-prec
>> save.exp ...
>> FAIL: gdb.reverse/solib-precsave.exp: reverse-step into solib function one
>> FAIL: gdb.reverse/solib-precsave.exp: reverse-step within solib function one
>> FAIL: gdb.reverse/solib-precsave.exp: reverse-step back to main one
>> FAIL: gdb.reverse/solib-precsave.exp: reverse-step into solib function two
>> FAIL: gdb.reverse/solib-precsave.exp: reverse-step within solib function two
>>
>> Running /work/luimac01/work/builds/binutils-gdb-arm64-jammy/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.reverse/solib-reve
>> rse.exp ...
>> FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function one
>> FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function one
>> FAIL: gdb.reverse/solib-reverse.exp: reverse-step back to main one
>> FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function two
>> FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function two
>>
>> Maybe it addresses a different issue, but what I'm seeing is possibly something else (the linetable issue? I vaguely recall the situation for that).
>>>
>
> Hi,
>
> that is very well possible. I'm not claiming to fix the test-case on aarch64 in general, I'm very specifically claiming to fix the FAILs I reported in a PR.
>
> BTW the first FAIL in the PR is also different than the one you report above, which is usually a hint that there may be a different root cause.
>
> I'll commit (using an updated commit message claiming both PRs tdep/30010 and tdep/30011) once I do another round of testing.
>
> Thanks,
> - Tom
>
Sounds good to me. Thanks for the patch.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] [gdb/testsuite] Analyze non-leaf fn in gdb.base/unwind-on-each-insn.exp
2023-01-19 10:46 [PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements Tom de Vries
` (2 preceding siblings ...)
2023-01-19 10:46 ` [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function Tom de Vries
@ 2023-01-19 10:46 ` Tom de Vries
2023-01-23 10:18 ` Luis Machado
2023-01-25 12:32 ` [PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements Tom de Vries
4 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2023-01-19 10:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Bruno Larsen, Andrew Burgess, Luis Machado
In test-case gdb.base/unwind-on-each-insn.exp, we stepi through function foo
to check various invariants at each insn, in particular hoping to excercise
insns that modify stack and frame pointers.
Function foo is a leaf function, so add a non-leaf function bar, and step
through it as well (using nexti instead of stepi).
On aarch64-linux, we run into PR tdep/30010:
...
FAIL: unwind-on-each-insn.exp: bar: insn 8: $fba_value == $fn_fba
FAIL: unwind-on-each-insn.exp: bar: insn 8: check frame-id matches
FAIL: unwind-on-each-insn.exp: bar: insn 8: bt 2
FAIL: unwind-on-each-insn.exp: bar: insn 8: up
FAIL: unwind-on-each-insn.exp: bar: insn 8: $sp_value == $::main_sp
FAIL: unwind-on-each-insn.exp: bar: insn 8: $fba_value == $::main_fba
FAIL: unwind-on-each-insn.exp: bar: insn 8: [string equal $fid $::main_fid]
...
Tested on:
- x86_64-linux (-m64 and -m32)
- s390x-linux (-m64 and -m31)
- aarch64-linux
---
.../gdb.base/unwind-on-each-insn-foo.c | 6 +
gdb/testsuite/gdb.base/unwind-on-each-insn.c | 2 +
.../gdb.base/unwind-on-each-insn.exp | 132 ++++++++++--------
3 files changed, 81 insertions(+), 59 deletions(-)
diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c b/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
index 635aa44135e..4a3b2946a3b 100644
--- a/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
+++ b/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
@@ -20,3 +20,9 @@ foo (const char *s)
{
/* Nothing. */
}
+
+void
+bar (const char *s)
+{
+ foo (s);
+}
diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.c b/gdb/testsuite/gdb.base/unwind-on-each-insn.c
index 821df375115..4aabc651c7b 100644
--- a/gdb/testsuite/gdb.base/unwind-on-each-insn.c
+++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.c
@@ -16,10 +16,12 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
extern void foo (const char *);
+extern void bar (const char *);
int
main ()
{
foo ("foo");
+ bar ("bar");
return 0;
}
diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
index c61ffe5d174..c8e3f95e63a 100644
--- a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
+++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
@@ -69,72 +69,86 @@ proc get_fid { } {
lassign [get_sp_and_fba "in main"] main_sp main_fba
set main_fid [get_fid]
-# Now enter the foo function.
-gdb_breakpoint "*foo"
-gdb_continue_to_breakpoint "enter foo"
+proc do_test { function step_cmd } {
+ # Now enter the function.
+ gdb_breakpoint "*$function"
+ gdb_continue_to_breakpoint "enter $function"
+ delete_breakpoints
+
+ # Record the current stack-pointer, and the frame base address.
+ lassign [get_sp_and_fba "in $function"] fn_sp fn_fba
+ set fn_fid [get_fid]
+
+ for { set i_count 1 } { true } { incr i_count } {
+ with_test_prefix "instruction ${i_count}" {
+
+ # The current stack-pointer value can legitimately change
+ # throughout the lifetime of a function, so we don't check the
+ # current stack-pointer value. But the frame base address
+ # should not change, so we do check for that.
+ lassign [get_sp_and_fba "for fn"] sp_value fba_value
+ gdb_assert { $fba_value == $fn_fba }
+
+ # The frame-id should never change within a function, so check
+ # that now.
+ set fid [get_fid]
+ gdb_assert { [string equal $fid $fn_fid] } \
+ "check frame-id matches"
+
+ # Check that the previous frame is 'main'.
+ gdb_test "bt 2" "\r\n#1\\s+\[^\r\n\]+ in main \\(\\) .*"
+
+ # Move up the stack (to main).
+ gdb_test "up" \
+ "\r\n#1\\s+\[^\r\n\]+ in main \\(\\) .*"
+
+ # Check we can unwind the stack-pointer and the frame base
+ # address correctly.
+ lassign [get_sp_and_fba "for main"] sp_value fba_value
+ if { $i_count == 1 } {
+ # The stack-pointer may have changed while running to *$function.
+ set ::main_sp $sp_value
+ } else {
+ gdb_assert { $sp_value == $::main_sp }
+ }
+ gdb_assert { $fba_value == $::main_fba }
-# Record the current stack-pointer, and the frame base address.
-lassign [get_sp_and_fba "in foo"] foo_sp foo_fba
-set foo_fid [get_fid]
-
-for { set i_count 1 } { true } { incr i_count } {
- with_test_prefix "instruction ${i_count}" {
-
- # The current stack-pointer value can legitimately change
- # throughout the lifetime of a function, so we don't check the
- # current stack-pointer value. But the frame base address
- # should not change, so we do check for that.
- lassign [get_sp_and_fba "for foo"] sp_value fba_value
- gdb_assert { $fba_value == $foo_fba }
-
- # The frame-id should never change within a function, so check
- # that now.
- set fid [get_fid]
- gdb_assert { [string equal $fid $foo_fid] } \
- "check frame-id matches"
-
- # Check that the previous frame is 'main'.
- gdb_test "bt 2" "\r\n#1\\s+\[^\r\n\]+ in main \\(\\) .*"
-
- # Move up the stack (to main).
- gdb_test "up" \
- "\r\n#1\\s+\[^\r\n\]+ in main \\(\\) .*"
-
- # Check we can unwind the stack-pointer and the frame base
- # address correctly.
- lassign [get_sp_and_fba "for main"] sp_value fba_value
- if { $i_count == 1 } {
- # The stack-pointer may have changed while running to *foo.
- set main_sp $sp_value
- } else {
- gdb_assert { $sp_value == $main_sp }
- }
- gdb_assert { $fba_value == $main_fba }
+ # Check we have a consistent value for main's frame-id.
+ set fid [get_fid]
+ gdb_assert { [string equal $fid $::main_fid] }
- # Check we have a consistent value for main's frame-id.
- set fid [get_fid]
- gdb_assert { [string equal $fid $main_fid] }
+ # Move back to the inner most frame.
+ gdb_test "frame 0" ".*"
- # Move back to the inner most frame.
- gdb_test "frame 0" ".*"
+ if { $i_count > 100 } {
+ # We expect a handful of instructions, if we reach 100,
+ # something is going wrong. Avoid an infinite loop.
+ fail "exceeded max number of instructions"
+ break
+ }
- if { $i_count > 100 } {
- # We expect a handful of instructions, if we reach 100,
- # something is going wrong. Avoid an infinite loop.
- fail "exceeded max number of instructions"
- break
- }
+ set in_fn 0
+ gdb_test_multiple $step_cmd "" {
+ -re -wrap "$::hex in $function \\(\\)" {
+ set in_fn 1
+ }
+ -re -wrap "" {}
+ }
- set in_foo 0
- gdb_test_multiple "stepi" "" {
- -re -wrap "$hex in foo \\(\\)" {
- set in_foo 1
+ if { ! $in_fn } {
+ break
}
- -re -wrap "" {}
}
+ }
+}
- if { ! $in_foo } {
- break
- }
+foreach {
+ function step_cmd
+} {
+ foo stepi
+ bar nexti
+} {
+ with_test_prefix $function {
+ do_test $function $step_cmd
}
}
--
2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] [gdb/testsuite] Analyze non-leaf fn in gdb.base/unwind-on-each-insn.exp
2023-01-19 10:46 ` [PATCH 4/4] [gdb/testsuite] Analyze non-leaf fn in gdb.base/unwind-on-each-insn.exp Tom de Vries
@ 2023-01-23 10:18 ` Luis Machado
0 siblings, 0 replies; 13+ messages in thread
From: Luis Machado @ 2023-01-23 10:18 UTC (permalink / raw)
To: Tom de Vries, gdb-patches; +Cc: Bruno Larsen, Andrew Burgess
On 1/19/23 10:46, Tom de Vries wrote:
> In test-case gdb.base/unwind-on-each-insn.exp, we stepi through function foo
> to check various invariants at each insn, in particular hoping to excercise
> insns that modify stack and frame pointers.
>
> Function foo is a leaf function, so add a non-leaf function bar, and step
> through it as well (using nexti instead of stepi).
>
> On aarch64-linux, we run into PR tdep/30010:
> ...
> FAIL: unwind-on-each-insn.exp: bar: insn 8: $fba_value == $fn_fba
> FAIL: unwind-on-each-insn.exp: bar: insn 8: check frame-id matches
> FAIL: unwind-on-each-insn.exp: bar: insn 8: bt 2
> FAIL: unwind-on-each-insn.exp: bar: insn 8: up
> FAIL: unwind-on-each-insn.exp: bar: insn 8: $sp_value == $::main_sp
> FAIL: unwind-on-each-insn.exp: bar: insn 8: $fba_value == $::main_fba
> FAIL: unwind-on-each-insn.exp: bar: insn 8: [string equal $fid $::main_fid]
> ...
>
> Tested on:
> - x86_64-linux (-m64 and -m32)
> - s390x-linux (-m64 and -m31)
> - aarch64-linux
> ---
> .../gdb.base/unwind-on-each-insn-foo.c | 6 +
> gdb/testsuite/gdb.base/unwind-on-each-insn.c | 2 +
> .../gdb.base/unwind-on-each-insn.exp | 132 ++++++++++--------
> 3 files changed, 81 insertions(+), 59 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c b/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
> index 635aa44135e..4a3b2946a3b 100644
> --- a/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
> +++ b/gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
> @@ -20,3 +20,9 @@ foo (const char *s)
> {
> /* Nothing. */
> }
> +
> +void
> +bar (const char *s)
> +{
> + foo (s);
> +}
> diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.c b/gdb/testsuite/gdb.base/unwind-on-each-insn.c
> index 821df375115..4aabc651c7b 100644
> --- a/gdb/testsuite/gdb.base/unwind-on-each-insn.c
> +++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.c
> @@ -16,10 +16,12 @@
> along with this program. If not, see <http://www.gnu.org/licenses/>. */
>
> extern void foo (const char *);
> +extern void bar (const char *);
>
> int
> main ()
> {
> foo ("foo");
> + bar ("bar");
> return 0;
> }
> diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
> index c61ffe5d174..c8e3f95e63a 100644
> --- a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
> +++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp
> @@ -69,72 +69,86 @@ proc get_fid { } {
> lassign [get_sp_and_fba "in main"] main_sp main_fba
> set main_fid [get_fid]
>
> -# Now enter the foo function.
> -gdb_breakpoint "*foo"
> -gdb_continue_to_breakpoint "enter foo"
> +proc do_test { function step_cmd } {
> + # Now enter the function.
> + gdb_breakpoint "*$function"
> + gdb_continue_to_breakpoint "enter $function"
> + delete_breakpoints
> +
> + # Record the current stack-pointer, and the frame base address.
> + lassign [get_sp_and_fba "in $function"] fn_sp fn_fba
> + set fn_fid [get_fid]
> +
> + for { set i_count 1 } { true } { incr i_count } {
> + with_test_prefix "instruction ${i_count}" {
> +
> + # The current stack-pointer value can legitimately change
> + # throughout the lifetime of a function, so we don't check the
> + # current stack-pointer value. But the frame base address
> + # should not change, so we do check for that.
> + lassign [get_sp_and_fba "for fn"] sp_value fba_value
> + gdb_assert { $fba_value == $fn_fba }
> +
> + # The frame-id should never change within a function, so check
> + # that now.
> + set fid [get_fid]
> + gdb_assert { [string equal $fid $fn_fid] } \
> + "check frame-id matches"
> +
> + # Check that the previous frame is 'main'.
> + gdb_test "bt 2" "\r\n#1\\s+\[^\r\n\]+ in main \\(\\) .*"
> +
> + # Move up the stack (to main).
> + gdb_test "up" \
> + "\r\n#1\\s+\[^\r\n\]+ in main \\(\\) .*"
> +
> + # Check we can unwind the stack-pointer and the frame base
> + # address correctly.
> + lassign [get_sp_and_fba "for main"] sp_value fba_value
> + if { $i_count == 1 } {
> + # The stack-pointer may have changed while running to *$function.
> + set ::main_sp $sp_value
> + } else {
> + gdb_assert { $sp_value == $::main_sp }
> + }
> + gdb_assert { $fba_value == $::main_fba }
>
> -# Record the current stack-pointer, and the frame base address.
> -lassign [get_sp_and_fba "in foo"] foo_sp foo_fba
> -set foo_fid [get_fid]
> -
> -for { set i_count 1 } { true } { incr i_count } {
> - with_test_prefix "instruction ${i_count}" {
> -
> - # The current stack-pointer value can legitimately change
> - # throughout the lifetime of a function, so we don't check the
> - # current stack-pointer value. But the frame base address
> - # should not change, so we do check for that.
> - lassign [get_sp_and_fba "for foo"] sp_value fba_value
> - gdb_assert { $fba_value == $foo_fba }
> -
> - # The frame-id should never change within a function, so check
> - # that now.
> - set fid [get_fid]
> - gdb_assert { [string equal $fid $foo_fid] } \
> - "check frame-id matches"
> -
> - # Check that the previous frame is 'main'.
> - gdb_test "bt 2" "\r\n#1\\s+\[^\r\n\]+ in main \\(\\) .*"
> -
> - # Move up the stack (to main).
> - gdb_test "up" \
> - "\r\n#1\\s+\[^\r\n\]+ in main \\(\\) .*"
> -
> - # Check we can unwind the stack-pointer and the frame base
> - # address correctly.
> - lassign [get_sp_and_fba "for main"] sp_value fba_value
> - if { $i_count == 1 } {
> - # The stack-pointer may have changed while running to *foo.
> - set main_sp $sp_value
> - } else {
> - gdb_assert { $sp_value == $main_sp }
> - }
> - gdb_assert { $fba_value == $main_fba }
> + # Check we have a consistent value for main's frame-id.
> + set fid [get_fid]
> + gdb_assert { [string equal $fid $::main_fid] }
>
> - # Check we have a consistent value for main's frame-id.
> - set fid [get_fid]
> - gdb_assert { [string equal $fid $main_fid] }
> + # Move back to the inner most frame.
> + gdb_test "frame 0" ".*"
>
> - # Move back to the inner most frame.
> - gdb_test "frame 0" ".*"
> + if { $i_count > 100 } {
> + # We expect a handful of instructions, if we reach 100,
> + # something is going wrong. Avoid an infinite loop.
> + fail "exceeded max number of instructions"
> + break
> + }
>
> - if { $i_count > 100 } {
> - # We expect a handful of instructions, if we reach 100,
> - # something is going wrong. Avoid an infinite loop.
> - fail "exceeded max number of instructions"
> - break
> - }
> + set in_fn 0
> + gdb_test_multiple $step_cmd "" {
> + -re -wrap "$::hex in $function \\(\\)" {
> + set in_fn 1
> + }
> + -re -wrap "" {}
> + }
>
> - set in_foo 0
> - gdb_test_multiple "stepi" "" {
> - -re -wrap "$hex in foo \\(\\)" {
> - set in_foo 1
> + if { ! $in_fn } {
> + break
> }
> - -re -wrap "" {}
> }
> + }
> +}
>
> - if { ! $in_foo } {
> - break
> - }
> +foreach {
> + function step_cmd
> +} {
> + foo stepi
> + bar nexti
> +} {
> + with_test_prefix $function {
> + do_test $function $step_cmd
> }
> }
This LGTM from aarch64's side. I see more PASSes and not FAIL's.
I see more FAIL's for arm-linux, but that is possibly broken in similar ways to what aarch64-linux was.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements
2023-01-19 10:46 [PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements Tom de Vries
` (3 preceding siblings ...)
2023-01-19 10:46 ` [PATCH 4/4] [gdb/testsuite] Analyze non-leaf fn in gdb.base/unwind-on-each-insn.exp Tom de Vries
@ 2023-01-25 12:32 ` Tom de Vries
4 siblings, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2023-01-25 12:32 UTC (permalink / raw)
To: gdb-patches; +Cc: Bruno Larsen, Andrew Burgess, Luis Machado
On 1/19/23 11:46, Tom de Vries via Gdb-patches wrote:
> Improving the test-case also detected a problem on powerpc64le, filed as PR
> tdep/30021 - "[gdb/tdep, powerpc64le] previous frame inner to this frame
> (corrupt stack?)".
>
> Due to unavailability I haven't tested the last patch on powerpc64le-linux.
I've done that now, and ran into yet another problem on
powerpc64le-linux: PR tdep/30049. I've mentioned it in the commit log.
I've pushed the remaining two patches.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 13+ messages in thread