V3 [PATCH] elf: Allow mixed ordered/unordered inputs for non-relocatable link

H.J. Lu hjl.tools@gmail.com
Tue Dec 22 13:24:30 GMT 2020


On Mon, Dec 21, 2020 at 8:21 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Dec 21, 2020 at 3:24 PM Fangrui Song <i@maskray.me> wrote:
> >
> > Really appreciate this patch! For RISC-V -mrelax, .gcc_except_table
> > has relocations referencing .text . I don't know whether GNU ld does
> > ad-hoc garbage collection for .gcc_except_table, if it doesn't, in the
> > future GCC can set SHF_LINK_ORDER on .gcc_except_table to allow a
> > non-adhoc garbage collection. I have a write-up about this
> > https://maskray.me/blog/2020-12-12-c++-exception-handling-abi#monolithic-.gcc_except_table
> > In particular, I mentioned on https://reviews.llvm.org/D83655
> > that Clang cannot use SHF_LINK_ORDER.
> >
> > On 2020-12-21, H.J. Lu via Binutils wrote:
> > >For non-relocatable link with SHF_LINK_ORDER inputs, allow mixed indirect
> > >and data inputs with ordered and unordered inputs:
> > >
> > >1. Add pattern to bfd_section for the matching section name pattern in
> > >linker script and update BFD_FAKE_SECTION.
> > >2. Sort the consecutive bfd_indirect_link_order sections with the same
> > >pattern to allow linker script to overdide input section order.
> > >3. Place unordered sections before ordered sections.
> > >4. Change the offsets of the indirect input sections only.
> > >
> > >bfd/
> > >
> > >       PR ld/26256
> > >       * elflink.c (compare_link_order): Place unordered sections before
> > >       ordered sections.
> > >       (elf_fixup_link_order): Add a link info argument.  Allow mixed
> > >       ordered and unordered input sections for non-relocatable link.
> > >       Sort the consecutive bfd_indirect_link_order sections with the
> > >       same pattern.  Change the offsets of the bfd_indirect_link_order
> > >       sections only.
> > >       (bfd_elf_final_link): Pass info to elf_fixup_link_order.
> > >       * section.c (bfd_section): Add pattern.
> > >       (BFD_FAKE_SECTION): Initialize pattern to NULL.
> > >       * bfd-in2.h: Regenerated.
> > >
> > >gas/
> > >
> > >       PR ld/26256
> > >       * config/obj-elf.c (obj_elf_change_section): Also filter out
> > >       SHF_LINK_ORDER.
> > >
> > >ld/
> > >
> > >       PR ld/26256
> > >       * ldlang.c (gc_section_callback): Set pattern.
> > >       * testsuite/ld-elf/pr26256-1.s: New file.
> > >       * testsuite/ld-elf/pr26256-1.t: Likewise.
> > >       * testsuite/ld-elf/pr26256-1a.d: Likewise.
> > >       * testsuite/ld-elf/pr26256-1b.d: Likewise.
> > >       * testsuite/ld-elf/pr26256-2.s: Likewise.
> > >       * testsuite/ld-elf/pr26256-2.t: Likewise.
> > >       * testsuite/ld-elf/pr26256-2a.d: Likewise.
> > >       * testsuite/ld-elf/pr26256-2b.d: Likewise.
> > >       * testsuite/ld-elf/pr26256-3.s: Likewise.
> > >       * testsuite/ld-elf/pr26256-3a.d: Likewise.
> > >       * testsuite/ld-elf/pr26256-3a.t: Likewise.
> > >       * testsuite/ld-elf/pr26256-3b.d: Likewise.
> > >       * testsuite/ld-elf/pr26256-3b.t: Likewise.
> > >---
> > > bfd/bfd-in2.h                    |  7 ++-
> > > bfd/elflink.c                    | 83 ++++++++++++++++++++++++--------
> > > bfd/section.c                    |  7 ++-
> > > gas/config/obj-elf.c             |  4 +-
> > > ld/ldlang.c                      |  4 +-
> > > ld/testsuite/ld-elf/pr26256-1.s  | 20 ++++++++
> > > ld/testsuite/ld-elf/pr26256-1.t  |  9 ++++
> > > ld/testsuite/ld-elf/pr26256-1a.d |  7 +++
> > > ld/testsuite/ld-elf/pr26256-1b.d |  7 +++
> > > ld/testsuite/ld-elf/pr26256-2.s  | 32 ++++++++++++
> > > ld/testsuite/ld-elf/pr26256-2.t  |  5 ++
> > > ld/testsuite/ld-elf/pr26256-2a.d | 19 ++++++++
> > > ld/testsuite/ld-elf/pr26256-2b.d | 19 ++++++++
> > > ld/testsuite/ld-elf/pr26256-3.s  | 18 +++++++
> > > ld/testsuite/ld-elf/pr26256-3a.d |  9 ++++
> > > ld/testsuite/ld-elf/pr26256-3a.t | 13 +++++
> > > ld/testsuite/ld-elf/pr26256-3b.d |  9 ++++
> > > ld/testsuite/ld-elf/pr26256-3b.t | 12 +++++
> > > 18 files changed, 259 insertions(+), 25 deletions(-)
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-1.s
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-1.t
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-1a.d
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-1b.d
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-2.s
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-2.t
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-2a.d
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-2b.d
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-3.s
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-3a.d
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-3a.t
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-3b.d
> > > create mode 100644 ld/testsuite/ld-elf/pr26256-3b.t
> > >
> > >diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> > >index f1bef5742a..cc5a3c9912 100644
> > >--- a/bfd/bfd-in2.h
> > >+++ b/bfd/bfd-in2.h
> > >@@ -1184,6 +1184,9 @@ typedef struct bfd_section
> > >   struct bfd_symbol *symbol;
> > >   struct bfd_symbol **symbol_ptr_ptr;
> > >
> > >+  /* The matching section name pattern in linker script.  */
> > >+  const char *pattern;
> > >+
> > >   /* Early in the link process, map_head and map_tail are used to build
> > >      a list of input sections attached to an output section.  Later,
> > >      output sections use these fields for a list of bfd_link_order
> > >@@ -1377,8 +1380,8 @@ discarded_section (const asection *sec)
> > >   /* target_index, used_by_bfd, constructor_chain, owner,           */ \
> > >      0,            NULL,        NULL,              NULL,               \
> > >                                                                        \
> > >-  /* symbol,                    symbol_ptr_ptr,                     */ \
> > >-     (struct bfd_symbol *) SYM, &SEC.symbol,                           \
> > >+  /* symbol,                    symbol_ptr_ptr, pattern,            */ \
> > >+     (struct bfd_symbol *) SYM, &SEC.symbol,    NULL,                  \
> > >                                                                        \
> > >   /* map_head, map_tail, already_assigned                           */ \
> > >      { NULL }, { NULL }, NULL                                          \
> > >diff --git a/bfd/elflink.c b/bfd/elflink.c
> > >index 1b3398126f..6930c7c741 100644
> > >--- a/bfd/elflink.c
> > >+++ b/bfd/elflink.c
> > >@@ -11867,8 +11867,21 @@ compare_link_order (const void *a, const void *b)
> > >   const struct bfd_link_order *blo = *(const struct bfd_link_order **) b;
> > >   asection *asec = elf_linked_to_section (alo->u.indirect.section);
> > >   asection *bsec = elf_linked_to_section (blo->u.indirect.section);
> > >-  bfd_vma apos = asec->output_section->lma + asec->output_offset;
> > >-  bfd_vma bpos = bsec->output_section->lma + bsec->output_offset;
> > >+  bfd_vma apos, bpos;
> > >+
> > >+  /* Check if any sections are unordered.  */
> > >+  if (asec == NULL || bsec == NULL)
> > >+    {
> > >+      /* Place unordered sections before ordered sections.  */
> > >+      if (bsec != NULL)
> > >+      return -1;
> > >+      else if (asec != NULL)
> > >+      return 1;
> > >+      return 0;
> > >+    }
> > >+
> >
> > I just commented that LLD places ordered sections before unordered sections.
> > https://sourceware.org/bugzilla/show_bug.cgi?id=26256#c10
> > Solaris folks appear to prefer this style.
>
> Changed.
>
> > >+  apos = asec->output_section->lma + asec->output_offset;
> > >+  bpos = bsec->output_section->lma + bsec->output_offset;
> > >
> > >   if (apos < bpos)
> > >     return -1;
> > >@@ -11903,14 +11916,14 @@ compare_link_order (const void *a, const void *b)
> > >    sections.  Ideally we'd do this in the linker proper.  */
> > >
> > > static bfd_boolean
> > >-elf_fixup_link_order (bfd *abfd, asection *o)
> > >+elf_fixup_link_order (struct bfd_link_info *info, bfd *abfd, asection *o)
> > > {
> > >   size_t seen_linkorder;
> > >   size_t seen_other;
> > >   size_t n;
> > >   struct bfd_link_order *p;
> > >   bfd *sub;
> > >-  struct bfd_link_order **sections;
> > >+  struct bfd_link_order **sections, **indirect_sections;
> > >   asection *other_sec, *linkorder_sec;
> > >   bfd_vma offset;  /* Octets.  */
> > >
> > >@@ -11941,7 +11954,9 @@ elf_fixup_link_order (bfd *abfd, asection *o)
> > >       else
> > >       seen_other++;
> > >
> > >-      if (seen_other && seen_linkorder)
> > >+      /* Allow mixed ordered and unordered input sections for
> > >+         non-relocatable link.  */
> > >+      if (bfd_link_relocatable (info) && seen_other && seen_linkorder)
> > >       {
> > >         if (other_sec && linkorder_sec)
> > >           _bfd_error_handler
> >
> > I don't take time investigating the logic here. But about -r &
> > SHF_LINK_ORDER:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=26256#c11
>
> "ld-r" also uses a linker script:
>
>   .rodata       0 : { *(.rodata) }
>   .rodata1      0 : { *(.rodata1) }
>
> What happens if 2 .rodata sections have different HF_LINK_ORDER?
> It is better for ld to reject it.
>
> OK for master?
>

Here is the updated patch to fix the sorting.  OK for master?

Thanks.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-elf-Allow-mixed-ordered-unordered-inputs-for-non-rel.patch
Type: text/x-patch
Size: 17789 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20201222/be96afdb/attachment-0001.bin>


More information about the Binutils mailing list