Committed: bfd/mmo.c: Fix massive gcc LTO testsuite failures for MMIX.

[Hans-Peter Nilsson <hp at bitrange dot com>]

First of all, I'm slightly amazed that gcc LTO works (worked)
for mmix-knuth-mmixware, as we're doing "cross-format-linking"
from 64-bit ELF to MMO (i.e. not just cross-linking but also
using the "generic linker" code with ELF input and outputting a
non-ELF-output-format; likely a unique linker plugin setup).

But (as alluded in
<http://gcc.gnu.org/ml/gcc-patches/2015-07/msg01144.html>), ever since
binutils commit 5ae0078cd2b6b69e61 (git-bisected for one of the
failing gcc tests) every gcc LTO test has failed with a SEGV in a
similar manner.  It seems that for a symbol s in a BFD marked with
BFD_PLUGIN, s->section->output_section is NULL, causing a a SEGV when
dereferencing s->section->output_section->vma.

I'm not sure whether s->section->output_section is actually supposed
to be valid and its NULL-ness is a bug, but I prefer to code
defensively.  Also, the bfd ELF code is riddled with (abfd->flags &
BFD_PLUGIN) conditions, so let's just do that here too.  I wish I
could cover this bug trivially in the ld testsuite, but the ld-plugin
tests require gcc, and I don't keep a suitable cross-gccs when
autotesting binutils.  And, along that path, this bug is already
extensively covered by the gcc testsuite.

This fixes the first inspected SEGV, not sure if this is complete but
either way it's a first step.  Committed.

(Most of the patch is just re-indentation after gating the
for-loop with "if ((abfd->flags & BFD_PLUGIN) == 0)".)

PS. LTO debugging is (still) a pain.  I guess I should add my
notes to the DebuggingGCC section in the gcc wiki.  Later...

bfd:
    	* mmo.c (mmo_write_symbols_and_terminator): Skip symbol-type
    	assignment loop for bfd plugin objects.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 5aa84e0..fd17ed6 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,8 @@
+2015-07-29  Hans-Peter Nilsson  <hp@bitrange.com>
+
+	* mmo.c (mmo_write_symbols_and_terminator): Skip symbol-type
+	assignment loop for bfd plugin objects.
+
 2015-07-28  Alan Modra  <amodra@gmail.com>

 	* elf.c (_bfd_elf_map_sections_to_segments): Do not make a new
diff --git a/bfd/mmo.c b/bfd/mmo.c
index bdf2a12..b269ffb 100644
--- a/bfd/mmo.c
+++ b/bfd/mmo.c
@@ -2934,59 +2934,65 @@ mmo_write_symbols_and_terminator (bfd *abfd)
       count++;
     }

-  for (i = 0, serno = 1; i < count && table[i] != NULL; i++)
+  /* Don't bother inspecting symbols in plugin dummy objects; their
+     symbols aren't fully inspectable.  */
+  if ((abfd->flags & BFD_PLUGIN) == 0)
     {
-      asymbol *s = table[i];
+      for (i = 0, serno = 1; i < count && table[i] != NULL; i++)
+	{
+	  asymbol *s = table[i];

-      /* It's not enough to consult bfd_is_local_label, since it does not
-	 mean "local" in the sense of linkable-and-observable-after-link.
-	 Let's just check the BSF_GLOBAL flag.
+	  /* It's not enough to consult bfd_is_local_label, since it does not
+	     mean "local" in the sense of linkable-and-observable-after-link.
+	     Let's just check the BSF_GLOBAL flag.

-	 Also, don't export symbols with characters not in the allowed set.  */
-      if ((s->flags & (BSF_DEBUGGING|BSF_GLOBAL)) == BSF_GLOBAL
-	  && strspn (s->name,
-		     valid_mmo_symbol_character_set) == strlen (s->name))
-	{
-	  struct mmo_symbol sym;
-	  memset (&sym, 0, sizeof (sym));
-
-	  /* Need to strip const here; strdup:ing would leak and the
-	     existing string must be safe to reuse.  */
-	  sym.name = (char *) s->name;
-	  sym.value =
-	    s->value
-	    + s->section->output_section->vma
-	    + s->section->output_offset;
-
-	  if (bfd_is_und_section (s->section))
-	    sym.sym_type = mmo_undef_sym;
-	  else if (strcmp (s->section->name, MMO_DATA_SECTION_NAME) == 0
-		   /* The encoding of data symbols require that the "rest"
-		      of the value fits in 6 bytes, so the upper two bytes
-		      must be 0x2000.  All other symbols get to be the
-		      absolute type.  */
-		   && (sym.value >> 48) == 0x2000)
-	    sym.sym_type = mmo_data_sym;
-	  else if (strcmp (s->section->name, MMIX_REG_SECTION_NAME) == 0)
-	    sym.sym_type = mmo_reg_sym;
-	  else if (strcmp (s->section->name,
-			   MMIX_REG_CONTENTS_SECTION_NAME) == 0)
+	     Also, don't export symbols with characters not in the
+	     allowed set.  */
+	  if ((s->flags & (BSF_DEBUGGING|BSF_GLOBAL)) == BSF_GLOBAL
+	      && strspn (s->name,
+			 valid_mmo_symbol_character_set) == strlen (s->name))
 	    {
-	      sym.sym_type = mmo_reg_sym;
-	      sym.value /= 8;
-	    }
-	  else
-	    sym.sym_type = mmo_abs_sym;
+	      struct mmo_symbol sym;
+	      memset (&sym, 0, sizeof (sym));
+
+	      /* Need to strip const here; strdup:ing would leak and the
+		 existing string must be safe to reuse.  */
+	      sym.name = (char *) s->name;
+	      sym.value =
+		s->value
+		+ s->section->output_section->vma
+		+ s->section->output_offset;
+
+	      if (bfd_is_und_section (s->section))
+		sym.sym_type = mmo_undef_sym;
+	      else if (strcmp (s->section->name, MMO_DATA_SECTION_NAME) == 0
+		       /* The encoding of data symbols require that the "rest"
+			  of the value fits in 6 bytes, so the upper two bytes
+			  must be 0x2000.  All other symbols get to be the
+			  absolute type.  */
+		       && (sym.value >> 48) == 0x2000)
+		sym.sym_type = mmo_data_sym;
+	      else if (strcmp (s->section->name, MMIX_REG_SECTION_NAME) == 0)
+		sym.sym_type = mmo_reg_sym;
+	      else if (strcmp (s->section->name,
+			       MMIX_REG_CONTENTS_SECTION_NAME) == 0)
+		{
+		  sym.sym_type = mmo_reg_sym;
+		  sym.value /= 8;
+		}
+	      else
+		sym.sym_type = mmo_abs_sym;

-	  /* FIXME: We assume the order of the received symbols is an
-	     ordered mapping of the serial numbers.  This is not
-	     necessarily true if we e.g. objcopy a mmo file to another and
-	     there are gaps in the numbering.  Not sure if this can
-	     happen.  Not sure what to do.  */
-	  sym.serno = serno++;
+	      /* FIXME: We assume the order of the received symbols is an
+		 ordered mapping of the serial numbers.  This is not
+		 necessarily true if we e.g. objcopy a mmo file to another and
+		 there are gaps in the numbering.  Not sure if this can
+		 happen.  Not sure what to do.  */
+	      sym.serno = serno++;

-	  if (! mmo_internal_add_3_sym (abfd, &root, &sym))
-	    return FALSE;
+	      if (! mmo_internal_add_3_sym (abfd, &root, &sym))
+		return FALSE;
+	    }
 	}
     }


brgds, H-P