Fix middle-end/81737

Message ID 20170808172345.GK17069@redhat.com
State New
Headers show

Commit Message

Marek Polacek Aug. 8, 2017, 5:23 p.m.
On Mon, Aug 07, 2017 at 04:07:49PM +0200, Richard Biener wrote:
> On August 7, 2017 11:09:59 AM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote:
> >On Mon, Aug 07, 2017 at 10:58:09AM +0200, Jakub Jelinek wrote:
> >> On Mon, Aug 07, 2017 at 10:47:51AM +0200, Marek Polacek wrote:
> >> > In my recent change I failed to check whether the type domain
> >> > of a type is non-NULL and this goof causes crashing on this
> >> > testcase.
> >> > 
> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> >> > 
> >> > 2017-08-07  Marek Polacek  <polacek@redhat.com>
> >> > 
> >> > 	PR middle-end/81737
> >> > 	* fold-const.c (fold_indirect_ref_1): Check type_domain.
> >> > 
> >> > 	* gcc.dg/pr81737.c: New test.
> >> 
> >> The old code was assuming size_zero_node if type_domain is NULL
> >> or TYPE_MIN_VALUE is NULL, which is reasonable for C/C++, but indeed
> >might
> >> be wrong perhaps for Fortran or Ada.
> 
> It's how the middle-end defines it, so please restore this behavior (sorry for missing this in the review).  IIRC there are at least one other 'copy' of the folding somewhere.

Sure, this patch should do it:

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2017-08-08  Marek Polacek  <polacek@redhat.com>

	PR middle/81695
	* fold-const.c (fold_indirect_ref_1): Restore original behavior
	regarding size_zero_node.


	Marek

Comments

Richard Biener Aug. 14, 2017, 8:22 a.m. | #1
On Tue, Aug 8, 2017 at 7:23 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Mon, Aug 07, 2017 at 04:07:49PM +0200, Richard Biener wrote:
>> On August 7, 2017 11:09:59 AM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote:
>> >On Mon, Aug 07, 2017 at 10:58:09AM +0200, Jakub Jelinek wrote:
>> >> On Mon, Aug 07, 2017 at 10:47:51AM +0200, Marek Polacek wrote:
>> >> > In my recent change I failed to check whether the type domain
>> >> > of a type is non-NULL and this goof causes crashing on this
>> >> > testcase.
>> >> >
>> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>> >> >
>> >> > 2017-08-07  Marek Polacek  <polacek@redhat.com>
>> >> >
>> >> >  PR middle-end/81737
>> >> >  * fold-const.c (fold_indirect_ref_1): Check type_domain.
>> >> >
>> >> >  * gcc.dg/pr81737.c: New test.
>> >>
>> >> The old code was assuming size_zero_node if type_domain is NULL
>> >> or TYPE_MIN_VALUE is NULL, which is reasonable for C/C++, but indeed
>> >might
>> >> be wrong perhaps for Fortran or Ada.
>>
>> It's how the middle-end defines it, so please restore this behavior (sorry for missing this in the review).  IIRC there are at least one other 'copy' of the folding somewhere.
>
> Sure, this patch should do it:
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-08-08  Marek Polacek  <polacek@redhat.com>
>
>         PR middle/81695
>         * fold-const.c (fold_indirect_ref_1): Restore original behavior
>         regarding size_zero_node.
>
> diff --git gcc/fold-const.c gcc/fold-const.c
> index 5a118ca50a1..2c47d1d16a4 100644
> --- gcc/fold-const.c
> +++ gcc/fold-const.c
> @@ -14109,22 +14109,21 @@ fold_indirect_ref_1 (location_t loc, tree type, tree op0)
>                    && type == TREE_TYPE (op00type))
>             {
>               tree type_domain = TYPE_DOMAIN (op00type);
> -             tree min;
> +             tree min = size_zero_node;
>               if (type_domain != NULL_TREE
> -                 && (min = TYPE_MIN_VALUE (type_domain))
> +                 && TYPE_MIN_VALUE (type_domain)
>                   && TREE_CODE (min) == INTEGER_CST)
> +               min = TYPE_MIN_VALUE (type_domain);

I think this is wrong for non-INTEGER_CST TYPE_MIN_VALUE.

Richard.

> +             offset_int off = wi::to_offset (op01);
> +             offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type));
> +             offset_int remainder;
> +             off = wi::divmod_trunc (off, el_sz, SIGNED, &remainder);
> +             if (remainder == 0)
>                 {
> -                 offset_int off = wi::to_offset (op01);
> -                 offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type));
> -                 offset_int remainder;
> -                 off = wi::divmod_trunc (off, el_sz, SIGNED, &remainder);
> -                 if (remainder == 0)
> -                   {
> -                     off = off + wi::to_offset (min);
> -                     op01 = wide_int_to_tree (sizetype, off);
> -                     return build4_loc (loc, ARRAY_REF, type, op00, op01,
> -                                        NULL_TREE, NULL_TREE);
> -                   }
> +                 off = off + wi::to_offset (min);
> +                 op01 = wide_int_to_tree (sizetype, off);
> +                 return build4_loc (loc, ARRAY_REF, type, op00, op01,
> +                                    NULL_TREE, NULL_TREE);
>                 }
>             }
>         }
>
>         Marek

Patch

diff --git gcc/fold-const.c gcc/fold-const.c
index 5a118ca50a1..2c47d1d16a4 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -14109,22 +14109,21 @@  fold_indirect_ref_1 (location_t loc, tree type, tree op0)
 		   && type == TREE_TYPE (op00type))
 	    {
 	      tree type_domain = TYPE_DOMAIN (op00type);
-	      tree min;
+	      tree min = size_zero_node;
 	      if (type_domain != NULL_TREE
-		  && (min = TYPE_MIN_VALUE (type_domain))
+		  && TYPE_MIN_VALUE (type_domain)
 		  && TREE_CODE (min) == INTEGER_CST)
+		min = TYPE_MIN_VALUE (type_domain);
+	      offset_int off = wi::to_offset (op01);
+	      offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type));
+	      offset_int remainder;
+	      off = wi::divmod_trunc (off, el_sz, SIGNED, &remainder);
+	      if (remainder == 0)
 		{
-		  offset_int off = wi::to_offset (op01);
-		  offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type));
-		  offset_int remainder;
-		  off = wi::divmod_trunc (off, el_sz, SIGNED, &remainder);
-		  if (remainder == 0)
-		    {
-		      off = off + wi::to_offset (min);
-		      op01 = wide_int_to_tree (sizetype, off);
-		      return build4_loc (loc, ARRAY_REF, type, op00, op01,
-					 NULL_TREE, NULL_TREE);
-		    }
+		  off = off + wi::to_offset (min);
+		  op01 = wide_int_to_tree (sizetype, off);
+		  return build4_loc (loc, ARRAY_REF, type, op00, op01,
+				     NULL_TREE, NULL_TREE);
 		}
 	    }
 	}