From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 2A/5HlG+3V/DDwAAWB0awg (envelope-from ) for ; Sat, 19 Dec 2020 03:48:17 -0500 Received: by simark.ca (Postfix, from userid 112) id 7CF4D1F0AA; Sat, 19 Dec 2020 03:48:17 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.3 required=5.0 tests=MAILING_LIST_MULTI,RDNS_NONE, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 042B41E99A for ; Sat, 19 Dec 2020 03:48:17 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9D0E43854829; Sat, 19 Dec 2020 08:48:16 +0000 (GMT) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id F34123854801 for ; Sat, 19 Dec 2020 08:48:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org F34123854801 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 1AB55AD45; Sat, 19 Dec 2020 08:48:13 +0000 (UTC) Subject: Re: [PATCH][gdb/testsuite] Fix gdb.base/endianity.exp with gcc-4.8 To: Simon Marchi , Joel Brobecker References: <20201210152945.GA16462@delia> <20201213135632.GE366101@adacore.com> <57aa2ba0-4590-a901-8de9-7f916f02ae1a@suse.de> <686f81a2-cc07-fe65-02c1-2cb9ec9377e4@simark.ca> From: Tom de Vries Message-ID: <208ea7a9-d382-7fe4-89b2-75a9b6aa4e10@suse.de> Date: Sat, 19 Dec 2020 09:48:11 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 MIME-Version: 1.0 In-Reply-To: <686f81a2-cc07-fe65-02c1-2cb9ec9377e4@simark.ca> Content-Type: multipart/mixed; boundary="------------8E771050DE9641C6253CC8DB" Content-Language: en-US X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" This is a multi-part message in MIME format. --------------8E771050DE9641C6253CC8DB Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 12/14/20 5:55 PM, Simon Marchi wrote: > On 2020-12-13 11:23 a.m., Tom de Vries wrote: >> 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 >> > > Just a small nit, it would be nice to have an "unsupported" call in the > if to record in the sum file that part of this file was skipped and why. > > Also, I don't like the (pre-existing) check: > > if { ([test_compiler_info {gcc-[0-5]-*}] || ![test_compiler_info gcc*]) } { > > That makes it so testing with any other compiler than gcc will result in > this part being skipped. What if clang gains support for this feature? > > Would it make sense to have a small > "supports_scalar_storage_order_attribute" util proc in lib/gdb.exp? How about this? Thanks, - Tom --------------8E771050DE9641C6253CC8DB Content-Type: text/x-patch; charset=UTF-8; name="0005-gdb-testsuite-Introduce-supports_scalar_storage_order_attribute.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0005-gdb-testsuite-Introduce-supports_scalar_storage_order_a"; filename*1="ttribute.patch" [gdb/testsuite] Introduce supports_scalar_storage_order_attribute Introduce support test procs: - supports_scalar_storage_order_attribute, and - supports_gnuc and use them in test-case gdb.base/endianity.exp. Tested on x86_64-linux with gcc-7.5.0, gcc-4.8.5, and clang 10.0.1. gdb/testsuite/ChangeLog: 2020-12-19 Tom de Vries * lib/gdb.exp (supports_scalar_storage_order_attribute) (supports_gnuc): New proc. * gdb.base/endianity.exp: Define TEST_SSO. Eliminate test_compiler_info calls. Add unsupported message. * gdb.base/endianity.c: Use TEST_SSO. --- gdb/testsuite/gdb.base/endianity.c | 2 +- gdb/testsuite/gdb.base/endianity.exp | 11 ++++++--- gdb/testsuite/lib/gdb.exp | 46 ++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/gdb/testsuite/gdb.base/endianity.c b/gdb/testsuite/gdb.base/endianity.c index ef3b6d4fdb..15cbdd9a26 100644 --- a/gdb/testsuite/gdb.base/endianity.c +++ b/gdb/testsuite/gdb.base/endianity.c @@ -26,7 +26,7 @@ struct otherendian __complex__ float cplx; double d; } -#if defined __GNUC__ && (__GNUC__ >= 6) +#if TEST_SSO #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ __attribute__( ( scalar_storage_order( "big-endian" ) ) ) #else diff --git a/gdb/testsuite/gdb.base/endianity.exp b/gdb/testsuite/gdb.base/endianity.exp index 4520799d04..d54c1b6973 100644 --- a/gdb/testsuite/gdb.base/endianity.exp +++ b/gdb/testsuite/gdb.base/endianity.exp @@ -15,7 +15,12 @@ standard_testfile .c -if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { +set test_sso [expr \ + [supports_scalar_storage_order_attribute] \ + && [supports_gnuc]] + +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \ + [list debug additional_flags=-DTEST_SSO=$test_sso]] } { return -1 } @@ -37,12 +42,12 @@ gdb_test "print o.d = -23.125" "= -23.125" 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*]) } { +if { !$test_sso } { # The rest of the testcase requires Scalar Storage Order support. # This compiler does not support it, so skip the rest. + unsupported "No scalar storage order support" return -1 } gdb_test "x/x &o.v" "0x04000000" gdb_test "x/xh &o.w" "0x0300" - diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index e812237d67..573d9cc802 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -7669,5 +7669,51 @@ gdb_caching_proc have_fuse_ld_gold { return [gdb_simple_compile $me $src executable $flags] } +# Return 1 if compiler supports scalar_storage_order attribute, otherwise +# return 0. +gdb_caching_proc supports_scalar_storage_order_attribute { + set me "supports_scalar_storage_order_attribute" + set src { + #include + struct sle { + int v; + } __attribute__((scalar_storage_order("little-endian"))); + struct sbe { + int v; + } __attribute__((scalar_storage_order("big-endian"))); + struct sle sle; + struct sbe sbe; + int main () { + sle.v = sbe.v = 0x11223344; + int same = memcmp (&sle, &sbe, sizeof (int)) == 0; + int sso = !same; + return sso; + } + } + if { ![gdb_simple_compile $me $src executable ""] } { + return 0 + } + + set result [remote_exec host $obj] + set status [lindex $result 0] + set output [lindex $result 1] + if { $output != "" } { + return 0 + } + + return $status +} + +# Return 1 if compiler supports __GNUC__, otherwise return 0. +gdb_caching_proc supports_gnuc { + set me "supports_gnuc" + set src { + #ifndef __GNUC__ + #error "No gnuc" + #endif + } + return [gdb_simple_compile $me $src object ""] +} + # Always load compatibility stuff. load_lib future.exp --------------8E771050DE9641C6253CC8DB--