public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: binutils@sourceware.org
Cc: amodra@gmail.com,	Andrew Burgess <andrew.burgess@embecosm.com>
Subject: [PATCHv2 2/3] ld: better handling of lma region for orphan sections
Date: Wed, 08 Mar 2017 21:12:00 -0000	[thread overview]
Message-ID: <3170b1580490e49fd65e2772d8f035eea86cb375.1489007204.git.andrew.burgess@embecosm.com> (raw)
In-Reply-To: <cover.1489007204.git.andrew.burgess@embecosm.com>
In-Reply-To: <cover.1489007204.git.andrew.burgess@embecosm.com>

When picking an lma_region for an orphan section we currently create a
new lang_output_section_statement_type and then populate this with the
orphan section.

The problem is that the lang_output_section_statement_type has a prev
pointer that links back to the previous output section.  For non-orphan
output sections, that are created in linker script order, the prev
pointer will point to the output section that appears previous in linker
script order, as you'd probably expect.

The problem is that orphan sections are placed after processing the
linker script, and so, in the case of an output section created for an
orphan input section, the prev pointer actually points to the last
output section created.

This causes some unexpected behaviour when the orphan section is not
placed after the last non-orphan section that was created.

For example, consider this linker script:

  MEMORY {
    TEXT   : ORIGIN = 0x200,  LENGTH = 0x10
    RODATA : ORIGIN = 0x400,  LENGTH = 0x10
  }

  SECTIONS {
    .text   :           {*(.text)    } AT>TEXT
    .data   : AT(0x300) { *(.data)   }
    .rodata :           { *(.rodata) } AT>RODATA
  }

If we are processing an orphan section '.data.1' and decide to place
this after '.data', then the output section created will have a prev
pointer that references the '.rodata' output section.  The result of
this is that '.data.1' will actually be assigned to the RODATA lma
region, which is probably not the expected behaviour.

The reason why '.data.1' is placed into the lma region of the '.rodata'
section is that lma region propagation is done at the time we create the
output section, based on the previous output section pointer, which is
really just a last-output-section-created pointer at that point in time,
though the prev point is fixed up later to reflect the true order of the
output sections.

The solution I propose in this commit is to move the propagation of lma
regions into a separate pass of the linker, rather than performing this
as part of the enter/exit of output sections during linker script
parsing.

During this later phase we have all of the output sections to hand, and
the prev/next points have been fixed up by this point to reflect the
actual placement ordering.

There's a new test to cover this issue that passes on a range of
targets, however, some targets generate additional sections, or have
stricter memory region size requirements that make it harder to come
up with a generic pass pattern, that still tests the required
features.  For now I've set the test to ignore these targets.

ld/ChangeLog:

	* ldlang.c (lang_leave_output_section_statement): Move lma_region
	logic to...
	(lang_propagate_lma_regions): ...this new function.
	(lang_process): Call new function.
	* testsuite/ld-elf/orphan-9.d: New file.
	* testsuite/ld-elf/orphan-9.ld: New file.
	* testsuite/ld-elf/orphan-9.s: New file.
	* NEWS: Mention change in behaviour.
---
 ld/ChangeLog                    | 11 +++++++++++
 ld/NEWS                         |  3 +++
 ld/ldlang.c                     | 34 ++++++++++++++++++++++++----------
 ld/testsuite/ld-elf/orphan-9.d  | 12 ++++++++++++
 ld/testsuite/ld-elf/orphan-9.ld | 28 ++++++++++++++++++++++++++++
 ld/testsuite/ld-elf/orphan-9.s  | 11 +++++++++++
 6 files changed, 89 insertions(+), 10 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/orphan-9.d
 create mode 100644 ld/testsuite/ld-elf/orphan-9.ld
 create mode 100644 ld/testsuite/ld-elf/orphan-9.s

diff --git a/ld/ChangeLog b/ld/ChangeLog
index 3883bcb..5a06195 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,14 @@
+2017-03-08  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* ldlang.c (lang_leave_output_section_statement): Move lma_region
+	logic to...
+	(lang_propagate_lma_regions): ...this new function.
+	(lang_process): Call new function.
+	* testsuite/ld-elf/orphan-9.d: New file.
+	* testsuite/ld-elf/orphan-9.ld: New file.
+	* testsuite/ld-elf/orphan-9.s: New file.
+	* NEWS: Mention change in behaviour.
+
 2017-03-07  Alan Modra  <amodra@gmail.com>
 
 	* ldlang.c (open_input_bfds): Check that lang_assignment_statement
diff --git a/ld/NEWS b/ld/NEWS
index 23ca25a..86e9073 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -5,6 +5,9 @@
 * When configuring for arc*-*-linux* targets the default linker emulation will
   change if --with-cpu=nps400 is used at configure time.
 
+* Improve assignment of LMAs to orphan sections in some edge cases where a
+  mixture of both AT>LMA_REGION and AT(LMA) are used.
+
 Changes in 2.28:
 
 * The EXCLUDE_FILE linker script construct can now be applied outside of the
diff --git a/ld/ldlang.c b/ld/ldlang.c
index ff6ef39..89b896f 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6844,6 +6844,27 @@ lang_check_relocs (void)
     }
 }
 
