Re: RFC: ld: Add --text-section-ordering-file (version 2)

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Mon, May 6, 2024 at 1:28 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sun, Apr 28, 2024 at 5:12 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Fri, Apr 26, 2024 at 10:13:48PM +0930, Alan Modra wrote:
> > > On Fri, Apr 26, 2024 at 10:59:00AM +0100, Nick Clifton wrote:
> > > > It could be done, but I like keeping things simple.
> > >
> > > Here's a completely untested implementation of the idea I'm proposing,
> > > written just to show that the linker modifications are relatively
> > > small.  (There is a very good chance I've messed up list handling..)
> > > What appears inside the --section-ordering-file output sections is
> > > spliced in to corresponding output sections in the default script,
> > > before the first input wild statement.
> > >
> > > I like keeping things simple too.  This should work without modifying
> > > default linker scripts (and user scripts!), and allows all the usual
> > > ways of specifying input sections.  That means you can use constructs
> > > like EXCLUDE_FILE, SORT, KEEP, *(.text .rdata) vs. *(.text) *(.rdata)
> > > and so on.
> >
> > The patch wasn't too far wrong.  I had some compile issues when plugin
> > support was disabled, and it is probably a bad idea to allow creation
> > of new output sections via --section-ordering-file, so that is now
> > disallowed.
> >
> > I'm not putting forward this patch as a finished work, it's just a
> > demonstrator.
> >
> > What I'd like to see is:
> > 1) Lose that horrible "INCLUDE section-ordering-file" requirement for
> >    existing scripts.  Do you really want that put into every output
> >    section in every script in scripttempl/?
> > 2) Parse the section ordering file in ldgram.y.  An entry under file:
> >    would be appropriate, like we do for version scripts.  That would
> >    at least lose the SECTIONS clause required by my demonstrator
> >    patch.  You wouldn't necessarily need to follow existing
> >    specification of output and input sections, or to support other
> >    things allowed in output sections, but I do think a syntax with an
> >    explicit output section is a good idea.  ie. not one where the
> >    output .text is deduced from the input .text.mumble.
> >
> >
> > diff --git a/ld/ld.h b/ld/ld.h
> > index fcdd9a2c083..15d9dc262b6 100644
> > --- a/ld/ld.h
> > +++ b/ld/ld.h
> > @@ -196,6 +196,8 @@ typedef struct
> >
> >    /* Default linker script.  */
> >    char *default_script;
> > +
> > +  char *section_ordering_file;
> >  } args_type;
> >
> >  extern args_type command_line;
> > @@ -325,6 +327,7 @@ extern ld_config_type config;
> >
> >  extern FILE * saved_script_handle;
> >  extern bool force_make_executable;
> > +extern bool in_section_ordering;
> >
> >  extern int yyparse (void);
> >  extern void add_cref (const char *, bfd *, asection *, bfd_vma);
> > diff --git a/ld/ldlang.c b/ld/ldlang.c
> > index 54d1af62ebe..6424d4764d1 100644
> > --- a/ld/ldlang.c
> > +++ b/ld/ldlang.c
> > @@ -1299,6 +1299,7 @@ output_section_statement_newfunc (struct bfd_hash_entry *entry,
> >    ret->s.output_section_statement.section_alignment = NULL;
> >    ret->s.output_section_statement.block_value = 1;
> >    lang_list_init (&ret->s.output_section_statement.children);
> > +  lang_list_init (&ret->s.output_section_statement.sort_children);
> >    lang_statement_append (stat_ptr, &ret->s, &ret->s.header.next);
> >
> >    /* For every output section statement added to the list, except the
> > @@ -7613,13 +7614,21 @@ lang_enter_output_section_statement (const char *output_section_statement_name,
> >    lang_output_section_statement_type *os;
> >
> >    os = lang_output_section_statement_lookup (output_section_statement_name,
> > -                                            constraint, 2);
> > +                                            constraint,
> > +                                            in_section_ordering ? 0 : 2);
> > +  if (os == NULL)
> > +    einfo (_("%F%P:%pS: error: output section must already exist\n"), NULL);
> >    current_section = os;
> >
> > +  /* Make next things chain into subchain of this.  */
> > +  push_stat_ptr (in_section_ordering ? &os->sort_children : &os->children);
> > +
> > +  if (in_section_ordering)
> > +    return os;
> > +
> >    if (os->addr_tree == NULL)
> > -    {
> > -      os->addr_tree = address_exp;
> > -    }
> > +    os->addr_tree = address_exp;
> > +
> >    os->sectype = sectype;
> >    if (sectype == type_section || sectype == typed_readonly_section)
> >      os->sectype_value = sectype_value;
> > @@ -7629,9 +7638,6 @@ lang_enter_output_section_statement (const char *output_section_statement_name,
> >      os->flags = SEC_NO_FLAGS;
> >    os->block_value = 1;
> >
> > -  /* Make next things chain into subchain of this.  */
> > -  push_stat_ptr (&os->children);
> > -
> >    os->align_lma_with_input = align_with_input == ALIGN_WITH_INPUT;
> >    if (os->align_lma_with_input && align != NULL)
> >      einfo (_("%F%P:%pS: error: align with input and explicit align specified\n"),
> > @@ -7971,21 +7977,6 @@ find_rescan_insertion (lang_input_statement_type *add)
> >    return iter;
> >  }
> >
> > -/* Insert SRCLIST into DESTLIST after given element by chaining
> > -   on FIELD as the next-pointer.  (Counterintuitively does not need
> > -   a pointer to the actual after-node itself, just its chain field.)  */
> > -
> > -static void
> > -lang_list_insert_after (lang_statement_list_type *destlist,
> > -                       lang_statement_list_type *srclist,
> > -                       lang_statement_union_type **field)
> > -{
> > -  *(srclist->tail) = *field;
> > -  *field = srclist->head;
> > -  if (destlist->tail == field)
> > -    destlist->tail = srclist->tail;
> > -}
> > -
> >  /* Detach new nodes added to DESTLIST since the time ORIGLIST
> >     was taken as a copy of it and leave them in ORIGLIST.  */
> >
> > @@ -8033,6 +8024,21 @@ find_next_input_statement (lang_statement_union_type **s)
> >  }
> >  #endif /* BFD_SUPPORTS_PLUGINS */
> >
> > +/* Insert SRCLIST into DESTLIST after given element by chaining
> > +   on FIELD as the next-pointer.  (Counterintuitively does not need
> > +   a pointer to the actual after-node itself, just its chain field.)  */
> > +
> > +static void
> > +lang_list_insert_after (lang_statement_list_type *destlist,
> > +                       lang_statement_list_type *srclist,
> > +                       lang_statement_union_type **field)
> > +{
> > +  *(srclist->tail) = *field;
> > +  *field = srclist->head;
> > +  if (destlist->tail == field)
> > +    destlist->tail = srclist->tail;
> > +}
> > +
> >  /* Add NAME to the list of garbage collection entry points.  */
> >
> >  void
> > @@ -8127,9 +8133,34 @@ reset_resolved_wilds (void)
> >    lang_for_each_statement (reset_one_wild);
> >  }
> >
> > +/* For each output section statement, splice any entries on the
> > +   sort_children list before the first wild statement on the children
> > +   list.  */
> > +
> > +static void
> > +lang_os_merge_sort_children (void)
> > +{
> > +  lang_output_section_statement_type *os;
> > +  for (os = (void *) lang_os_list.head; os != NULL; os = os->next)
> > +    {
> > +      if (os->sort_children.head != NULL)
> > +       {
> > +         lang_statement_union_type **where;
> > +         for (where = &os->children.head;
> > +              *where != NULL;
> > +              where = &(*where)->header.next)
> > +           if ((*where)->header.type == lang_wild_statement_enum)
> > +             break;
> > +         lang_list_insert_after (&os->children, &os->sort_children, where);
> > +       }
> > +    }
> > +}
> > +
> >  void
> >  lang_process (void)
> >  {
> > +  lang_os_merge_sort_children ();
> > +
> >    /* Finalize dynamic list.  */
> >    if (link_info.dynamic_list)
> >      lang_finalize_version_expr_head (&link_info.dynamic_list->head);
> > @@ -8817,6 +8848,10 @@ lang_leave_output_section_statement (fill_type *fill, const char *memspec,
> >                                      lang_output_section_phdr_list *phdrs,
> >                                      const char *lma_memspec)
> >  {
> > +  pop_stat_ptr ();
> > +  if (in_section_ordering)
> > +    return;
> > +
> >    lang_get_regions (&current_section->region,
> >                     &current_section->lma_region,
> >                     memspec, lma_memspec,
> > @@ -8825,7 +8860,6 @@ lang_leave_output_section_statement (fill_type *fill, const char *memspec,
> >
> >    current_section->fill = fill;
> >    current_section->phdrs = phdrs;
> > -  pop_stat_ptr ();
> >  }
> >
> >  /* Set the output format type.  -oformat overrides scripts.  */
> > diff --git a/ld/ldlang.h b/ld/ldlang.h
> > index ea1c26d00f3..91947c56fda 100644
> > --- a/ld/ldlang.h
> > +++ b/ld/ldlang.h
> > @@ -162,6 +162,8 @@ typedef struct lang_output_section_statement_struct
> >
> >    lang_output_section_phdr_list *phdrs;
> >
> > +  lang_statement_list_type sort_children;
> > +
> >    /* Used by ELF SHF_LINK_ORDER sorting.  */
> >    void *data;
> >
> > diff --git a/ld/ldlex.h b/ld/ldlex.h
> > index d575562a357..5708e6f5e34 100644
> > --- a/ld/ldlex.h
> > +++ b/ld/ldlex.h
> > @@ -62,6 +62,7 @@ enum option_values
> >    OPTION_SONAME,
> >    OPTION_SORT_COMMON,
> >    OPTION_SORT_SECTION,
> > +  OPTION_SECTION_ORDERING_FILE,
> >    OPTION_STATS,
> >    OPTION_SYMBOLIC,
> >    OPTION_SYMBOLIC_FUNCTIONS,
> > diff --git a/ld/ldmain.c b/ld/ldmain.c
> > index fe389681bd3..77b57a9fd32 100644
> > --- a/ld/ldmain.c
> > +++ b/ld/ldmain.c
> > @@ -90,6 +90,8 @@ bool version_printed;
> >  /* TRUE if we should demangle symbol names.  */
> >  bool demangling;
> >
> > +bool in_section_ordering;
> > +
> >  args_type command_line;
> >
> >  ld_config_type config;
> > @@ -446,6 +448,19 @@ main (int argc, char **argv)
> >        info_msg ("\n==================================================\n");
> >      }
> >
> > +  if (command_line.section_ordering_file)
> > +    {
> > +      FILE *hold_script_handle;
> > +
> > +      hold_script_handle = saved_script_handle;
> > +      ldfile_open_command_file (command_line.section_ordering_file);
> > +      saved_script_handle = hold_script_handle;
> > +      in_section_ordering = true;
> > +      parser_input = input_script;
> > +      yyparse ();
> > +      in_section_ordering = false;
> > +    }
> > +
> >    if (command_line.force_group_allocation
> >        || !bfd_link_relocatable (&link_info))
> >      link_info.resolve_section_groups = true;
> > diff --git a/ld/lexsup.c b/ld/lexsup.c
> > index dad3b6059ed..0c337a4d170 100644
> > --- a/ld/lexsup.c
> > +++ b/ld/lexsup.c
> > @@ -487,6 +487,9 @@ static const struct ld_option ld_options[] =
> >    { {"sort-section", required_argument, NULL, OPTION_SORT_SECTION},
> >      '\0', N_("name|alignment"),
> >      N_("Sort sections by name or maximum alignment"), TWO_DASHES },
> > +  { {"section-ordering-file", required_argument, NULL, OPTION_SECTION_ORDERING_FILE},
> > +    '\0', N_("FILE"),
> > +    N_("Sort sections by statements in FILE"), TWO_DASHES },
> >    { {"spare-dynamic-tags", required_argument, NULL, OPTION_SPARE_DYNAMIC_TAGS},
> >      '\0', N_("COUNT"), N_("How many tags to reserve in .dynamic section"),
> >      TWO_DASHES },
> > @@ -1400,6 +1403,9 @@ parse_args (unsigned argc, char **argv)
> >             einfo (_("%F%P: invalid section sorting option: %s\n"),
> >                    optarg);
> >           break;
> > +       case OPTION_SECTION_ORDERING_FILE:
> > +         command_line.section_ordering_file = optarg;
> > +         break;
> >         case OPTION_STATS:
> >           config.stats = true;
> >           break;
> > diff --git a/ld/testsuite/ld-scripts/section-order-1a.d b/ld/testsuite/ld-scripts/section-order-1a.d
> > new file mode 100644
> > index 00000000000..03d848b83ea
> > --- /dev/null
> > +++ b/ld/testsuite/ld-scripts/section-order-1a.d
> > @@ -0,0 +1,17 @@
> > +#source: section-order-1b.s
> > +#source: section-order-1a.s
> > +#source: start.s
> > +#ld: --section-ordering-file section-order-1a.t
> > +#nm: -n
> > +
> > +#...
> > +[0-9a-f]+ T yyy
> > +#...
> > +[0-9a-f]+ T bar
> > +#...
> > +[0-9a-f]+ T [_]+start
> > +#...
> > +[0-9a-f]+ T xxx
> > +#...
> > +[0-9a-f]+ T foo
> > +#pass
> > diff --git a/ld/testsuite/ld-scripts/section-order-1a.s b/ld/testsuite/ld-scripts/section-order-1a.s
> > new file mode 100644
> > index 00000000000..17d311fd58f
> > --- /dev/null
> > +++ b/ld/testsuite/ld-scripts/section-order-1a.s
> > @@ -0,0 +1,19 @@
> > +       .section .text.foo
> > +       .globl  foo
> > +foo:
> > +       .dc.a 0
> > +
> > +       .section .text.bar
> > +       .globl  bar
> > +bar:
> > +       .dc.a 0
> > +
> > +       .section .data.small
> > +       .globl small
> > +small:
> > +       .dc.a 0
> > +
> > +       .section .bar
> > +       .global bar
> > +bart:
> > +       .dc.a 0
> > diff --git a/ld/testsuite/ld-scripts/section-order-1a.t b/ld/testsuite/ld-scripts/section-order-1a.t
> > new file mode 100644
> > index 00000000000..e3786818fea
> > --- /dev/null
> > +++ b/ld/testsuite/ld-scripts/section-order-1a.t
> > @@ -0,0 +1,15 @@
> > +SECTIONS
> > +{
> > +  .text : {
> > +    *(.text.yyy)
> > +    *(.text.b?r)
> > +    *(.text)
> > +    *(.text.xxx .text.foo)
> > +  }
> > +
> > +  .data : {
> > +    *(.data.small)
> > +    *(.big*)
> > +    *(.bar .baz*)
> > +  }
> > +}
> > diff --git a/ld/testsuite/ld-scripts/section-order-1b.d b/ld/testsuite/ld-scripts/section-order-1b.d
> > new file mode 100644
> > index 00000000000..41b4d16097e
> > --- /dev/null
> > +++ b/ld/testsuite/ld-scripts/section-order-1b.d
> > @@ -0,0 +1,17 @@
> > +#source: section-order-1a.s
> > +#source: section-order-1b.s
> > +#source: start.s
> > +#ld: --section-ordering-file section-order-1b.t
> > +#nm: -n
> > +
> > +#...
> > +[0-9a-f]+ T yyy
> > +#...
> > +[0-9a-f]+ T bar
> > +#...
> > +[0-9a-f]+ T [_]+start
> > +#...
> > +[0-9a-f]+ T xxx
> > +#...
> > +[0-9a-f]+ T foo
> > +#pass
> > diff --git a/ld/testsuite/ld-scripts/section-order-1b.s b/ld/testsuite/ld-scripts/section-order-1b.s
> > new file mode 100644
> > index 00000000000..7fe103d0ea6
> > --- /dev/null
> > +++ b/ld/testsuite/ld-scripts/section-order-1b.s
> > @@ -0,0 +1,19 @@
> > +       .section .text.xxx
> > +       .globl  xxx
> > +xxx:
> > +       .dc.a 0
> > +
> > +       .section .text.yyy
> > +       .globl  yyy
> > +yyy:
> > +       .dc.a 0
> > +
> > +       .section .big
> > +       .global big
> > +big:
> > +       .dc.a 0
> > +
> > +       .section .baz
> > +       .global baz
> > +baz:
> > +       .dc.a 0
> > diff --git a/ld/testsuite/ld-scripts/section-order-1b.t b/ld/testsuite/ld-scripts/section-order-1b.t
> > new file mode 100644
> > index 00000000000..896653b2608
> > --- /dev/null
> > +++ b/ld/testsuite/ld-scripts/section-order-1b.t
> > @@ -0,0 +1,9 @@
> > +SECTIONS {
> > +  .text : {
> > +    *(.text.yyy)
> > +    *(.text.b?r)
> > +    *(*t)
> > +    *(.text.xxx)
> > +    *(.text.foo)
> > +  }
> > +}
> > diff --git a/ld/testsuite/ld-scripts/section-order-1c.d b/ld/testsuite/ld-scripts/section-order-1c.d
> > new file mode 100644
> > index 00000000000..65819cbcb63
> > --- /dev/null
> > +++ b/ld/testsuite/ld-scripts/section-order-1c.d
> > @@ -0,0 +1,13 @@
> > +#source: section-order-1b.s
> > +#source: section-order-1a.s
> > +#source: start.s
> > +#ld: --section-ordering-file section-order-1a.t
> > +#nm: -n
> > +
> > +#...
> > +[0-9a-f]+ D small
> > +#...
> > +[0-9a-f]+ D big
> > +#...
> > +[0-9a-f]+ D ba.*
> > +#pass
> > diff --git a/ld/testsuite/ld-scripts/section-order.exp b/ld/testsuite/ld-scripts/section-order.exp
> > new file mode 100644
> > index 00000000000..9b87e746cd1
> > --- /dev/null
> > +++ b/ld/testsuite/ld-scripts/section-order.exp
> > @@ -0,0 +1,45 @@
> > +# Test for --section-ordering-file FILE.
> > +# Copyright (C) 2024 Free Software Foundation, Inc.
> > +#
> > +# This file is part of the GNU Binutils.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 3 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write to the Free Software
> > +# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
> > +# MA 02110-1301, USA.
> > +
> > +# The --section-ordering-file option is only supported by ELF and
> > +# PE COFF linkers, which allow for arbitrarily named sections, eg:
> > +# .text.*
> > +if { !([is_elf_format] || [is_pecoff_format]) } {
> > +    return
> > +}
> > +
> > +set old_ldflags $LDFLAGS
> > +if { [istarget spu*-*-*] } then {
> > +    set LDFLAGS "$LDFLAGS --local-store 0:0 --no-overlays"
> > +} elseif { [is_pecoff_format] } then {
> > +    set LDFLAGS "$LDFLAGS --image-base 0"
> > +} elseif { [is_xcoff_format] } then {
> > +    set LDFLAGS "$LDFLAGS -bnogc"
> > +}
> > +
> > +set test_list [lsort [glob -nocomplain $srcdir/$subdir/section-order*.d]]
> > +foreach test_file $test_list {
> > +    set test_name [file rootname $test_file]
> > +    set map_file "tmpdir/[file tail $test_name].map"
> > +    verbose $test_name
> > +    run_dump_test $test_name
> > +}
> > +
> > +set LDFLAGS $old_ldflags
> > diff --git a/ld/testsuite/ld-scripts/start.s b/ld/testsuite/ld-scripts/start.s
> > new file mode 100644
> > index 00000000000..3f646267716
> > --- /dev/null
> > +++ b/ld/testsuite/ld-scripts/start.s
> > @@ -0,0 +1,14 @@
> > +       .text
> > +       .global start   /* Used by SH targets.  */
> > +start:
> > +       .global _start
> > +_start:
> > +       .global __start
> > +__start:
> > +       .global _mainCRTStartup /* Used by PE targets.  */
> > +_mainCRTStartup:
> > +       .global main    /* Used by HPPA targets.  */
> > +main:
> > +       .globl  _main   /* Used by LynxOS targets.  */
> > +_main:
> > +       .dc.a 0
> >
> > --
> > Alan Modra
>
> Noah is also working on something similar.
>
> --
> H.J.

Is there anything holding this patch up?