[PATCH] [gdb/testsuite] Fix gdb.xml/tdesc-regs.exp on riscv64

[Andrew Burgess]

Tom de Vries <tdevries@suse.de> writes:

> When running test-case gdb.xml/tdesc-regs.exp on riscv64-linux, I get:
> ...
> (gdb) set tdesc file single-reg.xml^M
> warning: Architecture rejected target-supplied description^M
> (gdb) FAIL: gdb.xml/tdesc-regs.exp: set tdesc file single-reg.xml
> UNSUPPORTED: gdb.xml/tdesc-regs.exp: register tests
> ...
>
> The FAIL and UNSUPPORTED are produced here:
> ...
>  # If no core registers were specified, assume this target does not
>  # support target-defined registers.  Verify that we get a warning if
>  # we try to use them.  This not only tests the warning, but also
>  # reminds maintainers to add test support when they add the feature.
>
> if {[string equal ${core-regs} ""]} {
>     gdb_test "set tdesc file $single_reg_xml" \
> 	"warning: Target-supplied registers are not supported.*" \
> 	"set tdesc file single-reg.xml"
>     unsupported "register tests"
>     return 0
> }
> ...
>
> The test-case contains target-specific setting of the core-regs variable, and
> adding this for riscv64 bypasses this code and makes the test-case pass.
>
> However, without that change, the test-case shouldn't produce a FAIL since
> gdb isn't doing anything wrong.

Yeah, you are technically correct, so I can't really argue against this
change, but I do think things were probably simpler before, and the FAIL
only exists when the test needs fixing for this architecture.  But, as I
said, you are technically correct.

I do have one tiny nit though...

>
> Fix this by producing instead:
> ...
> PASS: $exp: set tdesc file single-reg.xml
> UNSUPPORTED: $exp: register tests (missing architecture-specific core-regs setting)
> ...
>
> Tested on riscv64-linux.
> ---
>  gdb/testsuite/gdb.xml/tdesc-regs.exp | 36 +++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.xml/tdesc-regs.exp b/gdb/testsuite/gdb.xml/tdesc-regs.exp
> index 7c8b0b8cef8..fdd65fb3bbb 100644
> --- a/gdb/testsuite/gdb.xml/tdesc-regs.exp
> +++ b/gdb/testsuite/gdb.xml/tdesc-regs.exp
> @@ -80,21 +80,39 @@ switch -glob -- [istarget] {
>  	set regdir "i386/"
>          set core-regs {64bit-core.xml 64bit-sse.xml}
>      }
> +    "riscv64*-*-*" {
> +       set architecture "riscv:rv64"
> +       set regdir "riscv/"
> +       set core-regs 64bit-cpu.xml
> +    }
>  }
>  
> -# If no core registers were specified, assume this target does not
> -# support target-defined registers.  Verify that we get a warning if
> -# we try to use them.  This not only tests the warning, but also
> -# reminds maintainers to add test support when they add the feature.
> -
>  set single_reg_xml [gdb_remote_download host  \
>  			"$srcdir/$subdir/single-reg.xml"]
>  
>  if {[string equal ${core-regs} ""]} {
> -    gdb_test "set tdesc file $single_reg_xml" \
> -	"warning: Target-supplied registers are not supported.*" \
> -	"set tdesc file single-reg.xml"
> -    unsupported "register tests"
> +    set test "set tdesc file single-reg.xml"
> +    set feature_unsupported 0
> +    set feature_test_unsupported 0
> +    gdb_test_multiple "set tdesc file $single_reg_xml" $test {
> +	-re -wrap "warning: Target-supplied registers are not supported" {
> +	    set feature_unsupported 1
> +	    pass $gdb_test_name
> +	}
> +	-re -wrap "warning: Architecture rejected target-supplied description" {
> +	    set feature_test_unsupported 1
> +	    pass $gdb_test_name
> +	}
> +    }
> +
> +    if { $feature_unsupported } {
> +	# Remind maintainers to add the feature.

I don't think this comment is correct.  This branch is hit for targets
that don't support xml tdesc descriptions.  Now _maybe_ the maintainer
wants to add support, but then, maybe not.  In contrast ...

> +	unsupported "register tests"
> +    } elseif { $feature_test_unsupported } {
> +	# Remind maintainers to add test support.

... this comment is correct.  The target does support xml tdesc, but
we're missing an entry above, which the maintainer should fill in.

With the first comment removed:

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

> +	unsupported "register tests (missing architecture-specific core-regs setting)"
> +    }
> +
>      return 0
>  }
>  
>
> base-commit: 265ecaa3379fc6b95fcfd1c934d1e6c27d87439b
> -- 
> 2.35.3