diff mbox

[fortran] PR66310 Problems with intrinsic repeat for large number of copies

Message ID 625ff20d-7a64-c9e4-1b3d-6c85e93a3dcf@charter.net
State New
Headers show

Commit Message

Jerry DeLisle July 18, 2016, 12:02 a.m. UTC
On 07/12/2016 06:26 AM, Dominique d'Humières wrote:
>>> 2016-07-11  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
>>>
>>> 	PR fortran/66310
>>> 	* simplify.c (gfc_simplify_repeat): Set max repeat to huge - 1 to allow
>>> 	one byte for null terminating the resulting string constant.
>>
>> OK, thanks
> Please hold on. I still see several problem with the patch applied. One is
> 
>    program p
>       character :: z = 'z'
>       print *, repeat(z, huge(1)-2**9)
>    end

Please test this revised patch. See my comments in the PR.

I think we should commit this one.

Jerry

Comments

Dominique d'Humières July 19, 2016, 9:56 a.m. UTC | #1
> Le 18 juil. 2016 à 02:02, Jerry DeLisle <jvdelisle@charter.net> a écrit :
> 
> Please test this revised patch. See my comments in the PR.
> 
> I think we should commit this one.
> 
> Jerry

Jerry,

As said on IRC I think the limit should be documented and a TODO comment added to gcc/fortran/gfortran.h.

While trying to bootstrap with the patch I got

/opt/gcc/build_w/./prev-gcc/xg++ -B/opt/gcc/build_w/./prev-gcc/ -B/opt/gcc/gcc7w/x86_64-apple-darwin15.5.0/bin/ -nostdinc++ -B/opt/gcc/build_w/prev-x86_64-apple-darwin15.5.0/libstdc++-v3/src/.libs -B/opt/gcc/build_w/prev-x86_64-apple-darwin15.5.0/libstdc++-v3/libsupc++/.libs  -I/opt/gcc/build_w/prev-x86_64-apple-darwin15.5.0/libstdc++-v3/include/x86_64-apple-darwin15.5.0  -I/opt/gcc/build_w/prev-x86_64-apple-darwin15.5.0/libstdc++-v3/include  -I/opt/gcc/work/libstdc++-v3/libsupc++ -L/opt/gcc/build_w/prev-x86_64-apple-darwin15.5.0/libstdc++-v3/src/.libs -L/opt/gcc/build_w/prev-x86_64-apple-darwin15.5.0/libstdc++-v3/libsupc++/.libs -fno-PIE -c  -DIN_GCC_FRONTEND -g -O2   -gtoggle -DIN_GCC     -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -Ifortran -I../../work/gcc -I../../work/gcc/fortran -I../../work/gcc/../include -I./../intl -I../../work/gcc/../libcpp/include -I/opt/mp-new/include  -I../../work/gcc/../libdecnumber -I../../work/gcc/../libdecnumber/dpd -I../libdecnumber -I../../work/gcc/../libbacktrace -I/opt/mp-new/include  -o fortran/simplify.o -MT fortran/simplify.o -MMD -MP -MF fortran/.deps/simplify.TPo ../../work/gcc/fortran/simplify.c
../../work/gcc/fortran/simplify.c: In function 'gfc_expr* gfc_simplify_repeat(gfc_expr*, gfc_expr*)':
../../work/gcc/fortran/simplify.c:5089:11: error: variable 'i' set but not used [-Werror=unused-but-set-variable]
       int i;
           ^
cc1plus: all warnings being treated as errors

i.e., the lines

       int i;

and

       i = gfc_validate_kind (BT_INTEGER, gfc_charlen_int_kind, false);

must be deleted. also the comment

      /* Compute the maximum value allowed for NCOPIES:
	    huge(cl) - 1 / len.  */

should be updated.

Last point I have found, the limit does not seem to take into account CHARACTER(KIND=4):

