From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20091 invoked by alias); 20 Nov 2017 16:42:54 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 20079 invoked by uid 89); 20 Nov 2017 16:42:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.7 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=spirit, inadvertently, stdio, convoluted X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Nov 2017 16:42:52 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1770FC04AC59; Mon, 20 Nov 2017 16:42:51 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2B9E65EE14; Mon, 20 Nov 2017 16:42:48 +0000 (UTC) Subject: Re: [PATCH] Fix type casts losing typedefs and reimplement "whatis" typedef stripping To: Simon Marchi , Yao Qi References: <1498837699-20897-1-git-send-email-palves@redhat.com> <5225ca30-ef69-040d-3f96-be2e5e87b80b@redhat.com> <4b73fb98-b8c2-9d2f-1981-4660e3c202e6@simark.ca> Cc: GDB Patches , Phil Muldoon From: Pedro Alves Message-ID: <67474590-af49-a387-9f8b-044bdd689dc7@redhat.com> Date: Mon, 20 Nov 2017 16:42:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <4b73fb98-b8c2-9d2f-1981-4660e3c202e6@simark.ca> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-11/txt/msg00418.txt.bz2 On 11/18/2017 10:58 PM, Simon Marchi wrote: > On 2017-11-18 03:57 PM, Yao Qi wrote: >> On Mon, Aug 21, 2017 at 11:38 AM, Pedro Alves wrote: >> >> Hi Pedro, >> The new tests in gdb.base/whatis-ptype-typedefs.exp fail on 32-bit target. >> >> https://gdb-build.sergiodj.net/builders/Ubuntu-AArch32-m32/builds/1175/steps/test%20gdb/logs/stdio >> https://gdb-build.sergiodj.net/builders/Fedora-i686/builds/6867/steps/test%20gdb/logs/stdio >> https://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/6849/steps/test%20gdb/logs/stdio >> >> Can you take a look? >> > > I took a quick look. The issue (at least one of them) boils down to the fact > that on 64 bits, you can't do this: > > (gdb) p (float_typedef) v_uchar_array_t_struct_typedef > Invalid cast. > > but on 32 bits you can: > > (gdb) p (float_typedef) v_uchar_array_t_struct_typedef > $1 = 1.16251721e-41 > > The expression basically tries to cast an array (which decays to a pointer) to > a float. The cast works on 32 bits (doesn't give Invalid cast) because a float > and a pointer are of the same size, and the execution enters this if branch: > > https://github.com/bminor/binutils-gdb/blob/master/gdb/valops.c#L554 > > On 64 bits, they are not the same size, so it ends up in the invalid cast > branch. > > I don't really know what to do from there. Should we leave the behavior as-is > and update the test, or prevent this kind of cast (the compiler doesn't accept > that anyway, and I don't see any real use case to this). This function (value_cast) > is a bit convoluted, I'm always afraid to touch it... I'm not 100% sure either. value_cast is documented as: ~~~ More general than a C cast: accepts any two types of the same length, and if ARG2 is an lvalue it can be cast into anything at all. */ ~~~ and I've found this useful on several occasions (though for me it's usually more around converting an object to some structure). The point of the test was to cover as many of code paths in value_cast as possible, as a sort of documentation of the current behavior: # The main idea here is testing all the different paths in the # value casting code in GDB (value_cast), making sure typedefs are # preserved. ... # We try all combinations, even those that don't # parse, or are invalid, to catch the case of a # regression making them inadvertently valid. For # example, these convertions are invalid: ... So in that spirit, I propose starting my making the testcase adjust itself, like below, and also test floats of different sizes, leaving changing GDB's behavior for a separate consideration/change (using the fixed/extended test as baseline). This passes on x86 both 64-bit and 32-bit. >From 1a02eedaadcb7b62a5990f5838952b0600d4a8cc Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 20 Nov 2017 16:39:58 +0000 Subject: [PATCH] fix --- gdb/testsuite/gdb.base/whatis-ptype-typedefs.c | 10 +++++++ gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp | 37 ++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/gdb/testsuite/gdb.base/whatis-ptype-typedefs.c b/gdb/testsuite/gdb.base/whatis-ptype-typedefs.c index 5711a96..35c7279 100644 --- a/gdb/testsuite/gdb.base/whatis-ptype-typedefs.c +++ b/gdb/testsuite/gdb.base/whatis-ptype-typedefs.c @@ -56,6 +56,16 @@ DEF (int); typedef float float_typedef; DEF (float); +/* Double floats. */ + +typedef double double_typedef; +DEF (double); + +/* Long doubles. */ + +typedef long double long_double_typedef; +DEF (long_double); + /* Enums. */ typedef enum colors {red, green, blue} colors_typedef; diff --git a/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp b/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp index d333d81..c8fa2bd 100644 --- a/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp +++ b/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp @@ -92,6 +92,16 @@ set table { {"v_float_typedef" "float_typedef" "float"} {"v_float_typedef2" "float_typedef2" "float"} + {"double_typedef" "double" "double"} + {"double_typedef2" "double_typedef" "double"} + {"v_double_typedef" "double_typedef" "double"} + {"v_double_typedef2" "double_typedef2" "double"} + + {"long_double_typedef" "long double" "long double"} + {"long_double_typedef2" "long_double_typedef" "long double"} + {"v_long_double_typedef" "long_double_typedef" "long double"} + {"v_long_double_typedef2" "long_double_typedef2" "long double"} + {"colors_typedef" "(enum )?colors" "enum colors( : unsigned int)? {red, green, blue}"} {"colors_typedef2" "colors_typedef" "enum colors( : unsigned int)? {red, green, blue}"} {"v_colors_typedef" "colors_typedef" "enum colors( : unsigned int)? {red, green, blue}"} @@ -199,6 +209,20 @@ proc run_tests {lang} { } } + # If floats and pointers have he same on this architecture, then + # casting from array/function to float works, because + # arrays/functions first decay to pointers, and then GDB's cast is + # more general than a C cast and accepts any two types of the same + # length. + set float_ptr_same_size \ + [get_integer_valueof "sizeof (float) == sizeof (void *)" -1] + + set double_ptr_same_size \ + [get_integer_valueof "sizeof (double) == sizeof (void *)" -1] + + set long_double_ptr_same_size \ + [get_integer_valueof "sizeof (long double) == sizeof (void *)" -1] + # Test converting/casting all variables in the first column of the # table to all types (found in the first column of the table). # The aggregates are all defined to be the same size so that @@ -230,7 +254,7 @@ proc run_tests {lang} { # regression making them inadvertently valid. For # example, these convertions are invalid: # - # float <-> array + # float <-> array [iff sizeof pointer != sizeof float] # array -> function (not function pointer) # array -> member_ptr # @@ -247,8 +271,15 @@ proc run_tests {lang} { gdb_test "whatis ($to) $from" "syntax error.*" "whatis ($to) $from (syntax)" gdb_test "ptype ($to) $from" "syntax error.*" "ptype ($to) $from (syntax)" } elseif {([string match "*float*" $from] && [string match "*array*" $to]) - || ([string match "float*" $to] && [string match "*array*" $from]) - || ([string match "float*" $to] && [string match "*method" $from]) + || (!$float_ptr_same_size + && ([string match "float*" $to] && [string match "*array*" $from] + || [string match "float*" $to] && [string match "*method" $from])) + || (!$double_ptr_same_size + && ([string match "double*" $to] && [string match "*array*" $from] + || [string match "double*" $to] && [string match "*method" $from])) + || (!$long_double_ptr_same_size + && ([string match "long_double*" $to] && [string match "*array*" $from] + || [string match "long_double*" $to] && [string match "*method" $from])) || ([string match "*ftype" $to] && [string match "*array*" $from]) || ([string match "*ftype2" $to] && [string match "*array*" $from]) || ([string match "*ftype" $to] && [string match "*method" $from]) -- 2.5.5