From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92016 invoked by alias); 18 Sep 2017 16:21:05 -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 92000 invoked by uid 89); 18 Sep 2017 16:21:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 18 Sep 2017 16:21:02 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v8IGKsbm007546 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 18 Sep 2017 12:20:59 -0400 Received: by simark.ca (Postfix, from userid 112) id 7CBD31ECEE; Mon, 18 Sep 2017 12:20:54 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id EBECC1EA1D; Mon, 18 Sep 2017 12:20:53 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 18 Sep 2017 16:21:00 -0000 From: Simon Marchi To: Ulrich Weigand Cc: gdb-patches@sourceware.org Subject: Re: [RFC][02/19] Target FP: Simplify floatformat_from_type In-Reply-To: <20170918114951.28349D8083B@oc3748833570.ibm.com> References: <20170918114951.28349D8083B@oc3748833570.ibm.com> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 18 Sep 2017 16:20:54 +0000 X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00448.txt.bz2 On 2017-09-18 13:49, Ulrich Weigand wrote: > Simon Marchi wrote: > >> On 2017-09-05 20:20, Ulrich Weigand wrote: >> > size_t len = bit / TARGET_CHAR_BIT; >> > - gdb_assert (len >= floatformat_totalsize_bytes (floatformats[0])); >> > - gdb_assert (len >= floatformat_totalsize_bytes (floatformats[1])); >> > + gdb_assert (len * TARGET_CHAR_BIT >= floatformat->totalsize); >> >> That looks funny now. Is there a reason not to do >> >> gdb_assert (bit >= floatformat->totalsize); >> >> directly? > > Well, that would use different rounding ... Of course, this only > matters when floatformat->totalsize is not a multiple of > TARGET_CHAR_BIT, > which doesn't happen for any of the floatformats GDB supports, so it > is probably moot. Maybe we should instead use an assert to verify that > floatformat->totalsize is in fact a multiple of TARGET_CHAR_BIT? I indeed think we should add an assert that the type size in bits is a multiple of TARGET_CHAR_BIT. But it should not be in verify_floatformat, because it's not specific to float. In most calls to arch_type/init_type, we have that division that rounds down: t = init_type (objfile, TYPE_CODE_FLT, bit / TARGET_CHAR_BIT, name); Let's say we try to add an integer with bit == 24 and TARGET_CHAR_BIT == 16, we'll create a type with a size of 1 target byte, and things will likely break down the line. Since this is where we do the division by TARGET_CHAR_BIT, and therefore assume (implicitly) that bit is a multiple of TARGET_CHAR_BIT, I think this is where the assert should be added. To avoid adding them everywhere, we could make arch_type/init_type take a size in bits, and do the division and assert there. As long as verify_floatformat is concerned, its job is to verify (IIUC) that the type you want to create with BIT bits has enough room to hold a float of that kind. It doesn't have to know that we'll later divide that value by TARGET_CHAR_BIT. So here I think asserting "bit >= floatformat->totalsize)" is fine. I don't think floatformat->totalsize has any requirement to be a multiple of TARGET_CHAR_BIT. For example, my hardware could have a float of 29 bits, but when stored in memory it uses 32 bits (just like on x86 long double is an 80 bit float stored in a 96 bit data type). Then, bit would be 32 and floatformat->totalsize would be 29. Simon