[binutils-gdb] PR24311, FAIL: S-records with constructors

[Alan Modra <amodra at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ebd2263ba9a9124d93bbc0ece63d7e0fae89b40e

commit ebd2263ba9a9124d93bbc0ece63d7e0fae89b40e
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Mar 8 22:41:55 2019 +1030

    PR24311, FAIL: S-records with constructors
    
    Not padding string merge section output to its alignment can cause
    failures of the S-record tests when input string merge sections are
    padded, since the ELF linker output for the single string section
    would shrink compared to the SREC linker output.  That might result in
    following sections having different addresses.
    On the other hand, padding string merge section output when input
    string merge sections are *not* padded can also cause failures, in
    this case due to the ELF linker output for the string section being
    larger (due to padding) than the SREC linker output.
    
    It would be better to write a more robust test, but it is also nice
    to leave input unchanged when no string merges occur.
    
    	PR 24311
    	* merge.c (merge_strings): Return secinfo.  Don't pad section
    	to alignment here.
    	(_bfd_merge_sections): Pad section to alignment here, if input
    	sections contributing to merged output all pad to alignment.
    	Formatting.

Diff:
---
 bfd/ChangeLog |  9 +++++++++
 bfd/merge.c   | 52 ++++++++++++++++++++++++++++++----------------------
 2 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 15a5a99..18f1804 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,12 @@
+2019-03-08  Alan Modra  <amodra@gmail.com>
+
+	PR 24311
+	* merge.c (merge_strings): Return secinfo.  Don't pad section
+	to alignment here.
+	(_bfd_merge_sections): Pad section to alignment here, if input
+	sections contributing to merged output all pad to alignment.
+	Formatting.
+
 2019-03-06  Nick Clifton  <nickc@redhat.com>
 
 	* dwarf2.c (_bfd_dwarf2_find_symbol_bias): Check for a NULL symbol
diff --git a/bfd/merge.c b/bfd/merge.c
index fb7c085..632c852 100644
--- a/bfd/merge.c
+++ b/bfd/merge.c
@@ -621,7 +621,7 @@ is_suffix (const struct sec_merge_hash_entry *A,
 
 /* This is a helper function for _bfd_merge_sections.  It attempts to
    merge strings matching suffixes of longer strings.  */
-static bfd_boolean
+static struct sec_merge_sec_info *
 merge_strings (struct sec_merge_info *sinfo)
 {
   struct sec_merge_hash_entry **array, **a, *e;
@@ -633,7 +633,7 @@ merge_strings (struct sec_merge_info *sinfo)
   amt = sinfo->htab->size * sizeof (struct sec_merge_hash_entry *);
   array = (struct sec_merge_hash_entry **) bfd_malloc (amt);
   if (array == NULL)
-    return FALSE;
+    return NULL;
 
   for (e = sinfo->htab->first, a = array; e; e = e->next)
     if (e->alignment)
@@ -703,11 +703,6 @@ merge_strings (struct sec_merge_info *sinfo)
 	}
     }
   secinfo->sec->size = size;
-  if (secinfo->sec->alignment_power != 0)
-    {
-      bfd_size_type align = (bfd_size_type) 1 << secinfo->sec->alignment_power;
-      secinfo->sec->size = (secinfo->sec->size + align - 1) & -align;
-    }
 
   /* And now adjust the rest, removing them from the chain (but not hashtable)
      at the same time.  */
@@ -724,7 +719,7 @@ merge_strings (struct sec_merge_info *sinfo)
 	    e->u.index = e->u.suffix->u.index + (e->u.suffix->len - e->len);
 	  }
       }
-  return TRUE;
+  return secinfo;
 }
 
 /* This function is called once after all SEC_MERGE sections are registered
@@ -740,7 +735,8 @@ _bfd_merge_sections (bfd *abfd,
 
   for (sinfo = (struct sec_merge_info *) xsinfo; sinfo; sinfo = sinfo->next)
     {
-      struct sec_merge_sec_info * secinfo;
+      struct sec_merge_sec_info *secinfo;
+      bfd_size_type align;
 
       if (! sinfo->chain)
 	continue;
@@ -751,6 +747,7 @@ _bfd_merge_sections (bfd *abfd,
       secinfo->next = NULL;
 
       /* Record the sections into the hash table.  */
+      align = 1;
       for (secinfo = sinfo->chain; secinfo; secinfo = secinfo->next)
 	if (secinfo->sec->flags & SEC_EXCLUDE)
 	  {
@@ -758,18 +755,25 @@ _bfd_merge_sections (bfd *abfd,
 	    if (remove_hook)
 	      (*remove_hook) (abfd, secinfo->sec);
 	  }
-	else if (! record_section (sinfo, secinfo))
-	  return FALSE;
-
-      if (secinfo)
-	continue;
+	else
+	  {
+	    if (!record_section (sinfo, secinfo))
+	      return FALSE;
+	    if (align)
+	      {
+		align = (bfd_size_type) 1 << secinfo->sec->alignment_power;
+		if ((secinfo->sec->size & (align - 1)) != 0)
+		  align = 0;
+	      }
+	  }
 
       if (sinfo->htab->first == NULL)
 	continue;
 
       if (sinfo->htab->strings)
 	{
-	  if (!merge_strings (sinfo))
+	  secinfo = merge_strings (sinfo);
+	  if (!secinfo)
 	    return FALSE;
 	}
       else
@@ -789,8 +793,7 @@ _bfd_merge_sections (bfd *abfd,
 		  e->secinfo->first_str = e;
 		  size = 0;
 		}
-	      size = (size + e->alignment - 1)
-		     & ~((bfd_vma) e->alignment - 1);
+	      size = (size + e->alignment - 1) & ~((bfd_vma) e->alignment - 1);
 	      e->u.index = size;
 	      size += e->len;
 	      secinfo = e->secinfo;
@@ -798,11 +801,16 @@ _bfd_merge_sections (bfd *abfd,
 	  secinfo->sec->size = size;
 	}
 
-	/* Finally remove all input sections which have not made it into
-	   the hash table at all.  */
-	for (secinfo = sinfo->chain; secinfo; secinfo = secinfo->next)
-	  if (secinfo->first_str == NULL)
-	    secinfo->sec->flags |= SEC_EXCLUDE | SEC_KEEP;
+      /* If the input sections were padded according to their alignments,
+	 then pad the output too.  */
+      if (align)
+	secinfo->sec->size = (secinfo->sec->size + align - 1) & -align;
+
+      /* Finally remove all input sections which have not made it into
+	 the hash table at all.  */
+      for (secinfo = sinfo->chain; secinfo; secinfo = secinfo->next)
+	if (secinfo->first_str == NULL)
+	  secinfo->sec->flags |= SEC_EXCLUDE | SEC_KEEP;
     }
 
   return TRUE;