Re: [PATCH][gdb/testsuite] Fix gdb.base/endianity.exp with gcc-4.8

[Joel Brobecker <brobecker@adacore.com>]

Hi Tom,

> When running test-case gdb.base/endianity.exp using gcc-4.8, we get:
> ...
> (gdb) x/x &o.v^M
> 0x7fffffffd120: 0x00000004^M
> (gdb) XFAIL: gdb.base/endianity.exp: x/x &o.v
> x/xh &o.w^M
> 0x7fffffffd124: 0x0003^M
> (gdb) FAIL: gdb.base/endianity.exp: x/xh &o.w
> ...
> 
> The gcc 4.8 compiler does not support the scalar_storage_order attribute, so
> the testcase is compiled without that attribute, and the expected results are
> different.
> 
> This is why there's the first XFAIL, and we could xfail the second FAIL for the
> same reason.
> 
> Instead, fix this by adapting the expected values based on whether the attribute
> has been used in endianity.c.
> 
> Also, remove hard-coding of the byte order in the expected memory printing.
> 
> Tested on x86_64-linux, with gcc-4.8, gcc-7, and clang-10.
> 
> 
> Any comments?

For me, the whole point of this testcase is to test SSO, so if
the compiler doesn't support it, the testcase loses its value
entirely (to my eyes anyway). As a result of this, I dont' think
bringing the extra complexity that you are suggesting is bringing
any value -- I might argue that it's now hard to read the testcase
an understand what we're trying to do (sorry!).

In my opinion, rather than an XFAIL, we should just only do
the second half of the testcase if the compiler supports it,
than xfailing the tests. So I would do:

if { ([test_compiler_info {gcc-[0-5]-*}] || ![test_compiler_info gcc*]) } {
    # The rest of the testcase requires Scalar Storage Order support.
    # This compiler does not support it, so skip the rest.
    return
}

> [gdb/testsuite] Fix gdb.base/endianity.exp with gcc-4.8
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-12-10  Tom de Vries  <tdevries@suse.de>
> 
> 	PR testsuite/26953
> 	* gdb.base/endianity.c (ORDER_ATTR): New macro.
> 	(reverse): New variable.
> 	(struct otherendian): Use ORDER_ATTR.
> 	* gdb.base/endianity.exp: Use reverse variable to see if
> 	scalar_storage_order attribute was used.  Remove hard-coding of byte
> 	order in expected memory printing.
> 
> ---
>  gdb/testsuite/gdb.base/endianity.c   | 34 ++++++++++++++++------
>  gdb/testsuite/gdb.base/endianity.exp | 56 ++++++++++++++++++++++++++++++++----
>  2 files changed, 76 insertions(+), 14 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/endianity.c b/gdb/testsuite/gdb.base/endianity.c
> index ef3b6d4fdb..2bc3d09502 100644
> --- a/gdb/testsuite/gdb.base/endianity.c
> +++ b/gdb/testsuite/gdb.base/endianity.c
> @@ -17,6 +17,27 @@
>  
>  /* This tests the handling of dwarf attributes:
>      DW_AT_endianity, DW_END_big, and DW_END_little.  */
> +
> +
> +#if defined __GNUC__ && (__GNUC__ >= 6)
> +/* Scalar_storage_order attribute supported.  */
> +int reverse = 1;
> +
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +#define ORDER_ATTR \
> +  __attribute__( ( scalar_storage_order( "big-endian" ) ) )
> +#else
> +#define ORDER_ATTR \
> +  __attribute__( ( scalar_storage_order( "little-endian" ) ) )
> +#endif
> +
> +#else
> +
> +#define ORDER_ATTR
> +int reverse = 0;
> +
> +#endif
> +
>  struct otherendian
>  {
>    int v;
> @@ -25,21 +46,16 @@ struct otherendian
>    float f;
>    __complex__ float cplx;
>    double d;
> -}
> -#if defined __GNUC__ && (__GNUC__ >= 6)
> -#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> -__attribute__( ( scalar_storage_order( "big-endian" ) ) )
> -#else
> -__attribute__( ( scalar_storage_order( "little-endian" ) ) )
> -#endif
> -#endif
> -;
> +} ORDER_ATTR;
>  
>  void
>  do_nothing (struct otherendian *c)
>  {
>  }
>  
> +int v = 4;
> +short w = 3;
> +
>  int
>  main (void)
>  {
> diff --git a/gdb/testsuite/gdb.base/endianity.exp b/gdb/testsuite/gdb.base/endianity.exp
> index 2fa9ed3bf1..3f39be9239 100644
> --- a/gdb/testsuite/gdb.base/endianity.exp
> +++ b/gdb/testsuite/gdb.base/endianity.exp
> @@ -34,12 +34,58 @@ gdb_test "print o.x = 2" "= 2"
>  gdb_test "print o.f = 1.5" "= 1.5"
>  gdb_test "print o.d = -23.125" "= -23.125"
>  
> -# scalar_storage_order requires gcc >= 6
> -if { ([test_compiler_info {gcc-[0-5]-*}] || ![test_compiler_info gcc*]) } {
> -  setup_xfail "*-*-*"
> +proc reverse_hex { val } {
> +    set r {}
> +    foreach {a b} [split $val {}] {
> +	set two "$a$b"
> +	if { "$two" == "0x" } {
> +	    continue
> +	}
> +	lappend r "$two"
> +    }
> +    set r [lreverse $r]
> +    set r [join $r ""]
> +    return "0x$r"
> +}
> +
> +set reverse -1
> +gdb_test_multiple "p reverse" "" {
> +    -re -wrap "= 1" {
> +	set reverse 1
> +    }
> +    -re -wrap "= 0" {
> +	set reverse 0
> +    }
> +}
> +
> +if { $reverse != -1 } {
> +
> +    gdb_test_multiple "x/x &v" "" {
> +	-wrap -re "$hex <v>:.*($hex)" {
> +	    set v $expect_out(1,string)
> +	    pass $gdb_test_name
> +	    if { $reverse } {
> +		set o_v [reverse_hex $v]
> +	    } else {
> +		set o_v $v
> +	    }
> +	    gdb_test "x/x &o.v" $o_v
> +	}
> +    }
> +
> +    gdb_test_multiple "x/xh &w" "" {
> +	-wrap -re "$hex <w>:.*($hex)" {
> +	    set w $expect_out(1,string)
> +	    pass $gdb_test_name
> +	    if { $reverse } {
> +		set o_w [reverse_hex $w]
> +	    } else {
> +		set o_w $w
> +	    }
> +	    gdb_test "x/xh &o.w" $o_w
> +	}
> +    }
>  }
> -gdb_test "x/x &o.v" "0x04000000"
> -gdb_test "x/xh &o.w" "0x0300"
>  
>  gdb_test "print o" "= {v = 4, w = 3, x = 2, f = 1.5, cplx = 1.25 \\+ 7.25i, d = -23.125}" \
>      "print o after assignment"

-- 
Joel