On 10/30/2017 01:53 PM, Richard Biener wrote:
On October 30, 2017 4:19:25 PM GMT+01:00, Martin Sebor <mse...@gmail.com> wrote:
On 10/30/2017 05:45 AM, Richard Biener wrote:
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.

I haven't run into any such cases.  What would a more appropriate
place to detect out-of-bounds offsets?  I'm having a hard time
distinguishing what is appropriate and what isn't.  For instance,
if it's okay to detect some out of bounds offsets/indices in vrp
why is it wrong to do a better job of it in forwprop?


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

That's not the quite my intent, although it is close.


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.

Adding (or subtracting) an integer to/from a pointer to an array
is defined in both C and C++ only if the result points to an element
of the array or just past the last element of the array.  Otherwise
it's undefined. (A non-array object is considered an array of one
for this purpose.)

On GIMPLE this is indistinguishable from (short *) (p->a) + 3.

Sure, they're both the same:

  pa_3 = &p_2(D)->a;
  _1 = pa_3 + 6;

and that's fine because the implementation of the warning sees and
uses the byte offset from the beginning of a, so I don't understand
the problem you are pointing out.  Can you clarify what you mean?

Martin


GIMPLE is neither C nor C++.

Richard.


Martin


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';
       ~~~~^~~

Reply via email to