Bug 22353 - sysdeps/i386/i586/strcpy.S isn't maintainable
Summary: sysdeps/i386/i586/strcpy.S isn't maintainable
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: string (show other bugs)
Version: 2.27
: P2 normal
Target Milestone: 2.27
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-26 23:24 UTC by H.J. Lu
Modified: 2017-11-02 11:01 UTC (History)
1 user (show)

See Also:
Host:
Target: i586
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2017-10-26 23:24:28 UTC
sysdeps/i386/i586/strcpy.S has

       andl    $3, %ecx

#ifdef PIC
        call    2f  
        cfi_adjust_cfa_offset (4) 
2:      popl    %edx
        cfi_adjust_cfa_offset (-4)
        /* 0xb is the distance between 2: and 1: but we avoid writing
           1f-2b because the assembler generates worse code.  */
        leal    0xb(%edx,%ecx,8), %ecx
#else
        leal    1f(,%ecx,8), %ecx
#endif

        jmp     *%ecx

        .align 8
1:
        orb     (%esi), %al 
        jz      L(end)
        stosb
        xorl    %eax, %eax
        incl    %esi

        orb     (%esi), %al 
        jz      L(end)
        stosb
        xorl    %eax, %eax
        incl    %esi

        orb     (%esi), %al 
        jz      L(end)
        stosb
        xorl    %eax, %eax
        incl    %esi

L(1):   movl    (%esi), %ecx
        leal    4(%esi),%esi

Change a single instruction will break the jump table.
Comment 1 Sourceware Commits 2017-10-27 20:21:36 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, hjl/pr22353/master has been created
        at  5ed1b97d3112ce0f5d3cf57e7e50d6b1481740be (commit)

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=5ed1b97d3112ce0f5d3cf57e7e50d6b1481740be

commit 5ed1b97d3112ce0f5d3cf57e7e50d6b1481740be
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Oct 27 06:31:57 2017 -0700

    Add strcpy-stosb.S

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=cc4474bf8263e6621dff29a12d8a5ddd39ec800e

commit cc4474bf8263e6621dff29a12d8a5ddd39ec800e
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Oct 27 03:09:12 2017 -0700

    Add i386 strcpy.S

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=b44640054b78c21bf3176dd0e82c4c8f5c2b758c

