Message ID | 20220708065255.2316410-1-caiyinyu@loongson.cn |
---|---|
Headers | show |
Series | GLIBC LoongArch PATCHES | expand |
On Fri, 2022-07-08 at 14:52 +0800, caiyinyu wrote:
> binutils: https://github.com/loongson/binutils-gdb/tree/dev/new_reloc
There are too much changes in this branch. Could you isolate a minimal
binutils patch enough for eliminating R_LARCH_NONE and R_LARCH_IRELATIVE
from .rela.plt, get it reviewed, and push it into GNU binutils repo? I
don't think it's a good idea to make *GNU* libc depend on some external
linker code which is not reviewed by GNU binutils developers.
And we need to do this ASAP: we already missed binutils 2.39 release
branch cut date [1] and if we don't move fast we'll miss glibc-2.36
release date.
[1]: https://sourceware.org/pipermail/binutils/2022-July/121567.html
Resend to fix Adhemerval's email address :(. On Fri, 2022-07-08 at 15:13 +0800, Xi Ruoyao via Libc-alpha wrote: > On Fri, 2022-07-08 at 14:52 +0800, caiyinyu wrote: > > > binutils: > > https://github.com/loongson/binutils-gdb/tree/dev/new_reloc > > There are too much changes in this branch. Could you isolate a > minimal > binutils patch enough for eliminating R_LARCH_NONE and > R_LARCH_IRELATIVE > from .rela.plt, get it reviewed, and push it into GNU binutils repo? > I > don't think it's a good idea to make *GNU* libc depend on some > external > linker code which is not reviewed by GNU binutils developers. > > And we need to do this ASAP: we already missed binutils 2.39 release > branch cut date [1] and if we don't move fast we'll miss glibc-2.36 > release date. > > [1]: https://sourceware.org/pipermail/binutils/2022-July/121567.html > >
On 08/07/22 04:13, Xi Ruoyao via Libc-alpha wrote: > On Fri, 2022-07-08 at 14:52 +0800, caiyinyu wrote: > >> binutils: https://github.com/loongson/binutils-gdb/tree/dev/new_reloc > > There are too much changes in this branch. Could you isolate a minimal > binutils patch enough for eliminating R_LARCH_NONE and R_LARCH_IRELATIVE > from .rela.plt, get it reviewed, and push it into GNU binutils repo? I > don't think it's a good idea to make *GNU* libc depend on some external > linker code which is not reviewed by GNU binutils developers. > > And we need to do this ASAP: we already missed binutils 2.39 release > branch cut date [1] and if we don't move fast we'll miss glibc-2.36 > release date. > > [1]: https://sourceware.org/pipermail/binutils/2022-July/121567.html > > I finished my review for the port and it looks ok in general, however there some pieces that would require a v7: 1. Add R_LARCH_NONE handling on bootstrap, binutils 2.38 does generated it and from previous discussion it should be considered a missed optimizations instead of a linker error. 2. Remove the __loongarch_soft_float parts, since there is no support for soft floating-point. 3. Remove sysdeps/unix/sysv/linux/loongarch/ldconfig.h, this file is not required. 4. Some minor style issues. 5. Either remove HAVE_GETTIMEOFDAY_VSYSCALL or add a gettimeofday ifunc optimization. The only part really prevent port inclusion is 1. I am also assuming ifunc is support (at least you have added support on the Linux ABI part) with binutils 2.38. If not, you will need to remove support until you fix it on binutils. And also, you need to check and report the test results using the expected defined release branches, using out-of-tree branches are not acceptable for inclusion.
On Wed, 2022-07-13 at 16:55 -0300, Adhemerval Zanella Netto wrote: > I finished my review for the port and it looks ok in general, however there > some pieces that would require a v7: > > 1. Add R_LARCH_NONE handling on bootstrap, binutils 2.38 does generated it > and from previous discussion it should be considered a missed > optimizations instead of a linker error. > > 2. Remove the __loongarch_soft_float parts, since there is no support for > soft floating-point. > > 3. Remove sysdeps/unix/sysv/linux/loongarch/ldconfig.h, this file is not > required. > > 4. Some minor style issues. > > 5. Either remove HAVE_GETTIMEOFDAY_VSYSCALL or add a gettimeofday ifunc > optimization. > > The only part really prevent port inclusion is 1. I am also assuming ifunc > is support (at least you have added support on the Linux ABI part) with > binutils 2.38. If not, you will need to remove support until you fix it on > binutils. > > And also, you need to check and report the test results using the expected > defined release branches, using out-of-tree branches are not acceptable for > inclusion. The series tested with Linux-5.19-rc4, Binutils-2.38, and GCC-12.1. Some kernel patches used but they are only boot protocols and hardware drivers, not related to userspace ABI so Glibc should be unaffected. Some Binutils and GCC patches used but they are all already committed to upstream. To work around binutils issues, I added R_LARCH_NONE back to RTLD_BOOTSTRAP, and disabled IFUNC (by adding libc_cv_ld_gnu_indirect_function=no into preconfigure). Result: XPASS: conform/UNIX98/ndbm.h/linknamespace XPASS: conform/XOPEN2K/ndbm.h/linknamespace XPASS: conform/XOPEN2K8/ndbm.h/linknamespace XPASS: conform/XPG42/ndbm.h/linknamespace UNSUPPORTED: crypt/cert UNSUPPORTED: elf/tst-env-setuid UNSUPPORTED: elf/tst-env-setuid-tunables XPASS: elf/tst-protected1a XPASS: elf/tst-protected1b UNSUPPORTED: elf/tst-valgrind-smoke UNSUPPORTED: misc/tst-adjtimex UNSUPPORTED: misc/tst-clock_adjtime UNSUPPORTED: misc/tst-ntp_adjtime UNSUPPORTED: misc/tst-pkey UNSUPPORTED: misc/tst-rseq UNSUPPORTED: misc/tst-rseq-disable UNSUPPORTED: nptl/test-cond-printers UNSUPPORTED: nptl/test-condattr-printers UNSUPPORTED: nptl/test-mutex-printers UNSUPPORTED: nptl/test-mutexattr-printers UNSUPPORTED: nptl/test-rwlock-printers UNSUPPORTED: nptl/test-rwlockattr-printers UNSUPPORTED: nptl/tst-pthread-gdb-attach UNSUPPORTED: nptl/tst-pthread-gdb-attach-static UNSUPPORTED: nptl/tst-rseq-nptl UNSUPPORTED: stdlib/tst-secure-getenv UNSUPPORTED: time/tst-clock_settime UNSUPPORTED: time/tst-settimeofday Summary of test results: 4536 PASS 22 UNSUPPORTED 12 XFAIL 6 XPASS
On 14/07/22 08:33, Xi Ruoyao wrote: > On Wed, 2022-07-13 at 16:55 -0300, Adhemerval Zanella Netto wrote: > >> I finished my review for the port and it looks ok in general, however there >> some pieces that would require a v7: >> >> 1. Add R_LARCH_NONE handling on bootstrap, binutils 2.38 does generated it >> and from previous discussion it should be considered a missed >> optimizations instead of a linker error. >> >> 2. Remove the __loongarch_soft_float parts, since there is no support for >> soft floating-point. >> >> 3. Remove sysdeps/unix/sysv/linux/loongarch/ldconfig.h, this file is not >> required. >> >> 4. Some minor style issues. >> >> 5. Either remove HAVE_GETTIMEOFDAY_VSYSCALL or add a gettimeofday ifunc >> optimization. >> >> The only part really prevent port inclusion is 1. I am also assuming ifunc >> is support (at least you have added support on the Linux ABI part) with >> binutils 2.38. If not, you will need to remove support until you fix it on >> binutils. >> >> And also, you need to check and report the test results using the expected >> defined release branches, using out-of-tree branches are not acceptable for >> inclusion. > > The series tested with Linux-5.19-rc4, Binutils-2.38, and GCC-12.1. > Some kernel patches used but they are only boot protocols and hardware > drivers, not related to userspace ABI so Glibc should be unaffected. > Some Binutils and GCC patches used but they are all already committed to > upstream. Thanks. > > To work around binutils issues, I added R_LARCH_NONE back to > RTLD_BOOTSTRAP, and disabled IFUNC (by adding > libc_cv_ld_gnu_indirect_function=no into preconfigure). I do not think this is blocker, but if you need to explicit add libc_cv_ld_gnu_indirect_function=no it means your toolchain can potentially generate invalid binaries: $ cat test.c #include <stdio.h> static int impl (void) { return 42; } static void * resolver (void) { return impl; } int ifunc (void) __attribute__((ifunc ("resolver"))); int main (int argc, char *argv[]) { printf ("%d\n", (int) ifunc ()); return 0; } $ loongarch64-linux-gnu-hard/bin/loongarch64-glibc-linux-gnu-gcc test.c -o test $ qemu-loongarch64 elf/ld.so --library-path . ./test ./test: error while loading shared libraries: unexpected PLT reloc type 0x0c It would be better if you just disable ifunc support on static linker for now. > > Result: > > XPASS: conform/UNIX98/ndbm.h/linknamespace > XPASS: conform/XOPEN2K/ndbm.h/linknamespace > XPASS: conform/XOPEN2K8/ndbm.h/linknamespace > XPASS: conform/XPG42/ndbm.h/linknamespace > UNSUPPORTED: crypt/cert > UNSUPPORTED: elf/tst-env-setuid > UNSUPPORTED: elf/tst-env-setuid-tunables > XPASS: elf/tst-protected1a > XPASS: elf/tst-protected1b > UNSUPPORTED: elf/tst-valgrind-smoke > UNSUPPORTED: misc/tst-adjtimex > UNSUPPORTED: misc/tst-clock_adjtime > UNSUPPORTED: misc/tst-ntp_adjtime > UNSUPPORTED: misc/tst-pkey > UNSUPPORTED: misc/tst-rseq > UNSUPPORTED: misc/tst-rseq-disable > UNSUPPORTED: nptl/test-cond-printers > UNSUPPORTED: nptl/test-condattr-printers > UNSUPPORTED: nptl/test-mutex-printers > UNSUPPORTED: nptl/test-mutexattr-printers > UNSUPPORTED: nptl/test-rwlock-printers > UNSUPPORTED: nptl/test-rwlockattr-printers > UNSUPPORTED: nptl/tst-pthread-gdb-attach > UNSUPPORTED: nptl/tst-pthread-gdb-attach-static > UNSUPPORTED: nptl/tst-rseq-nptl > UNSUPPORTED: stdlib/tst-secure-getenv > UNSUPPORTED: time/tst-clock_settime > UNSUPPORTED: time/tst-settimeofday > Summary of test results: > 4536 PASS > 22 UNSUPPORTED > 12 XFAIL > 6 XPASS
On 7/8/22 02:52, caiyinyu wrote:
> Hello, these are LoongArch patches v6, and we really need your futher suggestions:
The v6 patches came up for review in the weekly Monday patch review meeting.
May you please post a v7 with all suggestions integrated?
Having a new v7 would help with review and inclusion into the release.
Thank you!