[PATCH 3/3] ld: Track changes to default region LMA even for empty sections

[PATCH 3/3] ld: Track changes to default region LMA even for empty sections

[Andrew Burgess]

Given a linker script fragment like this:

   SECTIONS {
     . = 0x1000;
     .text   : AT(0x100) { *(.text)   }
     .data   : AT(0x200) { *(.data)   }
     .rodata : AT(0x300) { *(.rodata) }
   }

and an input file containing sections, '.text', '.data.1', and
'.rodata', then we'd expect the linker to place '.text' and '.rodata' in
the obvious way, and the '.data.1' orphan section would be located after
the '.data' section (assuming similar section properties).

Further, I believe that the expectation would be that the LMA for the
orphan '.data.1' section would start from 0x200 (as there is no '.data'
content).

However, right now, the LMA for '.data.1' would be 0x101, following on
from the '.text' section, this is because the change in LMA for the
'.data' section is not noticed by the linker, if there's no content in
the '.data' section.

What can be even more confusing to a user (though the cause is obvious
once you understand what's going on) is that adding some content to
'.data' will cause the orphan '.data.1' to switch to an LMA based off of
0x200.

This commit changes the behaviour so that an empty section that is in
the default lma region, and sets its lma, will adjust the lma of the
default region, this change will then be reflected in following sections
within the default lma memory region.

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_size_sections_1): Shortcut loop only after
	tracking changes to the default regions LMA.
	* testsuite/ld-elf/orphan5.ld: Extend header comment.
	* testsuite/ld-elf/orphan6.d: New file.
	* testsuite/ld-elf/orphan6.s: New file.
	* NEWS: Mention change in behaviour.
---
 ld/ChangeLog                   |  9 +++++++++
 ld/NEWS                        |  3 +++
 ld/ldlang.c                    | 14 ++++++++++----
 ld/testsuite/ld-elf/orphan5.ld | 10 +++++++---
 ld/testsuite/ld-elf/orphan6.d  | 10 ++++++++++
 ld/testsuite/ld-elf/orphan6.s  |  8 ++++++++
 6 files changed, 47 insertions(+), 7 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/orphan6.d
 create mode 100644 ld/testsuite/ld-elf/orphan6.s

diff --git a/ld/ChangeLog b/ld/ChangeLog
index d011c74..c1ed5b2 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,5 +1,14 @@
 2017-02-21  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* ldlang.c (lang_size_sections_1): Shortcut loop only after
+	tracking changes to the default regions LMA.
+	* testsuite/ld-elf/orphan5.ld: Extend header comment.
+	* testsuite/ld-elf/orphan6.d: New file.
+	* testsuite/ld-elf/orphan6.s: New file.
+	* NEWS: Mention change in behaviour.
+
+2017-02-21  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.
diff --git a/ld/NEWS b/ld/NEWS
index 86e9073..972e7a8 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -8,6 +8,9 @@
 * Improve assignment of LMAs to orphan sections in some edge cases where a
   mixture of both AT>LMA_REGION and AT(LMA) are used.
 
+* Orphan sections placed after an empty section that has an AT(LMA) will now
+  take an load memory address starting from LMA.
+
 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 45dcd11..67b613f 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -5196,9 +5196,6 @@ lang_size_sections_1
 	      }
 	    os->processed_lma = TRUE;
 
