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.

2 files changed, 31 insertions, 10 deletions

diff --git a/ld/ChangeLog b/ld/ChangeLog
index 0a574db..b413418 100644
--- 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 c823a6d..2a840c1 100644
--- 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: