This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH to fix -Wrestrict ICE (PR middle-end/83463)


On 12/18/2017 05:27 PM, Jakub Jelinek wrote:
On Mon, Dec 18, 2017 at 05:03:22PM -0700, Martin Sebor wrote:
Your warning is about restrict and argument overlap, what does it have to do
with unprototyped calls?  Nothing.  There is no restrict in that case, and
it isn't handled as builtin if it doesn't match the builtin's prototype.

$ cat a.c && gcc -O2 -S -Wrestrict a.c
void* memcpy ();

char a[5];

void f (void)
{
  memcpy (a, a + 1, 3);
}
a.c: In function ‘f’:
a.c:7:3: warning: ‘memcpy’ accessing 3 bytes at offsets 0 and 1 overlaps 2
bytes at offset 1 [-Wrestrict]
   memcpy (a, a + 1, 3);
   ^~~~~~~~~~~~~~~~~~~~

By insisting on using gimple_call_builtin_p() you're effectively
arguing to disable the warning above, for no good reason that I
can see.

Because it is wrong.

I disagree.  Memcpy is a reserved external identifier and its
semantics are specified by the standard.  A program that defines
its own memcpy is undefined,  Even with -ffreestanding, GCC makes
assumptions about environments providing conforming memcpy and
memset.  Most of the other built-ins handled by the pass are also
reserved by C, same as memcpy.  But you know this at least as well
as I do.

Consider e.g.
void *mempcpy ();

void
foo (int *a)
{
  mempcpy (a, (float *) a + 1, ' ');
}

mempcpy isn't defined in ISO C99, so it is fine if you define it yourself
with void *mempcpy (int *, float *, char); prototype and do whatever you
want in there.  Warning about restrict doesn't make sense in that case.

Programs that define their own versions of GCC built-ins, whether
standard C or specified elsewhere, should disable the built-ins
from being recognized.  That's well understood, especially thanks
to -Wbuiltin-declaration-mismatch.  Otherwise, as the manual says,
they "may be handled as built-in functions."

As I already explained, the purpose of this pass is to help detect
bugs due to "honest" mistakes.  I see no value in trying to disable
such detection on the basis hypothetical use cases.  Doing so would
defeat the purpose of the pass.  I'm not at all concerned about
programs that don't disable built-ins but define functions with
the same names yet conflicting semantics, and on top of it declare
them without a prototype.  If those get diagnosed, so much
the better.  It will be a reminder to their authors to either
use -fno-builtin-xxx to disable the conflicting built-ins, at
least until -Wbuiltin-declaration-mismatch is enhanced to diagnose
such declarations.

With that said, while I value your input, I do not see the point
in continuing to debate whether the pass should diagnose these
cases.  In my mind the benefits are obvious and the downsides
are none.

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]