[PATCH 0/3] i586/i686: Fix multiple definitions in libc.a

[PATCH 0/3] i586/i686: Fix multiple definitions in libc.a

[Gabi Falk]

This patch fixes two issues related to multiple definitions of functions
in the static library built as PIC and adds a test to prevent such
issues from occurring in the future.

Gabi Falk (3):
  i586: Fix multiple definitions of __memcpy_chk and __mempcpy_chk
  i686: Fix multiple definitions of __memmove_chk and __memset_chk
  Add a test to check for duplicate definitions in the static library

 Makefile                    | 7 +++++++
 sysdeps/i386/i586/memcpy.S  | 2 +-
 sysdeps/i386/i686/memmove.S | 2 +-
 sysdeps/i386/i686/memset.S  | 2 +-
 4 files changed, 10 insertions(+), 3 deletions(-)

--
gabi

[PATCH 1/3] i586: Fix multiple definitions of __memcpy_chk and __mempcpy_chk

[Gabi Falk]

/home/bmg/install/compilers/x86_64-linux-gnu/lib/gcc/x86_64-glibc-linux-gnu/13.2.1/../../../../x86_64-glibc-linux-gnu/bin/ld: /home/bmg/build/glibcs/i586-linux-gnu/glibc/libc.a(memcpy_chk.o): in function `__memcpy_chk':
/home/bmg/src/glibc/debug/../sysdeps/i386/memcpy_chk.S:29: multiple definition of `__memcpy_chk';/home/bmg/build/glibcs/i586-linux-gnu/glibc/libc.a(memcpy.o):/home/bmg/src/glibc/string/../sysdeps/i386/i586/memcpy.S:31: first defined here /home/bmg/install/compilers/x86_64-linux-gnu/lib/gcc/x86_64-glibc-linux-gnu/13.2.1/../../../../x86_64-glibc-linux-gnu/bin/ld: /home/bmg/build/glibcs/i586-linux-gnu/glibc/libc.a(mempcpy_chk.o): in function `__mempcpy_chk': /home/bmg/src/glibc/debug/../sysdeps/i386/mempcpy_chk.S:28: multiple definition of `__mempcpy_chk'; /home/bmg/build/glibcs/i586-linux-gnu/glibc/libc.a(mempcpy.o):/home/bmg/src/glibc/string/../sysdeps/i386/i586/memcpy.S:31: first defined here

After this change, the static library built for i586, regardless of PIC
options, contains implementations of these functions respectively from
sysdeps/i386/memcpy_chk.S and sysdeps/i386/mempcpy_chk.S.  This ensures
that memcpy and mempcpy won't pull in __chk_fail and the routines it
calls.

Reported-by: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Gabi Falk <gabifalk@gmx.com>
---
 sysdeps/i386/i586/memcpy.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/i386/i586/memcpy.S b/sysdeps/i386/i586/memcpy.S
index 3e26f112d6..79856d498a 100644
--- a/sysdeps/i386/i586/memcpy.S
+++ b/sysdeps/i386/i586/memcpy.S
@@ -26,7 +26,7 @@
 #define LEN	SRC+4

         .text
-#if defined PIC && IS_IN (libc)
+#if defined SHARED && IS_IN (libc)
 ENTRY (__memcpy_chk)
 	movl	12(%esp), %eax
 	cmpl	%eax, 16(%esp)
--
gabi

[PATCH 2/3] i686: Fix multiple definitions of __memmove_chk and __memset_chk

[Gabi Falk]

Commit c73c96a4a1af1326df7f96eec58209e1e04066d8 updated memcpy.S and
mempcpy.S, but omitted memmove.S and memset.S.  As a result, the static
library built as PIC, whether with or without multiarch support,
contains two definitions for each of the __memmove_chk and __memset_chk
symbols.

/usr/lib/gcc/i686-pc-linux-gnu/14/../../../../i686-pc-linux-gnu/bin/ld: /usr/lib/gcc/i686-pc-linux-gnu/14/../../../../lib/libc.a(memset-ia32.o): in function `__memset_chk':
/var/tmp/portage/sys-libs/glibc-2.39-r3/work/glibc-2.39/string/../sysdeps/i386/i686/memset.S:32: multiple definition of `__memset_chk'; /usr/lib/gcc/i686-pc-linux-gnu/14/../../../../lib/libc.a(memset_chk.o):/var/tmp/portage/sys-libs/glibc-2.39-r3/work/glibc-2.39/debug/../sysdeps/i386/i686/multiarch/memset_chk.c:24: first defined here

After this change, regardless of PIC options, the static library, built
for i686 with multiarch contains implementations of these functions
respectively from debug/memmove_chk.c and debug/memset_chk.c, and
without multiarch contains implementations of these functions
respectively from sysdeps/i386/memmove_chk.S and
sysdeps/i386/memset_chk.S.  This ensures that memmove and memset won't
pull in __chk_fail and the routines it calls.

Reported-by: Sam James <sam@gentoo.org>
Fixes: c73c96a4a1 ("i686: Fix build with --disable-multiarch")
Signed-off-by: Gabi Falk <gabifalk@gmx.com>
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
---
 sysdeps/i386/i686/memmove.S | 2 +-
 sysdeps/i386/i686/memset.S  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sysdeps/i386/i686/memmove.S b/sysdeps/i386/i686/memmove.S
index f230359ad6..effd958120 100644
--- a/sysdeps/i386/i686/memmove.S
+++ b/sysdeps/i386/i686/memmove.S
@@ -29,7 +29,7 @@
 #define SRC	DEST+4
 #define LEN	SRC+4

-#if defined PIC && IS_IN (libc)
+#if defined SHARED && IS_IN (libc)
 ENTRY_CHK (__memmove_chk)
 	movl	12(%esp), %eax
 	cmpl	%eax, 16(%esp)
diff --git a/sysdeps/i386/i686/memset.S b/sysdeps/i386/i686/memset.S
index f02f5a6df7..ab06771ea0 100644
--- a/sysdeps/i386/i686/memset.S
+++ b/sysdeps/i386/i686/memset.S
@@ -27,7 +27,7 @@
 #define LEN	CHR+4

         .text
-#if defined PIC && IS_IN (libc)
+#if defined SHARED && IS_IN (libc)
 ENTRY_CHK (__memset_chk)
 	movl	12(%esp), %eax
 	cmpl	%eax, 16(%esp)
--
gabi

[PATCH 3/3] Add a test to check for duplicate definitions in the static library

[Gabi Falk]

This change follows two previous fixes addressing multiple definitions
of __memcpy_chk and __mempcpy_chk functions on i586, and __memmove_chk
and __memset_chk functions on i686.  The test is intended to prevent
such issues from occurring in the future.

Signed-off-by: Gabi Falk <gabifalk@gmx.com>
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
---
 Makefile | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Makefile b/Makefile
index b5ef98fb04..adf4749124 100644
--- a/Makefile
+++ b/Makefile
@@ -577,6 +577,13 @@ $(objpfx)lint-makefiles.out: scripts/lint-makefiles.sh
 	$(SHELL) $< "$(PYTHON)" `pwd` > $@ ; \
 	$(evaluate-test)

+# Link libc.a as a whole to verify that it does not contain multiple
+# definitions of any symbols.
+tests-special += $(objpfx)link-static-libc.out
+$(objpfx)link-static-libc.out:
+	$(LINK.o) $(whole-archive) -r $(objpfx)libc.a -o /dev/null > $@ 2>&1; \
+	$(evaluate-test)
+
 # Print test summary for tests in $1 .sum file;
 # $2 is optional test identifier.
 # Fail if there are unexpected failures in the test results.
--
gabi

Re: [PATCH 1/3] i586: Fix multiple definitions of __memcpy_chk and __mempcpy_chk

[H.J. Lu]

On Tue, Apr 30, 2024 at 1:05 PM Gabi Falk <gabifalk@gmx.com> wrote:
>
> /home/bmg/install/compilers/x86_64-linux-gnu/lib/gcc/x86_64-glibc-linux-gnu/13.2.1/../../../../x86_64-glibc-linux-gnu/bin/ld: /home/bmg/build/glibcs/i586-linux-gnu/glibc/libc.a(memcpy_chk.o): in function `__memcpy_chk':
> /home/bmg/src/glibc/debug/../sysdeps/i386/memcpy_chk.S:29: multiple definition of `__memcpy_chk';/home/bmg/build/glibcs/i586-linux-gnu/glibc/libc.a(memcpy.o):/home/bmg/src/glibc/string/../sysdeps/i386/i586/memcpy.S:31: first defined here /home/bmg/install/compilers/x86_64-linux-gnu/lib/gcc/x86_64-glibc-linux-gnu/13.2.1/../../../../x86_64-glibc-linux-gnu/bin/ld: /home/bmg/build/glibcs/i586-linux-gnu/glibc/libc.a(mempcpy_chk.o): in function `__mempcpy_chk': /home/bmg/src/glibc/debug/../sysdeps/i386/mempcpy_chk.S:28: multiple definition of `__mempcpy_chk'; /home/bmg/build/glibcs/i586-linux-gnu/glibc/libc.a(mempcpy.o):/home/bmg/src/glibc/string/../sysdeps/i386/i586/memcpy.S:31: first defined here
>
> After this change, the static library built for i586, regardless of PIC
> options, contains implementations of these functions respectively from
> sysdeps/i386/memcpy_chk.S and sysdeps/i386/mempcpy_chk.S.  This ensures
> that memcpy and mempcpy won't pull in __chk_fail and the routines it
> calls.
>
> Reported-by: Florian Weimer <fweimer@redhat.com>
> Signed-off-by: Gabi Falk <gabifalk@gmx.com>
> ---
>  sysdeps/i386/i586/memcpy.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sysdeps/i386/i586/memcpy.S b/sysdeps/i386/i586/memcpy.S
> index 3e26f112d6..79856d498a 100644
> --- a/sysdeps/i386/i586/memcpy.S
> +++ b/sysdeps/i386/i586/memcpy.S
> @@ -26,7 +26,7 @@
>  #define LEN    SRC+4
>
>          .text
> -#if defined PIC && IS_IN (libc)
> +#if defined SHARED && IS_IN (libc)
>  ENTRY (__memcpy_chk)
>         movl    12(%esp), %eax
>         cmpl    %eax, 16(%esp)
> --
> gabi
>

LGTM.

Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

Thanks.

-- 
H.J.

Re: [PATCH 2/3] i686: Fix multiple definitions of __memmove_chk and __memset_chk

[H.J. Lu]

On Tue, Apr 30, 2024 at 1:05 PM Gabi Falk <gabifalk@gmx.com> wrote:
>
> Commit c73c96a4a1af1326df7f96eec58209e1e04066d8 updated memcpy.S and
> mempcpy.S, but omitted memmove.S and memset.S.  As a result, the static
> library built as PIC, whether with or without multiarch support,
> contains two definitions for each of the __memmove_chk and __memset_chk
> symbols.
>
> /usr/lib/gcc/i686-pc-linux-gnu/14/../../../../i686-pc-linux-gnu/bin/ld: /usr/lib/gcc/i686-pc-linux-gnu/14/../../../../lib/libc.a(memset-ia32.o): in function `__memset_chk':
> /var/tmp/portage/sys-libs/glibc-2.39-r3/work/glibc-2.39/string/../sysdeps/i386/i686/memset.S:32: multiple definition of `__memset_chk'; /usr/lib/gcc/i686-pc-linux-gnu/14/../../../../lib/libc.a(memset_chk.o):/var/tmp/portage/sys-libs/glibc-2.39-r3/work/glibc-2.39/debug/../sysdeps/i386/i686/multiarch/memset_chk.c:24: first defined here
>
> After this change, regardless of PIC options, the static library, built
> for i686 with multiarch contains implementations of these functions
> respectively from debug/memmove_chk.c and debug/memset_chk.c, and
> without multiarch contains implementations of these functions
> respectively from sysdeps/i386/memmove_chk.S and
> sysdeps/i386/memset_chk.S.  This ensures that memmove and memset won't
> pull in __chk_fail and the routines it calls.
>
> Reported-by: Sam James <sam@gentoo.org>
> Fixes: c73c96a4a1 ("i686: Fix build with --disable-multiarch")
> Signed-off-by: Gabi Falk <gabifalk@gmx.com>
> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> ---
>  sysdeps/i386/i686/memmove.S | 2 +-
>  sysdeps/i386/i686/memset.S  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sysdeps/i386/i686/memmove.S b/sysdeps/i386/i686/memmove.S
> index f230359ad6..effd958120 100644
> --- a/sysdeps/i386/i686/memmove.S
> +++ b/sysdeps/i386/i686/memmove.S
> @@ -29,7 +29,7 @@
>  #define SRC    DEST+4
>  #define LEN    SRC+4
>
> -#if defined PIC && IS_IN (libc)
> +#if defined SHARED && IS_IN (libc)
>  ENTRY_CHK (__memmove_chk)
>         movl    12(%esp), %eax
>         cmpl    %eax, 16(%esp)
> diff --git a/sysdeps/i386/i686/memset.S b/sysdeps/i386/i686/memset.S
> index f02f5a6df7..ab06771ea0 100644
> --- a/sysdeps/i386/i686/memset.S
> +++ b/sysdeps/i386/i686/memset.S
> @@ -27,7 +27,7 @@
>  #define LEN    CHR+4
>
>          .text
> -#if defined PIC && IS_IN (libc)
> +#if defined SHARED && IS_IN (libc)
>  ENTRY_CHK (__memset_chk)
>         movl    12(%esp), %eax
>         cmpl    %eax, 16(%esp)
> --
> gabi
>

LGTM.

Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

Thanks.

-- 
H.J.

Re: [PATCH 3/3] Add a test to check for duplicate definitions in the static library

[H.J. Lu]

On Tue, Apr 30, 2024 at 1:05 PM Gabi Falk <gabifalk@gmx.com> wrote:
>
> This change follows two previous fixes addressing multiple definitions
> of __memcpy_chk and __mempcpy_chk functions on i586, and __memmove_chk
> and __memset_chk functions on i686.  The test is intended to prevent
> such issues from occurring in the future.
>
> Signed-off-by: Gabi Falk <gabifalk@gmx.com>
> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> ---
>  Makefile | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index b5ef98fb04..adf4749124 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -577,6 +577,13 @@ $(objpfx)lint-makefiles.out: scripts/lint-makefiles.sh
>         $(SHELL) $< "$(PYTHON)" `pwd` > $@ ; \
>         $(evaluate-test)
>
> +# Link libc.a as a whole to verify that it does not contain multiple
> +# definitions of any symbols.
> +tests-special += $(objpfx)link-static-libc.out
> +$(objpfx)link-static-libc.out:
> +       $(LINK.o) $(whole-archive) -r $(objpfx)libc.a -o /dev/null > $@ 2>&1; \
> +       $(evaluate-test)
> +
>  # Print test summary for tests in $1 .sum file;
>  # $2 is optional test identifier.
>  # Fail if there are unexpected failures in the test results.
> --
> gabi
>

LGTM.

Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

Thanks.

-- 
H.J.

Re: [PATCH 0/3] i586/i686: Fix multiple definitions in libc.a

[Dmitry V. Levin]

On Tue, Apr 30, 2024 at 08:05:01PM +0000, Gabi Falk wrote:
> This patch fixes two issues related to multiple definitions of functions
> in the static library built as PIC and adds a test to prevent such
> issues from occurring in the future.
> 
> Gabi Falk (3):
>   i586: Fix multiple definitions of __memcpy_chk and __mempcpy_chk
>   i686: Fix multiple definitions of __memmove_chk and __memset_chk
>   Add a test to check for duplicate definitions in the static library
> 
>  Makefile                    | 7 +++++++
>  sysdeps/i386/i586/memcpy.S  | 2 +-
>  sysdeps/i386/i686/memmove.S | 2 +-
>  sysdeps/i386/i686/memset.S  | 2 +-
>  4 files changed, 10 insertions(+), 3 deletions(-)

For all the series:
Reviewed-by: Dmitry V. Levin <ldv@altlinux.org>


-- 
ldv

Re: [PATCH 0/3] i586/i686: Fix multiple definitions in libc.a

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 732 bytes --]

Gabi Falk <gabifalk@gmx.com> writes:

> This patch fixes two issues related to multiple definitions of functions
> in the static library built as PIC and adds a test to prevent such
> issues from occurring in the future.
>
> Gabi Falk (3):
>   i586: Fix multiple definitions of __memcpy_chk and __mempcpy_chk
>   i686: Fix multiple definitions of __memmove_chk and __memset_chk
>   Add a test to check for duplicate definitions in the static library

Thanks! Pushed with Tested-by for the i686 commit specifically too.

>
>  Makefile                    | 7 +++++++
>  sysdeps/i386/i586/memcpy.S  | 2 +-
>  sysdeps/i386/i686/memmove.S | 2 +-
>  sysdeps/i386/i686/memset.S  | 2 +-
>  4 files changed, 10 insertions(+), 3 deletions(-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]