This is the mail archive of the libc-help@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

i686/memmove.S always copies backwards when dst > src


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]