Re: [PING #2] [PATCH] enhance -Wrestrict to handle string built-ins (PR 78918)

[Jeff Law <law at redhat dot com>]

On 08/28/2017 06:27 PM, Martin Sebor wrote:
>> Correct.  I wound my way through this mess a while back.  Essentially
>> Red Hat had a customer with code that had overlapped memcpy arguments.
>> We had them use the memstomp interposition library to start tracking
>> these problems down.
>>
>> One of the things that popped up was structure/class copies which were
>> implemented via calls to memcpy.    In the case of self assignment, the
>> interposition library would note the overlap and (rightly IMHO) complain.
> 
> Is this bug 32667?  I'm not having any luck reproducing it with
> any of the test cases there and varying struct sizes, or with
> the test case in the duplicate bug 65029 I filed for the same
> thing last year.  It would be nice to have a test case.
It certainly seems to be in the same class as 32667.  I'm not sure if
it's exactly the same root cause in terms of the front end interactions,
but the net result is the same.

I just reviewed the discussion in my outbox -- I didn't see sample code
or a reference to 32667, but I'm only looking at one side of the
conversation.


> 
>> One could argue that GCC should emit memmove by default for structure
>> assignments, only using memcpy when it knows its not doing self
>> assignment (which may be hard to determine).  Furthermore, GCC should
>> eliminate self structure/class assignment.
> 
> If it's still a problem emitting memmove seems like the right
> thing to do.  From what I've read the performance advantage of
> memcpy over memmove seems debatable at best.  Most performance
> sensitive code avoids making copies of very large objects so
> the only code that can be impacted doesn't care about efficiency
> quite so much.  For small enough objects, inlining the copy as
> GCC already does would obviate the efficiency concern altogether.
I personally feel we reached the point of diminishing returns years ago
--  in attempts to inline/unroll the copies in GCC, in terms of glibc's
copier implementations and in the gain of using memcpy over memmove vs
the pain of folks that can't be bothered to do it right WRT overlapped
copies.

But that's a bit of a digression.  THe issue at handle is structure
assignments that GCC is turning into memcpys.  That's a  much smaller
subset of calls to memcpy and more likely to not be a performance issue
to change.


>> A self-copy should just be folded away.  It's no different than x = x on
>> scalars except that we've dropped it to a memcpy in the IL.  Doing so
>> makes the code more efficient and removes false positives from tools
>> like the memstomp interposition library, making those tools more useful.
> 
> It's possible to do in simple cases but not in general.  I agree
> that in the general case when overlap is possible the only safe
> solution, short of actually testing for it at runtime, is to call
> memmove.
So let's zap those where we know its a self copy.  I'd look favorably on
a patch that use memmove on the others, but I won't immediately approve
it without wider buy-in as I expect it could well be controversial.
These should be separate patches as the former should go forward
immediately.

Jeff