This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, fortran] PR31217 - ICE using FORALL on character substrings


Tobias,

> > ==14821== Invalid read of size 4
> > ==14821==    at 0x459520: find_forall_index (resolve.c:4339)
>
> Ah yes.... I had seen that on my x86_ia64 and I forgot all about it.
> I even found a fix for it:)  If you look at the substring part of
> find_forall_index, you will find that it is completely screwed up.

As promised, here is the real patch:)  The junk has been removed from
resolve.c and all is well.

Bootstraps and regtests x86_ia64/fc5 - OK for trunk?

Cheers and thanks

Paul

2007-07-18  Paul Thomas <pault@gcc.gnu.org>

	PR fortran/31217
	PR fortran/33811
	* trans-array.c (gfc_conv_loop_setup): Send a complete type to
	gfc_trans_create_temp_array if the temporary is character.
	* trans-stmt.c (gfc_trans_assign_need_temp): Do likewise for
	allocate_temp_for_forall_nest.
	* dependency.c (gfc_check_dependency): Use gfc_dep_resolver for
	all references, not just REF_ARRAY.
	(gfc_dep_resolver): Correct the logic for substrings so that
	overlapping arrays are handled correctly.
	* resolve.c (find_forall_index): Remove extraneous code under
	REF_SUBSTRING.

2007-07-18  Paul Thomas <pault@gcc.gnu.org>

	PR fortran/31217
	PR fortran/33811
	* gfortran.dg/forall_12.f90: New test.
