MSP430: ld: Update output section tail when shuffling ".either" sections

The MSP430 linker shuffles input sections with names beginning with
".either" between the upper and lower memory regions, to try to avoid
one region overflowing when there is space in the other region.

However, when an ".either" input section attached to the tail of an
output section was moved to a different output section in the other
region, that tail wasn't being updated to the new section at the end
of the original output section.

This caused a bug where a shuffled section could end up in the
middle of another section in the output executable, resulting in
corrupted code or data.

When changing the output section of an input section attached to the
tail of its output section, that tail is now updated to point to
the new input section at the end of the section list.

ld/ChangeLog:

2020-08-06  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* emultempl/msp430.em (change_output_section): Update the tail
	of the output section statement list when moving the original
	tail to a different output section.
	(eval_upper_either_sections): Don't move sections from the upper
	region to the lower region unless the upper region is
	overflowing.

diff --git a/ld/ChangeLog b/ld/ChangeLog
index 0a574db7b030332225caf239c7a01f61a2007bef..b413418471441bf2dff6a988264cc2f93bfd175d 100644 (file)
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,12 @@
+2020-08-06  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
+
+       * emultempl/msp430.em (change_output_section): Update the tail
+       of the output section statement list when moving the original
+       tail to a different output section.
+       (eval_upper_either_sections): Don't move sections from the upper
+       region to the lower region unless the upper region is
+       overflowing.
+
 2020-08-03  Alan Modra  <amodra@gmail.com>
 
        * ldlex.l (yy_create_string_buffer): Use xmalloc rather than malloc.

diff --git a/ld/emultempl/msp430.em b/ld/emultempl/msp430.em
index c823a6d9b5868846ce3c00f662462bb27c5741c6..2a840c1e2bfcbe742994adcbb38d446b8800ebd4 100644 (file)
--- a/ld/emultempl/msp430.em
+++ b/ld/emultempl/msp430.em
@@ -339,9 +339,10 @@ fi
 fragment <<EOF
 
 static bfd_boolean
-change_output_section (lang_statement_union_type ** head,
+change_output_section (lang_statement_union_type **head,
                       asection *s,
-                      lang_output_section_statement_type * new_output_section)
+                      lang_output_section_statement_type *new_os,
+                      lang_output_section_statement_type *old_os)
 {
   asection *is;
   lang_statement_union_type * prev = NULL;
@@ -356,20 +357,27 @@ change_output_section (lang_statement_union_type ** head,
          is = curr->input_section.section;
          if (is == s)
            {
+             lang_statement_list_type *old_list
+               = (lang_statement_list_type *) &old_os->children;
              s->output_section = NULL;
-             lang_add_section (& (new_output_section->children), s, NULL,
-                               new_output_section);
+             lang_add_section (&new_os->children, s, NULL, new_os);
+
              /* Remove the section from the old output section.  */
              if (prev == NULL)
                *head = curr->header.next;
              else
                prev->header.next = curr->header.next;
+             /* If the input section we just moved is the tail of the old
+                output section, then we also need to adjust that tail.  */
+             if (old_list->tail == (lang_statement_union_type **) curr)
+               old_list->tail = (lang_statement_union_type **) prev;
+
              return TRUE;
            }
          break;
        case lang_wild_statement_enum:
          if (change_output_section (&(curr->wild_statement.children.head),
-                                    s, new_output_section))
+                                    s, new_os, old_os))
            return TRUE;
          break;
        default:
@@ -606,11 +614,15 @@ eval_upper_either_sections (bfd *abfd ATTRIBUTE_UNUSED,
       upper_size = &upper_size_ram;
     }
 
-  /* Move sections in the upper region that would fit in the lower
-     region to the lower region.  */
-  if (*lower_size + s->size < lower->region->length)
+  /* If the upper region is overflowing, try moving sections to the lower
+     region.
+     Note that there isn't any general benefit to using lower memory over upper
+     memory, so we only move sections around with the goal of making the program
+     fit.  */
+  if (*upper_size > upper->region->length
+      && *lower_size + s->size < lower->region->length)
     {
-      if (change_output_section (&(upper->children.head), s, lower))
+      if (change_output_section (&(upper->children.head), s, lower, upper))
        {
          *upper_size -= s->size;
          *lower_size += s->size;
@@ -700,7 +712,7 @@ eval_lower_either_sections (bfd *abfd ATTRIBUTE_UNUSED,
     }
   /* Move sections that cause the lower region to overflow to the upper region.  */
   if (*lower_size + s->size > output_sec->region->length)
-    change_output_section (&(output_sec->children.head), s, upper);
+    change_output_section (&(output_sec->children.head), s, upper, output_sec);
   else
     *lower_size += s->size;
  end: