[PATCH] fortran/arith.c -- Fix the range of integers in overflow checks

Steve Kargl sgk@troutmask.apl.washington.edu
Sun Aug 27 18:09:00 GMT 2006


Friends,

During my recent clean up of arith.[ch], I noticed that 
gfortran computed the integer ranges to check for underflow
and overflow.  For any integer kind, the correct range is

  - huge(i) - 1 <= i <= huge(i)

gfortran was using the range
 
  - huge(i) - 1 <= i <= 2 * huge(i) + 1

The confusion arose because the gfc_integer_info structure
includes a max_int = 2 * huge(i) + 1 member that is used as
a mask in the simplification of the NOT() intrinsic function.

This bugs exists in all versions of gfortran.

The attached patch fixes the problem and problems that where
exposed in the testsuite.  The test program intrinsic_set_exponent.f90
may still fail on 64-bit platforms because in essences it is
a bogus piece of code.  If it fails, please reported it to me
so that I can deleted it.

What does the patch do?  It eliminates the max_int structure
member in gfc_integer_info.  The construction of a mask has
been moved to gfc_simplify_not() and integer range checking is
done within the appropriate interval.

Note, this change has an interesting side effect.  I look
forward to the bogus bug reports for code of the form.

   program HaHa
     integer, parameter :: mini = -2147483648
   end program HaHa

Yes,  -2147483648 is the valid minimum integer(4) value.
However, the above code contains a unary minus expression,
so the integer constant 2147483648 overflows it range.


2006-08-27  Steven G. Kargl  <kargls@comcast.net>

	* gfortran.h (gfc_integer_info): Delete	max_int member
	* arith.c (gfc_arith_init_1): Remove initialization of max_int.
 	(gfc_arith_done_1): Remove clearing of max_int.
 	(gfc_check_integer_range): Use huge instead of max_int.
	* simplify.c (gfc_simplify_not): Build a mask in place of max_int.
 
2006-08-27  Steven G. Kargl  <kargls@comcast.net>

	* gfortran.fortran-torture/execute/intrinsic_set_exponent.f90:
	Comment out illegal code.
 	* gfortran.fortran-torture/compile/data_1.f90: Fix overflow.
	* gfortran.dg/enum_8.f90: Fix dg-error for overflow.
	* gfortran.dg/g77/20030326-1.f: Ditto.

-- 
Steve
-------------- next part --------------
Index: gcc/testsuite/gfortran.fortran-torture/execute/intrinsic_set_exponent.f90
===================================================================
--- gcc/testsuite/gfortran.fortran-torture/execute/intrinsic_set_exponent.f90	(revision 116486)
+++ gcc/testsuite/gfortran.fortran-torture/execute/intrinsic_set_exponent.f90	(working copy)
@@ -19,10 +19,11 @@ subroutine test_real4()
   y = set_exponent (x, n)
   if (exponent (y) .ne. n) call abort()
 
-  n = 128
-  i = o'00037777777'
-  y = set_exponent (x, n)
-  if (exponent (y) .ne. n) call abort()
+!  This is fubar.
+!  n = 128
+!  i = o'00037777777'
+!  y = set_exponent (x, n)
+!  if (exponent (y) .ne. n) call abort()
 
   n = -148
   x = -1024.0
@@ -35,10 +36,11 @@ subroutine test_real4()
   if (y .ne. -128.0) call abort()
   if (exponent (y) .ne. n) call abort()
 
-  n = 128
-  i = o'20037777777'
-  y = set_exponent (x, n)
-  if (exponent (y) .ne. n) call abort()
+!  This is fubar.
+!  n = 128
+!  i = o'20037777777'
+!  y = set_exponent (x, n)
+!  if (exponent (y) .ne. n) call abort()
 
 end
 
Index: gcc/testsuite/gfortran.fortran-torture/compile/data_1.f90
===================================================================
--- gcc/testsuite/gfortran.fortran-torture/compile/data_1.f90	(revision 116486)
+++ gcc/testsuite/gfortran.fortran-torture/compile/data_1.f90	(working copy)
@@ -7,5 +7,5 @@ DATA y /a(1.)/ ! used to give an error a
 END
 ! this tests the fix for PR 13940
 SUBROUTINE a
-DATA i /x'f95f95f9'/
+DATA i /z'f95f95'/
 END
Index: gcc/testsuite/gfortran.dg/enum_8.f90
===================================================================
--- gcc/testsuite/gfortran.dg/enum_8.f90	(revision 116486)
+++ gcc/testsuite/gfortran.dg/enum_8.f90	(working copy)
@@ -5,7 +5,7 @@
 program main
   implicit none
   enum, bind (c)