Index: /svn/trunk/gcc/fortran/trans-array.c
===================================================================
*** /svn/trunk/gcc/fortran/trans-array.c	(revision 129417)
--- /svn/trunk/gcc/fortran/trans-array.c	(working copy)
*************** gfc_conv_loop_setup (gfc_loopinfo * loop
*** 3376,3381 ****
--- 3376,3388 ----
    if (loop->temp_ss != NULL)
      {
        gcc_assert (loop->temp_ss->type == GFC_SS_TEMP);
+ 
+       /* Make absolutely sure that this is a complete type.  */
+       if (loop->temp_ss->string_length)
+ 	loop->temp_ss->data.temp.type
+ 		= gfc_get_character_type_len (gfc_default_character_kind,
+ 					      loop->temp_ss->string_length);
+ 
        tmp = loop->temp_ss->data.temp.type;
        len = loop->temp_ss->string_length;
        n = loop->temp_ss->data.temp.dimen;
Index: /svn/trunk/gcc/fortran/trans-stmt.c
===================================================================
*** /svn/trunk/gcc/fortran/trans-stmt.c	(revision 129417)
--- /svn/trunk/gcc/fortran/trans-stmt.c	(working copy)
*************** gfc_trans_assign_need_temp (gfc_expr * e
*** 2172,2178 ****
  					&lss, &rss);
  
    /* The type of LHS. Used in function allocate_temp_for_forall_nest */
!   type = gfc_typenode_for_spec (&expr1->ts);
  
    /* Allocate temporary for nested forall construct according to the
       information in nested_forall_info and inner_size.  */
--- 2172,2191 ----
  					&lss, &rss);
  
    /* The type of LHS. Used in function allocate_temp_for_forall_nest */
!   if (expr1->ts.type == BT_CHARACTER && expr1->ts.cl->length)
!     {
!       if (!expr1->ts.cl->backend_decl)
! 	{
! 	  gfc_se tse;
! 	  gfc_init_se (&tse, NULL);
! 	  gfc_conv_expr (&tse, expr1->ts.cl->length);
! 	  expr1->ts.cl->backend_decl = tse.expr;
! 	}
!       type = gfc_get_character_type_len (gfc_default_character_kind,
! 				         expr1->ts.cl->backend_decl);
!     }
!   else
!     type = gfc_typenode_for_spec (&expr1->ts);
  
    /* Allocate temporary for nested forall construct according to the
       information in nested_forall_info and inner_size.  */
Index: /svn/trunk/gcc/fortran/resolve.c
===================================================================
*** /svn/trunk/gcc/fortran/resolve.c	(revision 129417)
--- /svn/trunk/gcc/fortran/resolve.c	(working copy)
*************** find_forall_index (gfc_expr *expr, gfc_s
*** 4375,4383 ****
  	      break;
  
  	    case REF_SUBSTRING:
- 	      if (expr->symtree->n.sym == symbol)
- 		return SUCCESS;
- 	      tmp = expr->ref;
  	      /* Check if the symbol appears in the substring section.  */
  	      if (find_forall_index (tmp->u.ss.start, symbol) == SUCCESS)
  		return SUCCESS;
--- 4375,4380 ----
Index: /svn/trunk/gcc/fortran/dependency.c
===================================================================
*** /svn/trunk/gcc/fortran/dependency.c	(revision 129417)
--- /svn/trunk/gcc/fortran/dependency.c	(working copy)
*************** gfc_check_dependency (gfc_expr *expr1, g
*** 657,664 ****
  
        /* Identical and disjoint ranges return 0,
  	 overlapping ranges return 1.  */
!       /* Return zero if we refer to the same full arrays.  */
!       if (expr1->ref->type == REF_ARRAY && expr2->ref->type == REF_ARRAY)
  	return gfc_dep_resolver (expr1->ref, expr2->ref);
  
        return 1;
--- 657,663 ----
  
        /* Identical and disjoint ranges return 0,
  	 overlapping ranges return 1.  */
!       if (expr1->ref && expr2->ref)
  	return gfc_dep_resolver (expr1->ref, expr2->ref);
  
        return 1;
*************** gfc_dep_resolver (gfc_ref *lref, gfc_ref
*** 1197,1204 ****
  	  break;
  	  
  	case REF_SUBSTRING:
! 	  /* Substring overlaps are handled by the string assignment code.  */
! 	  return 0;
  	
  	case REF_ARRAY:
  	  if (lref->u.ar.dimen != rref->u.ar.dimen)
--- 1196,1205 ----
  	  break;
  	  
  	case REF_SUBSTRING:
! 	  /* Substring overlaps are handled by the string assignment code
! 	     if there is not an underlying dependency.  */
! 	  return ((fin_dep >= GFC_DEP_OVERLAP)
! 		    && (fin_dep != GFC_DEP_ERROR)) ? 1 : 0;
  	
  	case REF_ARRAY:
  	  if (lref->u.ar.dimen != rref->u.ar.dimen)
Index: /svn/trunk/gcc/testsuite/gfortran.dg/forall_12.f90
===================================================================
*** /svn/trunk/gcc/testsuite/gfortran.dg/forall_12.f90	(revision 0)
--- /svn/trunk/gcc/testsuite/gfortran.dg/forall_12.f90	(revision 0)
***************
*** 0 ****
--- 1,25 ----
+ ! { dg-do run }
+ ! Tests the fix for PR31217 and PR33811 , in which dependencies were not
+ ! correctly handled for the assignments below and, when this was fixed,
+ ! the last two ICEd on trying to create the temorary.
+ !
+ ! Contributed by Joost VandeVondele <jv244@cam.ac.uk>
+ !
+   character(len=1) :: a = "1"
+   character(len=1) :: b(4) = (/"1","2","3","4"/), c(4)
+   c = b
+   forall(i=1:1) a(i:i) = a(i:i)         ! This was the original PR31217
+   forall(i=1:1) b(i:i) = b(i:i)         ! The rest were found to be broken
+   forall(i=1:1) b(:)(i:i) = b(:)(i:i)
+   forall(i=1:1) b(1:3)(i:i) = b(2:4)(i:i)
+   if (any (b .ne. (/"2","3","4","4"/))) call abort ()
+   b = c
+   forall(i=1:1) b(2:4)(i:i) = b(1:3)(i:i)
+   if (any (b .ne. (/"1","1","2","3"/))) call abort ()
+   b = c
+   do i = 1, 1
+     b(2:4)(i:i) = b(1:3)(i:i)           ! This was PR33811
+   end do
+   if (any (b .ne. (/"1","1","2","3"/))) call abort ()
+ end
+ 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]