Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: Joel Brobecker <brobecker@adacore.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH][gdb/testsuite] Fix gdb.base/endianity.exp with gcc-4.8
Date: Sun, 13 Dec 2020 17:23:56 +0100	[thread overview]
Message-ID: <57aa2ba0-4590-a901-8de9-7f916f02ae1a@suse.de> (raw)
In-Reply-To: <20201213135632.GE366101@adacore.com>

[-- Attachment #1: Type: text/plain, Size: 1777 bytes --]

On 12/13/20 2:56 PM, Joel Brobecker wrote:
> 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!).
> 

Np, that's also good feedback, thanks.

> 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
> }
> 

Ack, committed as below.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-testsuite-Fix-gdb.base-endianity.exp-with-gcc-4.8.patch --]
[-- Type: text/x-patch, Size: 1938 bytes --]

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

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.

Fix this by rather than xfailing, skipping the tests if the compiler does not
support the scalar_storage_order attribute.

Tested on x86_64-linux, with gcc-4.8, gcc-7, and clang-10.

gdb/testsuite/ChangeLog:

2020-12-10  Tom de Vries  <tdevries@suse.de>

	PR testsuite/26953
	* gdb.base/endianity.exp: Skip tests requiring scalar_storage_order
	attribute support if compiler doesn't support it.

---
 gdb/testsuite/gdb.base/endianity.exp | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.base/endianity.exp b/gdb/testsuite/gdb.base/endianity.exp
index 2fa9ed3bf1..4520799d04 100644
--- a/gdb/testsuite/gdb.base/endianity.exp
+++ b/gdb/testsuite/gdb.base/endianity.exp
@@ -34,12 +34,15 @@ 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
+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"
+
 if { ([test_compiler_info {gcc-[0-5]-*}] || ![test_compiler_info gcc*]) } {
-  setup_xfail "*-*-*"
+    # The rest of the testcase requires Scalar Storage Order support.
+    # This compiler does not support it, so skip the rest.
+    return -1
 }
+
 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"

  reply	other threads:[~2020-12-13 16:24 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-10 15:29 Tom de Vries
2020-12-13 13:56 ` Joel Brobecker
2020-12-13 16:23   ` Tom de Vries [this message]
2020-12-14 16:55     ` Simon Marchi
2020-12-19  8:48       ` Tom de Vries
2020-12-19 13:42         ` Simon Marchi via Gdb-patches
2020-12-19 15:41           ` Tom de Vries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57aa2ba0-4590-a901-8de9-7f916f02ae1a@suse.de \
    --to=tdevries@suse.de \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox