Re: [PATCHv3 3/3] gdb: fix reg corruption from displaced stepping on amd64

[Pedro Alves <pedro@palves.net>]

Hi!

On 2023-03-27 1:32 p.m., Andrew Burgess wrote:

> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
> index a3fc0b9272b..b4d3beeaba2 100644
> --- a/gdb/gdbarch-gen.h
> +++ b/gdb/gdbarch-gen.h
> @@ -1068,9 +1068,9 @@ typedef bool (gdbarch_displaced_step_hw_singlestep_ftype) (struct gdbarch *gdbar
>  extern bool gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch);
>  extern void set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, gdbarch_displaced_step_hw_singlestep_ftype *displaced_step_hw_singlestep);
>  
> -/* Fix up the state resulting from successfully single-stepping a
> -   displaced instruction, to give the result we would have gotten from
> -   stepping the instruction in its original location.
> +/* Fix up the state after attempting to single-step a displaced
> +   instruction, to give the result we would have gotten from stepping the
> +   instruction in its original location.
>  
>     REGS is the register state resulting from single-stepping the
>     displaced instruction.
> @@ -1078,15 +1078,22 @@ extern void set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, g
>     CLOSURE is the result from the matching call to
>     gdbarch_displaced_step_copy_insn.
>  
> -   If you provide gdbarch_displaced_step_copy_insn.but not this
> -   function, then GDB assumes that no fixup is needed after
> -   single-stepping the instruction.
> +   FROM is the address where the instruction was original located, TO is
> +   the address of the displaced buffer where the instruction was copied
> +   to for stepping, and PC is the address at which the inferior stopped
> +   after stepping.
>  
>     For a general explanation of displaced stepping and how GDB uses it,
> -   see the comments in infrun.c. */
> +   see the comments in infrun.c.
> +
> +   This function will be called both when the single-step succeeded, and
> +   in the case where the single-step didn't succeed, for example, if the
> +   inferior was interrupted by a signal.  Within the function it is
> +   possible to use PC and TO to determine if the instruction was stepped
> +   or not. */

This comment seems stale, as we can't use PC/TO to determine whether stepped
or not.  I can't find the comment in the .py file, so, just need to regen?

