Re: [PATCH] fix ICEs in c-attribs.c (PR 88383, 89288, 89798, 89797)​

Martin Sebor msebor@gmail.com
Wed Apr 17 06:02:00 GMT 2019


On 4/15/19 7:12 AM, Christophe Lyon wrote:
> On Sat, 13 Apr 2019 at 00:38, Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 4/12/19 3:42 PM, Jakub Jelinek wrote:
>>> On Fri, Apr 12, 2019 at 10:45:25AM -0600, Jeff Law wrote:
>>>>> gcc/ChangeLog:
>>>>>
>>>>>      PR c/89797
>>>>>      * targhooks.c (default_vector_alignment): Avoid assuming
>>>>>      argument fits in SHWI.
>>>>>      * tree.h (TYPE_VECTOR_SUBPARTS): Avoid sign overflow in
>>>>>      a shift expression.
>>>>>
>>>>> gcc/c-family/ChangeLog:
>>>>>
>>>>>      PR c/88383
>>>>>      PR c/89288
>>>>>      PR c/89798
>>>>>      PR c/89797
>>>>>      * c-attribs.c (type_valid_for_vector_size): Detect excessively
>>>>>      large sizes.
>> ...
>>>
>>> Has the patch been tested at all?
>>
>> A few times.  The c-attribs.c change above didn't make it into
>> the commit.
> 
> Hi,
> Even with r270331, I'm still seeing the ICE on aarch64 (actually with
> trunk @r270370)
> 
> Is there still some commit missing?

I only fixed the TYPE_VECTOR_SUBPARTS bug behind the ICE in bug
89797 for targets where NUM_POLY_INT_COEFFS == 1, and only tested
it on x86_64.

The block of code that's compiled when NUM_POLY_INT_COEFFS == 1
is also buggy but from what I can tell in more ways than one.

   inline poly_uint64
   TYPE_VECTOR_SUBPARTS (const_tree node)
   {
     STATIC_ASSERT (NUM_POLY_INT_COEFFS <= 2);
     unsigned int precision = VECTOR_TYPE_CHECK 
(node)->type_common.precision;
     if (NUM_POLY_INT_COEFFS == 2)
       {
         poly_uint64 res = 0;
         res.coeffs[0] = 1 << (precision & 0xff);
         if (precision & 0x100)
   	  res.coeffs[1] = 1 << (precision & 0xff);
         return res;
       }
     else
       return (unsigned HOST_WIDE_INT)1 << precision;
   }

Shifting 1 to the left by more than 30 bits is undefined (that
was also the bug I fixed in the other branch).  In addition,
setting res.coeffs[1] to the same value as res.coeffs[0] doesn't
seem right either, but since precision should be less than 256
it may not actually matter.  In any case, I wasn't sure and
since I don't have easy access to speedy aarch64 hardware I
wasn't brave enough to mess with it at this stage.

Then there's another bug in aarch64_simd_vector_alignment
similar to the one the patch fixed in default_vector_alignment
where the code assumes that the vector alignment fits in SHWI.

The otherwise untested patch below fixes the ICE for aarch64
but not the other (suspected) bug.

Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 270402)
+++ gcc/tree.h	(working copy)
@@ -3735,9 +3735,9 @@ TYPE_VECTOR_SUBPARTS (const_tree node)
    if (NUM_POLY_INT_COEFFS == 2)
      {
        poly_uint64 res = 0;
-      res.coeffs[0] = 1 << (precision & 0xff);
+      res.coeffs[0] = (unsigned HOST_WIDE_INT)1 << (precision & 0xff);
        if (precision & 0x100)
-	res.coeffs[1] = 1 << (precision & 0xff);
+	res.coeffs[1] = (unsigned HOST_WIDE_INT)1 << (precision & 0xff);
        return res;
      }
    else
Index: gcc/config/aarch64/aarch64.c
===================================================================
--- gcc/config/aarch64/aarch64.c	(revision 270402)
+++ gcc/config/aarch64/aarch64.c	(working copy)
@@ -14924,7 +14924,10 @@ aarch64_simd_vector_alignment (const_tree type)
         be set for non-predicate vectors of booleans.  Modes are the most
         direct way we have of identifying real SVE predicate types.  */
      return GET_MODE_CLASS (TYPE_MODE (type)) == MODE_VECTOR_BOOL ? 16 
: 128;
-  HOST_WIDE_INT align = tree_to_shwi (TYPE_SIZE (type));
+  tree size = TYPE_SIZE (type);
+  unsigned HOST_WIDE_INT align = 128;
+  if (tree_fits_uhwi_p (size))
+    align = tree_to_uhwi (TYPE_SIZE (type));
    return MIN (align, 128);
  }

I'm guessing that the full fix for TYPE_VECTOR_SUBPARTS might look
something like this, but that's just a wild guess.

===================================================================
--- gcc/tree.h	(revision 270402)
+++ gcc/tree.h	(working copy)
@@ -3735,9 +3735,9 @@ TYPE_VECTOR_SUBPARTS (const_tree node)
    if (NUM_POLY_INT_COEFFS == 2)
      {
        poly_uint64 res = 0;
-      res.coeffs[0] = 1 << (precision & 0xff);
+      res.coeffs[0] = (unsigned HOST_WIDE_INT)1 << (precision & 0xff);
        if (precision & 0x100)
-	res.coeffs[1] = 1 << (precision & 0xff);
+	res.coeffs[1] = (unsigned HOST_WIDE_INT)1 << ((precision & 0x100) >> 16);
        return res;
      }
    else

Martin



More information about the Gcc-patches mailing list