public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Use size_type_node instead of sizetype for stride overflow checking (PR fortran/46945)
@ 2010-12-15 18:46 Jakub Jelinek
  2010-12-15 18:59 ` Janne Blomqvist
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2010-12-15 18:46 UTC (permalink / raw)
  To: gcc-patches, fortran

Hi!

Using sizetype for testing whether stride * element_size will overflow
is wrong, because unlike all sane unsigned types, sizetype doesn't have
modulo semantics, but instead has undefined overflow.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2010-12-15  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/46945
	* trans-array.c (gfc_array_init_size): Perform stride overflow
	checking and multiplication by element_size in size_type_node instead
	of sizetype, return value with size_type_node type instead of
	sometimes with sizetype and sometimes with gfc_array_index_type.

	* gfortran.dg/pr46945.f90: New test.

--- gcc/fortran/trans-array.c.jj	2010-12-14 08:11:31.000000000 +0100
+++ gcc/fortran/trans-array.c	2010-12-15 12:16:49.000000000 +0100
@@ -4154,15 +4154,15 @@ gfc_array_init_size (tree descriptor, in
      size of an element to get the total size.  */
   tmp = TYPE_SIZE_UNIT (gfc_get_element_type (type));
   /* Convert to size_t.  */
-  element_size = fold_convert (sizetype, tmp);
-  stride = fold_convert (sizetype, stride);
+  element_size = fold_convert (size_type_node, tmp);
+  stride = fold_convert (size_type_node, stride);
 
   /* First check for overflow. Since an array of type character can
      have zero element_size, we must check for that before
      dividing.  */
   tmp = fold_build2_loc (input_location, TRUNC_DIV_EXPR, 
-			 sizetype, 
-			 TYPE_MAX_VALUE (sizetype), element_size);
+			 size_type_node,
+			 TYPE_MAX_VALUE (size_type_node), element_size);
   tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node,
 			 gfc_unlikely (fold_build2_loc (input_location, LT_EXPR, 
 							boolean_type_node, tmp, 
@@ -4178,7 +4178,7 @@ gfc_array_init_size (tree descriptor, in
 			 *overflow, tmp);
   *overflow = gfc_evaluate_now (tmp, pblock);
 
-  size = fold_build2_loc (input_location, MULT_EXPR, sizetype,
+  size = fold_build2_loc (input_location, MULT_EXPR, size_type_node,
 			  stride, element_size);
 
   if (poffset != NULL)
@@ -4190,11 +4190,11 @@ gfc_array_init_size (tree descriptor, in
   if (integer_zerop (or_expr))
     return size;
   if (integer_onep (or_expr))
-    return gfc_index_zero_node;
+    return build_int_cst (size_type_node, 0);
 
   var = gfc_create_var (TREE_TYPE (size), "size");
   gfc_start_block (&thenblock);
-  gfc_add_modify (&thenblock, var, size_zero_node);
+  gfc_add_modify (&thenblock, var, build_int_cst (size_type_node, 0));
   thencase = gfc_finish_block (&thenblock);
 
   gfc_start_block (&elseblock);
--- gcc/testsuite/gfortran.dg/pr46945.f90.jj	2010-12-15 12:22:11.000000000 +0100
+++ gcc/testsuite/gfortran.dg/pr46945.f90	2010-12-15 12:21:46.000000000 +0100
@@ -0,0 +1,10 @@
+! PR fortran/46945
+! { dg-do run }
+! { dg-options "-O -ftree-vrp -fno-tree-ccp -fno-tree-fre" }
+
+program pr46945
+  real, allocatable :: a(:,:,:)
+  integer :: n
+  n = 0
+  allocate (a(n,n,n))
+end program pr46945

	Jakub

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Use size_type_node instead of sizetype for stride overflow checking (PR fortran/46945)
  2010-12-15 18:46 [PATCH] Use size_type_node instead of sizetype for stride overflow checking (PR fortran/46945) Jakub Jelinek
@ 2010-12-15 18:59 ` Janne Blomqvist
  2010-12-15 22:00   ` Jakub Jelinek
  0 siblings, 1 reply; 3+ messages in thread
From: Janne Blomqvist @ 2010-12-15 18:59 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, fortran

On Wed, Dec 15, 2010 at 20:04, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> Using sizetype for testing whether stride * element_size will overflow
> is wrong, because unlike all sane unsigned types, sizetype doesn't have
> modulo semantics, but instead has undefined overflow.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

You've converted size_zero_node into build_int_cst (size_type_node,
0), but you missed one case, so I presume you'd want

diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 84e6005..f93967e 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -4169,10 +4169,11 @@ gfc_array_init_size (tree descriptor, int
rank, int corank, tree * poffset,
                                                        stride)),
                         integer_one_node, integer_zero_node);
   tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node,
-                        gfc_unlikely (fold_build2_loc (input_location, EQ_EXPR,
-                                                       boolean_type_node,
-                                                       element_size,
-                                                       size_zero_node)),
+                        gfc_unlikely (fold_build2_loc
+                                      (input_location, EQ_EXPR,
+                                       boolean_type_node,
+                                       element_size,
+                                       build_int_cst (size_type_node, 0))),
                         integer_zero_node, tmp);
   tmp = fold_build2_loc (input_location, PLUS_EXPR, integer_type_node,
                         *overflow, tmp);

on top of your patch? With that change, ok for trunk



-- 
Janne Blomqvist

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Use size_type_node instead of sizetype for stride overflow checking (PR fortran/46945)
  2010-12-15 18:59 ` Janne Blomqvist
