i686/memmove.S always copies backwards when dst > src
Ondřej Bílka
neleai@seznam.cz
Thu Jan 2 20:17:00 GMT 2014
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?
More information about the Libc-alpha
mailing list