[PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements

[PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements

[Tom de Vries]

While analyzing PR record/29721 "[gdb, record, aarch64] FAIL:
gdb.reverse/solib-precsave.exp: reverse-next third shr1", I came to realize
that I was a looking at the aarch64 variant of x86_64 PR record/16678 (as
indeed suggested by Bruno in the PR).

The test-case gdb.base/unwind-on-each-insn.exp was added in the commit to fix
PR record/16678, to detect similar problems on other architectures, but it
passes on aarch64 and doesn't detect record/29721, because the function that
is checked is too simple on aarch64:
...
    00000000004005fc <foo>:
      4005fc:       d503201f        nop
      400600:       d65f03c0        ret
...

This series first simplifies, and then improves test-case
gdb.base/unwind-on-each-insn.exp to detect PR record/16678, or more precisely,
the two spinoff PRs I filed that have reproducers that do not involve reverse
execution:
- PR30010 - [gdb/tdep, aarch64] Incorrect frame address for last insn
  (non-leaf case)
- PR30011 - [gdb/tdep, aarch64] Incorrect frame address for last insn
  (leaf case)

In short, we have following patches:
- [gdb/testsuite] Simplify gdb.base/unwind-on-each-insn.exp
  Remove unnecessary and fragile complication of analyzing disassembly.
- [gdb/testsuite] Improve gdb.base/unwind-on-each-insn.exp
  Detect PR30011.
- [gdb/tdep, aarch64] Fix frame address of last insn in leaf function
  Fix for PR30011.
- [gdb/testsuite] Analyze non-leaf fn in gdb.base/unwind-on-each-insn.exp
  Detect PR30010.

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.

While doing this investigation I also ran into PR tdep/30019 -
"[gdb/tdep, i386] frame address at first insn in main is zero", but the
test-case doesn't trigger this.  I've not tried adding this.

Finally, I'm considering moving the test-case to gdb.arch, but I haven't
included a patch for this.

Tom de Vries (4):
  [gdb/testsuite] Simplify gdb.base/unwind-on-each-insn.exp
  [gdb/testsuite] Improve gdb.base/unwind-on-each-insn.exp
  [gdb/tdep, aarch64] Fix frame address of last insn in leaf function
  [gdb/testsuite] Analyze non-leaf fn in
    gdb.base/unwind-on-each-insn.exp

 gdb/aarch64-tdep.c                            |   6 +-
 .../gdb.base/unwind-on-each-insn-foo.c        |   8 +-
 gdb/testsuite/gdb.base/unwind-on-each-insn.c  |   6 +-
 .../gdb.base/unwind-on-each-insn.exp          | 167 ++++++++----------
 4 files changed, 89 insertions(+), 98 deletions(-)


base-commit: b8d21eb0cd10d6127e77cc437d82e949adb0c454
-- 
2.35.3

[PATCH 1/4] [gdb/testsuite] Simplify gdb.base/unwind-on-each-insn.exp

[Tom de Vries]

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

[PATCH 2/4] [gdb/testsuite] Improve gdb.base/unwind-on-each-insn.exp

[Tom de Vries]

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

[PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function

[Tom de Vries]

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

[PATCH 4/4] [gdb/testsuite] Analyze non-leaf fn in gdb.base/unwind-on-each-insn.exp

[Tom de Vries]

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

Re: [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function

[Tom de Vries]

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

Re: [PATCH 1/4] [gdb/testsuite] Simplify gdb.base/unwind-on-each-insn.exp

[Tom de Vries]

[-- 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

Re: [PATCH 2/4] [gdb/testsuite] Improve gdb.base/unwind-on-each-insn.exp

[Luis Machado]

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.

Re: [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function

[Luis Machado]

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.  */

Re: [PATCH 4/4] [gdb/testsuite] Analyze non-leaf fn in gdb.base/unwind-on-each-insn.exp

[Luis Machado]

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.

Re: [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function

[Tom de Vries]

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.  */
> 

Re: [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function

[Luis Machado]

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.

Re: [PATCH 0/4] [gdb] Test-case gdb.base/unwind-on-each-insn.exp improvements

[Tom de Vries]

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