* [PATCH] MIPS: Don't __gnu_lto_slim to .scommon @ 2023-07-03 10:36 YunQiang Su 2023-07-03 10:50 ` [PATCH v2] MIPS: Don't move " YunQiang Su 0 siblings, 1 reply; 11+ messages in thread From: YunQiang Su @ 2023-07-03 10:36 UTC (permalink / raw) To: binutils; +Cc: macro, YunQiang Su The LTO plugin doesn't expect __gnu_lto_slim is marked as COMMON, and in .scommon section. Let's skip __gnu_lto_slim when detect symbols that should be moved to .scommon. This patch can fix testcase: PR ld/15323 (3) PR ld/15323 (4) for MIPS. --- bfd/elfxx-mips.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c index 71f2dc9d779..63742de009c 100644 --- a/bfd/elfxx-mips.c +++ b/bfd/elfxx-mips.c @@ -7861,6 +7861,10 @@ _bfd_mips_elf_add_symbol_hook (bfd *abfd, struct bfd_link_info *info, switch (sym->st_shndx) { case SHN_COMMON: + /* __gnu_lto_slim shouldn't mark as COMMON and move to .scommon: + lto plugin doesn't expect so. */ + if (strcmp (*namep, "__gnu_lto_slim") == 0) + break; /* Common symbols less than the GP size are automatically treated as SHN_MIPS_SCOMMON symbols. */ if (sym->st_size > elf_gp_size (abfd) -- 2.30.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] MIPS: Don't move __gnu_lto_slim to .scommon 2023-07-03 10:36 [PATCH] MIPS: Don't __gnu_lto_slim to .scommon YunQiang Su @ 2023-07-03 10:50 ` YunQiang Su 2023-07-05 1:42 ` YunQiang Su 0 siblings, 1 reply; 11+ messages in thread From: YunQiang Su @ 2023-07-03 10:50 UTC (permalink / raw) To: binutils; +Cc: macro, YunQiang Su The LTO plugin doesn't expect __gnu_lto_slim is marked as COMMOM or moved .scommon section. Let's skip __gnu_lto_slim when detect symbols to be moved to .scommon. This patch can fix testcase: PR ld/15323 (3) PR ld/15323 (4) for MIPS. --- bfd/elfxx-mips.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c index 71f2dc9d779..63742de009c 100644 --- a/bfd/elfxx-mips.c +++ b/bfd/elfxx-mips.c @@ -7861,6 +7861,10 @@ _bfd_mips_elf_add_symbol_hook (bfd *abfd, struct bfd_link_info *info, switch (sym->st_shndx) { case SHN_COMMON: + /* __gnu_lto_slim shouldn't mark as COMMON and move to .scommon: + lto plugin doesn't expect so. */ + if (strcmp (*namep, "__gnu_lto_slim") == 0) + break; /* Common symbols less than the GP size are automatically treated as SHN_MIPS_SCOMMON symbols. */ if (sym->st_size > elf_gp_size (abfd) -- 2.30.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] MIPS: Don't move __gnu_lto_slim to .scommon 2023-07-03 10:50 ` [PATCH v2] MIPS: Don't move " YunQiang Su @ 2023-07-05 1:42 ` YunQiang Su 2023-07-13 5:39 ` YunQiang Su 2023-07-19 14:55 ` Maciej W. Rozycki 0 siblings, 2 replies; 11+ messages in thread From: YunQiang Su @ 2023-07-05 1:42 UTC (permalink / raw) To: YunQiang Su; +Cc: binutils, macro YunQiang Su <yunqiang.su@cipunited.com> 于2023年7月3日周一 18:51写道: > > The LTO plugin doesn't expect __gnu_lto_slim is marked as > COMMOM or moved .scommon section. > @Maciej W. Rozycki This is a tiny patch, and I think that it should be in 2.41 release. Any idea? > Let's skip __gnu_lto_slim when detect symbols to be moved to > .scommon. > > This patch can fix testcase: > PR ld/15323 (3) > PR ld/15323 (4) > for MIPS. > --- > bfd/elfxx-mips.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c > index 71f2dc9d779..63742de009c 100644 > --- a/bfd/elfxx-mips.c > +++ b/bfd/elfxx-mips.c > @@ -7861,6 +7861,10 @@ _bfd_mips_elf_add_symbol_hook (bfd *abfd, struct bfd_link_info *info, > switch (sym->st_shndx) > { > case SHN_COMMON: > + /* __gnu_lto_slim shouldn't mark as COMMON and move to .scommon: > + lto plugin doesn't expect so. */ > + if (strcmp (*namep, "__gnu_lto_slim") == 0) > + break; > /* Common symbols less than the GP size are automatically > treated as SHN_MIPS_SCOMMON symbols. */ > if (sym->st_size > elf_gp_size (abfd) > -- > 2.30.2 > -- YunQiang Su ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] MIPS: Don't move __gnu_lto_slim to .scommon 2023-07-05 1:42 ` YunQiang Su @ 2023-07-13 5:39 ` YunQiang Su 2023-07-19 14:55 ` Maciej W. Rozycki 1 sibling, 0 replies; 11+ messages in thread From: YunQiang Su @ 2023-07-13 5:39 UTC (permalink / raw) To: YunQiang Su, Nick Clifton; +Cc: binutils, macro YunQiang Su <wzssyqa@gmail.com> 于2023年7月5日周三 09:42写道: > > YunQiang Su <yunqiang.su@cipunited.com> 于2023年7月3日周一 18:51写道: > > > > The LTO plugin doesn't expect __gnu_lto_slim is marked as > > COMMOM or moved .scommon section. > > > > @Maciej W. Rozycki This is a tiny patch, and I think that it should be > in 2.41 release. > Any idea? > ping > > Let's skip __gnu_lto_slim when detect symbols to be moved to > > .scommon. > > > > This patch can fix testcase: > > PR ld/15323 (3) > > PR ld/15323 (4) > > for MIPS. > > --- > > bfd/elfxx-mips.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c > > index 71f2dc9d779..63742de009c 100644 > > --- a/bfd/elfxx-mips.c > > +++ b/bfd/elfxx-mips.c > > @@ -7861,6 +7861,10 @@ _bfd_mips_elf_add_symbol_hook (bfd *abfd, struct bfd_link_info *info, > > switch (sym->st_shndx) > > { > > case SHN_COMMON: > > + /* __gnu_lto_slim shouldn't mark as COMMON and move to .scommon: > > + lto plugin doesn't expect so. */ > > + if (strcmp (*namep, "__gnu_lto_slim") == 0) > > + break; > > /* Common symbols less than the GP size are automatically > > treated as SHN_MIPS_SCOMMON symbols. */ > > if (sym->st_size > elf_gp_size (abfd) > > -- > > 2.30.2 > > > > > -- > YunQiang Su -- YunQiang Su ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] MIPS: Don't move __gnu_lto_slim to .scommon 2023-07-05 1:42 ` YunQiang Su 2023-07-13 5:39 ` YunQiang Su @ 2023-07-19 14:55 ` Maciej W. Rozycki 2023-07-19 17:26 ` YunQiang Su 1 sibling, 1 reply; 11+ messages in thread From: Maciej W. Rozycki @ 2023-07-19 14:55 UTC (permalink / raw) To: YunQiang Su; +Cc: YunQiang Su, binutils On Wed, 5 Jul 2023, YunQiang Su wrote: > > The LTO plugin doesn't expect __gnu_lto_slim is marked as > > COMMOM or moved .scommon section. > > > > @Maciej W. Rozycki This is a tiny patch, and I think that it should be > in 2.41 release. > Any idea? What produces this symbol and why is it a common symbol in the first place if it's not supposed to? NB the mail server for your @gmail.com address rejects e-mail coming from my system. Please sort this out with your ISP. Maciej ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] MIPS: Don't move __gnu_lto_slim to .scommon 2023-07-19 14:55 ` Maciej W. Rozycki @ 2023-07-19 17:26 ` YunQiang Su 2023-07-20 9:05 ` Alan Modra 0 siblings, 1 reply; 11+ messages in thread From: YunQiang Su @ 2023-07-19 17:26 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: YunQiang Su, binutils Maciej W. Rozycki <macro@orcam.me.uk> 于2023年7月19日周三 22:55写道: > > On Wed, 5 Jul 2023, YunQiang Su wrote: > > > > The LTO plugin doesn't expect __gnu_lto_slim is marked as > > > COMMOM or moved .scommon section. > > > > > > > @Maciej W. Rozycki This is a tiny patch, and I think that it should be > > in 2.41 release. > > Any idea? > > What produces this symbol and why is it a common symbol in the first > place if it's not supposed to? > The symbol is produced by GCC LTO, and it has SHN_COMMON, while no SEC_IS_COMMON flag is set. When gcc -r -flto, this symbol is collected into .scommon and marked as SEC_IS_COMMON, which is rejected by linker. I have no idea why gcc lto uses SHN_COMMON, while it is used by other architectures. That's why I block `__gnu_lto_slim` out .scommon and SEC_IS_COMMON flag. See bfd/linker.c (_bfd_generic_link_add_one_symbol): else if (bfd_is_com_section (section)) { row = COMMON_ROW; if (!bfd_link_relocatable (info) && name != NULL && name[0] == '_' && name[1] == '_' && strcmp (name + (name[2] == '_'), "__gnu_lto_slim") == 0) _bfd_error_handler (_("%pB: plugin needed to handle lto object"), abfd); } > NB the mail server for your @gmail.com address rejects e-mail coming from > my system. Please sort this out with your ISP. > > Maciej -- YunQiang Su ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] MIPS: Don't move __gnu_lto_slim to .scommon 2023-07-19 17:26 ` YunQiang Su @ 2023-07-20 9:05 ` Alan Modra 2023-07-20 9:34 ` Maciej W. Rozycki 0 siblings, 1 reply; 11+ messages in thread From: Alan Modra @ 2023-07-20 9:05 UTC (permalink / raw) To: YunQiang Su; +Cc: Maciej W. Rozycki, YunQiang Su, binutils, Nick Clifton I think the patch is OK, but the same should be applied to another place to stop objcopy modifying __gnu_lto_slim. mips-linux-gnu testcase after running make check: binutils/objcopy ld/tmpdir/pr15323a-r.o ld/tmpdir/xxx then inspect ld/tmpdir/xxx with readelf. I'm going to apply the following to mainline, and will also apply to the 2.41 branch tomorrow if no one objects. * elfxx-mips.c (_bfd_mips_elf_symbol_processing): Don't treat __gnu_lto_slim as SHN_MIPS_SCOMMON. (_bfd_mips_elf_add_symbol_hook): Likewise. diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c index 71f2dc9d779..1e9851c3190 100644 --- a/bfd/elfxx-mips.c +++ b/bfd/elfxx-mips.c @@ -7176,10 +7176,11 @@ _bfd_mips_elf_symbol_processing (bfd *abfd, asymbol *asym) case SHN_COMMON: /* Common symbols less than the GP size are automatically - treated as SHN_MIPS_SCOMMON symbols on IRIX5. */ + treated as SHN_MIPS_SCOMMON symbols, with some exceptions. */ if (asym->value > elf_gp_size (abfd) || ELF_ST_TYPE (elfsym->internal_elf_sym.st_info) == STT_TLS - || IRIX_COMPAT (abfd) == ict_irix6) + || IRIX_COMPAT (abfd) == ict_irix6 + || strcmp (asym->name, "__gnu_lto_slim") == 0) break; /* Fall through. */ case SHN_MIPS_SCOMMON: @@ -7862,10 +7863,11 @@ _bfd_mips_elf_add_symbol_hook (bfd *abfd, struct bfd_link_info *info, { case SHN_COMMON: /* Common symbols less than the GP size are automatically - treated as SHN_MIPS_SCOMMON symbols. */ + treated as SHN_MIPS_SCOMMON symbols, with some exceptions. */ if (sym->st_size > elf_gp_size (abfd) || ELF_ST_TYPE (sym->st_info) == STT_TLS - || IRIX_COMPAT (abfd) == ict_irix6) + || IRIX_COMPAT (abfd) == ict_irix6 + || strcmp (*namep, "__gnu_lto_slim") == 0) break; /* Fall through. */ case SHN_MIPS_SCOMMON: -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] MIPS: Don't move __gnu_lto_slim to .scommon 2023-07-20 9:05 ` Alan Modra @ 2023-07-20 9:34 ` Maciej W. Rozycki 2023-07-20 11:23 ` Alan Modra 0 siblings, 1 reply; 11+ messages in thread From: Maciej W. Rozycki @ 2023-07-20 9:34 UTC (permalink / raw) To: Alan Modra; +Cc: YunQiang Su, YunQiang Su, binutils, Nick Clifton On Thu, 20 Jul 2023, Alan Modra wrote: > I think the patch is OK, but the same should be applied to another > place to stop objcopy modifying __gnu_lto_slim. > > mips-linux-gnu testcase after running make check: > binutils/objcopy ld/tmpdir/pr15323a-r.o ld/tmpdir/xxx > then inspect ld/tmpdir/xxx with readelf. > > I'm going to apply the following to mainline, and will also apply to > the 2.41 branch tomorrow if no one objects. You are the author of this code, so I guess you know what's going on here. I still don't understand the circumstances that cause linker.c to reject this symbol if it's in an SHN_MIPS_SCOMMON rather than SHN_COMMON section, and the relevant git change descriptions (going back to commit b794fc1d1c3a ("Warn for ar/nm/ranlib/ld on lto objects without plugin")) do not explain it, so I'd appreciate if you got me enlightened. Maciej ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] MIPS: Don't move __gnu_lto_slim to .scommon 2023-07-20 9:34 ` Maciej W. Rozycki @ 2023-07-20 11:23 ` Alan Modra 2023-07-20 14:32 ` Maciej W. Rozycki 0 siblings, 1 reply; 11+ messages in thread From: Alan Modra @ 2023-07-20 11:23 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: YunQiang Su, YunQiang Su, binutils, Nick Clifton On Thu, Jul 20, 2023 at 10:34:43AM +0100, Maciej W. Rozycki wrote: > On Thu, 20 Jul 2023, Alan Modra wrote: > > > I think the patch is OK, but the same should be applied to another > > place to stop objcopy modifying __gnu_lto_slim. > > > > mips-linux-gnu testcase after running make check: > > binutils/objcopy ld/tmpdir/pr15323a-r.o ld/tmpdir/xxx > > then inspect ld/tmpdir/xxx with readelf. > > > > I'm going to apply the following to mainline, and will also apply to > > the 2.41 branch tomorrow if no one objects. > > You are the author of this code, so I guess you know what's going on > here. I still don't understand the circumstances that cause linker.c to > reject this symbol if it's in an SHN_MIPS_SCOMMON rather than SHN_COMMON > section, and the relevant git change descriptions (going back to commit > b794fc1d1c3a ("Warn for ar/nm/ranlib/ld on lto objects without plugin")) > do not explain it, so I'd appreciate if you got me enlightened. It isn't anything in linker.c, I believe the problem occurs in the lto plugin which processes object files using libiberty/simple-object*. Code in libiberty/simple-object-elf.c removes SHN_COMMON symbols, which for most architectures removes __gnu_lto_slim, but not on mips if the symbol is moved to SHN_MIPS_SCOMMON. Then the symbol appears in lto output files, resulting in linker errors like: /tmp/ccg5JkW9.debug.temp.o: plugin needed to handle lto object I haven't actually looked at this under gdb to see exactly what is going on for mips, I'm relying on memory from doing so for powerpc a while ago. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] MIPS: Don't move __gnu_lto_slim to .scommon 2023-07-20 11:23 ` Alan Modra @ 2023-07-20 14:32 ` Maciej W. Rozycki 2023-07-20 22:44 ` Alan Modra 0 siblings, 1 reply; 11+ messages in thread From: Maciej W. Rozycki @ 2023-07-20 14:32 UTC (permalink / raw) To: Alan Modra; +Cc: YunQiang Su, YunQiang Su, binutils, Nick Clifton On Thu, 20 Jul 2023, Alan Modra wrote: > > > I think the patch is OK, but the same should be applied to another > > > place to stop objcopy modifying __gnu_lto_slim. > > > > > > mips-linux-gnu testcase after running make check: > > > binutils/objcopy ld/tmpdir/pr15323a-r.o ld/tmpdir/xxx > > > then inspect ld/tmpdir/xxx with readelf. > > > > > > I'm going to apply the following to mainline, and will also apply to > > > the 2.41 branch tomorrow if no one objects. > > > > You are the author of this code, so I guess you know what's going on > > here. I still don't understand the circumstances that cause linker.c to > > reject this symbol if it's in an SHN_MIPS_SCOMMON rather than SHN_COMMON > > section, and the relevant git change descriptions (going back to commit > > b794fc1d1c3a ("Warn for ar/nm/ranlib/ld on lto objects without plugin")) > > do not explain it, so I'd appreciate if you got me enlightened. > > It isn't anything in linker.c, I believe the problem occurs in the lto > plugin which processes object files using libiberty/simple-object*. > Code in libiberty/simple-object-elf.c removes SHN_COMMON symbols, > which for most architectures removes __gnu_lto_slim, but not on mips > if the symbol is moved to SHN_MIPS_SCOMMON. Then the symbol appears > in lto output files, resulting in linker errors like: > /tmp/ccg5JkW9.debug.temp.o: plugin needed to handle lto object It seems to me then we should be fixing libiberty instead, to also remove SHN_MIPS_SCOMMON symbols, for whatever intent it's done, shouldn't we? > I haven't actually looked at this under gdb to see exactly what is > going on for mips, I'm relying on memory from doing so for powerpc a > while ago. Completely understood. Maciej ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] MIPS: Don't move __gnu_lto_slim to .scommon 2023-07-20 14:32 ` Maciej W. Rozycki @ 2023-07-20 22:44 ` Alan Modra 0 siblings, 0 replies; 11+ messages in thread From: Alan Modra @ 2023-07-20 22:44 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: YunQiang Su, YunQiang Su, binutils, Nick Clifton On Thu, Jul 20, 2023 at 03:32:49PM +0100, Maciej W. Rozycki wrote: > On Thu, 20 Jul 2023, Alan Modra wrote: > > > > > I think the patch is OK, but the same should be applied to another > > > > place to stop objcopy modifying __gnu_lto_slim. > > > > > > > > mips-linux-gnu testcase after running make check: > > > > binutils/objcopy ld/tmpdir/pr15323a-r.o ld/tmpdir/xxx > > > > then inspect ld/tmpdir/xxx with readelf. > > > > > > > > I'm going to apply the following to mainline, and will also apply to > > > > the 2.41 branch tomorrow if no one objects. > > > > > > You are the author of this code, so I guess you know what's going on > > > here. I still don't understand the circumstances that cause linker.c to > > > reject this symbol if it's in an SHN_MIPS_SCOMMON rather than SHN_COMMON > > > section, and the relevant git change descriptions (going back to commit > > > b794fc1d1c3a ("Warn for ar/nm/ranlib/ld on lto objects without plugin")) > > > do not explain it, so I'd appreciate if you got me enlightened. > > > > It isn't anything in linker.c, I believe the problem occurs in the lto > > plugin which processes object files using libiberty/simple-object*. > > Code in libiberty/simple-object-elf.c removes SHN_COMMON symbols, > > which for most architectures removes __gnu_lto_slim, but not on mips > > if the symbol is moved to SHN_MIPS_SCOMMON. Then the symbol appears > > in lto output files, resulting in linker errors like: > > /tmp/ccg5JkW9.debug.temp.o: plugin needed to handle lto object > > It seems to me then we should be fixing libiberty instead, to also remove > SHN_MIPS_SCOMMON symbols, for whatever intent it's done, shouldn't we? It would be quite reasonable to change the libiberty code to test for EM_MIPS and SHN_MIPS_SCOMMON, or to discard __gnu_lto_slim by name. (The only reason that simple_object_elf_copy_lto_debug_sections removes SHN_COMMON symbols is to remove __gnu_lto_slim.) However the symbol is a marker only, so I think it is also entirely reasonable to stop mips ld -r or objcopy from modifying it unexpectedly. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-07-20 22:44 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-07-03 10:36 [PATCH] MIPS: Don't __gnu_lto_slim to .scommon YunQiang Su 2023-07-03 10:50 ` [PATCH v2] MIPS: Don't move " YunQiang Su 2023-07-05 1:42 ` YunQiang Su 2023-07-13 5:39 ` YunQiang Su 2023-07-19 14:55 ` Maciej W. Rozycki 2023-07-19 17:26 ` YunQiang Su 2023-07-20 9:05 ` Alan Modra 2023-07-20 9:34 ` Maciej W. Rozycki 2023-07-20 11:23 ` Alan Modra 2023-07-20 14:32 ` Maciej W. Rozycki 2023-07-20 22:44 ` Alan Modra
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).