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

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

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.