Re: [PATCH] avoid assuming every type has a size (PR 89662)

[Martin Sebor]

On 3/12/19 2:20 AM, Richard Biener wrote:
On Mon, Mar 11, 2019 at 9:16 PM Martin Sebor <mse...@gmail.com> wrote:

A -Warray-bounds enhancement committed last year into GCC 9
introduced an assumption that the MEM_REF type argument has
a size.  The test case submitted in PR89662 does pointer
addition on void*, in which the MEM_REF type is void*, which
breaks the assumption.

The attached change removes this assumption and considers such
types to have the size of 1.  (The result is used to scale
the offset in diagnostics after it has been determined to be
out of bounds.)

Why's this not catched here:

   if (POINTER_TYPE_P (reftype)
       || !COMPLETE_TYPE_P (reftype)
^^^

       || TREE_CODE (TYPE_SIZE_UNIT (reftype)) != INTEGER_CST
       || RECORD_OR_UNION_TYPE_P (reftype))
     return;

Reftype is the type of the argument referenced by the MEM_REF,
not the type of the MEM_REF itself.  The type examined in
the patch is the latter.  In the test case in PR 89662, reftype
is unsigned char but the MEM_REF type is void*.

  void *f (void *c) { return c; }
  void g () {
                    // unsigned char D.1930[1];
    int n = 1;
    char c[n];
    h (f(c) - 1);   // h (&MEM[(void *)&D.1930 + -1B]);
  }


and what avoids the bad situation for

   char (*a)[n];
   sink (a - 1);

?  That is, the code assumes TYPE_SIZE_UNIT is an INTEGER_CST
but the above should get you a non-constant type?  It's probably
easier to generate a gimple testcase with this.

I think it will depend on what a points to.  The only kind of VLA
the code works with is one whose size is known (for others, like
in some range, I haven't seen a MEM_REF) .  In the following for
instance we get:

  void f (void)
  {
    int n = 4;
    char a[n];      // unsigned char D.1935[4];
    int (*p)[n] = &a;
    sink (p - 1);   // &MEM[(void *)&D.1935 - 16B]
    // warning: array subscript -4 is outside array bounds of ‘unsigned 
char[4]’
    sink (*p - 1); // &MEM[(void *)&D.1935 - 4B]
    // warning: array subscript -1 is outside array bounds of ‘unsigned 
char[4]’
  }

I'm not sure what a GIMPLE test case might look like that would
make the code misbehave.  I tried a few variations but most of
them ICE somewhere else.

Martin