This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [ping * 4] PR35503 - warn for restrict
- From: Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>
- To: Jason Merrill <jason at redhat dot com>
- Cc: Joseph Myers <joseph at codesourcery dot com>, gcc Patches <gcc-patches at gcc dot gnu dot org>, Marek Polacek <polacek at redhat dot com>
- Date: Wed, 2 Nov 2016 23:17:37 +0530
- Subject: Re: [ping * 4] PR35503 - warn for restrict
- Authentication-results: sourceware.org; auth=none
- References: <CAAgBjMm+GuqQjXVS61gxGXZjeQCR5=r19njEU_xgXw312Bu6_Q@mail.gmail.com> <alpine.DEB.2.20.1611021242440.29297@digraph.polyomino.org.uk> <CADzB+2k=jyMbBR0bBFAa0qOBTcWLafuBTSaBTsODDTxDBU8hYA@mail.gmail.com> <CAAgBjM=gVP-MSJ76CZPDFYw7pRVmQdCBXJ69y_EwVz=aqRq2=A@mail.gmail.com> <CADzB+2mD6SguSfKqt=T2cv=tX+yYhaFrdNZzcUJ9J4bM3W9Zrw@mail.gmail.com>
On 2 November 2016 at 23:07, Jason Merrill <jason@redhat.com> wrote:
> On Wed, Nov 2, 2016 at 1:08 PM, Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> wrote:
>> On 2 November 2016 at 18:29, Jason Merrill <jason@redhat.com> wrote:
>>> Then I'll approve the whole patch.
>> Thanks!
>> Trying the patch on kernel build (allmodconfig) reveals the following
>> couple of warnings:
>> http://pastebin.com/Sv2HFDUv
>>
>> I think warning for str_error_r() is correct
>
> It's accurate, but unhelpful; snprintf isn't going to use the contents
> of buf via the variadic argument, so this warning is just noise.
Ah, indeed, it's just printing address of buf, not using the contents.
>
>> however I am not sure if
>> warning for pager_preexec() is legit or a false positive:
>>
>> pager.c: In function 'pager_preexec':
>> pager.c:35:12: warning: passing argument 2 to restrict-qualified
>> parameter aliases with argument 4 [-Wrestrict]
>> select(1, &in, NULL, &in, NULL);
>> ^~~ ~~~
>> Is the warning correct for the above call to select() syscall ?
>
> The warning looks correct based on the prototype
>
> extern int select (int __nfds, fd_set *__restrict __readfds,
> fd_set *__restrict __writefds,
> fd_set *__restrict __exceptfds,
> struct timeval *__restrict __timeout);
>
> But passing the same fd_set to both readfds and exceptfds seems
> reasonable to me, so this also seems like a false positive.
>
> Looking at C11, I see this example:
>
> EXAMPLE 3 The function parameter declarations
> void h(int n, int * restrict p, int * restrict q, int * restrict r)
> {
> int i;
> for (i = 0; i < n; i++)
> p[i] = q[i] + r[i];
> }
>
> illustrate how an unmodified object can be aliased through two
> restricted pointers. In particular, if a and b
> are disjoint arrays, a call of the form h(100, a, b, b) has defined
> behavior, because array b is not
> modified within function h.
>
> This is is another example of well-defined code that your warning will
> complain about.
Yes, that's a limitation of the patch, it just looks at the prototype, and
not how the arguments are used in the function.
>
>> Should we instead keep it in Wextra, or continue keeping it in Wall ?
>
> It seems that it doesn't belong in -Wall. I don't feel strongly about -Wextra.
Should I commit the patch by keeping Wrestrict "standalone",
ie, not including it in either Wall or Wextra ?
Thanks,
Prathamesh
>
> Jason