Re: [PATCH] gdb fix for catch-syscall.exp

[Simon Marchi <simark@simark.ca>]

On 2021-11-29 11:46 a.m., Carl Love via Gdb-patches wrote:
> Simon:
> 
> I split the latest patch so there is a separate revert patch and a
> patch to make the Powerepc test a xfail.  Note, the revert patch only
> reverts the part of the original patch that causes the regression test
> on amd64.  
> 
> Let me know if the first patch looks OK to commit to mainline.
> 
> We can continue to discuss the second patch to decide if there is a
> better implementation to add the xfail and if it really is a kernel
> failure.
> 
>                      Carl 
> --------------------------------------------------------------------
> [PATCH 1/2] gdb: Revert change to gdb.base/catch-syscall.exp
> 
> The previous commit:
>    commit ab198279120fe7937c0970a8bb881922726678f9
>    Author: Carl Love <cel@us.ibm.com>
>    Date:   Wed Nov 17 22:29:33 2021 +0000
> 
>        gdb fix for catch-syscall.exp
> 
> Fixed the test failure on PowerPC but broke the test on amd64.  The issue is
> both messages:
> 
>     Catchpoint 1 (call to syscall execve), ....
>     Catchpoint 1 (returned from syscall execve),  ....
> 
> are seen on amd64 whereas on PowerPC only the "call to syscall execve" is
> seen.  Based on the discussion on the mailing list, it is felt that this
> is really an issue with the PowerPC kernel support not reporting both events.
> 
> This patch just reverts the part of the commit that caused the regression
> failure on amd64.  The change for the istarget powerpc64-*-linux* is not
> being reverted.
> ---
>  gdb/testsuite/gdb.base/catch-syscall.exp | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
> index 016d0a698a6..cdd5e2aec47 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
> @@ -348,9 +348,7 @@ proc test_catch_syscall_execve {} {
>  	# Check for entry/return across the execve, making sure that the
>  	# syscall_state isn't lost when turning into a new process.
>  	insert_catch_syscall_with_arg "execve"
> -
> -	# Check that the execve is called.
> -	check_call_to_syscall "execve"
> +	check_continue "execve"
>  
>  	# Continue to main so extended-remote can read files as needed.
>  	# (Otherwise that "Reading" output confuses gdb_continue_to_end.)
> -- 
> 2.30.2

This was already done by this commit:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e19f8248297bb9864ff14368cc89d2446572837a

> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
> index cdd5e2aec47..0693f7afe27 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
> @@ -127,7 +127,24 @@ proc check_return_from_syscall { syscall { pattern "" } } {
>      }
>  
>      set thistest "syscall $syscall has returned"
> -    gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
> +    if { $pattern eq "execve" } {
> +	gdb_test_multiple "continue" $thistest {
> +	    -re -wrap "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" {
> +		pass $thistest
> +		return 1
> +	    }
> +	    -re -wrap ".*Breakpoint $decimal, main .*" {
> +		# On Powerpc kernel does not report the returned from syscall
> +		# as expected by the test.  GDB bugzilla 28623.
> +		xfail $thistest
> +		return 0

This should check that the target is powerpc before doing the xfail, because
getting that result on any other architecture should result in a FAIL.  Probably
"[istarget "powerpc*-*-*"]".


> +	    }
> +	}
> +
> +    } else {
> +	gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
> +	return 1
> +    }
>  }
>  
>  # Internal procedure that performs two 'continue' commands and checks if
> @@ -142,7 +159,11 @@ proc check_continue { syscall { pattern "" } } {
>      # Testing if the inferior has called the syscall.
>      check_call_to_syscall $syscall $pattern
>      # And now, that the syscall has returned.
> -    check_return_from_syscall $syscall $pattern
> +    if [check_return_from_syscall $syscall $pattern] {
> +	return 1
> +    } else {
> +	return 0
> +    }

Can this just be

  return [check_return_from_syscall $syscall $pattern] ?

Simon