Re: RFC: Possible tweak to MIPS16/microMIPS PLT choice

[Richard Sandiford <rdsandiford at googlemail dot com>]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> While writing some tests for the MIPS16 and microMIPS PLT stuff
> to address http://article.gmane.org/gmane.comp.gnu.binutils/61257
> there was one corner case where the handling seemed a bit counter-
> intuitive.  If a function is referenced by both HI/LO relocations
> and a JAL relocation, the ISA encoding of the JAL decides the encoding
> of the PLT, as intended.  But if a function is referenced by both HI/LO
> relocations and a GOT CALL relocation (but not by JAL relocations),
> the ISA encoding of the GOT CALL reference doesn't influence the encoding
> of the PLT.  This means that:
>
> 1. microMIPS CALL reloc + HI/LO ref -> microMIPS PLT
> 2. MIPS16 CALL reloc    + HI/LO ref -> MIPS PLT
> 3. MIPS CALL reloc      + HI/LO ref in non-microMIPS object -> MIPS PLT
> 4. MIPS CALL reloc      + HI/LO ref in microMIPS object     -> microMIPS PLT
>
> (1) and (3) are obviously right, but (2) and (4) were less obvious.
> With a patch like the attached, we'd use the GOT CALL relocs as a
> fall-back when choosing the encoding.  No change log, since it's an RFC.
>
> I don't really have a strong opinion either way.  It's just that these
> things are harder to change once they're enshrined in tests, so I'd like
> to be sure of the behaviour.

Er, please pretend that the patch I sent was this one.

Thanks,
Richard


Index: bfd/elfxx-mips.c
===================================================================
--- bfd/elfxx-mips.c	2013-10-05 15:50:03.060932500 +0100
+++ bfd/elfxx-mips.c	2013-10-06 11:46:40.496809801 +0100
@@ -341,6 +341,14 @@ struct plt_entry
 
   /* Whether we need a compressed PLT entry.  */
   unsigned int need_comp : 1;
+
+  /* Whether there are standard-encoding GOT CALL references
+     to the symbol.  */
+  unsigned int suggest_mips : 1;
+
+  /* Whether there are compressed-encoding GOT CALL references
+     to the symbol.  */
+  unsigned int suggest_comp : 1;
 };
 
 /* The MIPS ELF linker needs additional information for each symbol in
@@ -8181,6 +8189,15 @@ _bfd_mips_elf_check_relocs (bfd *abfd, s
 		 _bfd_elf_adjust_dynamic_symbol.  */
 	      h->needs_plt = 1;
 	      h->type = STT_FUNC;
+
+	      if (h->plt.plist == NULL)
+		h->plt.plist = mips_elf_make_plt_record (abfd);
+	      if (h->plt.plist == NULL)
+		return FALSE;
+	      if (mips16_reloc_p (r_type) || micromips_reloc_p (r_type))
+		h->plt.plist->suggest_comp = TRUE;
+	      else
+		h->plt.plist->suggest_mips = TRUE;
 	    }
 	  break;
 
@@ -8882,14 +8899,22 @@ _bfd_mips_elf_adjust_dynamic_symbol (str
 	}
 
       /* Otherwise, if there are no direct calls to the function, we
-         have a free choice of whether to use standard or compressed
-         entries.  Prefer microMIPS entries if the object is known to
-         contain microMIPS code, so that it becomes possible to create
-         pure microMIPS binaries.  Prefer standard entries otherwise,
-         because MIPS16 ones are no smaller and are usually slower.  */
+	 have a free choice of whether to use standard or compressed
+	 entries.  If there were GOT CALL relocations to the symbol,
+	 and all references were from standard or compressed code,
+	 use the same form for the PLT entry.  Otherwise prefer microMIPS
+	 entries if the object is known to contain microMIPS code, so that
+	 it becomes possible to create pure microMIPS binaries.  Prefer
+	 standard entries otherwise, because MIPS16 ones are no smaller
+	 and are usually slower.  */
       if (!h->plt.plist->need_mips && !h->plt.plist->need_comp)
 	{
-	  if (micromips_p)
+	  if (h->plt.plist->suggest_mips != h->plt.plist->suggest_comp)
+	    {
+	      h->plt.plist->need_mips = h->plt.plist->suggest_mips;
+	      h->plt.plist->need_comp = h->plt.plist->suggest_comp;
+	    }
+	  else if (micromips_p)
 	    h->plt.plist->need_comp = TRUE;
 	  else
 	    h->plt.plist->need_mips = TRUE;
@@ -9211,12 +9236,7 @@ mips_elf_allocate_lazy_stub (struct mips
 
       BFD_ASSERT (htab->root.dynobj != NULL);
       if (h->root.plt.plist == NULL)
-	h->root.plt.plist = mips_elf_make_plt_record (htab->sstubs->owner);
-      if (h->root.plt.plist == NULL)
-	{
-	  hti->error = TRUE;
-	  return FALSE;
-	}
+	abort ();
       h->root.root.u.def.section = htab->sstubs;
       h->root.root.u.def.value = htab->sstubs->size + isa_bit;
       h->root.plt.plist->stub_offset = htab->sstubs->size;