Home Original page

[PATCH v3 08/10] MIPS/BFD: Fix howto orphan REL HI16 relocation addend processing

Maciej W. Rozycki macro@orcam.me.uk
Tue Dec 9 12:40:34 GMT 2025 
Correct the addend being ignored for orphan REL HI16 relocations.

For assembly and non-ELF links `_bfd_mips_elf_hi16_reloc' is called from 
`bfd_install_relocation' and `bfd_perform_relocation' respectively via 
the respective howtos.  It caches the relocation for later processing as 
`_bfd_mips_elf_lo16_reloc' is called via the corresponding LO16 reloc's 
howto, at which point both the HI16 and the LO16 parts are calculated 
and installed.

If no matching LO16 relocation has been later encountered, then the 
cached entry is never processed, with the outstanding cached entries 
silently dropped at the conclusion of processing, resulting in zero 
addend being used for the field relocated.

Dropping of the entries only happens in `_bfd_mips_elf_free_cached_info' 
at the time the BFD is being successfully closed and section contents 
long written to output.  For non-ELF links dropping will also execute in 
`_bfd_elf_mips_get_relocated_section_contents' via a separate piece of 
code if the function has encountered an error.

Address the issues first by factoring out code to process outstanding 
cached entries to `mips_elf_free_hi16_list' and then by making the 
function actually install the relocations cached as required.  This has 
to happen before section contents have been written and therefore the 
success path wires the function call to `bfd_finalize_section_relocs', 
for assembly and `_bfd_elf_mips_get_relocated_section_contents' for 
non-ELF links.

For housekeeping purposes the latter call will just drop cached entries 
as it happens now in the case of an error, and likewise the call from 
`_bfd_mips_elf_free_cached_info' is retained in case a fatal error in 
the assembler prevents `bfd_finalize_section_relocs' from being called.

This also results in a warning being issued now about orphan REL HI16 
relocations encountered in non-ELF links.  Previously no such warning 
was produced since the cached entries were dropped.  For assembly we 
expect the tool to have issued its own warning, so we process orphan 
relocations silently if successful, but still issue a warning if an 
error is returned.

We are careful in `mips_elf_free_hi16_list' to retain any incoming BFD 
error as the function may be called under an error condition and if 
there's another failure in processing at this stage we don't want to 
clobber the original error.

Test cases will be added with a separate change.
---
Changes from v2:

- Resolve a trivial merge conflict in bfd/elfxx-mips.h.

- Mark `_bfd_mips_elf_finalize_section_relocs' ATTRIBUTE_HIDDEN.

New change in v2.
---
 bfd/elf32-mips.c  |    2 
 bfd/elf64-mips.c  |    2 
 bfd/elfn32-mips.c |    2 
 bfd/elfxx-mips.c  |  194 +++++++++++++++++++++++++++++++++++++++---------------
 bfd/elfxx-mips.h  |    2 
 5 files changed, 149 insertions(+), 53 deletions(-)

binutils-mips-reloc-hi16-orphan-addend.diff
Index: binutils-gdb/bfd/elf32-mips.c
===================================================================
--- binutils-gdb.orig/bfd/elf32-mips.c
+++ binutils-gdb/bfd/elf32-mips.c
@@ -4089,6 +4089,8 @@ static const struct ecoff_debug_swap mip
 #define bfd_elf32_bfd_is_target_special_symbol \
 					_bfd_mips_elf_is_target_special_symbol
 #define bfd_elf32_get_synthetic_symtab	_bfd_mips_elf_get_synthetic_symtab
+#define bfd_elf32_finalize_section_relocs \
+					_bfd_mips_elf_finalize_section_relocs
 #define bfd_elf32_find_nearest_line	_bfd_mips_elf_find_nearest_line
 #define bfd_elf32_find_nearest_line_with_alt \
 					_bfd_mips_elf_find_nearest_line_with_alt
Index: binutils-gdb/bfd/elf64-mips.c
===================================================================
--- binutils-gdb.orig/bfd/elf64-mips.c
+++ binutils-gdb/bfd/elf64-mips.c
@@ -4797,6 +4797,8 @@ static const struct elf_size_info mips_e
    ABI.  */
 #define bfd_elf64_bfd_is_target_special_symbol \
 					_bfd_mips_elf_is_target_special_symbol
+#define bfd_elf64_finalize_section_relocs \
+					_bfd_mips_elf_finalize_section_relocs
 #define bfd_elf64_find_nearest_line	_bfd_mips_elf_find_nearest_line
 #define bfd_elf64_find_nearest_line_with_alt \
 				_bfd_mips_elf_find_nearest_line_with_alt
Index: binutils-gdb/bfd/elfn32-mips.c
===================================================================
--- binutils-gdb.orig/bfd/elfn32-mips.c
+++ binutils-gdb/bfd/elfn32-mips.c
@@ -4184,6 +4184,8 @@ static const struct ecoff_debug_swap mip
 #define elf_backend_mips_rtype_to_howto	mips_elf_n32_rtype_to_howto
 #define bfd_elf32_bfd_is_target_special_symbol \
 					_bfd_mips_elf_is_target_special_symbol
+#define bfd_elf32_finalize_section_relocs \
+					_bfd_mips_elf_finalize_section_relocs
 #define bfd_elf32_find_nearest_line	_bfd_mips_elf_find_nearest_line
 #define bfd_elf32_find_nearest_line_with_alt \
 				_bfd_mips_elf_find_nearest_line_with_alt
Index: binutils-gdb/bfd/elfxx-mips.c
===================================================================
--- binutils-gdb.orig/bfd/elfxx-mips.c
+++ binutils-gdb/bfd/elfxx-mips.c
@@ -552,11 +552,13 @@ struct mips_htab_traverse_info
 /* Used to store a REL high-part relocation such as R_MIPS_HI16 or
    R_MIPS_GOT16.  REL is the relocation, INPUT_SECTION is the section
    that contains the relocation field and DATA points to the start of
-   INPUT_SECTION.  */
+   INPUT_SECTION.  OUTPUT_BFD is the output BFD for relocatable output
+   or a NULL pointer otherwise.  */
 
 struct mips_hi16
 {
   struct mips_hi16 *next;
+  bfd *output_bfd;
   bfd_byte *data;
   asection *input_section;
   arelent rel;
@@ -778,6 +780,8 @@ static bool mips_elf_create_dynamic_relo
    bfd_vma *, asection *);
 static bfd_vma mips_elf_adjust_gp
   (bfd *, struct mips_got_info *, bfd *);
+static bool mips_elf_free_hi16_list
+  (bfd *, bool, struct bfd_link_info *);
 
 /* This will be used when we sort the dynamic relocation records.  */
 static bfd *reldyn_sorting_bfd;
@@ -1392,12 +1396,7 @@ _bfd_mips_elf_free_cached_info (bfd *abf
       && (tdata = mips_elf_tdata (abfd)) != NULL)
     {
       BFD_ASSERT (tdata->root.object_id == MIPS_ELF_DATA);
-      while (tdata->mips_hi16_list != NULL)
-	{
-	  struct mips_hi16 *hi = tdata->mips_hi16_list;
-	  tdata->mips_hi16_list = hi->next;
-	  free (hi);
-	}
+      mips_elf_free_hi16_list (abfd, false, NULL);
       if (tdata->find_line_info != NULL)
 	_bfd_ecoff_free_ecoff_debug_info (&tdata->find_line_info->d);
     }
@@ -2551,6 +2550,7 @@ _bfd_mips_elf_hi16_reloc (bfd *abfd, are
   n->next = tdata->mips_hi16_list;
   n->data = data;
   n->input_section = input_section;
+  n->output_bfd = output_bfd;
   n->rel = *reloc_entry;
   tdata->mips_hi16_list = n;
 
@@ -2581,6 +2581,31 @@ _bfd_mips_elf_got16_reloc (bfd *abfd, ar
 				   input_section, output_bfd, error_message);
 }
 
+/* A helper function for REL high-part relocations that takes into account
+   local R_MIPS*_GOT16 relocations, which are something of a special case.
+   We want to install the addend in the same way as for a R_MIPS*_HI16
+   relocation (with a rightshift of 16).  However, since GOT16 relocations
+   can also be used with global symbols, their howto has a rightshift of 0.  */
+
+static bfd_reloc_status_type
+_bfd_mips_elf_shr16_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
+			   void *data, asection *input_section,
+			   bfd *output_bfd, char **error_message)
+{
+  reloc_howto_type **howto = &reloc_entry->howto;
+
+  if ((*howto)->type == R_MIPS_GOT16)
+    *howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS_HI16, false);
+  else if ((*howto)->type == R_MIPS16_GOT16)
+    *howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS16_HI16, false);
+  else if ((*howto)->type == R_MICROMIPS_GOT16)
+    *howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MICROMIPS_HI16, false);
+
+  return _bfd_mips_elf_generic_reloc (abfd, reloc_entry, symbol, data,
+				      input_section, output_bfd,
+				      error_message);
+}
+
 /* A howto special_function for REL *LO16 relocations.  The *LO16 itself
    is a straightforward 16 bit inplace relocation, but we must deal with
    any partnering high-part relocations as well.  */
