Mikael Pettersson <mi...@it.uu.se> writes:
> John David Anglin writes:
> > On Sun, 18 Nov 2012, Richard Sandiford wrote:
> >
> > > HOST_WIDE_INT start = bitpos_ - (bitpos_ % unit);
> > > if (bitregion_start_ && start < bitregion_start_)
> > > break;
> > > - if (bitregion_end_ && start + unit > bitregion_end_ + 1)
> > > + if (start + unit > bitregion_end_ + 1)
> >
> > This causes:
> >
> > /home/dave/gnu/gcc/objdir/./prev-gcc/g++
> -B/home/dave/gnu/gcc/objdir/./prev-gcc/
> -B/home/dave/opt/gnu/gcc/gcc-4.8.0/hppa-linux-gnu/bin/ -nostdinc++
> -B/home/dave/gnu/gcc/objdir/prev-hppa-linux-gnu/libstdc++-v3/src/.libs
> -B/home/dave/gnu/gcc/objdir/prev-hppa-linux-gnu/libstdc++-v3/libsupc++/.libs
> -I/home/dave/gnu/gcc/objdir/prev-hppa-linux-gnu/libstdc++-v3/include/hppa-linux-gnu
> -I/home/dave/gnu/gcc
> > /objdir/prev-hppa-linux-gnu/libstdc++-v3/include
> -I/home/dave/gnu/gcc/gcc/libstdc++-v3/libsupc++
> -L/home/dave/gnu/gcc/objdir/prev-hppa-linux-gnu/libstdc++-v3/sr
> > c/.libs
> -L/home/dave/gnu/gcc/objdir/prev-hppa-linux-gnu/libstdc++-v3/libsupc++/.libs
> -c -g -O2 -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tabl
> > es -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual
> -Wmissing-format-attribut
> > e -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings
> -Werror
> > -fno-common -DHAVE_CONFIG_H -I. -I. -I../../gcc/gcc -I../../gcc/gcc/.
> -I../../g
> > cc/gcc/../include -I../../gcc/gcc/../libcpp/include
> -I../../gcc/gcc/../libdecnu
> > mber -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber
> -I../../gcc/gcc/../li
> > bbacktrace ../../gcc/gcc/stor-layout.c -o
> stor-layout.o../../gcc/gcc/stor-layout.c: In member function 〘bool
> bit_field_mode_iterator::n
> > ext_mode(machine_mode*)〙:
> > ../../gcc/gcc/stor-layout.c:2690:43: error: comparison between signed and
> unsign
> > ed integer expressions [-Werror=sign-compare]
> > if (start + unit > bitregion_end_ + 1)
> > ^
> > cc1plus: all warnings being treated as errors
>
> This error also breaks m68k-linux bootstrap.
>
> HWI32 issue?
Yeah, I expect so, sorry.
Logically, everything here would be unsigned arithmetic, but as the
comment says:
/* We use signed values here because the bit position can be negative
for invalid input such as gcc.dg/pr48335-8.c. */
This is the patch I'm testing. There are three things being checked here:
- "unit", the size of the mode in isolation. This really is an unsigned
value, and is compared to unsigned values like GET_MODE_PRECISION.
- bitpos_ % unit (+ bitsize_), the start and end positions of the bitfield
relative to the start of the mode. The start position is supposed to be
[0, unit), so the modulus and result should be unsigned. (Using unsigned
modulus doesn't cope with negative bit positions combined with
non-power-of-2 units, but I don't think we support that.)
- bitregion_start_ and bitregion_end_. bitpos_ is signed and can be
negative, so the bitregion comparison should continue to be signed.
OK to commit if testing succeeds?
Richard
gcc/
* stor-layout.c (bit_field_mode_iterator::next_mode): Fix signedness.
Index: gcc/stor-layout.c
===================================================================
--- gcc/stor-layout.c 2012-11-20 10:15:39.000000000 +0000
+++ gcc/stor-layout.c 2012-11-20 10:15:39.464712715 +0000
@@ -2670,10 +2670,6 @@ bit_field_mode_iterator::next_mode (enum
if (unit != GET_MODE_PRECISION (mode_))
continue;
- /* Skip modes that are too small. */
- if ((bitpos_ % unit) + bitsize_ > unit)
- continue;
-
/* Stop if the mode is too wide to handle efficiently. */
if (unit > MAX_FIXED_MODE_SIZE)
break;
@@ -2683,11 +2679,18 @@ bit_field_mode_iterator::next_mode (enum
if (count_ > 0 && unit > BITS_PER_WORD)
break;
+ /* Skip modes that are too small. */
+ unsigned HOST_WIDE_INT substart = (unsigned HOST_WIDE_INT) bitpos_ %
unit;
+ unsigned HOST_WIDE_INT subend = substart + bitsize_;
+ if (subend > unit)
+ continue;
+
/* Stop if the mode goes outside the bitregion. */
- HOST_WIDE_INT start = bitpos_ - (bitpos_ % unit);
+ HOST_WIDE_INT start = bitpos_ - substart;
if (bitregion_start_ && start < bitregion_start_)
break;
- if (start + unit > bitregion_end_ + 1)
+ HOST_WIDE_INT end = start + unit;
+ if (end > bitregion_end_ + 1)
break;
/* Stop if the mode requires too much alignment. */