-    enumerator :: pp , qq = 4294967295, rr  ! { dg-error "not initialized with integer" }
+    enumerator :: pp, qq = 4294967295, rr ! { dg-error "too big for its kind" }
   end enum  ! { dg-error "has no ENUMERATORS" }
 
   enum, bind (c)
Index: gcc/testsuite/gfortran.dg/g77/20030326-1.f
===================================================================
--- gcc/testsuite/gfortran.dg/g77/20030326-1.f	(revision 116486)
+++ gcc/testsuite/gfortran.dg/g77/20030326-1.f	(working copy)
@@ -6,5 +6,5 @@
 ! For gfortran, see PR 13490
 !
        integer c
-       c = -2147483648 / (-1) ! { dg-warning "outside symmetric range" "" }
+       c = -2147483648 / (-1) ! { dg-error "too big for its kind" "" }
        end
Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 116486)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -1285,7 +1285,7 @@ gfc_expr;
 typedef struct
 {
   /* Values really representable by the target.  */
-  mpz_t huge, pedantic_min_int, min_int, max_int;
+  mpz_t huge, pedantic_min_int, min_int;
 
   int kind, radix, digits, bit_size, range;
 
Index: gcc/fortran/arith.c
===================================================================
--- gcc/fortran/arith.c	(revision 116486)
+++ gcc/fortran/arith.c	(working copy)
@@ -195,7 +195,7 @@ gfc_arith_init_1 (void)
       /* These are the numbers that are actually representable by the
          target.  For bases other than two, this needs to be changed.  */
       if (int_info->radix != 2)
-        gfc_internal_error ("Fix min_int, max_int calculation");
+        gfc_internal_error ("Fix min_int calculation");
 
       /* See PRs 13490 and 17912, related to integer ranges.
          The pedantic_min_int exists for range checking when a program
@@ -210,10 +210,6 @@ gfc_arith_init_1 (void)
       mpz_init (int_info->min_int);
       mpz_sub_ui (int_info->min_int, int_info->pedantic_min_int, 1);
 
-      mpz_init (int_info->max_int);
-      mpz_add (int_info->max_int, int_info->huge, int_info->huge);
-      mpz_add_ui (int_info->max_int, int_info->max_int, 1);
-
       /* Range  */
       mpfr_set_z (a, int_info->huge, GFC_RND_MODE);
       mpfr_log10 (a, a, GFC_RND_MODE);
@@ -321,7 +317,6 @@ gfc_arith_done_1 (void)
   for (ip = gfc_integer_kinds; ip->kind; ip++)
     {
       mpz_clear (ip->min_int);
-      mpz_clear (ip->max_int);
       mpz_clear (ip->pedantic_min_int);
       mpz_clear (ip->huge);
     }
@@ -356,7 +351,7 @@ gfc_check_integer_range (mpz_t p, int ki
     }
 
   if (mpz_cmp (p, gfc_integer_kinds[i].min_int) < 0
-      || mpz_cmp (p, gfc_integer_kinds[i].max_int) > 0)
+      || mpz_cmp (p, gfc_integer_kinds[i].huge) > 0)
     result = ARITH_OVERFLOW;
 
   return result;
Index: gcc/fortran/simplify.c
===================================================================
--- gcc/fortran/simplify.c	(revision 116486)
+++ gcc/fortran/simplify.c	(working copy)
@@ -2590,6 +2590,7 @@ gfc_simplify_not (gfc_expr * e)
 {
   gfc_expr *result;
   int i;
+  mpz_t mask;
 
   if (e->expr_type != EXPR_CONSTANT)
     return NULL;
@@ -2599,14 +2600,19 @@ gfc_simplify_not (gfc_expr * e)
   mpz_com (result->value.integer, e->value.integer);
 
   /* Because of how GMP handles numbers, the result must be ANDed with
-     the max_int mask.  For radices <> 2, this will require change.  */
+     a mask.  For radices <> 2, this will require change.  */
 
   i = gfc_validate_kind (BT_INTEGER, e->ts.kind, false);
 
-  mpz_and (result->value.integer, result->value.integer,
-	   gfc_integer_kinds[i].max_int);
+  mpz_init (mask);
+  mpz_add (mask, gfc_integer_kinds[i].huge, gfc_integer_kinds[i].huge);
+  mpz_add_ui (mask, mask, 1);
+
+  mpz_and (result->value.integer, result->value.integer, mask);
 
   twos_complement (result->value.integer, gfc_integer_kinds[i].bit_size);
+
+  mpz_clear (mask);
 
   return range_check (result, "NOT");
 }


More information about the Gcc-patches mailing list