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).