public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: Nick Clifton <nickc@redhat.com>
Cc: hjl.tools@gmail.com, binutils@sourceware.org, siddhesh@redhat.com
Subject: Re: RFC: ld: Add --text-section-ordering-file (version 2)
Date: Mon, 29 Apr 2024 09:42:26 +0930	[thread overview]
Message-ID: <Zi7l6irHuTIDwH8i@squeak.grove.modra.org> (raw)
In-Reply-To: <ZiuhhGfcqIUl6RbG@squeak.grove.modra.org>

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

  reply	other threads:[~2024-04-29  0:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-25 13:01 Nick Clifton
2024-04-25 15:32 ` H.J. Lu
2024-04-26  9:38   ` Nick Clifton
2024-04-26  1:46 ` Hans-Peter Nilsson
2024-04-26  9:46   ` Nick Clifton
2024-04-27  0:46     ` Hans-Peter Nilsson
2024-04-26  4:17 ` Alan Modra
2024-04-26  9:59   ` Nick Clifton
2024-04-26 12:43     ` Alan Modra
2024-04-29  0:12       ` Alan Modra [this message]
2024-05-02 15:16         ` Nick Clifton
2024-05-06 18:27         ` H.J. Lu
2024-05-10 21:46           ` Noah Goldstein
2024-05-11 13:01             ` H.J. Lu
2024-05-07 16:39         ` RFC: ld: Add --text-section-ordering-file (version 4) Nick Clifton
2024-05-10 12:25           ` Alan Modra
2024-05-10 16:00             ` Nick Clifton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zi7l6irHuTIDwH8i@squeak.grove.modra.org \
    --to=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=hjl.tools@gmail.com \
    --cc=nickc@redhat.com \
    --cc=siddhesh@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).