Re: [PATCH,v2] Harden gdb.base/step-over-syscall.exp

[Simon Marchi <simark@simark.ca>]

On 2020-01-24 11:35 a.m., Luis Machado wrote:
> New in v2:
> 
> - Set initial values to -1 instead of 0.
> - Rewrote RE to prevent unexpected matching when parsing one character at a
>   time.
> - Used gdb_assert for an additional check.
> - Validated with check-read1
> 
> Simon,
> 
> I did some research on checking the syscall numbers to make sure we're calling
> the right syscall, but there seems to be considerable variation in terms of
> what registers are used to pass the syscall number and also the syscall
> number itself.

I would expect it to have exactly one register and one syscall number per architecture.

> For example, aarch64 seems to use clone for fork/vfork, but arm does not. The
> syscall number comes in through w0, but it also gets passed via x8 if it goes
> into the kernel.

So when we are stopped about to execute the svc instruction, the value should
be in x8, that's the one we would care about.

> Given the added complexity and the fact that the test is already breaking into
> fork/vfork/clone, i think we can be reasonably sure that we are invoking the
> right syscall. What do you think? Any other ideas?

Yeah, it's probably overkill, forget about that.

> @@ -75,39 +76,70 @@ 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 -1
> +    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 ".*$gdb_prompt $" {
> +	    pass $test
> +	}
> +    }

Instead of setting the test variable, you can now just pass the test name to
gdb_test_multiple and access it inside the callbacks as $gdb_test_name.  See
commit:

gdb/testsuite: Add gdb_test_name variable
3d63690a0316d92cf248542ee12a3fc8b30152ea

The patch LGTM with that fixed.

Simon