[Bug libc/16394] New: i686/memmove.S always copies backwards when dst > src

["maxim.kuvyrkov at gmail dot com" <sourceware-bugzilla at sourceware dot org>]

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.