[PATCH] Fix up wi::lrshift (PR c++/69399)
Jakub Jelinek
jakub@redhat.com
Tue Jan 26 18:21:00 GMT 2016
Hi!
On Tue, Jan 26, 2016 at 04:54:43PM +0100, Richard Biener wrote:
> > Somehow PR 65656 miscompiled:
> >
> > if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
> > ? xi.len == 1 && xi.val[0] >= 0
> > : xi.precision <= HOST_BITS_PER_WIDE_INT)
> >
> > which turned the expression into true and hit
> >
> > val[0] = xi.to_uhwi () >> shift;
> > result.set_len (1);
>
> I think we need a better analysis as we use __builtin_constant_p
> elsewhere as well.
So, I had a look at this bug and it seems clearly a bug on the wide-int.h
side. wi::lrshift right now doesn't do what the comment says (which says
that for the larger precision fixed types it only cares about constant
shift).
Compared to wi::lshift, for the
STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
case lrshift doesn't bother to check the value of shift.
While for xi.precision <= HOST_BITS_PER_WIDE_INT, at least conforming code
should not perform out of bounds shifts and thus there is no need to check
the value of shift, for xi.precision > HOST_BITS_PER_WIDE_INT it is
completely valid to have large shift (in between HOST_BITS_PER_WIDE_INT
inclusive and xi.precision exclusive), and then we just trigger undefined
behavior for that case.
The question is, shall we do what wi::lshift does and have the fast path
only for the constant shifts, as the untested patch below does, or shall we
check shift dynamically (thus use
shift < HOST_BITS_PER_WIDE_INT
instead of
STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT)
in the patch), or shall we have another case for such shifts and set
val[0] = 0; then?
The __builtin_constant_p change affects whether we trigger this bug
only for fixed large precision instantiations (with trunk
__builtin_constant_p) or also by mistake for variable large precision
instantiations (with gcc 5 __builtin_constant_p), but the fixed large
precision instantiations are broken in any case.
2016-01-26 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/69399
* wide-int.h (wi::lrshift): For larger precisions, only
use fast path if shift is known to be < HOST_BITS_PER_WIDE_INT.
--- gcc/wide-int.h.jj 2016-01-04 14:55:50.000000000 +0100
+++ gcc/wide-int.h 2016-01-26 19:05:20.715269366 +0100
@@ -2909,7 +2909,9 @@ wi::lrshift (const T1 &x, const T2 &y)
For variable-precision integers like wide_int, handle HWI
and sub-HWI integers inline. */
if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
- ? xi.len == 1 && xi.val[0] >= 0
+ ? (STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT)
+ && xi.len == 1
+ && xi.val[0] >= 0)
: xi.precision <= HOST_BITS_PER_WIDE_INT)
{
val[0] = xi.to_uhwi () >> shift;
Jakub
More information about the Gcc-patches
mailing list