[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