Jakub Jelinek via Gcc <gcc@gcc.gnu.org> writes:
> On Tue, Oct 27, 2020 at 01:18:03PM -0400, Andrew MacLeod via Gcc wrote:
>> I was looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97596
>> 
>> and the ranger is constructing some 128 bit constants and calling
>> wide_int_to_tree to turn them into trees.
>> 
>> In particular, it starts with the value
>> 
>> p r.lower_bound(0)
>> {<wide_int_storage> = {val = {-65535, 9223372036854775807, 140737488257608},
>> len = 2, precision = 128}, static is_sign_extended = true}
>> 
>> p r.lower_bound(0).dump()
>> [0x7fffffffffffffff,0xffffffffffff0001], precision = 128
>> 
>> 
>>  and proceeds to call
>> 
>> wide_int new_lb = wi::set_bit (r.lower_bound (0), 127)
>> 
>> and creates the value:
>> 
>> p new_lb
>> {<wide_int_storage> = {val = {-65535, -1, 0}, len = 2, precision = 128},
>> static is_sign_extended = true}
>
> This is non-canonical and so invalid, if the low HWI has the MSB set
> and the high HWI is -1, it should have been just
> val = {-65535}, len = 1, precision = 128}
>
> I guess the bug is that wi::set_bit_large doesn't call canonize.

Yeah, looks like a micro-optimisation gone wrong.

> So perhaps totally untested:
>
> --- gcc/wide-int.cc.jj        2020-10-19 18:42:41.134426398 +0200
> +++ gcc/wide-int.cc   2020-10-27 18:33:38.546703763 +0100
> @@ -702,8 +702,11 @@ wi::set_bit_large (HOST_WIDE_INT *val, c
>        /* If the bit we just set is at the msb of the block, make sure
>        that any higher bits are zeros.  */
>        if (bit + 1 < precision && subbit == HOST_BITS_PER_WIDE_INT - 1)
> -     val[len++] = 0;
> -      return len;
> +     {
> +       val[len++] = 0;
> +       return len;
> +     }
> +      return canonize (val, len, precision);
>      }
>    else
>      {

LGTM, thanks.

Richard

Reply via email to