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

Message ID 20171218151022.GL2605@redhat.com
State New
Headers show
Series
  • PATCH to fix -Wrestrict ICE (PR middle-end/83463)
Related show

Commit Message

Marek Polacek Dec. 18, 2017, 3:10 p.m.
I'm not entirely up to speed with this code, but this one seemed sufficiently
obvious: check INTEGRAL_TYPE_P before looking at a tree's min/max value.
Otherwise, go with maxobjsize.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2017-12-18  Marek Polacek  <polacek@redhat.com>

	PR middle-end/83463
	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref):
	Check if TYPE is INTEGRAL_TYPE_P before accessing its min/max
	values.

	* gcc.dg/pr83463.c: New test.


	Marek

Comments

Jeff Law Dec. 18, 2017, 4:36 p.m. | #1
On 12/18/2017 08:10 AM, Marek Polacek wrote:
> I'm not entirely up to speed with this code, but this one seemed sufficiently

> obvious: check INTEGRAL_TYPE_P before looking at a tree's min/max value.

> Otherwise, go with maxobjsize.

> 

> Bootstrapped/regtested on x86_64-linux, ok for trunk?

> 

> 2017-12-18  Marek Polacek  <polacek@redhat.com>

> 

> 	PR middle-end/83463

> 	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref):

> 	Check if TYPE is INTEGRAL_TYPE_P before accessing its min/max

> 	values.

> 

> 	* gcc.dg/pr83463.c: New test.

OK.
jeff
Jakub Jelinek Dec. 18, 2017, 4:44 p.m. | #2
On Mon, Dec 18, 2017 at 09:36:46AM -0700, Jeff Law wrote:
> On 12/18/2017 08:10 AM, Marek Polacek wrote:

> > I'm not entirely up to speed with this code, but this one seemed sufficiently

> > obvious: check INTEGRAL_TYPE_P before looking at a tree's min/max value.

> > Otherwise, go with maxobjsize.

> > 

> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> > 

> > 2017-12-18  Marek Polacek  <polacek@redhat.com>

> > 

> > 	PR middle-end/83463

> > 	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref):

> > 	Check if TYPE is INTEGRAL_TYPE_P before accessing its min/max

> > 	values.

> > 

> > 	* gcc.dg/pr83463.c: New test.

> OK.


What about INTEGRAL_TYPE_Ps that have NULL TYPE_MIN_VALUE and/or
TYPE_MAX_VALUE?  Doesn't Ada create those?

	Jakub
Martin Sebor Dec. 18, 2017, 5 p.m. | #3
On 12/18/2017 08:10 AM, Marek Polacek wrote:
> I'm not entirely up to speed with this code, but this one seemed sufficiently

> obvious: check INTEGRAL_TYPE_P before looking at a tree's min/max value.

> Otherwise, go with maxobjsize.

>

> Bootstrapped/regtested on x86_64-linux, ok for trunk?


Thanks for the fix.  I noticed the problem only comes up when
memcpy is declared without a prototype.  Otherwise, the memcpy
call is eliminated early on (during gimplification?)  So while
avoiding the ICE is obviously a necessary change, I wonder if
this bug also suggests a missing optimization that might still
be relevant (i.e., eliminating memcpy() with a zero size for
a memcpy without a prototype):

   void* memcpy ();

   void p (void *d, const void *s)
   {
     memcpy (d, s, 0);
   }

GCC doesn't warn for this code which means it views the memcpy
as a built-in but it doesn't eliminate the call.  If I declare
memcpy for example like this, it complains about the mismatch
but it does eliminates the call:

   void* memcpy (long, long, long);

Martin

>

> 2017-12-18  Marek Polacek  <polacek@redhat.com>

>

> 	PR middle-end/83463

> 	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref):

> 	Check if TYPE is INTEGRAL_TYPE_P before accessing its min/max

> 	values.

>

> 	* gcc.dg/pr83463.c: New test.

>

> diff --git gcc/gimple-ssa-warn-restrict.c gcc/gimple-ssa-warn-restrict.c

> index 4d424735d2a..d1a376637a2 100644

> --- gcc/gimple-ssa-warn-restrict.c

> +++ gcc/gimple-ssa-warn-restrict.c

> @@ -287,13 +287,15 @@ builtin_memref::builtin_memref (tree expr, tree size)

