[PATCH v2 05/10] MIPS/GAS: Warn about unmatched REL HI16 relocations

[Maciej W. Rozycki]

Ever since commit 749b8d9d455c ("PATCH: ld/4208: `final link failed: Bad 
value' when building Linux MIPS kernels."), which went in back in 2007, 
<https://inbox.sourceware.org/binutils/20070320043504.GA7735@lucon.org/>, 
we've been warning about unmatched HI16 REL relocations in the linker, 
as it has been concluded in the discussion on PR ld/4208 that we have no 
need to be forgiving about compiler bugs that lead to object files to be 
made that are not compliant with the psABI.

However no corresponding update has ever been made to GAS, despite that 
it's always helpful to issue a warning as early as possible in the build 
process.  In this particular case GAS also has more information to hand 
as it always has the full original addend available that has been used 
for both the high-part and the low-part relocation, while the linker has 
to rely on the in-place value stored in the relocatable field by earlier 
processing, which has necessarily been truncated.  Therefore the linker 
may miss cases where unrelated high-part and low-part relocations have 
been put next to each other that had significantly different addends at 
assembly time (small differences are deliberately permitted where they 
are known not to affect any borrow from the high part).

Additionally GAS is able to point at the exact place in sources where 
any offending orphan relocations have been used.

Update GAS to issue the warning then where applicable and remove an old 
comment referring to GCC producing orphan HI16 relocations.

Test cases will be added with a separate change.
---
New change in v2.
---
 gas/config/tc-mips.c |   41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

binutils-mips-gas-hi16-orphan-warn.diff
Index: binutils-gdb/gas/config/tc-mips.c
===================================================================
--- binutils-gdb.orig/gas/config/tc-mips.c
+++ binutils-gdb/gas/config/tc-mips.c
@@ -7426,12 +7426,11 @@ micromips_add_label (void)
   S_SET_OTHER (s, ELF_ST_SET_MICROMIPS (S_GET_OTHER (s)));
 }
 
-/* If assembling microMIPS code, then return the microMIPS reloc
-   corresponding to the requested one if any.  Otherwise return
-   the reloc unchanged.  */
+/* Return the microMIPS relocation corresponding to RELOC if there is any
+   and MICROMIPS_P is true.  Otherwise return RELOC unchanged.  */
 
 static bfd_reloc_code_real_type
-micromips_map_reloc (bfd_reloc_code_real_type reloc)
+micromips_map_reloc (bfd_reloc_code_real_type reloc, bool micromips_p)
 {
   static const bfd_reloc_code_real_type relocs[][2] =
     {
@@ -7467,7 +7466,7 @@ micromips_map_reloc (bfd_reloc_code_real
   bfd_reloc_code_real_type r;
   size_t i;
 
-  if (!mips_opts.micromips)
+  if (!micromips_p)
     return reloc;
   for (i = 0; i < ARRAY_SIZE (relocs); i++)
     {
@@ -7948,7 +7947,8 @@ append_insn (struct mips_cl_insn *ip, ex
       /* Perform any necessary conversion to microMIPS relocations
 	 and find out how many relocations there actually are.  */
       for (i = 0; i < 3 && reloc_type[i] != BFD_RELOC_UNUSED; i++)
-	final_type[i] = micromips_map_reloc (reloc_type[i]);
+	final_type[i] = micromips_map_reloc (reloc_type[i],
+					     mips_opts.micromips);
 
       /* In a compound relocation, it is the final (outermost)
 	 operator that determines the relocated field.  */
@@ -14704,6 +14704,24 @@ parse_relocation (char **str, bfd_reloc_
   return false;
 }
 
+/* Return a relocation operator corresponding to RELOC.  */
+
+static const char *
+mips_op_for_relocation (bfd_reloc_code_real_type reloc)
+
+{
+  size_t i;
+
+  for (i = 0; i < ARRAY_SIZE (mips_percent_op); i++)
+    if (reloc == micromips_map_reloc (mips_percent_op[i].reloc,
+				      micromips_reloc_p (reloc)))
+      return mips_percent_op[i].str;
+  for (i = 0; i < ARRAY_SIZE (mips16_percent_op); i++)
+    if (reloc == mips16_percent_op[i].reloc)
+      return mips16_percent_op[i].str;
+
+  abort ();
+}
 
 /* Parse string STR as a 16-bit relocatable operand.  Store the
    expression in *EP and the relocations in the array starting
@@ -15565,11 +15583,7 @@ mips_frob_file (void)
 	 fixup_has_matching_lo_p() will return true.  For PC-relative
 	 relocations the distance between the offsets is retained
 	 according to expectations in `fixup_has_matching_lo_p',
-	 `_bfd_mips_elf_lo16_reloc' and `mips_elf_add_lo16_rel_addend'.
-
-	 We don't warn about unmatched high-part relocations since some
-	 versions of gcc have been known to emit dead "lui ...%hi(...)"
-	 instructions.  */
+	 `_bfd_mips_elf_lo16_reloc' and `mips_elf_add_lo16_rel_addend'.  */
       if (lo_pos != NULL)
 	{
 	  if (l->fixp->fx_r_type != BFD_RELOC_HI16_S_PCREL)
@@ -15581,6 +15595,11 @@ mips_frob_file (void)
 	      *lo_pos = l->fixp;
 	    }
 	}
+      else
+	as_warn_where
+	  (l->fixp->fx_file, l->fixp->fx_line,
+	   _("can't find matching low-part relocation for %s operator"),
+	   mips_op_for_relocation (l->fixp->fx_r_type));
     }
 }