@ 2010-12-15 22:00   ` Jakub Jelinek
  0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2010-12-15 22:00 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: gcc-patches, fortran

On Wed, Dec 15, 2010 at 08:22:20PM +0200, Janne Blomqvist wrote:
> You've converted size_zero_node into build_int_cst (size_type_node,
> 0), but you missed one case, so I presume you'd want

Thanks for catching it up.
This is what I've actually committed (note, I've also added cond temporary
so that the 80 chars length limit wouldn't be so big issue:

--- fortran/ChangeLog	(revision 167870)
+++ fortran/ChangeLog	(working copy)
@@ -1,3 +1,11 @@
+2010-12-15  Jakub Jelinek  <jakub@redhat.com>
+
+	PR fortran/46945
+	* trans-array.c (gfc_array_init_size): Perform stride overflow
+	checking and multiplication by element_size in size_type_node instead
+	of sizetype, return value with size_type_node type instead of
+	sometimes with sizetype and sometimes with gfc_array_index_type.
+
 2010-12-15  Janne Blomqvist  <jb@gcc.gnu.org>
 
 	* trans.c (gfc_allocate_with_status): Better error message for
--- testsuite/ChangeLog	(revision 167870)
+++ testsuite/ChangeLog	(working copy)
@@ -1,5 +1,8 @@
 2010-12-15  Jakub Jelinek  <jakub@redhat.com>
 
+	PR fortran/46945
+	* gfortran.dg/pr46945.f90: New test.
+
 	PR debug/46815
 	* g++.dg/guality/pr46815.C: New test.
 
--- fortran/trans-array.c	(revision 167870)
+++ fortran/trans-array.c	(working copy)
@@ -4006,6 +4006,7 @@ gfc_array_init_size (tree descriptor, in
   tree or_expr;
   tree thencase;
   tree elsecase;
+  tree cond;
   tree var;
   stmtblock_t thenblock;
   stmtblock_t elseblock;
@@ -4091,17 +4092,15 @@ gfc_array_init_size (tree descriptor, in
 			     fold_convert (gfc_array_index_type, 
 					   TYPE_MAX_VALUE (gfc_array_index_type)),
 					   size);
-      tmp = fold_build3_loc 
-	(input_location, COND_EXPR, integer_type_node,
-	 gfc_unlikely (fold_build2_loc (input_location, LT_EXPR, 
-					boolean_type_node, tmp, stride)),
-	 integer_one_node, integer_zero_node);
-      tmp = fold_build3_loc 
-	(input_location, COND_EXPR, integer_type_node,
-	 gfc_unlikely (fold_build2_loc (input_location, EQ_EXPR,
-					boolean_type_node, size, 
-					build_zero_cst (gfc_array_index_type))),
-	 integer_zero_node, tmp);
+      cond = gfc_unlikely (fold_build2_loc (input_location, LT_EXPR,
+					    boolean_type_node, tmp, stride));
+      tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node, cond,
+			     integer_one_node, integer_zero_node);
+      cond = gfc_unlikely (fold_build2_loc (input_location, EQ_EXPR,
+					    boolean_type_node, size,
+					    gfc_index_zero_node));
+      tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node, cond,
+			     integer_zero_node, tmp);
       tmp = fold_build2_loc (input_location, PLUS_EXPR, integer_type_node,
 			     *overflow, tmp);
       *overflow = gfc_evaluate_now (tmp, pblock);
@@ -4154,31 +4153,29 @@ gfc_array_init_size (tree descriptor, in
      size of an element to get the total size.  */
   tmp = TYPE_SIZE_UNIT (gfc_get_element_type (type));
   /* Convert to size_t.  */
-  element_size = fold_convert (sizetype, tmp);
-  stride = fold_convert (sizetype, stride);
+  element_size = fold_convert (size_type_node, tmp);
+  stride = fold_convert (size_type_node, stride);
 
   /* First check for overflow. Since an array of type character can
      have zero element_size, we must check for that before
      dividing.  */
   tmp = fold_build2_loc (input_location, TRUNC_DIV_EXPR, 
-			 sizetype, 
-			 TYPE_MAX_VALUE (sizetype), element_size);
-  tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node,
-			 gfc_unlikely (fold_build2_loc (input_location, LT_EXPR, 
-							boolean_type_node, tmp, 
-							stride)),
+			 size_type_node,
+			 TYPE_MAX_VALUE (size_type_node), element_size);
+  cond = gfc_unlikely (fold_build2_loc (input_location, LT_EXPR,
+					boolean_type_node, tmp, stride));
+  tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node, cond,
 			 integer_one_node, integer_zero_node);
-  tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node,
-			 gfc_unlikely (fold_build2_loc (input_location, EQ_EXPR,
-							boolean_type_node, 
-							element_size, 
-							size_zero_node)),
+  cond = gfc_unlikely (fold_build2_loc (input_location, EQ_EXPR,
+					boolean_type_node, element_size,
+					build_int_cst (size_type_node, 0)));
+  tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node, cond,
 			 integer_zero_node, tmp);
   tmp = fold_build2_loc (input_location, PLUS_EXPR, integer_type_node,
 			 *overflow, tmp);
   *overflow = gfc_evaluate_now (tmp, pblock);
 
-  size = fold_build2_loc (input_location, MULT_EXPR, sizetype,
+  size = fold_build2_loc (input_location, MULT_EXPR, size_type_node,
 			  stride, element_size);
 
   if (poffset != NULL)
@@ -4190,11 +4187,11 @@ gfc_array_init_size (tree descriptor, in
   if (integer_zerop (or_expr))
     return size;
   if (integer_onep (or_expr))
-    return gfc_index_zero_node;
+    return build_int_cst (size_type_node, 0);
 
   var = gfc_create_var (TREE_TYPE (size), "size");
   gfc_start_block (&thenblock);
-  gfc_add_modify (&thenblock, var, size_zero_node);
+  gfc_add_modify (&thenblock, var, build_int_cst (size_type_node, 0));
   thencase = gfc_finish_block (&thenblock);
 
   gfc_start_block (&elseblock);
--- testsuite/gfortran.dg/pr46945.f90	(revision 0)
+++ testsuite/gfortran.dg/pr46945.f90	(revision 0)
@@ -0,0 +1,10 @@
+! PR fortran/46945
+! { dg-do run }
+! { dg-options "-O -ftree-vrp -fno-tree-ccp -fno-tree-fre" }
+
+program pr46945
+  real, allocatable :: a(:,:,:)
+  integer :: n
+  n = 0
+  allocate (a(n,n,n))
+end program pr46945


	Jakub

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-12-15 20:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-15 18:46 [PATCH] Use size_type_node instead of sizetype for stride overflow checking (PR fortran/46945) Jakub Jelinek
2010-12-15 18:59 ` Janne Blomqvist
2010-12-15 22:00   ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).