Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095)

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, Jun 1, 2015 at 10:06 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Marek Polacek <polacek@redhat.com> writes:
>> +  /* Left-hand operand must be signed.  */
>> +  if (TYPE_UNSIGNED (type0))
>> +    return false;
>> +
>> +  /* Compute the result in infinite precision math (sort of).  */
>> +  widest_int w = wi::lshift (wi::to_widest (op0), wi::to_widest (op1));
>> +  unsigned int min_prec = wi::min_precision (w, SIGNED);
>> +  /* Watch out for shifting a negative value.  */
>> +  tree r = wide_int_to_tree (tree_int_cst_sgn (op0) >= 0
>> +                          ? unsigned_type_for (type0)
>> +                          : type0, w);
>> +  bool overflowed = wi::cmps (w, wi::to_widest (r));
>> +  if (overflowed && c_inhibit_evaluation_warnings == 0)
>> +    warning_at (loc, OPT_Wshift_overflow,
>> +             "result of %qE requires %u bits to represent, "
>> +             "but %qT only has %u bits",
>> +             build2_loc (loc, LSHIFT_EXPR, type0, op0, op1),
>> +             min_prec, type0, TYPE_PRECISION (type0));
>> +
>> +  return overflowed;
>
> Yeah, this "sort of" is a bit worrying :-)  Especially as the number
> of bits in a widest_int depends on the number of bits in the target's
> widest integer mode.  E.g. for i386 it's forced to 128, but for ARM
> it's 512 (IIRC).
>
> Could you do the check based on the wi::min_precision of the unshifted
> value?  I.e. see whether adding the shift amount to that gives a value
> greater than type's precision.

You could always use a FIXED_WIDE_INT like VRP does for its overflow
detection stuff.

Richard.

> Thanks,
> Richard