Patchwork [fortran] PR47348 - wrong string length with array constructor

login
register
mail settings
Submitter Paul Richard Thomas
Date Feb. 19, 2011, 8:55 a.m.
Message ID <AANLkTinkQBW_UnjbXMUgdaHHuNmG4PoBR4+1-g_F8BmY@mail.gmail.com>
Download mbox | patch
Permalink /patch/83688/
State New
Headers show

Comments

Paul Richard Thomas - Feb. 19, 2011, 8:55 a.m.
The attached is straight forward.  Instead of quitting the evaluation
of the length of a substring if it is not constant, we now evaluate
the expression for the constructor element.  Whilst not as efficient
as evaluating string-lengths directly, it is quite bomb-proof.
bounds_check_10.f90 needed a wildcard for the run-time error message
since the elements compared go from 1:2 to 1:3 as the level of
optimization increases, changing this part of the message from "1/2"
to "1/4".  I am not sure why this happens but the error is still
picked up and the message remains correct.

Bootstrapped and regtested on Ubuntu/i686 - OK for trunk?

Paul

2011-02-19  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/47348
	* trans-array.c (get_array_ctor_all_strlen): Move up in file.
	(get_array_ctor_var_strlen): Add block dummy and add call to
	get_array_ctor_all_strlen instead of giving up on substrings.
	Call gcc_unreachable for default case.
	(get_array_ctor_strlen): Add extra argument to in call to
	get_array_ctor_var_strlen.

2011-02-19  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/47348
	* gfortran.dg/array_constructor_36.f90 : New test.
	* gfortran.dg/bounds_check_10.f90 : Change dg-output message to
	allow for comparison between different elements of the array
	constructor at different levels of optimization.
Tobias Burnus - Feb. 19, 2011, 11:19 a.m.
Paul Richard Thomas wrote:
> Bootstrapped and regtested on Ubuntu/i686 - OK for trunk?

OK Thanks for the patch!

Tobias

> 2011-02-19  Paul Thomas<pault@gcc.gnu.org>
>
> 	PR fortran/47348
> 	* trans-array.c (get_array_ctor_all_strlen): Move up in file.
> 	(get_array_ctor_var_strlen): Add block dummy and add call to
> 	get_array_ctor_all_strlen instead of giving up on substrings.
> 	Call gcc_unreachable for default case.
> 	(get_array_ctor_strlen): Add extra argument to in call to
> 	get_array_ctor_var_strlen.
>
> 2011-02-19  Paul Thomas<pault@gcc.gnu.org>
>
> 	PR fortran/47348
> 	* gfortran.dg/array_constructor_36.f90 : New test.
> 	* gfortran.dg/bounds_check_10.f90 : Change dg-output message to
> 	allow for comparison between different elements of the array
> 	constructor at different levels of optimization.

Patch

Index: gcc/fortran/trans-array.c
===================================================================
*** gcc/fortran/trans-array.c	(revision 169916)
--- gcc/fortran/trans-array.c	(working copy)
*************** gfc_trans_array_constructor_value (stmtb
*** 1495,1505 ****
  }
  
  
  /* Figure out the string length of a variable reference expression.
     Used by get_array_ctor_strlen.  */
  
  static void
! get_array_ctor_var_strlen (gfc_expr * expr, tree * len)
  {
    gfc_ref *ref;
    gfc_typespec *ts;
--- 1495,1549 ----
  }
  
  
+ /* A catch-all to obtain the string length for anything that is not a
+    a substring of non-constant length, a constant, array or variable.  */
+ 
+ static void
+ get_array_ctor_all_strlen (stmtblock_t *block, gfc_expr *e, tree *len)
+ {
+   gfc_se se;
+   gfc_ss *ss;
+ 
+   /* Don't bother if we already know the length is a constant.  */
+   if (*len && INTEGER_CST_P (*len))
+     return;
+ 
+   if (!e->ref && e->ts.u.cl && e->ts.u.cl->length
+ 	&& e->ts.u.cl->length->expr_type == EXPR_CONSTANT)
+     {
+       /* This is easy.  */
+       gfc_conv_const_charlen (e->ts.u.cl);
+       *len = e->ts.u.cl->backend_decl;
+     }
+   else
+     {
+       /* Otherwise, be brutal even if inefficient.  */
+       ss = gfc_walk_expr (e);
+       gfc_init_se (&se, NULL);
+ 
+       /* No function call, in case of side effects.  */
+       se.no_function_call = 1;
+       if (ss == gfc_ss_terminator)
+ 	gfc_conv_expr (&se, e);
+       else
+ 	gfc_conv_expr_descriptor (&se, e, ss);
+ 
+       /* Fix the value.  */
+       *len = gfc_evaluate_now (se.string_length, &se.pre);
+ 
+       gfc_add_block_to_block (block, &se.pre);
+       gfc_add_block_to_block (block, &se.post);
+ 
+       e->ts.u.cl->backend_decl = *len;
+     }
+ }
+ 
+ 
  /* Figure out the string length of a variable reference expression.
     Used by get_array_ctor_strlen.  */
  
  static void