@@ -2626,18 +2651,6 @@ _bfd_mips_elf_lo16_reloc (bfd *abfd, are
 
       hi = tdata->mips_hi16_list;
 
-      /* R_MIPS*_GOT16 relocations are something of a special case.  We
-	 want to install the addend in the same way as for a R_MIPS*_HI16
-	 relocation (with a rightshift of 16).  However, since GOT16
-	 relocations can also be used with global symbols, their howto
-	 has a rightshift of 0.  */
-      if (hi->rel.howto->type == R_MIPS_GOT16)
-	hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS_HI16, false);
-      else if (hi->rel.howto->type == R_MIPS16_GOT16)
-	hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS16_HI16, false);
-      else if (hi->rel.howto->type == R_MICROMIPS_GOT16)
-	hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MICROMIPS_HI16, false);
-
       /* We will be applying (symbol + addend) & 0xffff to the low insn,
 	 and we want to apply (symbol + addend + 0x8000) >> 16 to the
 	 high insn (the +0x8000 adjusting for when the applied low part is
@@ -2652,14 +2665,15 @@ _bfd_mips_elf_lo16_reloc (bfd *abfd, are
 
       hi->rel.addend = addhi + _bfd_mips_elf_sign_extend (addlo & 0xffff, 16);
 
-      ret = _bfd_mips_elf_generic_reloc (abfd, &hi->rel, symbol, hi->data,
-					 hi->input_section, output_bfd,
-					 error_message);
-      if (ret != bfd_reloc_ok)
-	return ret;
+      ret = _bfd_mips_elf_shr16_reloc (abfd, &hi->rel, symbol, hi->data,
+				       hi->input_section, hi->output_bfd,
+				       error_message);
 
       tdata->mips_hi16_list = hi->next;
       free (hi);
+
+      if (ret != bfd_reloc_ok)
+	return ret;
     }
 
   return _bfd_mips_elf_generic_reloc (abfd, reloc_entry, symbol, data,
@@ -13229,6 +13243,94 @@ _bfd_mips_elf_is_target_special_symbol (
   return _bfd_elf_is_local_label_name (abfd, sym->name);
 }
 
+/* Helper for `mips_elf_free_hi16_list'.  Resolve an orphan REL high-part
+   relocation according to HI.  Return TRUE if succeeded, otherwise FALSE.  */
+
+static bool
+_bfd_mips_elf_orphan_shr16_reloc (bfd *abfd, struct mips_hi16 *hi,
+				  struct bfd_link_info *info)
+{
+  asymbol *symbol = *hi->rel.sym_ptr_ptr;
+  const char *name = hi->rel.howto->name;
+  bfd_reloc_status_type r;
+  char *error_message;
+
+  r = _bfd_mips_elf_shr16_reloc (abfd, &hi->rel, symbol, hi->data,
+				 hi->input_section, hi->output_bfd,
+				 &error_message);
+  if (r == bfd_reloc_ok)
+    {
+      if (info != NULL)
+	{
+	  /* xgettext:c-format */
+	  error_message = bfd_asprintf (_("can't find matching LO16 reloc"
+					  " against `%s' for %s at %#" PRIx64
+					  " in section `%s'"),
+					symbol->name, name,
+					(uint64_t) hi->rel.address,
+					hi->input_section->name);
+	  if (error_message == NULL)
+	    return false;
+	  info->callbacks->warning
+	    (info, error_message, symbol->name, hi->input_section->owner,
+	     hi->input_section, hi->rel.address);
+	}
+      return true;
+    }
+
+  if (info != NULL)
+    _bfd_link_reloc_status_error (abfd, info, hi->input_section,
+				  &hi->rel, error_message, r);
+  else
+    _bfd_error_handler (_("%pB(%pA+%#" PRIx64 "): %s relocation error"),
+			abfd, hi->input_section, (uint64_t) hi->rel.address,
+			hi->rel.howto->name);
+  bfd_set_error (bfd_error_bad_value);
+  return false;
+}
+
+/* Resolve any outstanding orphan REL high-part relocations if INSTALL
+   is TRUE, and release their occupied memory.  */
+
+static bool
+mips_elf_free_hi16_list (bfd *abfd, bool install,
+			 struct bfd_link_info *info)
+{
+  bfd_error_type error_tag = bfd_get_error ();
+  struct mips_elf_obj_tdata *tdata;
+  bool status = true;
+
+  BFD_ASSERT (is_mips_elf (abfd));
+  tdata = mips_elf_tdata (abfd);
+  while (tdata->mips_hi16_list != NULL)
+    {
+      struct mips_hi16 *hi = tdata->mips_hi16_list;
+
+      if (install)
+	status &= _bfd_mips_elf_orphan_shr16_reloc (abfd, hi, info);
+      if (!status && error_tag == bfd_error_no_error)
+	error_tag = bfd_get_error ();
+
+      tdata->mips_hi16_list = hi->next;
+      free (hi);
+    }
+
+  bfd_set_error (error_tag);
+  return status;
+}
+
+/* Resolve any outstanding orphan REL high-part relocations before
+   calling the generic BFD handler.  */
+
+bool
+_bfd_mips_elf_finalize_section_relocs (bfd *abfd, asection *asect,
+				       arelent **location, unsigned int count)
+{
+  if (!mips_elf_free_hi16_list (abfd, true, NULL))
+    return false;
+  return _bfd_generic_finalize_section_relocs (abfd, asect, location, count);
+}
+
 bool
 _bfd_mips_elf_find_nearest_line (bfd *abfd, asymbol **symbols,
 				 asection *section, bfd_vma offset,
@@ -13399,7 +13501,8 @@ _bfd_elf_mips_get_relocated_section_cont
   asection *input_section = link_order->u.indirect.section;
   long reloc_size;
   arelent **reloc_vector;
-  long reloc_count;
+  long reloc_count = 0;
+  bool install = true;
 
   reloc_size = bfd_get_reloc_upper_bound (input_bfd, input_section);
   if (reloc_size < 0)
@@ -13417,39 +13520,19 @@ _bfd_elf_mips_get_relocated_section_cont
     return data;
 
   reloc_vector = (arelent **) bfd_malloc (reloc_size);
-  if (reloc_vector == NULL)
+  if (reloc_vector != NULL)
+    reloc_count = bfd_canonicalize_reloc (input_bfd, input_section,
+					  reloc_vector, symbols);
+
+  if (reloc_vector == NULL || reloc_count < 0)
     {
-      struct mips_elf_obj_tdata *tdata;
-      struct mips_hi16 **hip, *hi;
-    error_return:
-      /* If we are going to return an error, remove entries on
-	 mips_hi16_list that point into this section's data.  Data
-	 will typically be freed on return from this function.  */
-      tdata = mips_elf_tdata (abfd);
-      hip = &tdata->mips_hi16_list;
-      while ((hi = *hip) != NULL)
-	{
-	  if (hi->input_section == input_section)
-	    {
-	      *hip = hi->next;
-	      free (hi);
-	    }
-	  else
-	    hip = &hi->next;
-	}
+      install = false;
       if (orig_data == NULL)
 	free (data);
       data = NULL;
       goto out;
     }
 
-  reloc_count = bfd_canonicalize_reloc (input_bfd,
-					input_section,
-					reloc_vector,
-					symbols);
-  if (reloc_count < 0)
-    goto error_return;
-
   if (reloc_count > 0)
     {
       arelent **parent;
@@ -13514,7 +13597,8 @@ _bfd_elf_mips_get_relocated_section_cont
 		/* xgettext:c-format */
 		(_("%X%P: %pB(%pA): error: relocation for offset %V has no value\n"),
 		 abfd, input_section, (* parent)->address);
-	      goto error_return;
+	      install = false;
+	      goto out;
 	    }
 
 	  /* Zap reloc field when the symbol is from a discarded
@@ -13579,12 +13663,16 @@ _bfd_elf_mips_get_relocated_section_cont
 	      _bfd_link_reloc_status_error (abfd, link_info, input_section,
 					    *parent, error_message, r);
 	      if (r == bfd_reloc_outofrange || r == bfd_reloc_notsupported)
-		goto error_return;
+		{
+		  install = false;
+		  goto out;
+		}
 	    }
 	}
     }
 
  out:
+  mips_elf_free_hi16_list (input_bfd, install, link_info);
   free (reloc_vector);
   return data;
 }
Index: binutils-gdb/bfd/elfxx-mips.h
===================================================================
--- binutils-gdb.orig/bfd/elfxx-mips.h
+++ binutils-gdb/bfd/elfxx-mips.h
@@ -103,6 +103,8 @@ extern bool _bfd_mips_elf_ignore_discard
   (asection *) ATTRIBUTE_HIDDEN;
 extern bool _bfd_mips_elf_is_target_special_symbol
   (bfd *abfd, asymbol *sym) ATTRIBUTE_HIDDEN;
+extern bool _bfd_mips_elf_finalize_section_relocs
+  (bfd *, asection *, arelent **, unsigned int) ATTRIBUTE_HIDDEN;
 extern bool _bfd_mips_elf_find_nearest_line
   (bfd *, asymbol **, asection *, bfd_vma,
    const char **, const char **, unsigned int *, unsigned int *)
More information about the Binutils mailing list