commit b44640054b78c21bf3176dd0e82c4c8f5c2b758c
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Oct 27 02:31:13 2017 -0700

    i586: Use a jump table in strcpy.S {BZ #22353]
    
    i586 strcpy.S used a clever trick with LEA to avoid jump table:
    
    /* ECX has the last 2 bits of the address of source - 1.  */
    	andl	$3, %ecx
    
            call    2f
    2:      popl    %edx
    	/* 0xb is the distance between 2: and 1:.  */
            leal    0xb(%edx,%ecx,8), %ecx
            jmp     *%ecx
    
            .align 8
    1:  /* ECX == 0 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 1 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 2 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 3 */
    L(1):   movl    (%esi), %ecx
            leal    4(%esi),%esi
    
    This may fail if there are instruction changes before L(1):.  This patch
    replaces it with a jump table which works with any instruction changes.
    
    Tested on i586 and i686 with and without --disable-multi-arch.
    
    	{BZ #22353]
    	* sysdeps/i386/i586/strcpy.S (JMPTBL): New.
    	(BRANCH_TO_JMPTBL_ENTRY): Likewise.
    	(STRCPY): Use it.
    	(1): Renamed to ...
    	(L(Src0)): This.
    	(L(Src1)): New.
    	(L(Src2)): Likewise.
    	(L(1)): Renamed to ...
    	(L(Src3)): This.
    	(L(SrcTable)): New.

-----------------------------------------------------------------------
Comment 2 Sourceware Commits 2017-10-29 16:59:08 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, hjl/pr22353/master has been created
        at  75588c0e4cd27ad868256d094b5f7646bc404ca8 (commit)

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=75588c0e4cd27ad868256d094b5f7646bc404ca8

commit 75588c0e4cd27ad868256d094b5f7646bc404ca8
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Oct 27 06:31:57 2017 -0700

    Add strcpy-stosb.S

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=2fa584ae979326fd9cb33e6df9c50976643cbd84

commit 2fa584ae979326fd9cb33e6df9c50976643cbd84
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Oct 27 03:09:12 2017 -0700

    Add i386 strcpy.S

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=d78abd26b8c8da5b9d67773343824606f9c96a6a

commit d78abd26b8c8da5b9d67773343824606f9c96a6a
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Oct 27 02:31:13 2017 -0700

    i586: Use a jump table in strcpy.S [BZ #22353]
    
    i586 strcpy.S used a clever trick with LEA to avoid jump table:
    
    /* ECX has the last 2 bits of the address of source - 1.  */
    	andl	$3, %ecx
    
            call    2f
    2:      popl    %edx
    	/* 0xb is the distance between 2: and 1:.  */
            leal    0xb(%edx,%ecx,8), %ecx
            jmp     *%ecx
    
            .align 8
    1:  /* ECX == 0 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 1 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 2 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 3 */
    L(1):   movl    (%esi), %ecx
            leal    4(%esi),%esi
    
    This may fail if there are instruction changes before L(1):.  This patch
    replaces it with a jump table which works with any instruction changes.
    
    Tested on i586 and i686 with and without --disable-multi-arch.
    
    	[BZ #22353]
    	* sysdeps/i386/i586/strcpy.S (JMPTBL): New.
    	(BRANCH_TO_JMPTBL_ENTRY): Likewise.
    	(STRCPY): Use it.
    	(1): Renamed to ...
    	(L(Src0)): This.
    	(L(Src1)): New.
    	(L(Src2)): Likewise.
    	(L(1)): Renamed to ...
    	(L(Src3)): This.
    	(L(SrcTable)): New.

-----------------------------------------------------------------------
Comment 3 Sourceware Commits 2017-10-30 12:45:30 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, hjl/pr22353/master has been created
        at  610a04b517cadd2a461515ffa8da77b42ccb596e (commit)

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=610a04b517cadd2a461515ffa8da77b42ccb596e

commit 610a04b517cadd2a461515ffa8da77b42ccb596e
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Oct 27 06:31:57 2017 -0700

    Add strcpy-stosb.S

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=9b733b73b096a7c2f685c427d89431444603ac9f

commit 9b733b73b096a7c2f685c427d89431444603ac9f
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Oct 27 03:09:12 2017 -0700

    Add i386 strcpy.S

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=9a89973274f901f4c3313e6e2b84d6b2108c7924

commit 9a89973274f901f4c3313e6e2b84d6b2108c7924
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Oct 27 02:31:13 2017 -0700

    i586: Use a jump table in strcpy.S [BZ #22353]
    
    i586 strcpy.S used a clever trick with LEA to avoid jump table:
    
    /* ECX has the last 2 bits of the address of source - 1.  */
    	andl	$3, %ecx
    
            call    2f
    2:      popl    %edx
    	/* 0xb is the distance between 2: and 1:.  */
            leal    0xb(%edx,%ecx,8), %ecx
            jmp     *%ecx
    
            .align 8
    1:  /* ECX == 0 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 1 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 2 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 3 */
    L(1):   movl    (%esi), %ecx
            leal    4(%esi),%esi
    
    This may fail if there are instruction length changes before L(1):.  This
    patch replaces it with a jump table which works with any instruction length
    changes.
    
    Tested on i586 and i686 with and without --disable-multi-arch.
    
    	[BZ #22353]
    	* sysdeps/i386/i586/strcpy.S (JMPTBL): New.
    	(BRANCH_TO_JMPTBL_ENTRY): Likewise.
    	(STRCPY): Use it.
    	(1): Renamed to ...
    	(L(Src0)): This.
    	(L(Src1)): New.
    	(L(Src2)): Likewise.
    	(L(1)): Renamed to ...
    	(L(Src3)): This.
    	(L(SrcTable)): New.

-----------------------------------------------------------------------
Comment 4 Sourceware Commits 2017-10-30 17:03:17 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  c5cc45148c89cc5c57d1946348dd242d4db5c5f5 (commit)
      from  ce12269fac8cb873df1a8785e4a6cde870855590 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c5cc45148c89cc5c57d1946348dd242d4db5c5f5

commit c5cc45148c89cc5c57d1946348dd242d4db5c5f5
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Oct 30 10:02:16 2017 -0700

    i586: Use conditional branches in strcpy.S [BZ #22353]
    
    i586 strcpy.S used a clever trick with LEA to implement jump table:
    
    /* ECX has the last 2 bits of the address of source - 1.  */
    	andl	$3, %ecx
    
            call    2f
    2:      popl    %edx
    	/* 0xb is the distance between 2: and 1:.  */
            leal    0xb(%edx,%ecx,8), %ecx
            jmp     *%ecx
    
            .align 8
    1:  /* ECX == 0 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 1 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 2 */
            orb     (%esi), %al
            jz      L(end)
            stosb
            xorl    %eax, %eax
            incl    %esi
        /* ECX == 3 */
    L(1):   movl    (%esi), %ecx
            leal    4(%esi),%esi
    
    This fails if there are instruction length changes before L(1):.  This
    patch replaces it with conditional branches:
    
    	cmpb	$2, %cl
    	je	L(Src2)
    	ja	L(Src3)
    	cmpb	$1, %cl
    	je	L(Src1)
    
    L(Src0):
    
    which have similar performance and work with any instruction lengths.
    
    Tested on i586 and i686 with and without --disable-multi-arch.
    
    	[BZ #22353]
    	* sysdeps/i386/i586/strcpy.S (STRCPY): Use conditional branches.
    	(1): Renamed to ...
    	(L(Src0)): This.
    	(L(Src1)): New.
    	(L(Src2)): Likewise.
    	(L(1)): Renamed to ...
    	(L(Src3)): This.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                  |   11 +++++++++++
 sysdeps/i386/i586/strcpy.S |   28 +++++++++++-----------------
 2 files changed, 22 insertions(+), 17 deletions(-)
Comment 5 H.J. Lu 2017-10-30 17:05:43 UTC
Fixed for 2.27.