Patchwork RFC patch for fold-const.c's fold_indirect_ref_1 / avoid type-checking ICE with TYPE_MIN_VALUE vs. offset signess mismatches

login
register
mail settings
Submitter Tobias Burnus
Date May 10, 2013, 8:44 a.m.
Message ID <20130510084404.GA29124@physik.fu-berlin.de>
Download mbox | patch
Permalink /patch/242948/
State New
Headers show

Comments

Tobias Burnus - May 10, 2013, 8:44 a.m.
I run into the following issue on the Fortran-Dev branch. I get an ICE due to a tree-checking assert for fold-const.c's fold_indirect_ref_1, which has

              tree min_val = size_zero_node;
              if (type_domain && TYPE_MIN_VALUE (type_domain))
                min_val = TYPE_MIN_VALUE (type_domain);
              op01 = size_binop_loc (loc, EXACT_DIV_EXPR, op01,
                                     TYPE_SIZE_UNIT (type));
              op01 = size_binop_loc (loc, PLUS_EXPR, op01, min_val);

The problem is that "op01" is unsigned (size_type_node) while "min_val" alias TYPE_MIN_VALUE is signed (gfc_array_index_type which is PTRDIFF_TYPE).


TYPE_MIN_VALUE gets set via tree.c's build_range_type_1, which has:

  TYPE_MIN_VALUE (itype) = fold_convert (type, lowval);
  TYPE_MAX_VALUE (itype) = highval ? fold_convert (type, highval) : NULL;


And "op01" becomes a size_type_node via tree.h's

fold_build_pointer_plus_loc (location_t loc, tree ptr, tree off)
{
  return fold_build2_loc (loc, POINTER_PLUS_EXPR, TREE_TYPE (ptr),
                          ptr, fold_convert_loc (loc, sizetype, off));


I am not completely sure what's the best way to solve that, but the attached patch avoids the ICE for me.

I did an all-all language bootstrap (all,ada,go,objc,obj-c++) and had no new failures with regresting.
OK? Or do you have a better idea how to solve this?

Tobias
Richard Guenther - May 10, 2013, 12:35 p.m.
On Fri, May 10, 2013 at 10:44 AM, Tobias Burnus
<tobias.burnus@physik.fu-berlin.de> wrote:
> I run into the following issue on the Fortran-Dev branch. I get an ICE due to a tree-checking assert for fold-const.c's fold_indirect_ref_1, which has
>
>               tree min_val = size_zero_node;
>               if (type_domain && TYPE_MIN_VALUE (type_domain))
>                 min_val = TYPE_MIN_VALUE (type_domain);
>               op01 = size_binop_loc (loc, EXACT_DIV_EXPR, op01,
>                                      TYPE_SIZE_UNIT (type));
>               op01 = size_binop_loc (loc, PLUS_EXPR, op01, min_val);
>
> The problem is that "op01" is unsigned (size_type_node) while "min_val" alias TYPE_MIN_VALUE is signed (gfc_array_index_type which is PTRDIFF_TYPE).
>
>
> TYPE_MIN_VALUE gets set via tree.c's build_range_type_1, which has:
>
>   TYPE_MIN_VALUE (itype) = fold_convert (type, lowval);
>   TYPE_MAX_VALUE (itype) = highval ? fold_convert (type, highval) : NULL;
>
>
> And "op01" becomes a size_type_node via tree.h's
>
> fold_build_pointer_plus_loc (location_t loc, tree ptr, tree off)
> {
>   return fold_build2_loc (loc, POINTER_PLUS_EXPR, TREE_TYPE (ptr),
>                           ptr, fold_convert_loc (loc, sizetype, off));
>
>
> I am not completely sure what's the best way to solve that, but the attached patch avoids the ICE for me.
>
> I did an all-all language bootstrap (all,ada,go,objc,obj-c++) and had no new failures with regresting.
> OK? Or do you have a better idea how to solve this?

I think the middle-end expects unsigned sizetype in many places, so this
isn't the only point where you may hit such an issue.  The code has the
related issue that it happily does an unsigned division on negative offsets
because it fails to interpret the POINTER_PLUS_EXPR offset as signed

Note that those foldings are highly suspicious (I've removed most similar
foldings from elsewhere) because they are susceptible to generate
out-of-bound array accesses for multi-dimensional arrays.  This one seems
to be bogus this way, too.

That said, the function needs some major TLC :/  (I'd simply try removing
that array case)

Richard.

> Tobias

Patch

2013-05-10  Tobias Burnus  <burnus@net-b.de>

	* fold-const.c (fold_indirect_ref_1): Fold offset
	to TREE_TYPE (min_val) before converting to array syntax.

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 74b451e..8ac873a 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -16591,10 +16591,11 @@  fold_indirect_ref_1 (location_t loc, tree type, tree op0)
 	      tree min_val = size_zero_node;
 	      if (type_domain && TYPE_MIN_VALUE (type_domain))
 		min_val = TYPE_MIN_VALUE (type_domain);
 	      op01 = size_binop_loc (loc, EXACT_DIV_EXPR, op01,
 				     TYPE_SIZE_UNIT (type));
+             op01 = fold_convert (TREE_TYPE (min_val), op01);
 	      op01 = size_binop_loc (loc, PLUS_EXPR, op01, min_val);
 	      return build4_loc (loc, ARRAY_REF, type, op00, op01,
 				 NULL_TREE, NULL_TREE);
 	    }
 	}