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]

[PATCH] PR fortran/12632: -fbounds-check ICE


The following patch is my proposed solution to PR fortran/12632,
which is an ICE on valid using the g77 compiler with -fbounds-check
that is present on both mainline and the 3.3 branch.  The problem
concerns how to generate code/RTL when we determine that an array
index is out of bounds at compile-time.

The test case looks deceptively simple:

      INTEGER I(1)
      I(2) = 0
      END

Array bounds checking uses the special "void type" semantics of a
COND_EXPR, that allows either the second or third operand of the
ternary operator to have void type, indicating that the evaluation
of the branch doesn't terminate naturally, i.e. throws an exception
or calls a "noreturn" function.

The immediate/obvious problem is that these semantics interact poorly
with GCC's constant folding optimizations; the expression "1 ? x : y"
can only be transformed into "x", if X has the same type as the
COND_EXPR.  Similarly, for "0 ? x : y", where Y must have the same
type as the COND_EXPR.   When the array index was safe at compile-time,
things worked perfectly eliminating the condition, but unfortunately
if the array index was unsafe, we'd optimize the index into a function
call of type void, which then creates havoc.  This problem is easily
fixed either by testing for a constant condition in the front-end and
creating a COMPOUND_EXPR of the correct type, or alternatively by
disallowing the "unsafe" optimizations in fold-const.c.


As discovered by Andrew Pinski however, that fix alone isn't sufficient
for the above testcase, when optimizations are specified.  The fortran
front-end has a extremely clever optimization that realizes that if an
array is always only indexed by constants it need not be forced into
memory.  For the test-case above, the array I(1) *is* only indexed by
integer constants, and therefore gets assigned during g77's RTL expansion
to a single SImode register.

Of course, expand_assignment in expr.c only allows ARRAY_REFs to be
anything other than MEMs if the offset is extremely simple, i.e. constant
zero.  For constant integer indices that are within bounds, they'll get
folded down to a constant, everything works fine.  However, COMPOUND_EXPRs
or COND_EXPRs or any other way of wrapping a "noreturn" function call are
treated by the RTL expanders as a non-trivial offsets, and we therefore
abort.


My proposed solution to this second far more subtle issue, is to require
any array that has a constant index that is out of bounds at compile-time,
to be forced into memory when -fbounds-check is specified.   The RTL
optimizers are then more than happy to expand the call to the noreturn
function, and later passes delete the following address calculations as
dead.  In the g77 front-end, this is achieved by calling the function
ffe_mark_addressable on the array's VAR_DECL.  The only minor glitch is
that this wasn't previously passed to ffecom_subscript_check_, which is
where we diagnose the potential problem;  So I've added an additional
argument, and updated all the callers of this function to pass in a
suitable value.


The following patch has been tested on i686-pc-linux-gnu with a bootstrap
including C and fortran, and regression tested by running "make check-g77"
in the gcc subdirectory, with no new failures.  The new testcase, passes
at all optimization levels, but fails without this patch.


Ok for mainline?  And 3.3 after a few days?

Many thanks in advance,


2003-12-21  Roger Sayle  <roger@eyesopen.com>

	PR fortran/12632
	* fold-const.c (fold) <COND_EXPR>: Don't fold a constant condition,
	if the type of the selected branch doesn't match its' parent.

	* com.c (ffecom_subscript_check_): Take as an extra argument the
	(possibly NULL) decl of the array.  Don't create unnecessary tree
	nodes if the array index is known to be safe at compile-time.
	If the array index is unsafe, force the array decl into memory to
	avoid RTL expansion problems.
	(ffecom_array_ref_): Update calls to ffecom_subscript_check_.
	(ffecom_char_args_x_): Likewise.

	* g77.dg/12632.f: New test case.


Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.318
diff -c -3 -p -r1.318 fold-const.c
*** fold-const.c	20 Dec 2003 01:40:41 -0000	1.318
--- fold-const.c	22 Dec 2003 05:29:15 -0000
*************** fold (tree expr)
*** 7864,7872 ****
        /* Pedantic ANSI C says that a conditional expression is never an lvalue,
  	 so all simple results must be passed through pedantic_non_lvalue.  */
        if (TREE_CODE (arg0) == INTEGER_CST)
