Richard Sandiford <rdsandif...@googlemail.com> writes: > 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?
Now bootstrapped & regression-tested on x86_64-linux-gnu. > > 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. */
