From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 127497 invoked by alias); 17 Nov 2015 05:09:58 -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 127438 invoked by uid 89); 17 Nov 2015 05:09:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 17 Nov 2015 05:09:54 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 523CFAA7; Tue, 17 Nov 2015 05:09:53 +0000 (UTC) Received: from pinnacle.lan (ovpn-113-161.phx2.redhat.com [10.3.113.161]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tAH59qZb013079 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA256 bits=256 verify=NO); Tue, 17 Nov 2015 00:09:53 -0500 Date: Tue, 17 Nov 2015 05:09:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Cc: Dominik Vogt Subject: Re: [PATCH 0/2] Fix invalid left shift of negative value. Message-ID: <20151116220950.1e0f4a89@pinnacle.lan> In-Reply-To: <20151111122708.69c496d3@pinnacle.lan> References: <20151110154243.43d38f49@pinnacle.lan> <20151111172327.383F51407@oc7340732750.ibm.com> <20151111122708.69c496d3@pinnacle.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg00331.txt.bz2 On Wed, 11 Nov 2015 12:27:08 -0700 Kevin Buettner wrote: > On Wed, 11 Nov 2015 18:23:27 +0100 (CET) > "Ulrich Weigand" wrote: > > > Kevin Buettner wrote: > > > > > Looking at one of your changes from part 1/2... > > > > > > - (LONGEST) -1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1); > > > + -((LONGEST) 1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1)); > > > > > > What aspect of the original expression is not defined by the C standard? > > > > The C standard (either C99 or C11) says: > > > > The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits > > are filled with zeros. If E1 has an unsigned type, the value of the result > > is E1 * 2^E2, reduced modulo one more than the maximum value representable > > in the result type. If E1 has a signed type and nonnegative value, and > > E1 * 2^E2 is representable in the result type, then that is the resulting > > value; otherwise, the behavior is undefined. > > > > Note the "otherwise" case includes any E1 of signed type and negative value. > > > > (For >>, the behavior in the latter case is at least implementation- > > defined, and not undefined.) > > Thank you for providing the relevant text from the standard. > > Do you (or anyone else) know the rationale for specifying that the > behavior of << is undefined for (signed) negative values? > > My guess is that it's due to the fact that there are several ways > to represent signed numbers and that the standard has to account for > all of them. > > If that guess is correct, then it seems to me that using the unary > minus operator to help construct a mask is most likely broken for some > signed number representations. (I.e. we won't get the mask that > we've come to expect from the two's complement representation.) If so, > we should consider whether we want to find a more portable way to > construct these masks. > > Regardless, I want to have a better understanding of this matter > before approving Dominik's patch. I've been pondering this some more. It seems to me that there are more than a few places in GDB that assume that two's complement is being used as the representation for signed integers. I came across this comment in defs.h: /* Defaults for system-wide constants (if not defined by xm.h, we fake it). FIXME: Assumes 2's complement arithmetic. */ Is this something that we really want to fix? Can anyone think of a host which can't run GDB (and upon which we'd like to run GDB) due the fact that it uses something other than the two's complement representation for signed integers? My opinion: Assumptions about two's complement in GDB should not be fixed. I can't think of any architecture that I'd care to use which uses something other than two's complement. My limited research on the matter shows that really archaic machines used one's complement or signed magnitude representations. If we all agree that this is something we don't want to fix, then I think we should remove that FIXME and assert somewhere that GDB is expected to be hosted on platforms which use two's complement representation for signed integers. With that in mind... I've looked over both of Dominik's patches. They look okay to me. Kevin