On Fri, Aug 22, 2025 at 03:43:48PM +0530, Surya Kumari Jangala wrote:
> On 01/08/25 4:48 am, Kishan Parmar wrote:
> > This patch adds missing guards on shift amounts to prevent UB when the
> > shift count equals or exceeds HOST_BITS_PER_WIDE_INT.
> >
> > Although the main undefined behavior issues were addressed in a previous
> > patch (r16-2666-g647bd0a02789f1), two cases remained where shift counts
> > were only checked for nonzero but not for being within valid bounds.
> > This patch tightens those conditions by enforcing that shift counts are
> > greater than zero and less than HOST_BITS_PER_WIDE_INT.
>
> You can modify the commit message as:
> "This patch adds missing guards on shift amounts to prevent UB when the
> shift count equals or exceeds HOST_BITS_PER_WIDE_INT.
>
> In the patch (r16-2666-g647bd0a02789f1), shift counts
> were only checked for nonzero but not for being within valid bounds.
> This patch tightens those conditions by enforcing that shift counts are
> greater than zero and less than HOST_BITS_PER_WIDE_INT."
>
> With the above change, the patch looks good to me.
>
> Segher needs to approve before you can upstream it.
Yes! Approved for trunk (as well as any needed backports). Thanks!
(But see note below).
Segher
> > 2025-08-01 Kishan Parmar <kis...@linux.ibm.com>
> >
> > gc
(you need to fix this if you want the commit scripts to pick this up
automatically!)
> > PR target/118890
> > * config/rs6000/rs6000.cc (can_be_rotated_to_negative_lis): Add bounds
> > checks for shift counts to prevent undefined behavior.
> > (rs6000_emit_set_long_const): Likewise.
> > ---
> > gcc/config/rs6000/rs6000.cc | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> > index 764b4992fb5..8dd23f8619c 100644
> > --- a/gcc/config/rs6000/rs6000.cc
> > +++ b/gcc/config/rs6000/rs6000.cc
> > @@ -10322,7 +10322,7 @@ can_be_rotated_to_negative_lis (HOST_WIDE_INT c,
> > int *rot)
> > rotated over the highest bit. */
> > unsigned HOST_WIDE_INT uc = c;
> > int pos_one = clz_hwi ((HOST_WIDE_INT) (uc << 16) >> 16);
> > - if (pos_one != 0)
> > + if (pos_one > 0 && pos_one < HOST_BITS_PER_WIDE_INT)
> > {
> > middle_zeros = ctz_hwi (c >> (HOST_BITS_PER_WIDE_INT - pos_one));
> > int middle_ones = clz_hwi (~(uc << pos_one));
> > @@ -10585,7 +10585,7 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT
> > c, int *num_insns)
> > {
> > /* li/lis; rldicX */
> > unsigned HOST_WIDE_INT imm = (c | ~mask);
> > - if (shift != 0)
> > + if (shift > 0 && shift < HOST_BITS_PER_WIDE_INT)
> > imm = (imm >> shift) | (imm << (HOST_BITS_PER_WIDE_INT - shift));
> >
> > count_or_emit_insn (temp, GEN_INT (imm));