>  		  else

>  		    {

>  		      gimple *stmt = SSA_NAME_DEF_STMT (offset);

> +		      tree type;

>  		      if (is_gimple_assign (stmt)

> -			  && gimple_assign_rhs_code (stmt) == NOP_EXPR)

> +			  && gimple_assign_rhs_code (stmt) == NOP_EXPR

> +			  && (type = TREE_TYPE (gimple_assign_rhs1 (stmt)))

> +			  && INTEGRAL_TYPE_P (type))

>  			{

>  			  /* Use the bounds of the type of the NOP_EXPR operand

>  			     even if it's signed.  The result doesn't trigger

>  			     warnings but makes their output more readable.  */

> -			  tree type = TREE_TYPE (gimple_assign_rhs1 (stmt));

>  			  offrange[0] = wi::to_offset (TYPE_MIN_VALUE (type));

>  			  offrange[1] = wi::to_offset (TYPE_MAX_VALUE (type));

>  			}

> diff --git gcc/testsuite/gcc.dg/pr83463.c gcc/testsuite/gcc.dg/pr83463.c

> index e69de29bb2d..735ef3c6dc8 100644

> --- gcc/testsuite/gcc.dg/pr83463.c

> +++ gcc/testsuite/gcc.dg/pr83463.c

> @@ -0,0 +1,17 @@

> +/* PR middle-end/83463 */

> +/* { dg-do compile } */

> +/* { dg-options "-O2 -Wrestrict" } */

> +

> +int *a;

> +void *memcpy ();

> +void

> +m (void *p1)

> +{

> +  memcpy (0, p1, 0);

> +}

> +

> +void

> +p ()

> +{

> +  m (p + (long) a);

> +}

>

> 	Marek

>
Jakub Jelinek Dec. 18, 2017, 5:07 p.m. | #4
On Mon, Dec 18, 2017 at 10:00:36AM -0700, Martin Sebor wrote:
> On 12/18/2017 08:10 AM, Marek Polacek wrote:

> > I'm not entirely up to speed with this code, but this one seemed sufficiently

> > obvious: check INTEGRAL_TYPE_P before looking at a tree's min/max value.

> > Otherwise, go with maxobjsize.

> > 

> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> 

> Thanks for the fix.  I noticed the problem only comes up when

> memcpy is declared without a prototype.  Otherwise, the memcpy

> call is eliminated early on (during gimplification?)  So while

> avoiding the ICE is obviously a necessary change, I wonder if

> this bug also suggests a missing optimization that might still

> be relevant (i.e., eliminating memcpy() with a zero size for

> a memcpy without a prototype):


No, we shouldn't try to optimize if people mess up stuff.
What happens in this case is that
gimple_builtin_call_types_compatible_p
will fail, thus gimple_call_builtin_p and we won't be treating it specially.

That just shows a problem in the new pass:
  tree func = gimple_call_fndecl (call);
  if (!func || DECL_BUILT_IN_CLASS (func) != BUILT_IN_NORMAL)
    return;
is just wrong, it should have been:
  if (!gimple_call_builtin_p (call, BUILT_IN_NORMAL))
    return;

Then you don't need to worry about float or _Complex int third argument,
or just 2 or 27 arguments instead of 3 for memcpy, etc.

	Jakub
Martin Sebor Dec. 18, 2017, 5:37 p.m. | #5
On 12/18/2017 10:07 AM, Jakub Jelinek wrote:
> On Mon, Dec 18, 2017 at 10:00:36AM -0700, Martin Sebor wrote:

>> On 12/18/2017 08:10 AM, Marek Polacek wrote:

>>> I'm not entirely up to speed with this code, but this one seemed sufficiently

>>> obvious: check INTEGRAL_TYPE_P before looking at a tree's min/max value.

>>> Otherwise, go with maxobjsize.

>>>

>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?

>>

>> Thanks for the fix.  I noticed the problem only comes up when

>> memcpy is declared without a prototype.  Otherwise, the memcpy

>> call is eliminated early on (during gimplification?)  So while

>> avoiding the ICE is obviously a necessary change, I wonder if

>> this bug also suggests a missing optimization that might still

>> be relevant (i.e., eliminating memcpy() with a zero size for

>> a memcpy without a prototype):

>

> No, we shouldn't try to optimize if people mess up stuff.


