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

["H.J. Lu" <hjl.tools@gmail.com>]

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.