On Tue, Mar 12, 2019 at 5:36 PM Martin Sebor <mse...@gmail.com> wrote:
>
> 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.
Ugh how misleading...
> 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.
Thanks for trying. The patch is OK if you adjust it to verify
TYPE_SIZE_UNIT is an INTEGER_CST before throwing it
at wi::to_offset.
Richard.
> Martin
