There is nothing in this bug except saying there is wrong code happening (not even with what options or with anything else).
Really sorry about that, I managed to accidentally hit the Enter key halfway through writing the title. Here is the actual bug description: -- Wine provides freestanding libraries, including manual definitions of memcpy() and memmove() [1]. Those are defined in C, and while our definitions are *technically* non-compliant C (violating the requirement that the pointers must point to the same object), they should be fine for our targets, and anyway, the case I'm running into is failure to handle overlap where the pointers *do* in fact point into the same object. I can't find fault with the definitions themselves, although I may be missing something. We also, contrary to standards, give memcpy() the semantics of memmove(), because some Windows programs are buggy and make that assumption. We do this by copy-pasting the definition (I'm not sure why we do this rather than just calling one function from the other, but it is what it is). I recently started compiling with -march=native, and found that gcc was failing to correctly handle overlap in memmove. Further investigation revealed that, somehow, memmove() was being incorrectly optimized to *not* check for overlap, while memcpy() remained in its unoptimized form. I ran into this originally with the i686-w64-mingw32 target, but I've adjusted the target to i686-linux-gnu since it happens there too. It does *not* happen on x86_64. [1] https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/ntdll/string.c#l98
Created attachment 56072 [details] testcase Attaching a reduced-ish testcase, that contains the unmodified code of memcpy() and memmove(), plus two callers. The callers seem to be necessary to trigger the incorrect optimization. Compile with '-c -O2 -march=bdver2 -m32'.
There is no bug here. ICF finds that your definition of memcpy is the same as memmove and merges the 2 and then calls memcpy from your memmove and then inlines the normal memcpy because well it says it is the same. You can just use -fno-builtin to fix the issue by saying memcpy and memmove are not builtins and treat them like normal functions. That fixes the issue by not inlining the target defined memcpy.
(In reply to Andrew Pinski from comment #4) > There is no bug here. > ICF finds that your definition of memcpy is the same as memmove and merges > the 2 and then calls memcpy from your memmove and then inlines the normal > memcpy because well it says it is the same. I suppose I understand this explanation, but it does not feel like a very intuitive behaviour. The ICF part makes sense. The choice to optimize a builtin memcpy/memmove call into a different instruction sequence (which doesn't match the original) also makes sense. I would not really expect these two to be combined in this manner, though. memmove() is not calling builtin memcpy(), it is calling our implementation of memcpy(), which doesn't have the same semantics as builtin memcpy(). [It also seems odd to me that func2() would be replaced with a builtin memcpy() rather than a builtin memmove()?] > You can just use -fno-builtin to fix the issue by saying memcpy and memmove > are not builtins and treat them like normal functions. > > That fixes the issue by not inlining the target defined memcpy. Fair enough, I guess. I suppose that's the right thing to do anyway...