Extra error checking around TLS relocations

We add the following extra error checking:
  1) That TLS relocations (including SIZE relocations, but excluding
     Local-Exec relocations) are not requested against a symbol plus
     addend.
  2) That SIZE relocations are requested against a defined symbol in the
     current binary (i.e. one that the static linker knows the size of).
  3) A TLS Local-Exec relocation must be against a symbol in the current
     binary.

All the above also have error messages that describe the problem so that
the user could fix it.

Treating a relocation against a "symbol plus addend" as an error is due
to a combination of factors.
  - The linker implementation does not have any way to represent a GOT
    entry of "symbol plus addend".  Hence we currently just have silent
    bugs if asked to implement those relocations which require a GOT
    entry if they have a "symbol plus addend" relocation.
  - It would be wasteful anyway to have multiple entries in the GOT for
    e.g. sym+off1, sym+off2.
  - Morello size relocations don't support "symbol plus addend" since
    the meaning would have to be defined (is this the *remaining* size
    of the symbol?) and there is no known use for this.

We allow local-exec relocation on "symbol plus addend" since then the
addend just implements an offset into the object we're accessing (rather
than a new GOT entry for the location of "symbol plus addend").
There is also an existing testcase in the BFD linker to allow such
relocations.  The compiler can always avoid emitting these if it wants.

Notes on implementation:
  - We choose to check errors in final_link_relocate rather than
    check_relocs since this is where most existing error checking is
    done.
  - We check for errors around addends in relocate_section rather than
    final_link_relocate or check_relocs since final_link_relocate does
    not get told the *original* relocation (before TLS relaxation) and
    check_relocs does not know about addends coming from the result of
    previous relocations on the same code.
    N.b. in order to emit multiple errors when there are multiple
    relocations with an addend we change things in relocate_section to
    store a "return value" in a local variable and set it to false if
    any problem was seen but not return early.

1 files changed, 71 insertions, 9 deletions

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index 446a9af70f9..92a539afcc8 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -203,7 +203,14 @@
    || (R_TYPE) == BFD_RELOC_AARCH64_TLSLD_MOVW_DTPREL_G1	\
    || (R_TYPE) == BFD_RELOC_AARCH64_TLSLD_MOVW_DTPREL_G1_NC	\
    || (R_TYPE) == BFD_RELOC_AARCH64_TLSLD_MOVW_DTPREL_G2	\
