diff mbox

PR 63427: Missing wrapping operation in wi::from_mpz

Message ID 8761ascmqh.fsf@googlemail.com
State New
Headers show

Commit Message

Richard Sandiford Feb. 23, 2015, 10 p.m. UTC 
The comment for wi::from_mpz says:

/* Returns X converted to TYPE.  If WRAP is true, then out-of-range
   values of VAL will be wrapped; otherwise, they will be set to the
   appropriate minimum or maximum TYPE bound.  */

The problem in PR 63427 was that we didn't actually implement the WRAP
case; we just read the whole thing into a wide_int and hoped that it
was already in range of the target precision.  In the testcase this
resulted in the "len" field being too big for the precision, which
"only" showed up as a failure when ubsan was enabled.  But in more
extreme cases we could walk well beyond the end up of the buffer,
as shown by a segfault in the attached testcase.

Tested on x86_64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
	PR fortran/63427
	* wide-int.cc (wi::from_mpz): Cope with unwrapped values that are
	too big for a wide_int.  Implement missing wrapping operation.

gcc/testsuite/
	PR fortran/63427
	* gfortran.dg/integer_exponentiation_6.F90: New test.

Comments

Jeff Law Feb. 23, 2015, 10:44 p.m. UTC | #1 
On 02/23/15 15:00, Richard Sandiford wrote:
> The comment for wi::from_mpz says:
>
> /* Returns X converted to TYPE.  If WRAP is true, then out-of-range
>     values of VAL will be wrapped; otherwise, they will be set to the
>     appropriate minimum or maximum TYPE bound.  */
>
> The problem in PR 63427 was that we didn't actually implement the WRAP
> case; we just read the whole thing into a wide_int and hoped that it
> was already in range of the target precision.  In the testcase this
> resulted in the "len" field being too big for the precision, which
> "only" showed up as a failure when ubsan was enabled.  But in more
> extreme cases we could walk well beyond the end up of the buffer,
> as shown by a segfault in the attached testcase.
>
> Tested on x86_64-linux-gnu.  OK to install?
>
> Thanks,
> Richard
>
>
> gcc/
> 	PR fortran/63427
> 	* wide-int.cc (wi::from_mpz): Cope with unwrapped values that are
> 	too big for a wide_int.  Implement missing wrapping operation.
>
> gcc/testsuite/
> 	PR fortran/63427
> 	* gfortran.dg/integer_exponentiation_6.F90: New test.
OK.
jeff
diff mbox

Patch

Index: gcc/wide-int.cc
===================================================================
--- gcc/wide-int.cc	2015-02-21 14:33:53.493206747 +0000
+++ gcc/wide-int.cc	2015-02-21 14:47:02.546425643 +0000
@@ -237,7 +237,7 @@  wide_int
 wi::from_mpz (const_tree type, mpz_t x, bool wrap)
 {
   size_t count, numb;
-  int prec = TYPE_PRECISION (type);
+  unsigned int prec = TYPE_PRECISION (type);
   wide_int res = wide_int::create (prec);
 
   if (!wrap)
@@ -261,16 +261,28 @@  wi::from_mpz (const_tree type, mpz_t x,
      for representing the value.  The code to calculate count is
      extracted from the GMP manual, section "Integer Import and Export":
      http://gmplib.org/manual/Integer-Import-and-Export.html  */
-  numb = 8 * sizeof(HOST_WIDE_INT);
+  numb = CHAR_BIT * sizeof (HOST_WIDE_INT);
   count = (mpz_sizeinbase (x, 2) + numb - 1) / numb;
   HOST_WIDE_INT *val = res.write_val ();
-  mpz_export (val, &count, -1, sizeof (HOST_WIDE_INT), 0, 0, x);
+  /* Write directly to the wide_int storage if possible, otherwise leave
+     GMP to allocate the memory for us.  It might be slightly more efficient
+     to use mpz_tdiv_r_2exp for the latter case, but the situation is
+     pathological and it seems safer to operate on the original mpz value
+     in all cases.  */
+  void *valres = mpz_export (count <= WIDE_INT_MAX_ELTS ? val : 0,
+			     &count, -1, sizeof (HOST_WIDE_INT), 0, 0, x);
   if (count < 1)
     {
       val[0] = 0;
       count = 1;
     }
-  res.set_len (count);
+  count = MIN (count, BLOCKS_NEEDED (prec));
+  if (valres != val)
+    {
+      memcpy (val, valres, count * sizeof (HOST_WIDE_INT));
+      free (valres);
+    }
+  res.set_len (canonize (val, count, prec));
 
   if (mpz_sgn (x) < 0)
     res = -res;
Index: gcc/testsuite/gfortran.dg/integer_exponentiation_6.F90
===================================================================
--- /dev/null	2015-02-14 10:03:47.263968597 +0000
+++ gcc/testsuite/gfortran.dg/integer_exponentiation_6.F90	2015-02-21 14:35:47.110411071 +0000
@@ -0,0 +1,4 @@ 
+! { dg-options "-fno-range-check" }
+program test
+  write (*), (2_8 ** 64009999_8) / 2
+end program test