[PING] [PATCH] Harden gdb.base/step-over-syscall.exp

[Luis Machado <luis.machado@linaro.org>]

ping?

https://sourceware.org/ml/gdb-patches/2020-01/msg00432.html

On 1/15/20 5:36 PM, Luis Machado wrote:
> There are a couple problems with this test.
> 
> First
> --
> 
> gdb.base/step-over-syscall.exp records the address of a syscall instruction
> within fork/vfork/clone functions and also the address of the instruction
> after that syscall instruction.
> 
> It uses these couples addresses to make sure we stepped over a syscall
> instruction (fork/vfork/clone events) correctly.
> 
> The way the test fetches the addresses of the instructions is by stepi-ing
> its way through the fork/vfork/clone functions until it finds a match for
> a syscall. Then it stepi's once again to get the address of the next
> instruction.
> 
> This assumes that stepi-ing over a syscall is working correctly and landing
> in the right PC. This is not the case for AArch64/Linux, where we're
> landing a couple instructions after the syscall in some cases.
> 
> The following patch lets the test execute as before, but adds a new instruction
> address check using the x command as opposed to stepi.
> 
> I didn't want to change how the test works since we may also be
> interested in checking if stepi-ing over the syscall under different
> conditions (displaced stepping on/off) yields the same results. I don't
> feel strongly about this, so i'm OK with changing how we compare PC's for
> the entire test if folks decide it is reasonable.
> 
> Second
> --
> 
> FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: continue to vfork (3rd time) (the program exited)
> FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: continue to syscall insn vfork (the program is no longer running)
> FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: single step over vfork (the program is no longer running)
> 
> Depending on the glibc version we may have different code generated for the
> fork/vfork/clone functions.
> 
> I ran into the situation where vfork for newer glibc's on AArch64/Linux is
> very short, so "break vfork" will put a breakpoint right at the syscall
> instruction, which is something the testcase isn't expecting (a off-by-1
> of sorts).
> 
> The patch adds extra code to handle this case. If the test detects we're
> already sitting at a syscall instruction, it records the address and moves
> on to record the address after that particular instruction.
> 
> Another measure is to "break *$syscall" instead of "break $syscall". That
> guarantees we're stopping at the first instruction of the syscall function,
> if it ever happens that the syscall instruction is the first instruction of
> those functions.
> 
> With these changes i can fix some failures for aarch64-linux-gnu and also
> expose the problems i've reported here:
> 
> https://sourceware.org/ml/gdb-patches/2019-12/msg01071.html
> 
> These tests now fail for aarch64-linux-gnu (patch for this is going through
> reviews):
> 
> FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: pc after stepi matches insn addr after syscall
> FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=on: pc after stepi matches insn addr after syscall
> 
> I've queued a test run on the buildbot to make sure things are sane for other
> architectures, and i've tested it on aarch64-linux-gnu.
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-01-15  Luis Machado  <luis.machado@linaro.org>
> 
> 	* gdb.base/step-over-syscall.exp (setup): Check if we're already
> 	sitting at a syscall instruction when we hit the syscall function's
> 	breakpoint.
> 	Check PC against one obtained with the x command.
> 	(step_over_syscall): Don't continue to the syscall instruction if
> 	we're already there.
> ---
>   gdb/testsuite/gdb.base/step-over-syscall.exp | 91 ++++++++++++++------
>   1 file changed, 66 insertions(+), 25 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/step-over-syscall.exp b/gdb/testsuite/gdb.base/step-over-syscall.exp
> index b373c169c0..4d9488b1d4 100644
> --- a/gdb/testsuite/gdb.base/step-over-syscall.exp
> +++ b/gdb/testsuite/gdb.base/step-over-syscall.exp
> @@ -46,7 +46,8 @@ proc_with_prefix check_pc_after_cross_syscall { syscall syscall_insn_next_addr }
>   
>   proc setup { syscall } {
>       global gdb_prompt syscall_insn
> -
> +    global hex
> +    set next_insn_addr 0
>       set testfile "step-over-$syscall"
>   
>       clean_restart $testfile
> @@ -62,7 +63,7 @@ proc setup { syscall } {
>       gdb_test_no_output "set displaced-stepping off" \
>   	"set displaced-stepping off during test setup"
>   
> -    gdb_test "break $syscall" "Breakpoint \[0-9\]* at .*"
> +    gdb_test "break \*$syscall" "Breakpoint \[0-9\]* at .*"
>   
>       gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, (.* in |__libc_|)$syscall \\(\\).*" \
>   	"continue to $syscall (1st time)"
> @@ -75,37 +76,72 @@ proc setup { syscall } {
>       # Hit the breakpoint on $syscall for the second time.  In this time,
>       # the address of syscall insn and next insn of syscall are recorded.
>   
> -    gdb_test "display/i \$pc" ".*"
> -
> -    # Single step until we see a syscall insn or we reach the
> -    # upper bound of loop iterations.
> -    set msg "find syscall insn in $syscall"
> -    set steps 0
> -    set max_steps 1000
> -    gdb_test_multiple "stepi" $msg {
> -	-re ".*$syscall_insn.*$gdb_prompt $" {
> -	    pass $msg
> +    # Check if the first instruction we stopped at is the syscall one.
> +    set syscall_insn_addr 0
> +    set test "fetch first stop pc"
> +    gdb_test_multiple "display/i \$pc" $test {
> +	-re "display/i .*: x/i .*=> ($hex) .*:.*$syscall_insn.*$gdb_prompt $" {
> +	    set syscall_insn_addr $expect_out(1,string)
> +	    pass $test
>   	}
> -	-re "x/i .*=>.*\r\n$gdb_prompt $" {
> -	    incr steps
> -	    if {$steps == $max_steps} {
> -		fail $msg
> -	    } else {
> -		send_gdb "stepi\n"
> -		exp_continue
> +	-re "display/i.*" {
> +	    pass $test
> +	}
> +    }
> +
> +    # If we are not at the syscall instruction yet, keep looking for it with
> +    # stepi commands.
> +    if {$syscall_insn_addr == 0} {
> +	# Single step until we see a syscall insn or we reach the
> +	# upper bound of loop iterations.
> +	set msg "find syscall insn in $syscall"
> +	set steps 0
> +	set max_steps 1000
> +	gdb_test_multiple "stepi" $msg {
> +	    -re ".*$syscall_insn.*$gdb_prompt $" {
> +		pass $msg
> +	    }
> +	    -re "x/i .*=>.*\r\n$gdb_prompt $" {
> +		incr steps
> +		if {$steps == $max_steps} {
> +		    fail $msg
> +		} else {
> +		    send_gdb "stepi\n"
> +		    exp_continue
> +		}
>   	    }
>   	}
> +
> +	if {$steps == $max_steps} {
> +	    return { -1, -1 }
> +	}
> +
> +	set syscall_insn_addr [get_hexadecimal_valueof "\$pc" "0" \
> +				  "pc before stepi"]
>       }
>   
> -    if {$steps == $max_steps} {
> -	return { -1, -1 }
> +    # We have found the syscall instruction.  Now record the next instruction.
> +    # Use the X command instead of stepi since we can't guarantee
> +    # stepi is working properly.
> +    set test "pc after syscall instruction"
> +    gdb_test_multiple "x/2i \$pc" $test {
> +	-re "x/2i .*=> $hex .*:.*$syscall_insn.* ($hex) .*:.*$gdb_prompt $" {
> +	    set next_insn_addr $expect_out(2,string)
> +	    pass $test
> +	}
>       }
>   
> -    set syscall_insn_addr [get_hexadecimal_valueof "\$pc" "0" \
> -			       "pc before stepi"]
>       if {[gdb_test "stepi" "x/i .*=>.*" "stepi $syscall insn"] != 0} {
>   	return { -1, -1 }
>       }
> +
> +    set pc_after_stepi [get_hexadecimal_valueof "\$pc" "0" \
> +			    "pc after stepi with x command"]
> +
> +    if {$next_insn_addr != $pc_after_stepi} {
> +      fail "pc after stepi matches insn addr after syscall"
> +    }
> +
>       return [list $syscall_insn_addr [get_hexadecimal_valueof "\$pc" \
>   					 "0" "pc after stepi"]]
>   }
> @@ -156,8 +192,13 @@ proc step_over_syscall { syscall } {
>   		}
>   	    }
>   
> -	    gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .*" \
> -		"continue to syscall insn $syscall"
> +	    # Check if the syscall breakpoint is at the syscall instruction
> +	    # address.  If so, no need to continue, otherwise we will run the
> +	    # inferior to completion.
> +	    if {$syscall_insn_addr != [get_hexadecimal_valueof "\$pc" "0"]} {
> +		gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .*" \
> +		    "continue to syscall insn $syscall"
> +	    }
>   
>   	    gdb_test_no_output "set displaced-stepping $displaced"
>   
>