! 	return pedantic_non_lvalue
! 	  (TREE_OPERAND (t, (integer_zerop (arg0) ? 2 : 1)));
!       else if (operand_equal_p (arg1, TREE_OPERAND (expr, 2), 0))
  	return pedantic_omit_one_operand (type, arg1, arg0);

        /* If we have A op B ? A : C, we may be able to convert this to a
--- 7864,7879 ----
        /* Pedantic ANSI C says that a conditional expression is never an lvalue,
  	 so all simple results must be passed through pedantic_non_lvalue.  */
        if (TREE_CODE (arg0) == INTEGER_CST)
! 	{
! 	  tem = TREE_OPERAND (t, (integer_zerop (arg0) ? 2 : 1));
! 	  /* Only optimize constant conditions when the selected branch
! 	     has the same type as the COND_EXPR.  This avoids optimizing
! 	     away "c ? x : throw", where the throw has a void type.  */
! 	  if (TREE_TYPE (tem) == TREE_TYPE (t))
! 	    return pedantic_non_lvalue (tem);
! 	  return t;
! 	}
!       if (operand_equal_p (arg1, TREE_OPERAND (expr, 2), 0))
  	return pedantic_omit_one_operand (type, arg1, arg0);

        /* If we have A op B ? A : C, we may be able to convert this to a
Index: f/com.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/f/com.c,v
retrieving revision 1.220
diff -c -3 -p -r1.220 com.c
*** f/com.c	31 Oct 2003 10:34:03 -0000	1.220
--- f/com.c	22 Dec 2003 05:29:21 -0000
*************** static GTY(()) tree shadowed_labels;
*** 638,652 ****

  /* Return the subscript expression, modified to do range-checking.

!    `array' is the array to be checked against.
     `element' is the subscript expression to check.
     `dim' is the dimension number (starting at 0).
     `total_dims' is the total number of dimensions (0 for CHARACTER substring).
  */

  static tree
  ffecom_subscript_check_ (tree array, tree element, int dim, int total_dims,
! 			 const char *array_name)
  {
    tree low = TYPE_MIN_VALUE (TYPE_DOMAIN (array));
    tree high = TYPE_MAX_VALUE (TYPE_DOMAIN (array));
--- 638,653 ----

  /* Return the subscript expression, modified to do range-checking.

!    `array' is the array type to be checked against.
     `element' is the subscript expression to check.
     `dim' is the dimension number (starting at 0).
     `total_dims' is the total number of dimensions (0 for CHARACTER substring).
+    `item' is the array decl or NULL_TREE.
  */

  static tree
  ffecom_subscript_check_ (tree array, tree element, int dim, int total_dims,
! 			 const char *array_name, tree item)
  {
    tree low = TYPE_MIN_VALUE (TYPE_DOMAIN (array));
    tree high = TYPE_MAX_VALUE (TYPE_DOMAIN (array));
*************** ffecom_subscript_check_ (tree array, tre
*** 713,718 ****
--- 714,723 ----
          }
      }

+   /* If the array index is safe at compile-time, return element.  */
+   if (integer_nonzerop (cond))
+     return element;
+
    {
      int len;
      char *proc;
*************** ffecom_subscript_check_ (tree array, tre
*** 807,819 ****
    TREE_SIDE_EFFECTS (die) = 1;
    die = convert (void_type_node, die);

!   element = ffecom_3 (COND_EXPR,
! 		      TREE_TYPE (element),
! 		      cond,
! 		      element,
! 		      die);

!   return element;
  }

  /* Return the computed element of an array reference.
--- 812,821 ----
    TREE_SIDE_EFFECTS (die) = 1;
    die = convert (void_type_node, die);

!   if (integer_zerop (cond) && item)
!     ffe_mark_addressable (item);

!   return ffecom_3 (COND_EXPR, TREE_TYPE (element), cond, element, die);
  }

  /* Return the computed element of an array reference.
*************** ffecom_arrayref_ (tree item, ffebld expr
*** 899,905 ****
  	  element = ffecom_expr_ (dims[i], NULL, NULL, NULL, FALSE, TRUE);
  	  if (flag_bounds_check)
  	    element = ffecom_subscript_check_ (array, element, i, total_dims,
! 					       array_name);
  	  if (element == error_mark_node)
  	    return element;

--- 901,907 ----
  	  element = ffecom_expr_ (dims[i], NULL, NULL, NULL, FALSE, TRUE);
  	  if (flag_bounds_check)
  	    element = ffecom_subscript_check_ (array, element, i, total_dims,
! 					       array_name, item);
  	  if (element == error_mark_node)
  	    return element;

*************** ffecom_arrayref_ (tree item, ffebld expr
*** 945,951 ****
  	  element = ffecom_expr_ (dims[i], NULL, NULL, NULL, FALSE, TRUE);
  	  if (flag_bounds_check)
  	    element = ffecom_subscript_check_ (array, element, i, total_dims,
! 					       array_name);
  	  if (element == error_mark_node)
  	    return element;

--- 947,953 ----
  	  element = ffecom_expr_ (dims[i], NULL, NULL, NULL, FALSE, TRUE);
  	  if (flag_bounds_check)
  	    element = ffecom_subscript_check_ (array, element, i, total_dims,
! 					       array_name, item);
  	  if (element == error_mark_node)
  	    return element;

*************** ffecom_char_args_x_ (tree *xitem, tree *
*** 2037,2043 ****
  		end_tree = ffecom_expr (end);
  		if (flag_bounds_check)
  		  end_tree = ffecom_subscript_check_ (array, end_tree, 1, 0,
! 						      char_name);
  		end_tree = convert (ffecom_f2c_ftnlen_type_node,
  				    end_tree);

--- 2039,2045 ----
  		end_tree = ffecom_expr (end);
  		if (flag_bounds_check)
  		  end_tree = ffecom_subscript_check_ (array, end_tree, 1, 0,
! 						      char_name, NULL_TREE);
  		end_tree = convert (ffecom_f2c_ftnlen_type_node,
  				    end_tree);

*************** ffecom_char_args_x_ (tree *xitem, tree *
*** 2055,2061 ****
  	    start_tree = ffecom_expr (start);
  	    if (flag_bounds_check)
  	      start_tree = ffecom_subscript_check_ (array, start_tree, 0, 0,
! 						    char_name);
  	    start_tree = convert (ffecom_f2c_ftnlen_type_node,
  				  start_tree);

--- 2057,2063 ----
  	    start_tree = ffecom_expr (start);
  	    if (flag_bounds_check)
  	      start_tree = ffecom_subscript_check_ (array, start_tree, 0, 0,
! 						    char_name, NULL_TREE);
  	    start_tree = convert (ffecom_f2c_ftnlen_type_node,
  				  start_tree);

*************** ffecom_char_args_x_ (tree *xitem, tree *
*** 2088,2094 ****
  		end_tree = ffecom_expr (end);
  		if (flag_bounds_check)
  		  end_tree = ffecom_subscript_check_ (array, end_tree, 1, 0,
! 						      char_name);
  		end_tree = convert (ffecom_f2c_ftnlen_type_node,
  				    end_tree);

--- 2090,2096 ----
  		end_tree = ffecom_expr (end);
  		if (flag_bounds_check)
  		  end_tree = ffecom_subscript_check_ (array, end_tree, 1, 0,
! 						      char_name, NULL_TREE);
  		end_tree = convert (ffecom_f2c_ftnlen_type_node,
  				    end_tree);


C { dg-do compile }
C { dg-options "-fbounds-check" }
       INTEGER I(1)
       I(2) = 0  ! { dg-error "out of defined range" "out of defined range" }
       END


Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833


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