+/* Look through all output sections looking for places where we can
+   propagate forward the lma region.  */
+
+static void
+lang_propagate_lma_regions (void)
+{
+  lang_output_section_statement_type *os;
+
+  for (os = &lang_output_section_statement.head->output_section_statement;
+       os != NULL;
+       os = os->next)
+    {
+      if (os->prev != NULL
+	  && os->lma_region == NULL
+	  && os->load_base == NULL
+	  && os->addr_tree == NULL
+	  && os->region == os->prev->region)
+	os->lma_region = os->prev->lma_region;
+    }
+}
+
 void
 lang_process (void)
 {
@@ -7022,6 +7043,9 @@ lang_process (void)
 	}
     }
 
+  /* Copy forward lma regions for output sections in same lma region.  */
+  lang_propagate_lma_regions ();
+
   /* Do anything special before sizing sections.  This is where ELF
      and other back-ends size dynamic sections.  */
   ldemul_before_allocation ();
@@ -7302,16 +7326,6 @@ lang_leave_output_section_statement (fill_type *fill, const char *memspec,
 		    current_section->load_base != NULL,
 		    current_section->addr_tree != NULL);
 
-  /* If this section has no load region or base, but uses the same
-     region as the previous section, then propagate the previous
-     section's load region.  */
-
-  if (current_section->lma_region == NULL
-      && current_section->load_base == NULL
-      && current_section->addr_tree == NULL
-      && current_section->region == current_section->prev->region)
-    current_section->lma_region = current_section->prev->lma_region;
-
   current_section->fill = fill;
   current_section->phdrs = phdrs;
   pop_stat_ptr ();
diff --git a/ld/testsuite/ld-elf/orphan-9.d b/ld/testsuite/ld-elf/orphan-9.d
new file mode 100644
index 0000000..637e8c5
--- /dev/null
+++ b/ld/testsuite/ld-elf/orphan-9.d
@@ -0,0 +1,12 @@
+#source: orphan-9.s
+#ld: -N -T orphan-9.ld
+#objdump: -h
+#notarget: d30v-* dlx-* fr30-* frv-* ft32-* iq2000-* mn10200-* moxie-* ms1-* msp430-* mt-* pj-*
+
+#...
+  . \.text         0+(08|10)  [0-9a-f]+  0+200 .*
+#...
+  . \.data         0+(08|10)  [0-9a-f]+  0+300 .*
+#...
+  . \.data\.1       0+8  [0-9a-f]+  0+3(0|1)[0-9a-f] .*
+#pass
diff --git a/ld/testsuite/ld-elf/orphan-9.ld b/ld/testsuite/ld-elf/orphan-9.ld
new file mode 100644
index 0000000..1a6773d
--- /dev/null
+++ b/ld/testsuite/ld-elf/orphan-9.ld
@@ -0,0 +1,28 @@
+/* This linker script is used for orphan-9 test.
+
+   We have a single byte in .data, and an orphan .data.1
+   section.  We are checking that the .data.1 orphan is assigned an
+   LMA after .data rather than picking up the lma region of .rodata.  */
+
+MEMORY
+{
+   MEM    : ORIGIN = 0x1000, LENGTH = 0x100
+   TEXT   : ORIGIN = 0x200,  LENGTH = 0x50
+   DATA   : ORIGIN = 0x300,  LENGTH = 0x50
+   RODATA : ORIGIN = 0x400,  LENGTH = 0x50
+}
+
+SECTIONS
+{
+  .text : {
+    *(.text)
+  } >MEM AT>TEXT
+
+  .data : AT(0x300) {
+    *(.data)
+  } >MEM
+
+  .rodata : {
+    *(.rodata)
+  } >MEM AT>RODATA
+}
diff --git a/ld/testsuite/ld-elf/orphan-9.s b/ld/testsuite/ld-elf/orphan-9.s
new file mode 100644
index 0000000..27efad8
--- /dev/null
+++ b/ld/testsuite/ld-elf/orphan-9.s
@@ -0,0 +1,11 @@
+        .text
+        .byte 0, 0, 0, 0, 0, 0, 0, 0
+
+        .data
+        .byte 0, 0, 0, 0, 0, 0, 0, 0
+
+        .section ".data.1", "aw"
+        .byte 0, 0, 0, 0, 0, 0, 0, 0
+
+        .section ".rodata", "a"
+        .byte 0, 0, 0, 0, 0, 0, 0, 0
-- 
2.5.1

  parent reply	other threads:[~2017-03-08 21:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-08 21:12 [PATCHv2 0/3] ld: Orphan section placement tweaks Andrew Burgess
2017-02-22  0:04 ` [PATCH " Andrew Burgess
2017-02-22  0:04   ` [PATCH 1/3] ld: Orphan section documentation Andrew Burgess
2017-02-22  6:28     ` Alan Modra
2017-02-22  0:04   ` [PATCH 2/3] ld: better handling of lma region for orphan sections Andrew Burgess
2017-02-22  6:32     ` Alan Modra
2017-02-22  0:04   ` [PATCH 3/3] ld: Track changes to default region LMA even for empty sections Andrew Burgess
2017-02-22  6:34     ` Alan Modra
2017-03-08 21:12   ` [PATCHv2 " Andrew Burgess
2017-03-08 21:12   ` Andrew Burgess [this message]
2017-03-08 21:12   ` [PATCHv2 1/3] ld: Orphan section documentation Andrew Burgess
2017-03-09  0:44     ` Alan Modra
2017-03-09  0:48 ` [PATCHv2 0/3] ld: Orphan section placement tweaks Alan Modra
2017-03-09 21:43   ` Andrew Burgess

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=3170b1580490e49fd65e2772d8f035eea86cb375.1489007204.git.andrew.burgess@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    /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).