Declaring memcpy() without a prototype isn't messing up.  It may
be poor practice but there's nothing inherently wrong with it.
GCC accepts it, treats it as a built-in (i.e., doesn't complain
about the declaration being incompatible with the built-in), but
doesn't optimize it.  On the other hand, as I said, when someone
really does mess up and declares memcpy like so:

   void* memcpy (long, long, long);

GCC optimizes calls to it as if the function were declared with
the right prototype.

> What happens in this case is that

> gimple_builtin_call_types_compatible_p

> will fail, thus gimple_call_builtin_p and we won't be treating it specially.

> That just shows a problem in the new pass:

>   tree func = gimple_call_fndecl (call);

>   if (!func || DECL_BUILT_IN_CLASS (func) != BUILT_IN_NORMAL)

>     return;

> is just wrong, it should have been:

>   if (!gimple_call_builtin_p (call, BUILT_IN_NORMAL))

>     return;

>

> Then you don't need to worry about float or _Complex int third argument,

> or just 2 or 27 arguments instead of 3 for memcpy, etc.


Yes, but only at the cost of not checking calls to memcpy() and
other built ins declared without a prototype.  I didn't consider
those when I wrote the code but now that I have, since GCC allows
built-ins to be declared without a prototype and doesn't warn even
with -Wpedantic, it doesn't seem like a good approach to me.

Martin
Jakub Jelinek Dec. 18, 2017, 5:45 p.m. | #6
On Mon, Dec 18, 2017 at 10:37:19AM -0700, Martin Sebor wrote:
> On 12/18/2017 10:07 AM, Jakub Jelinek wrote:

> > On Mon, Dec 18, 2017 at 10:00:36AM -0700, Martin Sebor wrote:

> > > On 12/18/2017 08:10 AM, Marek Polacek wrote:

> > > > I'm not entirely up to speed with this code, but this one seemed sufficiently

> > > > obvious: check INTEGRAL_TYPE_P before looking at a tree's min/max value.

> > > > Otherwise, go with maxobjsize.

> > > > 

> > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> > > 

> > > Thanks for the fix.  I noticed the problem only comes up when

> > > memcpy is declared without a prototype.  Otherwise, the memcpy

> > > call is eliminated early on (during gimplification?)  So while

> > > avoiding the ICE is obviously a necessary change, I wonder if

> > > this bug also suggests a missing optimization that might still

> > > be relevant (i.e., eliminating memcpy() with a zero size for

> > > a memcpy without a prototype):

> > 

> > No, we shouldn't try to optimize if people mess up stuff.

> 

> Declaring memcpy() without a prototype isn't messing up.  It may

> be poor practice but there's nothing inherently wrong with it.


There is.  Because the last argument is size_t, so calling it with 0
is UB (on LP64, on ILP32 I think it will be optimized normally).

	Jakub
Martin Sebor Dec. 18, 2017, 6:08 p.m. | #7
On 12/18/2017 10:45 AM, Jakub Jelinek wrote:
> On Mon, Dec 18, 2017 at 10:37:19AM -0700, Martin Sebor wrote:

>> On 12/18/2017 10:07 AM, Jakub Jelinek wrote:

>>> On Mon, Dec 18, 2017 at 10:00:36AM -0700, Martin Sebor wrote:

>>>> On 12/18/2017 08:10 AM, Marek Polacek wrote:

>>>>> I'm not entirely up to speed with this code, but this one seemed sufficiently

>>>>> obvious: check INTEGRAL_TYPE_P before looking at a tree's min/max value.

>>>>> Otherwise, go with maxobjsize.

>>>>>

>>>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?

>>>>

>>>> Thanks for the fix.  I noticed the problem only comes up when

>>>> memcpy is declared without a prototype.  Otherwise, the memcpy

>>>> call is eliminated early on (during gimplification?)  So while

>>>> avoiding the ICE is obviously a necessary change, I wonder if

>>>> this bug also suggests a missing optimization that might still

>>>> be relevant (i.e., eliminating memcpy() with a zero size for

>>>> a memcpy without a prototype):

>>>

>>> No, we shouldn't try to optimize if people mess up stuff.

>>

>> Declaring memcpy() without a prototype isn't messing up.  It may

>> be poor practice but there's nothing inherently wrong with it.

>

> There is.  Because the last argument is size_t, so calling it with 0

