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