Re: 2.29.1 Release Imminent

[Alan Modra <amodra at gmail dot com>]

On Thu, Sep 14, 2017 at 05:42:30PM +0930, Alan Modra wrote:
> On Wed, Sep 13, 2017 at 05:43:05PM +0200, Matthias Klose wrote:
> > On 13.09.2017 16:16, Alan Modra wrote:
> > > On Mon, Sep 11, 2017 at 06:36:36PM +0200, Matthias Klose wrote:
> > >> this weekend I got https://bugs.debian.org/874674, which might be a regression
> > >> compared to 2.28. I don't have further information on this yet.
> > > 
> > > Hmm, aborting at ../../bfd/merge.c:908.  llvm built with gcc-7 using
> > > -g.  So you'll have a very likely huge .debug_str in total for the
> > > merge code to handle.  I wonder if you are getting a memory alloc
> > > failure, with binutils somewhere not checking malloc return status?
> > 
> > that might be related to another issue I'm seeing with llvm builds on aarch64
> > and ppc64le, where the size of the debug info explodes by a factor of five, but
> > the linker doesn't crash. It's llvm building with
> > -DCMAKE_BUILD_TYPE=RelWithDebInfo, and it's going away with
> > -DCMAKE_CXX_FLAGS_RELWITHDEBINFO="-g1 -O2".  So the original problem might be a
> > GCC 7 issue.
> 
> Another possibility occurs to me.  We've seen the merge.c abort before
> when ld read an object file before it had been completely written.  So
> the llvm build may have a dependency problem.

I was looking at this issue again today, and think I might have
spotted the problem.  It appears that merge.c tries to cope with
memory allocation failures in some circumstances, but doesn't quite
manage to get everything right.  This patch will make ld report memory
allocation failures instead of silently not merging strings.

	* merge.c (merge_strings): Return FALSE on malloc failure.
	(_bfd_merge_sections): Return failures from record_section and
	merge_strings.

diff --git a/bfd/merge.c b/bfd/merge.c
index a1792a8..ad8db83 100644
--- a/bfd/merge.c
+++ b/bfd/merge.c
@@ -609,7 +609,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 void
+static bfd_boolean
 merge_strings (struct sec_merge_info *sinfo)
 {
   struct sec_merge_hash_entry **array, **a, *e;
@@ -621,7 +621,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)
-    goto alloc_failure;
+    return FALSE;
 
   for (e = sinfo->htab->first, a = array; e; e = e->next)
     if (e->alignment)
@@ -666,9 +666,7 @@ merge_strings (struct sec_merge_info *sinfo)
 	}
     }
 
-alloc_failure:
-  if (array)
-    free (array);
+  free (array);
 
   /* Now assign positions to the strings we want to keep.  */
   size = 0;
@@ -714,6 +712,7 @@ alloc_failure:
 	    e->u.index = e->u.suffix->u.index + (e->u.suffix->len - e->len);
 	  }
       }
+  return TRUE;
 }
 
 /* This function is called once after all SEC_MERGE sections are registered
@@ -748,7 +747,7 @@ _bfd_merge_sections (bfd *abfd,
 	      (*remove_hook) (abfd, secinfo->sec);
 	  }
 	else if (! record_section (sinfo, secinfo))
-	  break;
+	  return FALSE;
 
       if (secinfo)
 	continue;
@@ -757,7 +756,10 @@ _bfd_merge_sections (bfd *abfd,
 	continue;
 
       if (sinfo->htab->strings)
-	merge_strings (sinfo);
+	{
+	  if (!merge_strings (sinfo))
+	    return FALSE;
+	}
       else
 	{
 	  struct sec_merge_hash_entry *e;


-- 
Alan Modra
Australia Development Lab, IBM