This is the mail archive of the
`gdb-patches@sourceware.org`
mailing list for the GDB project.

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |

Other format: | [Raw text] |

*From*: Kevin Buettner <kevinb at redhat dot com>*To*: gdb-patches at sourceware dot org*Cc*: Dominik Vogt <vogt at linux dot vnet dot ibm dot com>*Date*: Mon, 16 Nov 2015 22:09:50 -0700*Subject*: Re: [PATCH 0/2] Fix invalid left shift of negative value.*Authentication-results*: sourceware.org; auth=none*References*: <20151110154243 dot 43d38f49 at pinnacle dot lan> <20151111172327 dot 383F51407 at oc7340732750 dot ibm dot com> <20151111122708 dot 69c496d3 at pinnacle dot lan>

On Wed, 11 Nov 2015 12:27:08 -0700 Kevin Buettner <kevinb@redhat.com> wrote: > On Wed, 11 Nov 2015 18:23:27 +0100 (CET) > "Ulrich Weigand" <uweigand@de.ibm.com> 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

**Follow-Ups**:**Re: [PATCH 0/2] Fix invalid left shift of negative value.***From:*Pedro Alves

**References**:**Re: [PATCH 0/2] Fix invalid left shift of negative value.***From:*Kevin Buettner

**Re: [PATCH 0/2] Fix invalid left shift of negative value.***From:*Ulrich Weigand

**Re: [PATCH 0/2] Fix invalid left shift of negative value.***From:*Kevin Buettner

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |