* [PATCH] i686: Fix multiple definitions of __memmove_chk and __memset_chk
@ 2024-04-26 10:50 Gabi Falk
2024-04-26 14:17 ` H.J. Lu
0 siblings, 1 reply; 6+ messages in thread
From: Gabi Falk @ 2024-04-26 10:50 UTC (permalink / raw)
To: libc-alpha; +Cc: Gabi Falk, Sam James
Commit c73c96a4a1af1326df7f96eec58209e1e04066d8 updated memcpy.S and
mempcpy.S, but omitted memmove.S` and memset.S. As a result, the
static library built with enabled multiarch (the default) 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
Reported-by: Sam James <sam@gentoo.org>
Fixes: c73c96a4a1 ("i686: Fix build with --disable-multiarch")
Signed-off-by: Gabi Falk <gabifalk@gmx.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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] i686: Fix multiple definitions of __memmove_chk and __memset_chk
2024-04-26 10:50 [PATCH] i686: Fix multiple definitions of __memmove_chk and __memset_chk Gabi Falk
@ 2024-04-26 14:17 ` H.J. Lu
2024-04-27 16:42 ` [PATCH v2] " Gabi Falk
0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2024-04-26 14:17 UTC (permalink / raw)
To: Gabi Falk; +Cc: libc-alpha, Sam James
On Fri, Apr 26, 2024 at 3:52 AM 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 with enabled multiarch (the default) 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
Please include at least one testcase to verify the fix.
Thanks.
> Reported-by: Sam James <sam@gentoo.org>
> Fixes: c73c96a4a1 ("i686: Fix build with --disable-multiarch")
> Signed-off-by: Gabi Falk <gabifalk@gmx.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
>
--
H.J.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] i686: Fix multiple definitions of __memmove_chk and __memset_chk
2024-04-26 14:17 ` H.J. Lu
@ 2024-04-27 16:42 ` Gabi Falk
2024-04-28 14:18 ` H.J. Lu
2024-04-28 19:27 ` Florian Weimer
0 siblings, 2 replies; 6+ messages in thread
From: Gabi Falk @ 2024-04-27 16:42 UTC (permalink / raw)
To: H.J. Lu; +Cc: Gabi Falk, libc-alpha, Dmitry V. Levin, Sam James
Commit c73c96a4a1af1326df7f96eec58209e1e04066d8 updated memcpy.S and
mempcpy.S, but omitted memmove.S and memset.S. As a result, the
static library built 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
Add a test to check that the static libc does not contain multiple
definitions of any symbols.
Reported-by: Sam James <sam@gentoo.org>
Fixes: c73c96a4a1 ("i686: Fix build with --disable-multiarch")
Signed-off-by: Gabi Falk <gabifalk@gmx.com>
---
Makefile | 7 +++++++
sysdeps/i386/i686/memmove.S | 2 +-
sysdeps/i386/i686/memset.S | 2 +-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 7052b46df8..2e351c0321 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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] i686: Fix multiple definitions of __memmove_chk and __memset_chk
2024-04-27 16:42 ` [PATCH v2] " Gabi Falk
@ 2024-04-28 14:18 ` H.J. Lu
2024-04-28 19:27 ` Florian Weimer
1 sibling, 0 replies; 6+ messages in thread
From: H.J. Lu @ 2024-04-28 14:18 UTC (permalink / raw)
To: Gabi Falk; +Cc: libc-alpha, Dmitry V. Levin, Sam James
On Sat, Apr 27, 2024 at 9:42 AM 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 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
>
> Add a test to check that the static libc does not contain multiple
> definitions of any symbols.
>
> Reported-by: Sam James <sam@gentoo.org>
> Fixes: c73c96a4a1 ("i686: Fix build with --disable-multiarch")
> Signed-off-by: Gabi Falk <gabifalk@gmx.com>
> ---
> Makefile | 7 +++++++
> sysdeps/i386/i686/memmove.S | 2 +-
> sysdeps/i386/i686/memset.S | 2 +-
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 7052b46df8..2e351c0321 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.
> 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)
When PIE is enabled by default, libc.a is compiled with PIC.
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] i686: Fix multiple definitions of __memmove_chk and __memset_chk
2024-04-27 16:42 ` [PATCH v2] " Gabi Falk
2024-04-28 14:18 ` H.J. Lu
@ 2024-04-28 19:27 ` Florian Weimer
2024-04-29 23:29 ` Dmitry V. Levin
1 sibling, 1 reply; 6+ messages in thread
From: Florian Weimer @ 2024-04-28 19:27 UTC (permalink / raw)
To: Gabi Falk; +Cc: H.J. Lu, libc-alpha, Dmitry V. Levin, Sam James
* Gabi Falk:
> Commit c73c96a4a1af1326df7f96eec58209e1e04066d8 updated memcpy.S and
> mempcpy.S, but omitted memmove.S and memset.S. As a result, the
> static library built 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
> diff --git a/Makefile b/Makefile
> index 7052b46df8..2e351c0321 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)
> +
I do like the new test, but it still fails on i586-linux-gnu (as defined
by scripts/build-many-glibs.py):
/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
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] i686: Fix multiple definitions of __memmove_chk and __memset_chk
2024-04-28 19:27 ` Florian Weimer
@ 2024-04-29 23:29 ` Dmitry V. Levin
0 siblings, 0 replies; 6+ messages in thread
From: Dmitry V. Levin @ 2024-04-29 23:29 UTC (permalink / raw)
To: Florian Weimer; +Cc: Gabi Falk, H.J. Lu, libc-alpha, Sam James
On Sun, Apr 28, 2024 at 09:27:47PM +0200, Florian Weimer wrote:
> * Gabi Falk:
>
> > Commit c73c96a4a1af1326df7f96eec58209e1e04066d8 updated memcpy.S and
> > mempcpy.S, but omitted memmove.S and memset.S. As a result, the
> > static library built 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
>
> > diff --git a/Makefile b/Makefile
> > index 7052b46df8..2e351c0321 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)
> > +
>
> I do like the new test, but it still fails on i586-linux-gnu (as defined
> by scripts/build-many-glibs.py):
>
> /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
Apparently, sysdeps/i386/i586/memcpy.S needs the same treatment as
sysdeps/i386/i686/memmove.S and sysdeps/i386/i686/memset.S:
$ git grep -Fl '#if defined PIC && IS_IN (libc)'
sysdeps/i386/i586/memcpy.S
sysdeps/i386/i686/memmove.S
sysdeps/i386/i686/memset.S
--
ldv
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-29 23:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-26 10:50 [PATCH] i686: Fix multiple definitions of __memmove_chk and __memset_chk Gabi Falk
2024-04-26 14:17 ` H.J. Lu
2024-04-27 16:42 ` [PATCH v2] " Gabi Falk
2024-04-28 14:18 ` H.J. Lu
2024-04-28 19:27 ` Florian Weimer
2024-04-29 23:29 ` Dmitry V. Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).