This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: i686/memmove.S always copies backwards when dst > src
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Maxim Kuvyrkov <maxim at kugelworks dot com>
- Cc: Yuriy Kaminskiy <yumkam at gmail dot com>, libc-alpha at sourceware dot org
- Date: Thu, 2 Jan 2014 21:17:11 +0100
- 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> <956CAFFD-2EA0-4AAB-BC41-94EA0D5531D4 at kugelworks dot com> <l98vji$ufi$1 at ger dot gmane dot org>
On Mon, Dec 23, 2013 at 01:26:20PM +0400, Yuriy Kaminskiy wrote:
> Maxim Kuvyrkov wrote:
> > On 23/12/2013, at 11:50 am, Yuriy Kaminskiy <yumkam@gmail.com> wrote:
snip
> >>
> >> 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 the 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.
>
> [including patch this time]
>
> --- 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
Looks good. As bugfix it needs a bugzilla entry and update
changelog/news. Maxim could you do that?