Re: [PATCH] enhance -Warray-bounds to detect out-of-bounds offsets (PR 82455)

[Richard Biener <rguenther@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2648 bytes --]

On Sun, 29 Oct 2017, Martin Sebor wrote:

> In my work on -Wrestrict, to issue meaningful warnings, I found
> it important to detect both out of bounds array indices as well
> as offsets in calls to restrict-qualified functions like strcpy.
> GCC already detects some of these cases but my tests for
> the enhanced warning exposed a few gaps.
> 
> The attached patch enhances -Warray-bounds to detect more instances
> out-of-bounds indices and offsets to member arrays and non-array
> members.  For example, it detects the out-of-bounds offset in the
> call to strcpy below.
> 
> The patch is meant to be applied on top posted here but not yet
> committed:
>   https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01304.html
> 
> Richard, since this also touches tree-vrp.c I look for your comments.

You fail to tell what you are changing and why - I have to reverse
engineer this from the patch which a) isn't easy in this case, b) feels
like a waste of time.  Esp. since the patch does many things.

My first question is why do you add a warning from forwprop?  It
_feels_ like you're trying to warn about arbitrary out-of-bound
addresses at the point they are folded to MEM_REFs.  And it looks
like you're warning about pointer arithmetic like &p->a + 6.
That doesn't look correct to me.  Pointer arithmetic in GIMPLE
is not restricted to operate within fields that are appearantly
accessed here - the only restriction is with respect to the
whole underlying pointed-to-object.

By doing the warning from forwprop you'll run into all such cases
introduced by GCC itself during quite late optimization passes.

You're trying to re-do __builtin_object_size even when that wasn't
used.

So it looks like you're on the wrong track.  Yes,

 strcpy (p->a + 6, "y");

_may_ be "invalid" C (I'm not even sure about that!) but it
is certainly not invalid GIMPLE.

Richard.

> Jeff, this is the enhancement you were interested in when we spoke
> last week.
> 
> Thanks
> Martin
> 
> $ cat a.c && gcc -O2 -S -Wall a.c
>   struct A { char a[4]; void (*pf)(void); };
> 
>   void f (struct A *p)
>   {
>     p->a[5] = 'x';            // existing -Warray-bounds
> 
>     strcpy (p->a + 6, "y");   // enhanced -Warray-bounds
>   }
> 
>   a.c: In function ‘f’:
>   a.c:7:3: warning: offset 6 is out of bounds of ‘char[4]’ [-Warray-bounds]
>    strcpy (p->a + 6, "y");
>    ^~~~~~~~~~~~~~~~~~~~~~
>   a.c:1:17: note: member declared here
>    struct A { char a[4]; void (*pf)(void); };
>                    ^
>   a.c:5:7: warning: array subscript 5 is above array bounds of ‘char[4]’
> [-Warray-bounds]
>      p->a[5] = 'x';
>      ~~~~^~~