[fortran,patch] Fix PR 34956 by not checking bounds of absent optional arguments

[fortran,patch] Fix PR 34956 by not checking bounds of absent optional arguments

[FX Coudert]

[-- Attachment #1: Type: text/plain, Size: 794 bytes --]

valgrind revealed the use of an uninitialized value in gfortran.dg/ 
bounds_check_9.f90 compiled with -fbounds-check, due to bounds- 
checking being performend on an absent optional argument. There  
already was some code to check for optional args, but it only covered  
half of the bound-checking; IIRC, I was the one who wrote it in the  
first place, so I can only apologize for the bad initial fix.

Bootstrapped and regtested on x86_64-linux, checked the tree dump and  
with valgrind that the issue was take care of. No testcase, because I  
can't seem to think of a way to get it to appear without valgrind  
(and the original testcase, of course, is already in the testsuite).

OK to commit?

FX

-- 
François-Xavier Coudert
http://www.homepages.ucl.ac.uk/~uccafco/

[-- Attachment #2: pr34956.ChangeLog --]
[-- Type: application/octet-stream, Size: 195 bytes --]

2008-02-29  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>

	PR fortran/34956
	* trans-array.c (gfc_conv_ss_startstride): Fix the logic to avoid
	checking bounds of absent optional arguments.


[-- Attachment #3: pr34956.diff --]
[-- Type: application/octet-stream, Size: 4446 bytes --]

Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c	(revision 132773)
+++ gcc/fortran/trans-array.c	(working copy)
@@ -2924,9 +2924,13 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 
       for (ss = loop->ss; ss != gfc_ss_terminator; ss = ss->loop_chain)
 	{
+	  stmtblock_t inner;
+
 	  if (ss->type != GFC_SS_SECTION)
 	    continue;
 
+	  gfc_start_block (&inner);
+
 	  /* TODO: range checking for mapped dimensions.  */
 	  info = &ss->data.info;
 
@@ -2953,7 +2957,7 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 	      asprintf (&msg, "Zero stride is not allowed, for dimension %d "
 			"of array '%s'", info->dim[n]+1,
 			ss->expr->symtree->name);
-	      gfc_trans_runtime_check (tmp, &block, &ss->expr->where, msg);
+	      gfc_trans_runtime_check (tmp, &inner, &ss->expr->where, msg);
 	      gfc_free (msg);
 
 	      desc = ss->data.info.descriptor;
@@ -2995,7 +2999,7 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 	      asprintf (&msg, "%s, lower bound of dimension %d of array '%s'"
 			" exceeded (%%ld < %%ld)", gfc_msg_fault,
 			info->dim[n]+1, ss->expr->symtree->name);
-	      gfc_trans_runtime_check (tmp, &block, &ss->expr->where, msg,
+	      gfc_trans_runtime_check (tmp, &inner, &ss->expr->where, msg,
 				       fold_convert (long_integer_type_node,
 						     info->start[n]),
 				       fold_convert (long_integer_type_node,
@@ -3011,7 +3015,7 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 	          asprintf (&msg, "%s, upper bound of dimension %d of array "
 			    "'%s' exceeded (%%ld > %%ld)", gfc_msg_fault,
 			    info->dim[n]+1, ss->expr->symtree->name);
-		  gfc_trans_runtime_check (tmp, &block, &ss->expr->where, msg,
+		  gfc_trans_runtime_check (tmp, &inner, &ss->expr->where, msg,
 			fold_convert (long_integer_type_node, info->start[n]),
 			fold_convert (long_integer_type_node, ubound));
 		  gfc_free (msg);
@@ -3033,7 +3037,7 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 	      asprintf (&msg, "%s, lower bound of dimension %d of array '%s'"
 			" exceeded (%%ld < %%ld)", gfc_msg_fault,
 			info->dim[n]+1, ss->expr->symtree->name);
-	      gfc_trans_runtime_check (tmp, &block, &ss->expr->where, msg,
+	      gfc_trans_runtime_check (tmp, &inner, &ss->expr->where, msg,
 				       fold_convert (long_integer_type_node,
 						     tmp2),
 				       fold_convert (long_integer_type_node,
@@ -3048,7 +3052,7 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 		  asprintf (&msg, "%s, upper bound of dimension %d of array "
 			    "'%s' exceeded (%%ld > %%ld)", gfc_msg_fault,
 			    info->dim[n]+1, ss->expr->symtree->name);
-		  gfc_trans_runtime_check (tmp, &block, &ss->expr->where, msg,
+		  gfc_trans_runtime_check (tmp, &inner, &ss->expr->where, msg,
 			fold_convert (long_integer_type_node, tmp2),
 			fold_convert (long_integer_type_node, ubound));
 		  gfc_free (msg);
@@ -3066,30 +3070,30 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 		  tree tmp3;
 
 		  tmp3 = fold_build2 (NE_EXPR, boolean_type_node, tmp, size[n]);
-
-		  /* For optional arguments, only check bounds if the
-		     argument is present.  */
-		  if (ss->expr->symtree->n.sym->attr.optional
-		      || ss->expr->symtree->n.sym->attr.not_always_present)
-		    {
-		      tree cond;
-
-		      cond = gfc_conv_expr_present (ss->expr->symtree->n.sym);
-		      tmp3 = fold_build2 (TRUTH_AND_EXPR, boolean_type_node,
-					  cond, tmp3);
-		    }
-
 		  asprintf (&msg, "%s, size mismatch for dimension %d "
 			    "of array '%s' (%%ld/%%ld)", gfc_msg_bounds,
 			    info->dim[n]+1, ss->expr->symtree->name);
-		  gfc_trans_runtime_check (tmp3, &block, &ss->expr->where, msg,
+		  gfc_trans_runtime_check (tmp3, &inner, &ss->expr->where, msg,
 			fold_convert (long_integer_type_node, tmp),
 			fold_convert (long_integer_type_node, size[n]));
 		  gfc_free (msg);
 		}
 	      else
-		size[n] = gfc_evaluate_now (tmp, &block);
+		size[n] = gfc_evaluate_now (tmp, &inner);
 	    }
+
+	  tmp = gfc_finish_block (&inner);
+
+	  /* For optional arguments, only check bounds if the argument is
+	     present.  */
+	  if (ss->expr->symtree->n.sym->attr.optional
+	      || ss->expr->symtree->n.sym->attr.not_always_present)
+	    tmp = build3_v (COND_EXPR,
+			    gfc_conv_expr_present (ss->expr->symtree->n.sym),
+			    tmp, build_empty_stmt ());
+
+	  gfc_add_expr_to_block (&block, tmp);
+
 	}
 
       tmp = gfc_finish_block (&block);

Fwd: [fortran,patch] Fix PR 34956 by not checking bounds of absent optional arguments

[FX Coudert]

[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]

ping

Début du message réexpédié :

> De : FX Coudert <fxcoudert@gmail.com>
> Date : 29 février 2008 22:14:28 GMT+00:00
> À : Fortran List <fortran@gcc.gnu.org>, gcc patches <gcc- 
> patches@gcc.gnu.org>
> Objet : [fortran,patch] Fix PR 34956 by not checking bounds of  
> absent optional arguments
>
> valgrind revealed the use of an uninitialized value in gfortran.dg/ 
> bounds_check_9.f90 compiled with -fbounds-check, due to bounds- 
> checking being performend on an absent optional argument. There  
> already was some code to check for optional args, but it only  
> covered half of the bound-checking; IIRC, I was the one who wrote  
> it in the first place, so I can only apologize for the bad initial  
> fix.
>
> Bootstrapped and regtested on x86_64-linux, checked the tree dump  
> and with valgrind that the issue was take care of. No testcase,  
> because I can't seem to think of a way to get it to appear without  
> valgrind (and the original testcase, of course, is already in the  
> testsuite).
>
> OK to commit?
>
> FX
>
> -- 
> François-Xavier Coudert
> http://www.homepages.ucl.ac.uk/~uccafco/

[-- Attachment #2: pr34956.ChangeLog --]
[-- Type: application/octet-stream, Size: 195 bytes --]

2008-02-29  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>

	PR fortran/34956
	* trans-array.c (gfc_conv_ss_startstride): Fix the logic to avoid
	checking bounds of absent optional arguments.


[-- Attachment #3: pr34956.diff --]
[-- Type: application/octet-stream, Size: 4446 bytes --]

Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c	(revision 132773)
+++ gcc/fortran/trans-array.c	(working copy)
@@ -2924,9 +2924,13 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 
       for (ss = loop->ss; ss != gfc_ss_terminator; ss = ss->loop_chain)
 	{
+	  stmtblock_t inner;
+
 	  if (ss->type != GFC_SS_SECTION)
 	    continue;
 
+	  gfc_start_block (&inner);
+
 	  /* TODO: range checking for mapped dimensions.  */
 	  info = &ss->data.info;
 
@@ -2953,7 +2957,7 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 	      asprintf (&msg, "Zero stride is not allowed, for dimension %d "
 			"of array '%s'", info->dim[n]+1,
 			ss->expr->symtree->name);
-	      gfc_trans_runtime_check (tmp, &block, &ss->expr->where, msg);
+	      gfc_trans_runtime_check (tmp, &inner, &ss->expr->where, msg);
 	      gfc_free (msg);
 
 	      desc = ss->data.info.descriptor;
@@ -2995,7 +2999,7 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 	      asprintf (&msg, "%s, lower bound of dimension %d of array '%s'"
 			" exceeded (%%ld < %%ld)", gfc_msg_fault,
 			info->dim[n]+1, ss->expr->symtree->name);
-	      gfc_trans_runtime_check (tmp, &block, &ss->expr->where, msg,
+	      gfc_trans_runtime_check (tmp, &inner, &ss->expr->where, msg,
 				       fold_convert (long_integer_type_node,
 						     info->start[n]),
 				       fold_convert (long_integer_type_node,
@@ -3011,7 +3015,7 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 	          asprintf (&msg, "%s, upper bound of dimension %d of array "
 			    "'%s' exceeded (%%ld > %%ld)", gfc_msg_fault,
 			    info->dim[n]+1, ss->expr->symtree->name);
-		  gfc_trans_runtime_check (tmp, &block, &ss->expr->where, msg,
+		  gfc_trans_runtime_check (tmp, &inner, &ss->expr->where, msg,
 			fold_convert (long_integer_type_node, info->start[n]),
 			fold_convert (long_integer_type_node, ubound));
 		  gfc_free (msg);
@@ -3033,7 +3037,7 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 	      asprintf (&msg, "%s, lower bound of dimension %d of array '%s'"
 			" exceeded (%%ld < %%ld)", gfc_msg_fault,
 			info->dim[n]+1, ss->expr->symtree->name);
-	      gfc_trans_runtime_check (tmp, &block, &ss->expr->where, msg,
+	      gfc_trans_runtime_check (tmp, &inner, &ss->expr->where, msg,
 				       fold_convert (long_integer_type_node,
 						     tmp2),
 				       fold_convert (long_integer_type_node,
@@ -3048,7 +3052,7 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 		  asprintf (&msg, "%s, upper bound of dimension %d of array "
 			    "'%s' exceeded (%%ld > %%ld)", gfc_msg_fault,
 			    info->dim[n]+1, ss->expr->symtree->name);
-		  gfc_trans_runtime_check (tmp, &block, &ss->expr->where, msg,
+		  gfc_trans_runtime_check (tmp, &inner, &ss->expr->where, msg,
 			fold_convert (long_integer_type_node, tmp2),
 			fold_convert (long_integer_type_node, ubound));
 		  gfc_free (msg);
@@ -3066,30 +3070,30 @@ gfc_conv_ss_startstride (gfc_loopinfo * 
 		  tree tmp3;
 
 		  tmp3 = fold_build2 (NE_EXPR, boolean_type_node, tmp, size[n]);
-
-		  /* For optional arguments, only check bounds if the
-		     argument is present.  */
-		  if (ss->expr->symtree->n.sym->attr.optional
-		      || ss->expr->symtree->n.sym->attr.not_always_present)
-		    {
-		      tree cond;
-
-		      cond = gfc_conv_expr_present (ss->expr->symtree->n.sym);
-		      tmp3 = fold_build2 (TRUTH_AND_EXPR, boolean_type_node,
-					  cond, tmp3);
-		    }
-
 		  asprintf (&msg, "%s, size mismatch for dimension %d "
 			    "of array '%s' (%%ld/%%ld)", gfc_msg_bounds,
 			    info->dim[n]+1, ss->expr->symtree->name);
-		  gfc_trans_runtime_check (tmp3, &block, &ss->expr->where, msg,
+		  gfc_trans_runtime_check (tmp3, &inner, &ss->expr->where, msg,
 			fold_convert (long_integer_type_node, tmp),
 			fold_convert (long_integer_type_node, size[n]));
 		  gfc_free (msg);
 		}
 	      else
-		size[n] = gfc_evaluate_now (tmp, &block);
+		size[n] = gfc_evaluate_now (tmp, &inner);
 	    }
+
+	  tmp = gfc_finish_block (&inner);
+
+	  /* For optional arguments, only check bounds if the argument is
+	     present.  */
+	  if (ss->expr->symtree->n.sym->attr.optional
+	      || ss->expr->symtree->n.sym->attr.not_always_present)
+	    tmp = build3_v (COND_EXPR,
+			    gfc_conv_expr_present (ss->expr->symtree->n.sym),
+			    tmp, build_empty_stmt ());
+
+	  gfc_add_expr_to_block (&block, tmp);
+
 	}
 
       tmp = gfc_finish_block (&block);

[-- Attachment #4: Type: text/plain, Size: 2 bytes --]

Re: Fwd: [fortran,patch] Fix PR 34956 by not checking bounds of absent optional arguments

[Jerry DeLisle]

FX Coudert wrote:
> ping
> 
> Début du message réexpédié :
> 
>> De : FX Coudert <fxcoudert@gmail.com>
>> Date : 29 février 2008 22:14:28 GMT+00:00
>> À : Fortran List <fortran@gcc.gnu.org>, gcc patches 
>> <gcc-patches@gcc.gnu.org>
>> Objet : [fortran,patch] Fix PR 34956 by not checking bounds of absent 
>> optional arguments
>>
>> valgrind revealed the use of an uninitialized value in 
>> gfortran.dg/bounds_check_9.f90 compiled with -fbounds-check, due to 
>> bounds-checking being performend on an absent optional argument. There 
>> already was some code to check for optional args, but it only covered 
>> half of the bound-checking; IIRC, I was the one who wrote it in the 
>> first place, so I can only apologize for the bad initial fix.
>>
>> Bootstrapped and regtested on x86_64-linux, checked the tree dump and 
>> with valgrind that the issue was take care of. No testcase, because I 
>> can't seem to think of a way to get it to appear without valgrind (and 
>> the original testcase, of course, is already in the testsuite).
>>
>> OK to commit?


OK, thanks,

Jerry