This is the mail archive of the
mailing list for the glibc project.
Re: i686/memmove.S always copies backwards when dst > src
- From: Maxim Kuvyrkov <maxim at kugelworks dot com>
- To: Yuriy Kaminskiy <yumkam at gmail dot com>
- Cc: GLIBC Devel <libc-alpha at sourceware dot org>
- Date: Mon, 23 Dec 2013 13:25:20 +1300
- Subject: Re: i686/memmove.S always copies backwards when dst > src
- Authentication-results: sourceware.org; auth=none
- References: <l97qbr$m1$1 at ger dot gmane dot org>
On 23/12/2013, at 11:50 am, Yuriy Kaminskiy <email@example.com> wrote:
> === cut string/memmove.c ===
> 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 ...]
> [...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.
Good job on catching this problem and even providing solution!
Libc-alpha@ (CC'ed) is the right list to post patches to; libc-help@ is for user questions about glibc. Please post your patch to libc-alpha@, e.g., by reply-all to this email.