Re: [PATCH] avoid warning for memcpy to self (PR 83456)

[Jeff Law <law at redhat dot com>]

On 03/08/2018 05:46 PM, Martin Sebor wrote:
> On 03/08/2018 01:59 AM, Richard Biener wrote:
>> On Thu, Mar 8, 2018 at 12:01 AM, Martin Sebor <msebor@gmail.com> wrote:
>>> I have become convinced that issuing -Wrestrict in gimple-fold
>>> for calls to memcpy() where the source pointer is the same as
>>> the destination causes more trouble than it's worth, especially
>>> when inlining is involved, as in:
>>>
>>>   inline void bar (void *d, void *s, unsigned N)
>>>   {
>>>     if (s != d)
>>>       memcpy (d, s, N);
>>>   }
>>>
>>>   void foo (void* src)
>>>   {
>>>     bar (src, src, 1);
>>>   }
>>>
>>> It seems that there should be a way to teach GCC to avoid
>>> folding statements in dead blocks (e.g., in a block controlled
>>> by 'if (0 != 0)' as the one below), and that it might even speed
>>> up compilation, but in the meantime it leads to false positive
>>> -Wrestrict warnings.
>>>
>>> The attached patch removes this instance of the warning and
>>> adjusts tests not to expect it.
>>
>> Ok.
> 
> I'm sorry, I just discovered that simply removing the code from
> gimple-fold would cause a regression with respect to GCC 7.0
> which does diagnose memcpy(p, p, N) calls, and the diagnostic
> is useful to users (pr60256 and pr65452 request one for all
> string functions, although for reasons other than to detect
> overlapping accesses).
> 
> To maintain the warning and at the same time eliminate the pesky
> false positives from gimple-fold I've partially restored the
> detection in the front-end.  That in turn exposed some missing
> checking of the no-warning bit, leading to duplicate warnings.
> I've fixed all that up in the attached slightly bigger patch.
> 
> Martin
> 
> PS Even with the change to the front end above, removing
> the memcpy warning from gimple-fold has the downside of
> failing to detect bugs like:
> 
>   struct S { char a[32]; } s;
>   memcpy (&s, s.a, sizeof s.a);
> 
> because the front end doesn't know that &s and s.a are at
> the same address (copying between distinct members of the
> same union causes another false negative).  I want to
> revisit this in stage 1 and see about handling it.  If
> you have suggestions/preference for how or where I'd
> appreciate your input.
I suspect the too-early folding is going to get in the way here.  In
some ways what you want is to either warn early or have a way to recover
the original arguments.  I'm not aware of a good way to do either for
this scenario though.


> 
> gcc-83456.diff
> 
> 
> PR tree-optimization/83456 - -Wrestrict false positive on a non-overlapping memcpy in an inline function
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/83456
> 	* gimple-fold.c (gimple_fold_builtin_memory_op): Avoid warning
> 	for perfectly overlapping calls to memcpy.
> 	(gimple_fold_builtin_memory_chk): Same.
> 	(gimple_fold_builtin_strcpy): Handle no-warning.
> 	(gimple_fold_builtin_stxcpy_chk): Same.
> 	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Handle no-warning.
> 
> gcc/c-family/ChangeLog:
> 
> 	PR tree-optimization/83456
> 	* gcc/c-family/c-common.c (check_function_restrict): Return bool.
> 	Restore checking of bounded built-in functions.
> 	(check_function_arguments): Also return the result
> 	of warn_for_restrict.
> 	* gcc/c-family/c-common.c (check_function_restrict): Return bool.
> 	* gcc/c-family/c-warn.c (warn_for_restrict): Return bool.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/83456
> 	* c-c++-common/Wrestrict-2.c: Remove test cases.
> 	* c-c++-common/Wrestrict.c: Same.
> 	* gcc.dg/Wrestrict-12.c: New test.
> 	* gcc.dg/Wrestrict-14.c: New test.
The self-copy via memcpy is particularly annoying.   In an ideal world
we'd never generate them, but we often do and it's been a source of many
complaints.

Ideally we'd never generate them.  Second best option is to fold them
away -- at least those generated by the compiler itself.

Anyway, this is OK for the trunk.  Feel free to open BZs for issues you
want to tackle during gcc-9.

Thanks,
jeff