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