> --- a/gdb/s390-tdep.c
> +++ b/gdb/s390-tdep.c
> @@ -482,8 +482,19 @@ static void
>  s390_displaced_step_fixup (struct gdbarch *gdbarch,
>  			   displaced_step_copy_insn_closure *closure_,
>  			   CORE_ADDR from, CORE_ADDR to,
> -			   struct regcache *regs)
> +			   struct regcache *regs, bool completed_p)
>  {
> +  CORE_ADDR pc = regcache_read_pc (regs);
> +
> +  /* If the displaced instruction didn't complete successfully then all we
> +     need to do is restore the program counter.  */
> +  if (!completed_p)
> +    {
> +      pc = from + (pc - to);
> +      regcache_write_pc (regs, pc);
> +      return;
> +    }
> +
>    /* Our closure is a copy of the instruction.  */
>    s390_displaced_step_copy_insn_closure *closure
>      = (s390_displaced_step_copy_insn_closure *) closure_;
> @@ -496,7 +507,6 @@ s390_displaced_step_fixup (struct gdbarch *gdbarch,
>    int i2, d2;
>  
>    /* Get current PC and addressing mode bit.  */

Comment slightly stale now.  No longer getting current PC here.

> -  CORE_ADDR pc = regcache_read_pc (regs);
>    ULONGEST amode = 0;
>  
>    if (register_size (gdbarch, S390_PSWA_REGNUM) == 4)
> diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-self-call-alarm.c b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call-alarm.c
> new file mode 100644
> index 00000000000..aec3d294b15
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call-alarm.c
> @@ -0,0 +1,24 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> +   Copyright 2023 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <unistd.h>
> +
> +void
> +setup_alarm (void)
> +{
> +  alarm (300);
> +}
> diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.S b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.S
> new file mode 100644
> index 00000000000..a227ba21917
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.S
> @@ -0,0 +1,50 @@
> +/* Copyright 2009-2023 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +   This file is part of the gdb testsuite.
> +   It tests displaced stepping over various insns that require special
> +   handling.  */
> +
> +	.text
> +
> +	.global main
> +main:
> +	nop
> +
> +	callq	setup_alarm
> +
> +	nop
> +
> +/***********************************************/
> +
> +/* test call/ret */
> +
> +	.global test_call
> +test_call:
> +	call test_call
> +	nop
> +	.global test_ret_end
> +test_ret_end:
> +	nop
> +
> +/***********************************************/
> +
> +/* all done */
> +
> +done:
> +	mov $0,%rdi
> +	call exit
> +	hlt
> +
> diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.exp b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.exp
> new file mode 100644
> index 00000000000..9f625b65b1f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.exp
> @@ -0,0 +1,81 @@
> +# Copyright 2023 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test amd64 displaced stepping over a call instruction that calls to
> +# itself.  This is pretty unlikely to be seen in the wild, but does
> +# test a corner case of our displaced step handling.
> +
> +require is_x86_64_m64_target
> +
> +set newline "\[\r\n\]*"
> +
> +set opts {debug nopie}
> +standard_testfile .S -alarm.c
> +
> +if { [prepare_for_testing "failed to prepare" $testfile "$srcfile $srcfile2" $opts] } {
> +    return -1
> +}
> +
> +gdb_test "set displaced-stepping on" ""
> +gdb_test "show displaced-stepping" ".* displaced stepping .* is on.*"
> +
> +if {![runto_main]} {
> +    return 0
> +}
> +
> +# Proceed to the test function.
> +gdb_breakpoint "test_call"
> +gdb_continue_to_breakpoint "test_call"
> +
> +# Get the current stack pointer value.
> +set sp [get_hexadecimal_valueof "\$sp" "*UNKNOWN*"]
> +
> +# Get the address of the next instruction.
> +set next_insn_addr ""
> +gdb_test_multiple "x/2i \$pc" "get address of next insn" {
> +    -re "\r\n=> $hex \[^\r\n\]+\r\n" {
> +	exp_continue
> +    }
> +    -re "^   ($hex) \[^\r\n\]+\r\n" {
> +	set next_insn_addr $expect_out(1,string)
> +	exp_continue
> +    }
> +    -re "^$::gdb_prompt $" {
> +	gdb_assert {![string equal $next_insn_addr ""]} \
> +	    $gdb_test_name
> +    }
> +}
> +
> +# Clear the slot on the stack and confirm it was set to zero.
> +set sp [expr $sp - 0x8]
> +gdb_test_no_output "set {unsigned long long} $sp = 0"
> +set zero_val 0x[format %016x 0]
> +gdb_test "x/1gx 0x[format %x $sp]" "$hex:\\s+${zero_val}" \
> +    "check return address slot was set to zero"
> +
> +# Single step.
> +gdb_test "stepi" \
> +    "Breakpoint $decimal, test_call \\(\\) at .*"
> +
> +# Check stack pointer was updated to the expected value

Missing period.

> +set new_sp [get_hexadecimal_valueof "\$sp" "*UNKNOWN*" \
> +	       "get stack pointer after step"]
> +gdb_assert {[expr $sp == $new_sp]} \
> +    "check stack pointer was updated as expected"
> +
> +# Check the contents of the stack were updated to the expected value.
> +set next_insn_addr 0x[format %016X $next_insn_addr]
> +gdb_test "x/1gx 0x[format %x $sp]" "$hex:\\s+$next_insn_addr" \
> +    "check return address was updated correctly"
> diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-signal.c b/gdb/testsuite/gdb.arch/amd64-disp-step-signal.c
> new file mode 100644
> index 00000000000..c968146624a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-disp-step-signal.c
> @@ -0,0 +1,30 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> +   Copyright 2023 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <signal.h>
> +#include <stdio.h>
> +
> +static void
> +sigalrm_handler (int sig)
> +{
> +}
> +
> +void
> +setup_signal_handler ()
> +{
> +  signal(SIGALRM, sigalrm_handler);

Space before parens.

The downside of using SIGALRM is that the gdb.arch/amd64-disp-step*.exp testcases won't be usable
with targets that have no such concept like Windows or embedded systems or GPUs anymore.  I think
just wrapping the two functions in #ifdef SIGLRM, and then skipping the signals-related parts
of the testcases with [target_info exists gdb,nosignals] || [istarget "*-*-mingw*"]
would be sufficient.

> +}
> diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.S b/gdb/testsuite/gdb.arch/amd64-disp-step.S
> index b25e292bdf0..bf73778cf43 100644
> --- a/gdb/testsuite/gdb.arch/amd64-disp-step.S
> +++ b/gdb/testsuite/gdb.arch/amd64-disp-step.S
> @@ -23,6 +23,10 @@
>  main:
>  	nop
>  
> +	callq	setup_signal_handler
> +
> +	nop
> +
>  /***********************************************/
>  
>  /* test call/ret */
> @@ -135,6 +139,14 @@ test_rip_rdi:
>  test_rip_rdi_end:
>  	nop
>  
> +	.global test_jmp
> +test_jmp:
> +	jmpq 	*jmp_dest(%rip)
> +	nop
> +	.global test_jmp_end
> +test_jmp_end:
> +	nop
> +
>  	/* skip over test data */
>  	jmp done
>  
> @@ -142,6 +154,9 @@ test_rip_rdi_end:
>  
>  answer:	.8byte 42
>  
> +jmp_dest:
> +	.8byte	test_jmp_end
> +
>  /***********************************************/
>  
>  /* all done */
> diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.exp b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
> index 2aee1e05774..09ba56e490e 100644
> --- a/gdb/testsuite/gdb.arch/amd64-disp-step.exp
> +++ b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
> @@ -17,15 +17,16 @@
>  
>  # Test amd64 displaced stepping.
>  
> +load_lib gdb-python.exp
>  
>  require is_x86_64_m64_target
>  
>  set newline "\[\r\n\]*"
>  
>  set opts {debug nopie}
> -standard_testfile .S
> +standard_testfile .S -signal.c
>  
> -if { [prepare_for_testing "failed to prepare" $testfile $srcfile $opts] } {
> +if { [prepare_for_testing "failed to prepare" $testfile "$srcfile $srcfile2" $opts] } {
>      return -1
>  }
>  
> @@ -154,9 +155,13 @@ proc set_regs { regs val } {
>      }
>  }
>  
> -# Verify all REGS equal VAL, except REG which equals REG_VAL.
> +# Verify all REGS equal VAL, except EXCEPT_REG which equals
> +# EXCEPT_REG_VAL.
> +#
> +# It is fine for EXCEPT_REG to be the empty string, in which case no
> +# register will be checked for EXCEPT_REG_VAL.
>  
> -proc verify_regs { test_name regs val except_reg except_reg_val } {
> +proc_with_prefix verify_regs { regs val except_reg except_reg_val } {
>      global newline
>  
>      foreach reg ${regs} {
> @@ -165,36 +170,101 @@ proc verify_regs { test_name regs val except_reg except_reg_val } {
>  	    set expected ${except_reg_val}
>  	}
>  	# The cast to (int) is because RBP is printed as a pointer.
> -	gdb_test "p (int) \$${reg}" " = ${expected}${newline}" "${test_name} ${reg} expected value"
> +	gdb_test "p (int) \$${reg}" " = ${expected}${newline}" "${reg} expected value"
>      }
>  }
>  
> -proc rip_test { reg } {
> +# Run the rip-relative tests.
> +#
> +# TEST_START_LABEL and TEST_END_LABEL are two labels that delimit the
> +# test in the srcfile.
> +#
> +# REG is either the name of a register which is the destiation

destiation -> destination

> +# location (when testing the add instruction), otherwise REG should be
> +# the empty string, when testing the 'jmpq*' instruction.
> +#
> +# SIGNAL_MODES is a list which always contains 'off' and optionally
> +# might also contain 'on'.  The 'on' value is only included if GDB
> +# supports Python.  The test is repeated for each signal mode.  With
> +# signal mode 'on' GDB uses Python to have a signal sent to the
> +# inferior.
> +proc rip_test { reg test_start_label test_end_label signal_modes } {
>      global srcfile rip_regs
>  
> -    set test_start_label "test_rip_${reg}"
> -    set test_end_label "test_rip_${reg}_end"
> -
>      gdb_test "break ${test_start_label}" \
>  	"Breakpoint.*at.* file .*$srcfile, line.*"
>      gdb_test "break ${test_end_label}" \
>  	"Breakpoint.*at.* file .*$srcfile, line.*"
>  
> -    gdb_test "continue" \
> -	"Continuing.*Breakpoint.*, ${test_start_label} ().*" \
> -	"continue to ${test_start_label}"
> +    foreach_with_prefix send_signal $signal_modes {
> +	if {$send_signal eq [lindex $signal_modes 0]} {
> +	    # The first time through we can just continue to the
> +	    # breakpoint.
> +	    gdb_test "continue" \
> +		"Continuing.*Breakpoint.*, ${test_start_label} ().*" \
> +		"continue to ${test_start_label}"
> +	} else {
> +	    # For the second time through the test we need to jump
> +	    # back to the beginning.
> +	    gdb_test "jump ${test_start_label}" \
> +		"Breakpoint.*, ${test_start_label} ().*" \
> +		"jump back to ${test_start_label}"
> +	}
> +
> +	set_regs ${rip_regs} 0
>  
> -    set_regs ${rip_regs} 0
> +	if {$send_signal} {
> +	    gdb_test_no_output "python signal_inferior()" \
> +		"send signal"
> +	}
> +
> +	gdb_test "continue" \
> +	    "Continuing.*Breakpoint.*, ${test_end_label} ().*" \
> +	    "continue to ${test_end_label}"
>  
> -    gdb_test "continue" \
> -	"Continuing.*Breakpoint.*, ${test_end_label} ().*" \
> -	"continue to ${test_end_label}"
> +	verify_regs ${rip_regs} 0 ${reg} 42
> +    }
> +}
>  
> -    verify_regs "test rip w/${reg}" ${rip_regs} 0 ${reg} 42
> +if {[allow_python_tests] && ![is_remote target]} {
> +    # The signal sending tests require that the signal appear to
> +    # arrive from an outside source, i.e. we can't use GDB's 'signal'
> +    # command to deliver it.
> +    #
> +    # The signal must arrive while GDB is processing the displaced
> +    # step instruction.
> +    #
> +    # If we use 'signal' to send the signal GDB doesn't actually do
> +    # the displaced step, but instead just delivers the signal.
> +    #
> +    # By having Python ask the OS to deliver us a signal we will
> +    # (hopefully) see the signal while processing the displaced step
> +    # instruction.
> +    #
> +    # Obviously non of this will work if the target is remote.

non -> none

[remote_exec target "kill ...] instead of Python would work with
remote though.  Like:

  remote_exec target "kill -ALRM $inferior_pid"

Several other tests do that already to send other signals to the
inferior.  Any reason not to here?

> +    gdb_test_multiline "Create function to send SIGALRM" \
> +	"python" "" \
> +	"import os, signal" "" \
> +	"def signal_inferior():" "" \
> +	"  os.kill(gdb.selected_inferior().pid, signal.SIGALRM)" "" \
> +	"end" ""
> +
> +    set signal_modes { off on }
> +} else {
> +    set signal_modes { off }
>  }
>  
> +# The the rip-relative add instructions.  There's a test writing to

Double "The the".

> +# each register in RIP_REGS in turn.
>  foreach reg ${rip_regs} {
> -    rip_test $reg
> +    with_test_prefix "add into ${reg}" {
> +	rip_test $reg "test_rip_${reg}" "test_rip_${reg}_end" $signal_modes
> +    }
> +}
> +
> +# Now test the rip-relative 'jmpq*' instruction.
> +with_test_prefix "rip-relative jmpq*" {
> +    rip_test "" "test_jmp" "test_jmp_end" $signal_modes
>  }
>  
>  ##########################################
> diff --git a/gdb/testsuite/gdb.arch/i386-disp-step-self-call-alarm.c b/gdb/testsuite/gdb.arch/i386-disp-step-self-call-alarm.c
> new file mode 100644
> index 00000000000..aec3d294b15
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-disp-step-self-call-alarm.c
> @@ -0,0 +1,24 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> +   Copyright 2023 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <unistd.h>
> +
> +void
> +setup_alarm (void)
> +{
> +  alarm (300);
> +}
> diff --git a/gdb/testsuite/gdb.arch/i386-disp-step-self-call.S b/gdb/testsuite/gdb.arch/i386-disp-step-self-call.S
> new file mode 100644
> index 00000000000..0b4255c36eb
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-disp-step-self-call.S
> @@ -0,0 +1,50 @@
> +/* Copyright 2009-2023 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +   This file is part of the gdb testsuite.
> +   It tests displaced stepping over various insns that require special
> +   handling.  */
> +
> +	.text
> +
> +	.global main
> +main:
> +	nop
> +
> +	call	setup_alarm
> +
> +	nop
> +
> +/***********************************************/
> +
> +/* test call/ret */
> +
> +	.global test_call
> +test_call:
> +	call test_call
> +	nop
> +	.global test_ret_end
> +test_ret_end:
> +	nop
> +
> +/***********************************************/
> +
> +/* all done */
> +
> +done:
> +	pushl $0
> +	call exit
> +	hlt
> +
> diff --git a/gdb/testsuite/gdb.arch/i386-disp-step-self-call.exp b/gdb/testsuite/gdb.arch/i386-disp-step-self-call.exp
> new file mode 100644
> index 00000000000..d676e62e15c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/i386-disp-step-self-call.exp
> @@ -0,0 +1,81 @@
> +# Copyright 2023 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test i386 displaced stepping over a call instruction that calls to
> +# itself.  This is pretty unlikely to be seen in the wild, but does
> +# test a corner case of our displaced step handling.
> +
> +require is_x86_like_target
> +
> +set newline "\[\r\n\]*"
> +
> +set opts {debug nopie}
> +standard_testfile .S -alarm.c
> +
> +if { [prepare_for_testing "failed to prepare" $testfile "$srcfile $srcfile2" $opts] } {
> +    return -1
> +}
> +
> +gdb_test "set displaced-stepping on" ""
> +gdb_test "show displaced-stepping" ".* displaced stepping .* is on.*"
> +
> +if {![runto_main]} {
> +    return 0
> +}
> +
> +# Proceed to the test function.
> +gdb_breakpoint "test_call"
> +gdb_continue_to_breakpoint "test_call"
> +
> +# Get the current stack pointer value.
> +set sp [get_hexadecimal_valueof "\$sp" "*UNKNOWN*"]
> +
> +# Get the address of the next instruction.
> +set next_insn_addr ""
> +gdb_test_multiple "x/2i \$pc" "get address of next insn" {
> +    -re "\r\n=> $hex \[^\r\n\]+\r\n" {
> +	exp_continue
> +    }
> +    -re "^   ($hex) \[^\r\n\]+\r\n" {
> +	set next_insn_addr $expect_out(1,string)
> +	exp_continue
> +    }
> +    -re "^$::gdb_prompt $" {
> +	gdb_assert {![string equal $next_insn_addr ""]} \
> +	    $gdb_test_name
> +    }
> +}
> +
> +# Clear the slot on the stack and confirm it was set to zero.
> +set sp [expr $sp - 0x4]
> +gdb_test_no_output "set {unsigned long long} $sp = 0"
> +set zero_val 0x[format %08x 0]
> +gdb_test "x/1wx 0x[format %x $sp]" "$hex:\\s+${zero_val}" \
> +    "check return address slot was set to zero"
> +
> +# Single step.
> +gdb_test "stepi" \
> +    "Breakpoint $decimal, test_call \\(\\) at .*"
> +
> +# Check stack pointer was updated to the expected value

Missing period.

> +set new_sp [get_hexadecimal_valueof "\$sp" "*UNKNOWN*" \
> +	       "get stack pointer after step"]
> +gdb_assert {[expr $sp == $new_sp]} \
> +    "check stack pointer was updated as expected"
> +
> +# Check the contents of the stack were updated to the expected value.
> +set next_insn_addr 0x[format %08X $next_insn_addr]
> +gdb_test "x/1wx 0x[format %x $sp]" "$hex:\\s+$next_insn_addr" \
> +    "check return address was updated correctly"
>