| Summary: | Memcheck needs to replace memcpy/memmove (and most certainly others) in VG_Z_LIBC_SONAME on Darwin | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Alexander Potapenko <glider> |
| Component: | memcheck | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | gparker, jseward, rhyskidd, timurrrr, tom |
| Priority: | NOR | ||
| Version: | 3.7 SVN | ||
| Target Milestone: | --- | ||
| Platform: | Unlisted Binaries | ||
| OS: | macOS | ||
| Latest Commit: | Version Fixed In: | ||
This code triggers a very subtle optimization in memcpy, which sometimes reads 64-byte blocks of memory if the source pointer is properly aligned: 0xffff0d65: movdqa 0x3(%esi,%edx,1),%xmm1 0xffff0d6b: movdqa 0x13(%esi,%edx,1),%xmm2 0xffff0d71: movdqa 0x23(%esi,%edx,1),%xmm3 0xffff0d77: movdqa 0x33(%esi,%edx,1),%xmm4 I haven't found any kernel sources containing this, but have spent some time debugging the case. If necessary, I can give more details. The full listing of the kernel memcpy implementation can be obtained from gdb by disassembling at 0xffff07a0 This happens because the line replacing memcpy on Darwin is commented out. The following patch fixes the problem for me: $ svn diff memcheck/mc_replace_strmem.c Index: memcheck/mc_replace_strmem.c =================================================================== --- memcheck/mc_replace_strmem.c (revision 12257) +++ memcheck/mc_replace_strmem.c (working copy) @@ -849,7 +849,7 @@ MEMCPY(NONE, ZuintelZufastZumemcpy) #elif defined(VGO_darwin) - //MEMCPY(VG_Z_LIBC_SONAME, memcpy) + MEMCPY(VG_Z_LIBC_SONAME, memcpy) //MEMCPY(VG_Z_DYLD, memcpy) MEMCPY(VG_Z_LIBC_SONAME, memcpyZDVARIANTZDsse3x) /* memcpy$VARIANT$sse3x */ MEMCPY(VG_Z_LIBC_SONAME, memcpyZDVARIANTZDsse42) /* memcpy$VARIANT$sse42 */ $ inst/bin/valgrind ./p --40937-- ./p: --40937-- dSYM directory is missing; consider using --dsymutil=yes &m: 0x100356000 m: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ==40937== The problem affects at least Mac OS 10.5 and 10.6 We have first encountered this problem running Chromium tests on Mac OS 10.5. Valgrind reported a number of invalid reads in __sfvwrite, which were related to incorrect handling of memcpy(), and some similar invalid reads in __CFStringCreateImmutableFunnel3, which were related to memmove(). This makes me think that the bunch of other string functions from VG_Z_LIBC_SONAME should be uncommented in memcheck/mc_replace_strmem.c instead of cleaning them up (see the comment to r12043) Because on Lion memcpy falls back to memmove this patch is invalid and should be applied iff DARWIN_VERS < DARWIN_10_7. See http://code.google.com/p/valgrind-variant/issues/detail?id=5 for additional info. I can't reproduce this at all, on my old low end Mac Mini. The code you mention .. 0xffff0d65: movdqa 0x3(%esi,%edx,1),%xmm1 0xffff0d6b: movdqa 0x13(%esi,%edx,1),%xmm2 0xffff0d71: movdqa 0x23(%esi,%edx,1),%xmm3 0xffff0d77: movdqa 0x33(%esi,%edx,1),%xmm4 is very similar to code in _memcpy$VARIANT$sse42 and _memcpy$VARIANT$sse3x. But memcheck intercepts both of those. In the stack trace you show, it's not clear what function the offending access is in: ==38912== Invalid read of size 8 ==38912== at 0x7FFFFFE00D78: ??? I guess the really bad scenario is that that is a page of code created by the kernel; it has no name (and hence is uninterceptable) and it is dynamically dispatched to from plain "memcpy". But I'm just guessing here. I committed a patch that reinstates the memcpy and memmove intercepts
for <= 10.6, and that seems to stop the problem. Thanks for the
test case btw.
> This makes me think that the bunch of other string functions from
> VG_Z_LIBC_SONAME should be uncommented in
> memcheck/mc_replace_strmem.c instead of cleaning them up
Which particular string functions are you thinking of?
Fixed, r12423. Bug can probably be closed now? |
$ cat p.cc #include <stdio.h> #include <stdlib.h> #define SIZE 142 int main() { int i; char *m = (char*)valloc(SIZE); for (i = 0; i < SIZE; i++) m[i] = 'a'; m[SIZE-1] = 0; printf("&m: %p\n", m); printf("m: %s\n", m); return 0; } $ g++ p.cc -o p $ inst/bin/valgrind ./p ==38912== Memcheck, a memory error detector --38912-- ./p: --38912-- dSYM directory is missing; consider using --dsymutil=yes &m: 0x100356000 ==38912== Invalid read of size 8 ==38912== at 0x7FFFFFE00D78: ??? ==38912== by 0x10010DE74: __sfvwrite (in /usr/lib/libSystem.B.dylib) ==38912== by 0x10010D7DB: __vfprintf (in /usr/lib/libSystem.B.dylib) ==38912== by 0x10014B8AA: vfprintf_l (in /usr/lib/libSystem.B.dylib) ==38912== by 0x100179006: printf (in /usr/lib/libSystem.B.dylib) ==38912== by 0x100000EF5: main (in ./p) ==38912== Address 0x100356088 is 136 bytes inside a block of size 142 alloc'd ==38912== at 0x1000101F3: memalign (vg_replace_malloc.c:697) ==38912== by 0x100010A84: valloc (vg_replace_malloc.c:735) ==38912== by 0x100000E99: main (in ./p) ==38912== m: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ==38912==