> is UB (on LP64, on ILP32 I think it will be optimized normally).


It isn't optimized either way.  In fact, the only indication
of a problem in the code below is the new -Wrestrict warning:

   void* memcpy ();

   void p ()
   {
     memcpy (0, 0, 0);
   }

   warning: ‘memcpy’ source argument is the same as destination [-Wrestrict]
      memcpy (0, 0, 0);
      ^~~~~~~~~~~~~~~~

   ;; Function p (p, funcdef_no=0, decl_uid=1892, cgraph_uid=0, 
symbol_order=0)

   p ()
   {
     <bb 2> [local count: 1073741825]:
     memcpy (0, 0, 0); [tail call]
     return;
   }

But you are conflating the ability to call such a built-in with
arguments of the wrong types with declaring it.  There really is
nothing wrong with declaring it as long as GCC checks the calls.

To ameliorate the undefined behavior you point out, GCC could do
one of three things: a) complain about declaring a built-in with
no prototype, or b) complain about calls to such built-ins that
pass arguments of the wrong type (or the wrong number of
arguments), or c) convert the arguments to the expected type
and still warn on mismatches).

In all cases all the information necessary to detect and diagnose
or even avoid the problem is available.  In fact, one might argue
that optimizing such calls (expanding them inline) would be
preferable to doing nothing and allowing the undefined behavior
to cause a bug at runtime.

Martin
Jakub Jelinek Dec. 18, 2017, 7:04 p.m. | #8
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.

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.

	Jakub
Martin Sebor Dec. 18, 2017, 11:04 p.m. | #9
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
the essential purpose of the pass (although this particular
bug wasn't necessarily its original focus).  Using an API that
would result in compromising this goal without offering other
benefits would be self-defeating.

Martin
Jakub Jelinek Dec. 18, 2017, 11:41 p.m. | #10
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.

The user can use -Wstrict-prototypes and -Wbuiltin-declaration-mismatch already,
and if needed, we can resurrect the "Add new warning -Wunprototyped-calls" thread
for the rest.  That is actually that can help (the remaining few users that
actually still use unprototyped calls).

	Jakub
Martin Sebor Dec. 19, 2017, 12:03 a.m. | #11
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
Jakub Jelinek Dec. 19, 2017, 12:27 a.m. | #12
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.

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.

	Jakub
Martin Sebor Dec. 19, 2017, 4:38 a.m. | #13
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

Patch

diff --git gcc/gimple-ssa-warn-restrict.c gcc/gimple-ssa-warn-restrict.c
index 4d424735d2a..d1a376637a2 100644
--- gcc/gimple-ssa-warn-restrict.c
+++ gcc/gimple-ssa-warn-restrict.c
@@ -287,13 +287,15 @@  builtin_memref::builtin_memref (tree expr, tree size)
 		  else
 		    {
 		      gimple *stmt = SSA_NAME_DEF_STMT (offset);
+		      tree type;
 		      if (is_gimple_assign (stmt)
-			  && gimple_assign_rhs_code (stmt) == NOP_EXPR)
+			  && gimple_assign_rhs_code (stmt) == NOP_EXPR
+			  && (type = TREE_TYPE (gimple_assign_rhs1 (stmt)))
+			  && INTEGRAL_TYPE_P (type))
 			{
 			  /* Use the bounds of the type of the NOP_EXPR operand
 			     even if it's signed.  The result doesn't trigger
 			     warnings but makes their output more readable.  */
-			  tree type = TREE_TYPE (gimple_assign_rhs1 (stmt));
 			  offrange[0] = wi::to_offset (TYPE_MIN_VALUE (type));
 			  offrange[1] = wi::to_offset (TYPE_MAX_VALUE (type));
 			}
diff --git gcc/testsuite/gcc.dg/pr83463.c gcc/testsuite/gcc.dg/pr83463.c
index e69de29bb2d..735ef3c6dc8 100644
--- gcc/testsuite/gcc.dg/pr83463.c
+++ gcc/testsuite/gcc.dg/pr83463.c
@@ -0,0 +1,17 @@ 
+/* PR middle-end/83463 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wrestrict" } */
+
+int *a;
+void *memcpy ();
+void
+m (void *p1)
+{
+  memcpy (0, p1, 0);
+}
+
+void
+p ()
+{
+  m (p + (long) a);
+}