[Book15] f90/bug% cat pr66310_par_4.f90
   program p
      character(len=2,kind=4), parameter :: z = 'yz'
      print *, repeat(z, 2**25)
   end
[Book15] f90/bug% gfc pr66310_par_4.f90
f951(3881,0x7fff74e38000) malloc: *** mach_vm_map(size=18446744073441116160) failed (error code=3)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug

f951: out of memory allocating 18446744073441116160 bytes after a total of 0 bytes

Thanks for working on this PR.

Dominique
diff mbox

Patch

diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 77831ab..dcacae8 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -59,6 +59,8 @@  not after.
 
 #define MAX_SUBRECORD_LENGTH 2147483639   /* 2**31-9 */
 
+/* Practical limit on size of character constants.  */
+#define MAX_CHAR_LENGTH 268435455    /* 2**28-1  */
 
 #define gfc_is_whitespace(c) ((c==' ') || (c=='\t'))
 
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 8096a92..4010753 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -5085,24 +5085,29 @@  gfc_simplify_repeat (gfc_expr *e, gfc_expr *n)
   /* Check that NCOPIES isn't too large.  */
   if (len)
     {
-      mpz_t max, mlen;
+      mpz_t max, mlen, limit;
       int i;
 
-      /* Compute the maximum value allowed for NCOPIES: huge(cl) / len.  */
+      /* Compute the maximum value allowed for NCOPIES:
+	    huge(cl) - 1 / len.  */
       mpz_init (max);
       i = gfc_validate_kind (BT_INTEGER, gfc_charlen_int_kind, false);
 
+      /* Set the limit on size to avoid unsigned integer
+	 wrapping and resource limits.  */
+      mpz_init_set_si (limit, MAX_CHAR_LENGTH);
+
       if (have_length)
 	{
-	  mpz_tdiv_q (max, gfc_integer_kinds[i].huge,
-		      e->ts.u.cl->length->value.integer);
+	  mpz_tdiv_q (max, limit, e->ts.u.cl->length->value.integer);
 	}
       else
 	{
 	  mpz_init_set_si (mlen, len);
-	  mpz_tdiv_q (max, gfc_integer_kinds[i].huge, mlen);
+	  mpz_tdiv_q (max, limit, mlen);
 	  mpz_clear (mlen);
 	}
+      mpz_clear (limit);
 
       /* The check itself.  */
       if (mpz_cmp (ncopies, max) > 0)
diff --git a/gcc/testsuite/gfortran.dg/repeat_4.f90 b/gcc/testsuite/gfortran.dg/repeat_4.f90
index e5b5acc..a6ee75b 100644
--- a/gcc/testsuite/gfortran.dg/repeat_4.f90
+++ b/gcc/testsuite/gfortran.dg/repeat_4.f90
@@ -22,17 +22,17 @@  program test
 
   ! Check for too large NCOPIES argument and limit cases
   print *, repeat(t0, huge(0))
-  print *, repeat(t1, huge(0))
+  print *, repeat(t1, huge(0)) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
   print *, repeat(t2, huge(0)) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
   print *, repeat(s2, huge(0)) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
 
-  print *, repeat(t0, huge(0)/2)
-  print *, repeat(t1, huge(0)/2)
-  print *, repeat(t2, huge(0)/2)
+  print *, repeat(t0, huge(0)/8)
+  print *, repeat(t1, huge(0)/8)
+  print *, repeat(t2, huge(0)/16)
 
   print *, repeat(t0, huge(0)/2+1)
-  print *, repeat(t1, huge(0)/2+1)
-  print *, repeat(t2, huge(0)/2+1) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
-  print *, repeat(s2, huge(0)/2+1) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
+  print *, repeat(t1, huge(0)/8+1) ! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
+  print *, repeat(t2, huge(0)/16+1)! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
+  print *, repeat(s2, huge(0)/16+1)! { dg-error "Argument NCOPIES of REPEAT intrinsic is too large " }
 
 end program test