! get_array_ctor_var_strlen (stmtblock_t *block, gfc_expr * expr, tree * len)
  {
    gfc_ref *ref;
    gfc_typespec *ts;
*************** get_array_ctor_var_strlen (gfc_expr * ex
*** 1526,1532 ****
  	case REF_SUBSTRING:
  	  if (ref->u.ss.start->expr_type != EXPR_CONSTANT
  	      || ref->u.ss.end->expr_type != EXPR_CONSTANT)
! 	    break;
  	  mpz_init_set_ui (char_len, 1);
  	  mpz_add (char_len, char_len, ref->u.ss.end->value.integer);
  	  mpz_sub (char_len, char_len, ref->u.ss.start->value.integer);
--- 1570,1580 ----
  	case REF_SUBSTRING:
  	  if (ref->u.ss.start->expr_type != EXPR_CONSTANT
  	      || ref->u.ss.end->expr_type != EXPR_CONSTANT)
! 	    {
! 	      /* Note that this might evaluate expr.  */
! 	      get_array_ctor_all_strlen (block, expr, len);
! 	      return;
! 	    }
  	  mpz_init_set_ui (char_len, 1);
  	  mpz_add (char_len, char_len, ref->u.ss.end->value.integer);
  	  mpz_sub (char_len, char_len, ref->u.ss.start->value.integer);
*************** get_array_ctor_var_strlen (gfc_expr * ex
*** 1536,1545 ****
  	  return;
  
  	default:
! 	  /* TODO: Substrings are tricky because we can't evaluate the
! 	     expression more than once.  For now we just give up, and hope
! 	     we can figure it out elsewhere.  */
! 	  return;
  	}
      }
  
--- 1584,1590 ----
  	  return;
  
  	default:
! 	 gcc_unreachable ();
  	}
      }
  
*************** get_array_ctor_var_strlen (gfc_expr * ex
*** 1547,1595 ****
  }
  
  
- /* A catch-all to obtain the string length for anything that is not a
-    constant, array or variable.  */
- static void
- get_array_ctor_all_strlen (stmtblock_t *block, gfc_expr *e, tree *len)
- {
-   gfc_se se;
-   gfc_ss *ss;
- 
-   /* Don't bother if we already know the length is a constant.  */
-   if (*len && INTEGER_CST_P (*len))
-     return;
- 
-   if (!e->ref && e->ts.u.cl && e->ts.u.cl->length
- 	&& e->ts.u.cl->length->expr_type == EXPR_CONSTANT)
-     {
-       /* This is easy.  */
-       gfc_conv_const_charlen (e->ts.u.cl);
-       *len = e->ts.u.cl->backend_decl;
-     }
-   else
-     {
-       /* Otherwise, be brutal even if inefficient.  */
-       ss = gfc_walk_expr (e);
-       gfc_init_se (&se, NULL);
- 
-       /* No function call, in case of side effects.  */
-       se.no_function_call = 1;
-       if (ss == gfc_ss_terminator)
- 	gfc_conv_expr (&se, e);
-       else
- 	gfc_conv_expr_descriptor (&se, e, ss);
- 
-       /* Fix the value.  */
-       *len = gfc_evaluate_now (se.string_length, &se.pre);
- 
-       gfc_add_block_to_block (block, &se.pre);
-       gfc_add_block_to_block (block, &se.post);
- 
-       e->ts.u.cl->backend_decl = *len;
-     }
- }
- 
- 
  /* Figure out the string length of a character array constructor.
     If len is NULL, don't calculate the length; this happens for recursive calls
     when a sub-array-constructor is an element but not at the first position,
--- 1592,1597 ----
*************** get_array_ctor_strlen (stmtblock_t *bloc
*** 1633,1639 ****
  	case EXPR_VARIABLE:
  	  is_const = false;
  	  if (len)
! 	    get_array_ctor_var_strlen (c->expr, len);
  	  break;
  
  	default:
--- 1635,1641 ----
  	case EXPR_VARIABLE:
  	  is_const = false;
  	  if (len)
! 	    get_array_ctor_var_strlen (block, c->expr, len);
  	  break;
  
  	default:
Index: gcc/testsuite/gfortran.dg/array_constructor_36.f90
===================================================================
*** gcc/testsuite/gfortran.dg/array_constructor_36.f90	(revision 0)
--- gcc/testsuite/gfortran.dg/array_constructor_36.f90	(revision 0)
***************
*** 0 ****
--- 1,21 ----
+ ! { dg-do run }
+ ! Test the fix for PR47348, in which the substring length
+ ! in the array constructor at line 19 would be missed and
+ ! the length of q used instead.
+ !
+ ! Contributed by Thomas Koenig  <tkoenig@netcologne.de>
+ !
+ program main
+   implicit none
+   character(len = *), parameter :: fmt='(2(A,"|"))'
+   character(len = *), parameter :: test='xyc|aec|'
+   integer :: i
+   character(len = 4) :: q
+   character(len = 8) :: buffer
+   q = 'xy'
+   i = 2
+   write (buffer, fmt) (/ trim(q), 'ae' /)//'c'
+   if (buffer .ne. test) Call abort
+   write (buffer, FMT) (/ q(1:i), 'ae' /)//'c'
+   if (buffer .ne. test) Call abort
+ end program main
Index: gcc/testsuite/gfortran.dg/bounds_check_10.f90
===================================================================
*** gcc/testsuite/gfortran.dg/bounds_check_10.f90	(revision 169916)
--- gcc/testsuite/gfortran.dg/bounds_check_10.f90	(working copy)
*************** z = [y(1:1), y(1:1), x(1:len(trim(x)))] 
*** 12,15 ****
  z = [trim(x), trim(y), "aaaa"] ! [ "a", "cd", "aaaa" ] should catch first error
  end program array_char
  
! ! { dg-output "Different CHARACTER lengths .1/2. in array constructor" }
--- 12,15 ----
  z = [trim(x), trim(y), "aaaa"] ! [ "a", "cd", "aaaa" ] should catch first error
  end program array_char
  
! ! { dg-output "Different CHARACTER lengths .1/.. in array constructor" }