-	    if (bfd_is_abs_section (os->bfd_section) || os->ignored)
-	      break;
-
 	    /* Keep track of normal sections using the default
 	       lma region.  We use this to set the lma for
 	       following sections.  Overlays or other linker
@@ -5206,7 +5203,13 @@ lang_size_sections_1
 	       default lma == vma is incorrect.
 	       To avoid warnings about dot moving backwards when using
 	       -Ttext, don't start tracking sections until we find one
-	       of non-zero size or with lma set differently to vma.  */
+	       of non-zero size or with lma set differently to vma.
+	       Do this tracking before we short-cut the loop so that we
+	       track changes for the case where the section size is zero,
+	       but the lma is set differently to the vma.  This is
+	       important, if an orphan section is placed after an
+	       otherwise empty output section that has an explicit lma
+	       set, we want that lma reflected in the orphans lma.  */
 	    if (!IGNORE_SECTION (os->bfd_section)
 		&& (os->bfd_section->size != 0
 		    || (r->last_os == NULL
@@ -5218,6 +5221,9 @@ lang_size_sections_1
 		&& !bfd_link_relocatable (&link_info))
 	      r->last_os = s;
 
+	    if (bfd_is_abs_section (os->bfd_section) || os->ignored)
+	      break;
+
 	    /* .tbss sections effectively have zero size.  */
 	    if (!IS_TBSS (os->bfd_section)
 		|| bfd_link_relocatable (&link_info))
diff --git a/ld/testsuite/ld-elf/orphan5.ld b/ld/testsuite/ld-elf/orphan5.ld
index 1c9873f..a676876 100644
--- a/ld/testsuite/ld-elf/orphan5.ld
+++ b/ld/testsuite/ld-elf/orphan5.ld
@@ -1,8 +1,12 @@
-/* This linker script is used for orphan5 test.
+/* This linker script is used for orphan5 and orphan6 test.
 
-   We have a single byte in .data, and an orphan .data.1
+   orphan5: 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.  */
+   LMA after .data rather than picking up the lma region of .rodata.
+
+   orphan6: In this case we have nothing in .data and an orphan
+   .data.1, we are checking that .data.1 is assigned an LMA after
+   .data, rather than defaulting to take LMA == VMA.  */
 
 MEMORY
 {
diff --git a/ld/testsuite/ld-elf/orphan6.d b/ld/testsuite/ld-elf/orphan6.d
new file mode 100644
index 0000000..fc94b26
--- /dev/null
+++ b/ld/testsuite/ld-elf/orphan6.d
@@ -0,0 +1,10 @@
+#source: orphan6.s
+#ld: -N -T orphan5.ld
+#objdump: -h
+#notarget: cr16* crx* d30v* dlx* epiphany* fido* fr30* frv* ft32* ip2k* iq2000* m68k* mips* mn10200* moxie* ms1* msp430* mt* pj* powerpc* spu* tile* tx39*
+
+#...
+  . \.text         0+[14]  [0-9a-f]+  0+200 .*
+#...
+  . \.data\.1       0+[14]  [0-9a-f]+  0+300 .*
+#pass
diff --git a/ld/testsuite/ld-elf/orphan6.s b/ld/testsuite/ld-elf/orphan6.s
new file mode 100644
index 0000000..da83927
--- /dev/null
+++ b/ld/testsuite/ld-elf/orphan6.s
@@ -0,0 +1,8 @@
+        .text
+        .byte 0
+
+        .section ".data.1", "aw"
+        .byte 0
+
+        .section ".rodata", "a"
+        .byte 0
-- 
2.5.1

[PATCH 2/3] ld: better handling of lma region for orphan sections

[Andrew Burgess]

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/orphan5.d: New file.
	* testsuite/ld-elf/orphan5.ld: New file.
	* testsuite/ld-elf/orphan5.s: New file.
	* NEWS: Mention change in behaviour.
---
 ld/ChangeLog                   | 11 +++++++++++
 ld/NEWS                        |  3 +++
 ld/ldlang.c                    | 34 ++++++++++++++++++++++++----------
 ld/testsuite/ld-elf/orphan5.d  | 12 ++++++++++++
 ld/testsuite/ld-elf/orphan5.ld | 28 ++++++++++++++++++++++++++++
 ld/testsuite/ld-elf/orphan5.s  | 11 +++++++++++
 6 files changed, 89 insertions(+), 10 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/orphan5.d
 create mode 100644 ld/testsuite/ld-elf/orphan5.ld
 create mode 100644 ld/testsuite/ld-elf/orphan5.s

diff --git a/ld/ChangeLog b/ld/ChangeLog
index c73a5c3..d011c74 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,14 @@
+2017-02-21  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/orphan5.d: New file.
+	* testsuite/ld-elf/orphan5.ld: New file.
+	* testsuite/ld-elf/orphan5.s: New file.
+	* NEWS: Mention change in behaviour.
+
 2017-02-21  Nick Clifton  <nickc@redhat.com>
 
 	* testsuite/ld-ifunc/pr18808b.c (bar): Fix compile time warning
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 1396c5b..45dcd11 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6843,6 +6843,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)
 {
@@ -7021,6 +7042,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 ();
@@ -7301,16 +7325,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/orphan5.d b/ld/testsuite/ld-elf/orphan5.d
new file mode 100644
index 0000000..d67f0d7
--- /dev/null
+++ b/ld/testsuite/ld-elf/orphan5.d
@@ -0,0 +1,12 @@
+#source: orphan5.s
+#ld: -N -T orphan5.ld
+#objdump: -h
+#notarget: cr16* crx* d30v* dlx* epiphany* fido* fr30* frv* ft32* ip2k* iq2000* m68k* mips* mn10200* moxie* ms1* msp430* mt* pj* powerpc* spu* tile* tx39*
+
+#...
+  . \.text         0+[14]  [0-9a-f]+  0+200 .*
+#...
+  . \.data         0+[14]  [0-9a-f]+  0+300 .*
+#...
+  . \.data\.1       0+[14]  [0-9a-f]+  0+30[0-9a-f] .*
+#pass
diff --git a/ld/testsuite/ld-elf/orphan5.ld b/ld/testsuite/ld-elf/orphan5.ld
new file mode 100644
index 0000000..1c9873f
--- /dev/null
+++ b/ld/testsuite/ld-elf/orphan5.ld
@@ -0,0 +1,28 @@
+/* This linker script is used for orphan5 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 = 0x10
+   DATA   : ORIGIN = 0x300,  LENGTH = 0x10
+   RODATA : ORIGIN = 0x400,  LENGTH = 0x10
+}
+
+SECTIONS
+{
+  .text : {
+    *(.text)
+  } >MEM AT>TEXT
+
+  .data : AT(0x300) {
+    *(.data)
+  } >MEM
+
+  .rodata : {
+    *(.rodata)
+  } >MEM AT>RODATA
+}
diff --git a/ld/testsuite/ld-elf/orphan5.s b/ld/testsuite/ld-elf/orphan5.s
new file mode 100644
index 0000000..3811799
--- /dev/null
+++ b/ld/testsuite/ld-elf/orphan5.s
@@ -0,0 +1,11 @@
+        .text
+        .byte 0
+
+        .data
+        .byte 0
+
+        .section ".data.1", "aw"
+        .byte 0
+
+        .section ".rodata", "a"
+        .byte 0
-- 
2.5.1

[PATCH 1/3] ld: Orphan section documentation

[Andrew Burgess]

Make more explicit mention of the fact that orphan sections cause a new
output section to be created.  Though this information is clearly
implied in the manual it might not be clear enough.

A user _might_ (incorrectly) think that orphan sections could be
inserted into an existing output section.

ld/ChangeLog:

	* ld.texinfo (Orphan Sections): Add more detail.
---
 ld/ld.texinfo | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index b128451..24504da 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -5762,10 +5762,12 @@
 Orphan sections are sections present in the input files which
 are not explicitly placed into the output file by the linker
 script.  The linker will still copy these sections into the
-output file, but it has to guess as to where they should be
-placed.  The linker uses a simple heuristic to do this.  It
-attempts to place orphan sections after non-orphan sections of the
-same attribute, such as code vs data, loadable vs non-loadable, etc.
+output file, creating a new output section to hold the orphans,
+but it has to guess as to where the new output section should
+be placed.  The linker uses a simple heuristic to do this.  It
+attempts to place orphan sections after non-orphan sections of
+the same attribute, such as code vs data, loadable vs
+non-loadable, etc.
 If there is not enough room to do this then it places
 at the end of the file.
 
-- 
2.5.1

[PATCH 0/3] ld: Orphan section placement tweaks

[Andrew Burgess]

This series is primarily intended to address two edge cases relating
to the placement of orphan sections where I think the current linker
behaviour is non-obvious.  Both cases relate to how LMAs are assigned
to orphan sections.

The first patch is a small documentation extension to make crystal
clear somethng which I don't think is spelled out clearly enough in
the manual right now.

I have run this series against a wide range of targets for testing,
with no regressions.  I did however, block the new tests from running
on a moderately sized sub-set of targets; the new tests make use of
rather restrictive linker scripts, the blocked targets all failed to
link or pass due to additional target-specific sections messing up the
results, or the target not being happy with the memory regions in the
test.  I figure that the large number of targets the tests do run on
should be enough - let me know if this is a problem.

Thanks,
Andrew

---

Andrew Burgess (3):
  ld: Orphan section documentation
  ld: better handling of lma region for orphan sections
  ld: Track changes to default region LMA even for empty sections

 ld/ChangeLog                   | 20 ++++++++++++++++++
 ld/NEWS                        |  6 ++++++
 ld/ld.texinfo                  | 10 +++++----
 ld/ldlang.c                    | 48 ++++++++++++++++++++++++++++++------------
 ld/testsuite/ld-elf/orphan5.d  | 12 +++++++++++
 ld/testsuite/ld-elf/orphan5.ld | 32 ++++++++++++++++++++++++++++
 ld/testsuite/ld-elf/orphan5.s  | 11 ++++++++++
 ld/testsuite/ld-elf/orphan6.d  | 10 +++++++++
 ld/testsuite/ld-elf/orphan6.s  |  8 +++++++
 9 files changed, 139 insertions(+), 18 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/orphan5.d
 create mode 100644 ld/testsuite/ld-elf/orphan5.ld
 create mode 100644 ld/testsuite/ld-elf/orphan5.s
 create mode 100644 ld/testsuite/ld-elf/orphan6.d
 create mode 100644 ld/testsuite/ld-elf/orphan6.s

-- 
2.5.1

Re: [PATCH 1/3] ld: Orphan section documentation

[Alan Modra]

On Wed, Feb 22, 2017 at 12:04:05AM +0000, Andrew Burgess wrote:
> Make more explicit mention of the fact that orphan sections cause a new
> output section to be created.  Though this information is clearly
> implied in the manual it might not be clear enough.
> 
> A user _might_ (incorrectly) think that orphan sections could be
> inserted into an existing output section.
> 
> ld/ChangeLog:
> 
> 	* ld.texinfo (Orphan Sections): Add more detail.

OK, with one change.

> ---
>  ld/ld.texinfo | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/ld/ld.texinfo b/ld/ld.texinfo
> index b128451..24504da 100644
> --- a/ld/ld.texinfo
> +++ b/ld/ld.texinfo
> @@ -5762,10 +5762,12 @@
>  Orphan sections are sections present in the input files which
>  are not explicitly placed into the output file by the linker
>  script.  The linker will still copy these sections into the
> -output file, but it has to guess as to where they should be
> -placed.  The linker uses a simple heuristic to do this.  It
> -attempts to place orphan sections after non-orphan sections of the
> -same attribute, such as code vs data, loadable vs non-loadable, etc.
> +output file, creating a new output section to hold the orphans,

s/a new output section/new output sections/

> +but it has to guess as to where the new output section should
> +be placed.  The linker uses a simple heuristic to do this.  It
> +attempts to place orphan sections after non-orphan sections of
> +the same attribute, such as code vs data, loadable vs
> +non-loadable, etc.
>  If there is not enough room to do this then it places
>  at the end of the file.
>  
> -- 
> 2.5.1
> 

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/3] ld: better handling of lma region for orphan sections

[Alan Modra]

On Wed, Feb 22, 2017 at 12:04:06AM +0000, Andrew Burgess wrote:
> 	* 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/orphan5.d: New file.
> 	* testsuite/ld-elf/orphan5.ld: New file.
> 	* testsuite/ld-elf/orphan5.s: New file.
> 	* NEWS: Mention change in behaviour.

OK, but

> --- /dev/null
> +++ b/ld/testsuite/ld-elf/orphan5.s
> @@ -0,0 +1,11 @@
> +        .text
> +        .byte 0

Make this 8 bytes instead, ditto below.  You will of course need to
adjust orphan5.d to suit, and that should allow most of the notarget
entries to be removed.

> +
> +        .data
> +        .byte 0
> +
> +        .section ".data.1", "aw"
> +        .byte 0
> +
> +        .section ".rodata", "a"
> +        .byte 0

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 3/3] ld: Track changes to default region LMA even for empty sections

[Alan Modra]

On Wed, Feb 22, 2017 at 12:04:07AM +0000, Andrew Burgess wrote:
> 	* ldlang.c (lang_size_sections_1): Shortcut loop only after
> 	tracking changes to the default regions LMA.
> 	* testsuite/ld-elf/orphan5.ld: Extend header comment.
> 	* testsuite/ld-elf/orphan6.d: New file.
> 	* testsuite/ld-elf/orphan6.s: New file.
> 	* NEWS: Mention change in behaviour.

OK, but please adjust the testcase as per my comments on orphan5.

-- 
Alan Modra
Australia Development Lab, IBM

[PATCHv2 3/3] ld: Track changes to default region LMA even for empty sections

[Andrew Burgess]

Given a linker script fragment like this:

   SECTIONS {
     . = 0x1000;
     .text   : AT(0x100) { *(.text)   }
     .data   : AT(0x200) { *(.data)   }
     .rodata : AT(0x300) { *(.rodata) }
   }

and an input file containing sections, '.text', '.data.1', and
'.rodata', then we'd expect the linker to place '.text' and '.rodata' in
the obvious way, and the '.data.1' orphan section would be located after
the '.data' section (assuming similar section properties).

Further, I believe that the expectation would be that the LMA for the
orphan '.data.1' section would start from 0x200 (as there is no '.data'
content).

However, right now, the LMA for '.data.1' would be 0x101, following on
from the '.text' section, this is because the change in LMA for the
'.data' section is not noticed by the linker, if there's no content in
the '.data' section.

What can be even more confusing to a user (though the cause is obvious
once you understand what's going on) is that adding some content to
'.data' will cause the orphan '.data.1' to switch to an LMA based off of
0x200.

This commit changes the behaviour so that an empty section that is in
the default lma region, and sets its lma, will adjust the lma of the
default region, this change will then be reflected in following sections
within the default lma memory region.

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_size_sections_1): Shortcut loop only after
	tracking changes to the default regions LMA.
	* testsuite/ld-elf/orphan-9.ld: Extend header comment.
	* testsuite/ld-elf/orphan-10.d: New file.
	* testsuite/ld-elf/orphan-10.s: New file.
	* NEWS: Mention change in behaviour.
---
 ld/ChangeLog                    |  9 +++++++++
 ld/NEWS                         |  3 +++
 ld/ldlang.c                     | 14 ++++++++++----
 ld/testsuite/ld-elf/orphan-10.d | 10 ++++++++++
 ld/testsuite/ld-elf/orphan-10.s |  8 ++++++++
 ld/testsuite/ld-elf/orphan-9.ld | 10 +++++++---
 6 files changed, 47 insertions(+), 7 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/orphan-10.d
 create mode 100644 ld/testsuite/ld-elf/orphan-10.s

diff --git a/ld/ChangeLog b/ld/ChangeLog
index 5a06195..8df9394 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,5 +1,14 @@
 2017-03-08  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* ldlang.c (lang_size_sections_1): Shortcut loop only after
+	tracking changes to the default regions LMA.
+	* testsuite/ld-elf/orphan-9.ld: Extend header comment.
+	* testsuite/ld-elf/orphan-10.d: New file.
+	* testsuite/ld-elf/orphan-10.s: New file.
+	* NEWS: Mention change in behaviour.
+
+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.
diff --git a/ld/NEWS b/ld/NEWS
index 86e9073..972e7a8 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -8,6 +8,9 @@
 * Improve assignment of LMAs to orphan sections in some edge cases where a
   mixture of both AT>LMA_REGION and AT(LMA) are used.
 
+* Orphan sections placed after an empty section that has an AT(LMA) will now
+  take an load memory address starting from LMA.
+
 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 89b896f..6011a00 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -5197,9 +5197,6 @@ lang_size_sections_1
 	      }
 	    os->processed_lma = TRUE;
 
-	    if (bfd_is_abs_section (os->bfd_section) || os->ignored)
-	      break;
-
 	    /* Keep track of normal sections using the default
 	       lma region.  We use this to set the lma for
 	       following sections.  Overlays or other linker
@@ -5207,7 +5204,13 @@ lang_size_sections_1
 	       default lma == vma is incorrect.
 	       To avoid warnings about dot moving backwards when using
 	       -Ttext, don't start tracking sections until we find one
-	       of non-zero size or with lma set differently to vma.  */
+	       of non-zero size or with lma set differently to vma.
+	       Do this tracking before we short-cut the loop so that we
+	       track changes for the case where the section size is zero,
+	       but the lma is set differently to the vma.  This is
+	       important, if an orphan section is placed after an
+	       otherwise empty output section that has an explicit lma
+	       set, we want that lma reflected in the orphans lma.  */
 	    if (!IGNORE_SECTION (os->bfd_section)
 		&& (os->bfd_section->size != 0
 		    || (r->last_os == NULL
@@ -5219,6 +5222,9 @@ lang_size_sections_1
 		&& !bfd_link_relocatable (&link_info))
 	      r->last_os = s;
 
+	    if (bfd_is_abs_section (os->bfd_section) || os->ignored)
+	      break;
+
 	    /* .tbss sections effectively have zero size.  */
 	    if (!IS_TBSS (os->bfd_section)
 		|| bfd_link_relocatable (&link_info))
diff --git a/ld/testsuite/ld-elf/orphan-10.d b/ld/testsuite/ld-elf/orphan-10.d
new file mode 100644
index 0000000..a5cf6d9
--- /dev/null
+++ b/ld/testsuite/ld-elf/orphan-10.d
@@ -0,0 +1,10 @@
+#source: orphan-10.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\.1       0+8  [0-9a-f]+  0+300 .*
+#pass
diff --git a/ld/testsuite/ld-elf/orphan-10.s b/ld/testsuite/ld-elf/orphan-10.s
new file mode 100644
index 0000000..b198966
--- /dev/null
+++ b/ld/testsuite/ld-elf/orphan-10.s
@@ -0,0 +1,8 @@
+        .text
+        .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
diff --git a/ld/testsuite/ld-elf/orphan-9.ld b/ld/testsuite/ld-elf/orphan-9.ld
index 1a6773d..d8a3917 100644
--- a/ld/testsuite/ld-elf/orphan-9.ld
+++ b/ld/testsuite/ld-elf/orphan-9.ld
@@ -1,8 +1,12 @@
-/* This linker script is used for orphan-9 test.
+/* This linker script is used for orphan-9 and orphan-10 test.
 
-   We have a single byte in .data, and an orphan .data.1
+   orphan-9: 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.  */
+   LMA after .data rather than picking up the lma region of .rodata.
+
+   orphan-10: In this case we have nothing in .data and an orphan
+   .data.1, we are checking that .data.1 is assigned an LMA after
+   .data, rather than defaulting to take LMA == VMA.  */
 
 MEMORY
 {
-- 
2.5.1

[PATCHv2 0/3] ld: Orphan section placement tweaks

[Andrew Burgess]

This patch series is mostly the same as the original series I sent.
I've addressed all of the points that Alan raised, so really I think
patches 2 & 3 are pre-approved.

The change is in patch 1.  In my "excitment" to make sure that the
documentation mentioned how orphan placement might create a new ouput
section, I made it sound like creating a new output section was the
_only_ option when placing an orphaned section, which isn't true at all.

This new version of patch 1 tries to make all of the options for
orphan placement clearer.

Feedback welcome.

Thanks,
Andrew


---

Andrew Burgess (3):
  ld: Orphan section documentation
  ld: better handling of lma region for orphan sections
  ld: Track changes to default region LMA even for empty sections

 ld/ChangeLog                    | 20 +++++++++++++++++
 ld/NEWS                         |  6 ++++++
 ld/ld.texinfo                   | 26 ++++++++++++++++------
 ld/ldlang.c                     | 48 +++++++++++++++++++++++++++++------------
 ld/testsuite/ld-elf/orphan-10.d | 10 +++++++++
 ld/testsuite/ld-elf/orphan-10.s |  8 +++++++
 ld/testsuite/ld-elf/orphan-9.d  | 12 +++++++++++
 ld/testsuite/ld-elf/orphan-9.ld | 32 +++++++++++++++++++++++++++
 ld/testsuite/ld-elf/orphan-9.s  | 11 ++++++++++
 9 files changed, 153 insertions(+), 20 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/orphan-10.d
 create mode 100644 ld/testsuite/ld-elf/orphan-10.s
 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

-- 
2.5.1

[PATCHv2 1/3] ld: Orphan section documentation

[Andrew Burgess]

Make more explicit mention of the fact that orphan sections can cause a
new output section to be created.  Though this information is clearly
implied in the manual it might not be clear enough.

A user _might_ (incorrectly) think that orphan sections can only be
inserted into an existing output section.

ld/ChangeLog:

	* ld.texinfo (Orphan Sections): Add more detail.
---
 ld/ld.texinfo | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index b128451..1abc9a7 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -5762,12 +5762,26 @@
 Orphan sections are sections present in the input files which
 are not explicitly placed into the output file by the linker
 script.  The linker will still copy these sections into the
-output file, but it has to guess as to where they should be
-placed.  The linker uses a simple heuristic to do this.  It
-attempts to place orphan sections after non-orphan sections of the
-same attribute, such as code vs data, loadable vs non-loadable, etc.
-If there is not enough room to do this then it places
-at the end of the file.
+output file by either finding, or creating a suitable output section
+in which to place the orphaned input section.
+
+If the name of an orphaned input section exactly matches the name of
+an existing output section, then the orphaned input section will be
+placed at the end of that output section.
+
+If there is no output section with a matching name then new output
+sections will be created.  Each new output section will have the same
+name as the orphan section placed within it.  If there are multiple
+orphan sections with the same name, these will all be combined into
+one new output section.
+
+If new output sections are created to hold orphaned input sections,
+then the linker must decide where to place these new output sections
+in relation to existing output sections.  The linker uses a simple
+heuristic to do this.  It attempts to place orphan sections after
+non-orphan sections of the same attribute, such as code vs data,
+loadable vs non-loadable, etc.  If there is not enough room to do this
+then it places at the end of the file.
 
 For ELF targets, the attribute of the section includes section type as
 well as section flag.
-- 
2.5.1

[PATCHv2 2/3] ld: better handling of lma region for orphan sections

[Andrew Burgess]

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

Re: [PATCHv2 1/3] ld: Orphan section documentation

[Alan Modra]

On Wed, Mar 08, 2017 at 09:12:37PM +0000, Andrew Burgess wrote:
> Make more explicit mention of the fact that orphan sections can cause a
> new output section to be created.  Though this information is clearly
> implied in the manual it might not be clear enough.
> 
> A user _might_ (incorrectly) think that orphan sections can only be
> inserted into an existing output section.
> 
> ld/ChangeLog:
> 
> 	* ld.texinfo (Orphan Sections): Add more detail.
> ---
>  ld/ld.texinfo | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/ld/ld.texinfo b/ld/ld.texinfo
> index b128451..1abc9a7 100644
> --- a/ld/ld.texinfo
> +++ b/ld/ld.texinfo
> @@ -5762,12 +5762,26 @@
>  Orphan sections are sections present in the input files which
>  are not explicitly placed into the output file by the linker
>  script.  The linker will still copy these sections into the
> -output file, but it has to guess as to where they should be
> -placed.  The linker uses a simple heuristic to do this.  It
> -attempts to place orphan sections after non-orphan sections of the
> -same attribute, such as code vs data, loadable vs non-loadable, etc.
> -If there is not enough room to do this then it places
> -at the end of the file.
> +output file by either finding, or creating a suitable output section
> +in which to place the orphaned input section.
> +
> +If the name of an orphaned input section exactly matches the name of
> +an existing output section, then the orphaned input section will be
> +placed at the end of that output section.
> +
> +If there is no output section with a matching name then new output
> +sections will be created.  Each new output section will have the same
> +name as the orphan section placed within it.  If there are multiple
> +orphan sections with the same name, these will all be combined into
> +one new output section.
> +
> +If new output sections are created to hold orphaned input sections,
> +then the linker must decide where to place these new output sections
> +in relation to existing output sections.  The linker uses a simple
> +heuristic to do this.  It attempts to place orphan sections after
> +non-orphan sections of the same attribute, such as code vs data,

Not a problem with doc you've added, but please delete "non-orphan"
here.  An orphan may well be placed after another orphan of the same
attribute.

> +loadable vs non-loadable, etc.  If there is not enough room to do this
> +then it places at the end of the file.

The last sentence here is bogus too.  What does room have to do with
it?  Maybe "If no sections with matching attributes are found, the
orphan section is placed at the end of the file."

Otherwise looks good.  Please apply.

>  For ELF targets, the attribute of the section includes section type as
>  well as section flag.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCHv2 0/3] ld: Orphan section placement tweaks

[Alan Modra]

On Wed, Mar 08, 2017 at 09:12:36PM +0000, Andrew Burgess wrote:
> This patch series is mostly the same as the original series I sent.
> I've addressed all of the points that Alan raised, so really I think
> patches 2 & 3 are pre-approved.

Yes, patch 2 and 3 are OK, and I've commented separately on patch 1.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCHv2 0/3] ld: Orphan section placement tweaks

[Andrew Burgess]

* Alan Modra <amodra@gmail.com> [2017-03-09 11:18:43 +1030]:

> On Wed, Mar 08, 2017 at 09:12:36PM +0000, Andrew Burgess wrote:
> > This patch series is mostly the same as the original series I sent.
> > I've addressed all of the points that Alan raised, so really I think
> > patches 2 & 3 are pre-approved.
> 
> Yes, patch 2 and 3 are OK, and I've commented separately on patch 1.

Thanks for your reviews.  I've pushed all 3 patches with the doc fix
you suggested.

Thanks again,
Andrew