public inbox for glibc-bugs@sourceware.org help / color / mirror / Atom feed
* [Bug libc/16394] New: i686/memmove.S always copies backwards when dst > src @ 2014-01-05 3:14 maxim.kuvyrkov at gmail dot com 2014-01-05 3:23 ` [Bug libc/16394] " maxim.kuvyrkov at gmail dot com ` (2 more replies) 0 siblings, 3 replies; 4+ messages in thread From: maxim.kuvyrkov at gmail dot com @ 2014-01-05 3:14 UTC (permalink / raw) To: glibc-bugs https://sourceware.org/bugzilla/show_bug.cgi?id=16394 Bug ID: 16394 Summary: i686/memmove.S always copies backwards when dst > src Product: glibc Version: unspecified Status: NEW Severity: enhancement Priority: P2 Component: libc Assignee: unassigned at sourceware dot org Reporter: maxim.kuvyrkov at gmail dot com CC: drepper.fsp at gmail dot com Submitted by Yuriy Kaminskiy on libc-alpha@: Compare: === cut string/memmove.c === rettype MEMMOVE (a1, a2, len) a1const void *a1; a2const void *a2; size_t len; { unsigned long int dstp = (long int) dest; unsigned long int srcp = (long int) src; /* This test makes the forward copying code be used whenever possible. Reduces the working set. */ if (dstp - srcp >= len) /* *Unsigned* compare! */ { /* Copy from the beginning to the end. */ === cut sysdeps/i386/i686/memmove.S === .... movl LEN(%esp), %ecx movl DEST(%esp), %edi ... movl SRC(%esp), %esi ... movl %edi, %eax subl %esi, %eax cmpl %eax, %edi jae 3f [...copy forward ...] ret 3: [...copy backward...] === cut === Obviously, the assembler code checks 'dstp - srcp >= dstp' (an awkward way to check for dstp > srcp) instead of 'dstp - srcp > len', as was in the C code; apparently this was /supposed/ to replicate same logic as in the C code, but registers names was mixed up, and as "it works", nobody noticed. Fortunately, it seems only result in choosing suboptimal backward copy in non-overlapping case when dst > src. Git blame says this mistaken check was already present when this code was first committed. Patch attached: --- glibc/sysdeps/i386/i686/memmove.S.orig +++ glibc/sysdeps/i386/i686/memmove.S @@ -63,8 +63,8 @@ movl %edi, %eax subl %esi, %eax - cmpl %eax, %edi - jae 3f + cmpl %eax, %ecx + ja 3f cld shrl $1, %ecx -- You are receiving this mail because: You are on the CC list for the bug. ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Bug libc/16394] i686/memmove.S always copies backwards when dst > src 2014-01-05 3:14 [Bug libc/16394] New: i686/memmove.S always copies backwards when dst > src maxim.kuvyrkov at gmail dot com @ 2014-01-05 3:23 ` maxim.kuvyrkov at gmail dot com 2014-01-08 20:55 ` maxim.kuvyrkov at gmail dot com 2014-06-13 9:13 ` fweimer at redhat dot com 2 siblings, 0 replies; 4+ messages in thread From: maxim.kuvyrkov at gmail dot com @ 2014-01-05 3:23 UTC (permalink / raw) To: glibc-bugs https://sourceware.org/bugzilla/show_bug.cgi?id=16394 Maxim Kuvyrkov <maxim.kuvyrkov at gmail dot com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |maxim.kuvyrkov at gmail dot com --- Comment #1 from Maxim Kuvyrkov <maxim.kuvyrkov at gmail dot com> --- Note that if (dstp - srcp >= len) /* *Unsigned* compare! */ is the right way to check for whether to use forward or backward copy. We still want to use forward copy when "dstp - srcp == len" as memory regions don't overlap for any LEN > 0. Your patch and the use of "ja" is correct though. -- You are receiving this mail because: You are on the CC list for the bug. ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Bug libc/16394] i686/memmove.S always copies backwards when dst > src 2014-01-05 3:14 [Bug libc/16394] New: i686/memmove.S always copies backwards when dst > src maxim.kuvyrkov at gmail dot com 2014-01-05 3:23 ` [Bug libc/16394] " maxim.kuvyrkov at gmail dot com @ 2014-01-08 20:55 ` maxim.kuvyrkov at gmail dot com 2014-06-13 9:13 ` fweimer at redhat dot com 2 siblings, 0 replies; 4+ messages in thread From: maxim.kuvyrkov at gmail dot com @ 2014-01-08 20:55 UTC (permalink / raw) To: glibc-bugs https://sourceware.org/bugzilla/show_bug.cgi?id=16394 Maxim Kuvyrkov <maxim.kuvyrkov at gmail dot com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RESOLVED Resolution|--- |FIXED --- Comment #2 from Maxim Kuvyrkov <maxim.kuvyrkov at gmail dot com> --- Fixed in 66671c84d58d6ae705bac39dc476c3d3b2b81116 -- You are receiving this mail because: You are on the CC list for the bug. ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Bug libc/16394] i686/memmove.S always copies backwards when dst > src 2014-01-05 3:14 [Bug libc/16394] New: i686/memmove.S always copies backwards when dst > src maxim.kuvyrkov at gmail dot com 2014-01-05 3:23 ` [Bug libc/16394] " maxim.kuvyrkov at gmail dot com 2014-01-08 20:55 ` maxim.kuvyrkov at gmail dot com @ 2014-06-13 9:13 ` fweimer at redhat dot com 2 siblings, 0 replies; 4+ messages in thread From: fweimer at redhat dot com @ 2014-06-13 9:13 UTC (permalink / raw) To: glibc-bugs https://sourceware.org/bugzilla/show_bug.cgi?id=16394 Florian Weimer <fweimer at redhat dot com> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |security- -- You are receiving this mail because: You are on the CC list for the bug. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-06-13 9:13 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-01-05 3:14 [Bug libc/16394] New: i686/memmove.S always copies backwards when dst > src maxim.kuvyrkov at gmail dot com 2014-01-05 3:23 ` [Bug libc/16394] " maxim.kuvyrkov at gmail dot com 2014-01-08 20:55 ` maxim.kuvyrkov at gmail dot com 2014-06-13 9:13 ` fweimer at redhat dot com
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).