With glibc 2.28 (and maybe earlier versions) on amd64 memcheck/tests/overlap fails. --- overlap.stderr.exp 2018-01-23 21:34:02.318995262 +0100 +++ overlap.stderr.out 2018-12-22 21:43:11.181679045 +0100 @@ -1,11 +1,3 @@ -Source and destination overlap in memcpy(0x........, 0x........, 21) - at 0x........: memcpy (vg_replace_strmem.c:...) - by 0x........: main (overlap.c:40) - -Source and destination overlap in memcpy(0x........, 0x........, 21) - at 0x........: memcpy (vg_replace_strmem.c:...) - by 0x........: main (overlap.c:42) - Source and destination overlap in strncpy(0x........, 0x........, 21) at 0x........: strncpy (vg_replace_strmem.c:...) by 0x........: main (overlap.c:45) Looking at the REDIRS it might be caused by redirecting a memcpy variant to memmove? --38108-- REDIR: 0x48f0210 (libc.so.6:memmove) redirected to 0x482b1be (_vgnU_ifunc_wrapper) --38108-- REDIR: 0x48f0680 (libc.so.6:memcpy@@GLIBC_2.14) redirected to 0x482b1be (_vgnU_ifunc_wrapper) --38108-- REDIR: 0x49c3b50 (libc.so.6:__memcpy_avx_unaligned_erms) redirected to 0x483ca60 (memmove)
It also started to fail for me some months ago. Debian GLIBC 2.24-11+deb9u3 gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) See https://sourceforge.net/p/valgrind/mailman/message/36034448/ At that time, IIRC, I vaguely contemplated this could be fixed by changing the REDIR mechanism by making it 'better ifunc aware, and do the redirection at an earlier stage.
Is there any progress here? How important will it be to fix this for 3.15.0?
(In reply to Julian Seward from comment #2) > Is there any progress here? How important will it be to fix this for 3.15.0? I believe this will be a non neglectible change in the REDIR mechanism, as the REDIR will have to be done at at ifunc level. As far as I can see; not fixing this means some false negative for memcpy, not detecting overlap args. So, not that critical IMO
I get a false positive: a call to memmove (which should cope with overlapping memory areas) is erroneously checked as if it were memcpy: ==3546== Source and destination overlap in memcpy_chk(0x1ffeffc460, 0x1ffeffc462, 9) ==3546== at 0x4C383D0: __memcpy_chk (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==3546== by 0x6DC9481: memmove (string_fortified.h:40) Comparing this with previous comments in this ticket, it seems like the redirects for memmove and memcpy are simply swapped.
I don't understand, I've looked at the code and at #275284 We have this redir MEMMOVE(VG_Z_LIBC_SONAME, memcpyZAGLIBCZu2Zd2Zd5) /* memcpy@GLIBC_2.2.5 */ It long predates glibc 2.28. The overlap testcase hasn't changed for donkeys years. Here's the 1st missing call 401283: ba 15 00 00 00 mov $0x15,%edx 401288: 48 89 ce mov %rcx,%rsi 40128b: 48 89 c7 mov %rax,%rdi 40128e: e8 ed fd ff ff call 401080 <memcpy@plt> (Fedora 37, an old Xeon CPU)
For now I added the following "workaround" to Fedora because we keep getting bug reports of memmove calls reporting src/dst overlaps: diff --git a/shared/vg_replace_strmem.c b/shared/vg_replace_strmem.c index b32f13f76..aa7f88ca2 100644 --- a/shared/vg_replace_strmem.c +++ b/shared/vg_replace_strmem.c @@ -1128,7 +1128,7 @@ static inline void my_exit ( int x ) MEMMOVE_OR_MEMCPY(20181, soname, fnname, 0) #define MEMCPY(soname, fnname) \ - MEMMOVE_OR_MEMCPY(20180, soname, fnname, 1) + MEMMOVE_OR_MEMCPY(20180, soname, fnname, 0) /* See KDE bug #402833 */ #if defined(VGO_linux) /* For older memcpy we have to use memmove-like semantics and skip We really should not do ifunc resolving for these two functions, just intercept them directly
*** Bug 453084 has been marked as a duplicate of this bug. ***
The "workaround from comment #6 isn't complete since memmove_chk and memcpy_chk can also alias possibly causing memmove_chk doing an overlap check (because it gets confused with memcpy_chk). So the current patch as used for Fedora is: diff --git a/shared/vg_replace_strmem.c b/shared/vg_replace_strmem.c index b32f13f76..464e8d4ca 100644 --- a/shared/vg_replace_strmem.c +++ b/shared/vg_replace_strmem.c @@ -1128,7 +1128,7 @@ static inline void my_exit ( int x ) MEMMOVE_OR_MEMCPY(20181, soname, fnname, 0) #define MEMCPY(soname, fnname) \ - MEMMOVE_OR_MEMCPY(20180, soname, fnname, 1) + MEMMOVE_OR_MEMCPY(20180, soname, fnname, 0) /* See KDE bug #402833 */ #if defined(VGO_linux) /* For older memcpy we have to use memmove-like semantics and skip @@ -1714,8 +1714,6 @@ static inline void my_exit ( int x ) RECORD_COPY(len); \ if (len == 0) \ return dst; \ - if (is_overlap(dst, src, len, len)) \ - RECORD_OVERLAP_ERROR("memcpy_chk", dst, src, len); \ if ( dst > src ) { \ d = (HChar *)dst + len - 1; \ s = (const HChar *)src + len - 1; \
Created attachment 162737 [details] disable overlap check and test for linux-amd64 More subtle workaround patch that only disables the overlap check (and test) on linux-amd64
(In reply to Mark Wielaard from comment #9) > Created attachment 162737 [details] > disable overlap check and test for linux-amd64 > > More subtle workaround patch that only disables the overlap check (and test) > on linux-amd64 This workaround is now committed: commit 53e101f562fa89bbf92d658fba626e2397862a16 Author: Mark Wielaard <mark@klomp.org> Date: Mon Oct 30 23:30:06 2023 +0100 Disable memcpy overlap check and test on amd64 linux Almost all newer distros have ifunc based memcpy/memmove glibc implementation which cause false positives. Disable the overlap check and test on these systems for now. https://bugs.kde.org/show_bug.cgi?id=402833 But it is just a workaround to get rid of the false positives. Real solution still needed. So keeping bug open.