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

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

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.