On 12/18/2017 04:41 PM, Jakub Jelinek wrote:
On Mon, Dec 18, 2017 at 04:04:11PM -0700, Martin Sebor wrote:
On 12/18/2017 12:04 PM, Jakub Jelinek wrote:
On Mon, Dec 18, 2017 at 11:08:19AM -0700, Martin Sebor wrote:
It isn't optimized either way.  In fact, the only indication
of a problem in the code below is the new -Wrestrict warning:

So just call it as memcpy (0, 0, (size_t) 0); or memcpy (0, 0, 0UL);
on targets where size_t is unsigned long int, and it will be optimized away,
because it will be compatible with the builtin's prototype then.

A call to memcpy(0, 0, (size_t)0) is not eliminated on any target,
presumably because the type of 0 is int and not void*.  "Don't
write bad code" is also not an answer to the question of why
shouldn't GCC eliminate the bad code (null pointer) when there
is no prototype given that it eliminates it when the function
is declared with one.  But I'm not insisting on it.  What
I think will much more helpful is diagnosing such bad calls,
similarly to how attribute sentinel diagnoses the analogous
subset of the problem involving variadic functions.

gimple_call_builtin_p is the API that has been agreed upon to be used for
checking for builtins several years ago, there is absolutely no reason why
this pass needs to treat them differently.

I already explained the reason: to help detect bugs.  That's

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.

I'm happy to take suggestions to improve the pass and fix bugs
in it.  But I see no point in debating whether it should be
changed in ways that would prevent it from diagnosing bugs like
the one above.  That would be counter-productive.

Martin

Reply via email to