-   || (R_TYPE) == BFD_RELOC_AARCH64_TLSLE_ADD_TPREL_HI12	\
+   || (R_TYPE) == BFD_RELOC_AARCH64_TLS_DTPMOD			\
+   || (R_TYPE) == BFD_RELOC_AARCH64_TLS_DTPREL			\
+   || (R_TYPE) == BFD_RELOC_AARCH64_TLS_TPREL			\
+   || IS_AARCH64_TLSLE_RELOC ((R_TYPE))				\
+   || IS_AARCH64_TLSDESC_RELOC ((R_TYPE)))
+
+#define IS_AARCH64_TLSLE_RELOC(R_TYPE)                          \
+   ((R_TYPE) == BFD_RELOC_AARCH64_TLSLE_ADD_TPREL_HI12		\
    || (R_TYPE) == BFD_RELOC_AARCH64_TLSLE_ADD_TPREL_LO12	\
    || (R_TYPE) == BFD_RELOC_AARCH64_TLSLE_ADD_TPREL_LO12_NC	\
    || (R_TYPE) == BFD_RELOC_AARCH64_TLSLE_LDST16_TPREL_LO12	\
@@ -218,11 +225,7 @@
    || (R_TYPE) == BFD_RELOC_AARCH64_TLSLE_MOVW_TPREL_G0_NC	\
    || (R_TYPE) == BFD_RELOC_AARCH64_TLSLE_MOVW_TPREL_G1		\
    || (R_TYPE) == BFD_RELOC_AARCH64_TLSLE_MOVW_TPREL_G1_NC	\
-   || (R_TYPE) == BFD_RELOC_AARCH64_TLSLE_MOVW_TPREL_G2		\
-   || (R_TYPE) == BFD_RELOC_AARCH64_TLS_DTPMOD			\
-   || (R_TYPE) == BFD_RELOC_AARCH64_TLS_DTPREL			\
-   || (R_TYPE) == BFD_RELOC_AARCH64_TLS_TPREL			\
-   || IS_AARCH64_TLSDESC_RELOC ((R_TYPE)))
+   || (R_TYPE) == BFD_RELOC_AARCH64_TLSLE_MOVW_TPREL_G2)
 
 #define IS_AARCH64_TLS_RELAX_RELOC(R_TYPE)			\
   ((R_TYPE) == BFD_RELOC_AARCH64_TLSDESC_ADD			\
@@ -7564,7 +7567,24 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto,
     case BFD_RELOC_MORELLO_MOVW_SIZE_G2:
     case BFD_RELOC_MORELLO_MOVW_SIZE_G2_NC:
     case BFD_RELOC_MORELLO_MOVW_SIZE_G3:
-      BFD_ASSERT (!weak_undef_p && !signed_addend);
+      if (weak_undef_p || !SYMBOL_REFERENCES_LOCAL (info, h))
+	{
+	  int howto_index = bfd_r_type - BFD_RELOC_AARCH64_RELOC_START;
+	  const char *name;
+	  if (h && h->root.root.string)
+	    name = h->root.root.string;
+	  else
+	    name = bfd_elf_sym_name (input_bfd, symtab_hdr, sym, NULL);
+	  _bfd_error_handler
+	    /* xgettext:c-format */
+	    (_("%pB: relocation %s against `%s' must be used against a "
+	       "non-interposable defined symbol"),
+	     input_bfd, elfNN_aarch64_howto_table[howto_index].name, name);
+	  bfd_set_error (bfd_error_bad_value);
+	  return bfd_reloc_continue;
+	}
+      /* signed addend should have been handled by relocate_section.  */
+      BFD_ASSERT (!signed_addend);
       value = sym ? sym->st_size : h->size;
       /* N.b. the call to resolve relocation is not really necessary since
 	 the relocation does not allow any addend, the relocation is not
@@ -7930,6 +7950,20 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto,
 	    bfd_set_error (bfd_error_bad_value);
 	    return bfd_reloc_notsupported;
 	  }
+	/* Cannot have a Local-Exec relocation against a symbol that is not in
+	   the current binary.  */
+	if (!TLS_SYMBOL_REFERENCES_LOCAL (info, h))
+	  {
+	    int howto_index = bfd_r_type - BFD_RELOC_AARCH64_RELOC_START;
+	    _bfd_error_handler
+	      /* xgettext:c-format */
+	      (_("%pB: Local-Exec TLS relocation %s against non-local "
+		 "symbol `%s'"),
+		 input_bfd, elfNN_aarch64_howto_table[howto_index].name,
+		 h->root.root.string);
+	    bfd_set_error (bfd_error_bad_value);
+	    return bfd_reloc_notsupported;
+	  }
 
 	bfd_vma def_value
 	  = weak_undef_p ? 0 : signed_addend - tpoff_base (info);
@@ -8677,6 +8711,7 @@ elfNN_aarch64_relocate_section (bfd *output_bfd,
 				Elf_Internal_Sym *local_syms,
 				asection **local_sections)
 {
+  bfd_boolean ret = TRUE;
   Elf_Internal_Shdr *symtab_hdr;
   struct elf_link_hash_entry **sym_hashes;
   Elf_Internal_Rela *rel;
@@ -8816,6 +8851,33 @@ elfNN_aarch64_relocate_section (bfd *output_bfd,
 	   so just skip over them.  */
 	continue;
 
+      /* We check TLS relocations with an addend here rather than in the
+	 final_link_relocate function since we want to base our decision on if
+	 the original relocation was a TLS relocation and the relaxation below
+	 could turn this relocation into a plain MORELLO_ADR_PREL_PG_HI20 or
+	 AARCH64_ADD_ABS_LO12_NC.  */
+      if (((IS_AARCH64_TLS_RELOC (bfd_r_type)
+	    && !IS_AARCH64_TLSLE_RELOC (bfd_r_type))
+	   || IS_MORELLO_SIZE_RELOC (bfd_r_type))
+	  && (rel->r_addend != 0 || addend != 0))
+	{
+	  _bfd_error_handler (_("%pB(%pA+%#" PRIx64 "): "
+				"relocation %s against `%s' is disallowed "
+				"with addend"),
+			      input_bfd, input_section,
+			      (uint64_t) rel->r_offset, howto->name, name);
+	  /* It could be confusing if there's not a TLS relocation with an
+	     addend but there was a TLS relocation with the previous relocation
+	     at the same spot.  */
+	  if (rel->r_addend == 0 && addend != 0)
+	    info->callbacks->warning
+	      (info,
+	       _("note: addend comes from previous relocation"),
+	       name, input_bfd, input_section, rel->r_offset);
+	  ret = FALSE;
+	  continue;
+	}
+
       /* We relax only if we can see that there can be a valid transition
 	 from a reloc type to another.
 	 We call elfNN_aarch64_final_link_relocate unless we're completely
@@ -8841,7 +8903,7 @@ elfNN_aarch64_relocate_section (bfd *output_bfd,
 	    }
 	  r = elfNN_aarch64_tls_relax (input_bfd, info, input_section,
 				       contents, rel, h);
-	  unresolved_reloc = 0;
+	  unresolved_reloc = FALSE;
 	}
       else
 	r = bfd_reloc_continue;
@@ -9234,7 +9296,7 @@ alignment than was declared where it was defined"),
 	addend = 0;
     }
 
-  return TRUE;
+  return ret;
 }
 
 /* Set the right machine number.  */