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 <mse...@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
