arm: Do not insert stubs needing Arm code on Thumb-only cores.

We recently fixed a bug in libgcc
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115360)
where a symbol was missing a %function .type decoration.

This meant the linker would silently pick the wrong type of 'farcall
stub', involving Arm-mode instructions on Thumb-only CPUs.

This patch emits an error instead, and warns in some other cases, to
encourage users to add the missing '.type foo,%function' directive.

In practice: in arm_type_of_stub() we no longer try to infer which
stub to use if the destination is of unknown type and the CPU is
Thumb-only; so we won't lie to elf32_arm_size_stubs() which does not
check branch_type.

If branch_type is ST_BRANCH_TO_ARM but the CPU is Thumb-only, we now
convert it to ST_BRANCH_TO_THUMB only if the destination is of
absolute type.  This is to support the case where the destination of
the branch is defined by the linker script (see thumb-b-lks-sym.s and
thumb-bl-lks-sym.s testcases for instance).

The motivating case is covered by the new farcall-missing-type
testcase, where we now emit an error message.  We do not emit an error
when branch_type is ST_BRANCH_UNKNOWN and the CPU supports Arm-mode: a
lot of legacy code (e.g. newlib's crt0.S) lacks the corresponding
'.type foo, %function' directives and even a (too verbose) warning
could be perceived as a nuisance.

Existing testcases where such a warning would trigger:
arm-static-app.s (app_func, app_func2)
arm-rel32.s (foo)
arm-app.s (app_func)
rel32-reject.s () main)
fix-arm1176.s (func_to_branch_to)
but this list is not exhaustive.

For the sake of clarity, the patch replaces occurrences of
sym.st_target_internal = 0; with
sym.st_target_internal = ST_BRANCH_TO_ARM;

enum arm_st_branch_type is defined in include/elf/arm.h, and relies on
ST_BRANCH_TO_ARM==0, as sym.st_target_internal is also initialized to
0 in other target-independent parts of BFD code.  (For instance,
swapping the ST_BRANCH_TO_ARM and ST_BRANCH_TO_THUMB entries in the
enum definition leads to 'interesting' results...)

Regarding the testsuite:
* new expected warning for thumb-b-lks-sym and thumb-bl-lks-sym
* new testcase farcall-missing-type to check the new error case
* attr-merge-arch-2b.s, branch-futures (and bfs-1.s) updated to avoid
  a diagnostic

Tested on arm-eabi and arm-pe with no regression.

1 files changed, 87 insertions, 12 deletions

diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index ca76bee..7441ee2 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -4226,12 +4226,33 @@ arm_type_of_stub (struct bfd_link_info *info,
 
   r_type = ELF32_R_TYPE (rel->r_info);
 
+  /* Don't pretend we know what stub to use (if any) when we target a
+     Thumb-only target and we don't know the actual destination
+     type.  */
+  if (branch_type == ST_BRANCH_UNKNOWN && thumb_only)
+    return stub_type;
+
   /* ST_BRANCH_TO_ARM is nonsense to thumb-only targets when we
      are considering a function call relocation.  */
   if (thumb_only && (r_type == R_ARM_THM_CALL || r_type == R_ARM_THM_JUMP24
 		     || r_type == R_ARM_THM_JUMP19)
       && branch_type == ST_BRANCH_TO_ARM)
-    branch_type = ST_BRANCH_TO_THUMB;
+    {
+      if (sym_sec == bfd_abs_section_ptr)
+	/* As an exception, assume that absolute symbols are of the
+	   right kind (Thumb).  They are presumably defined in the
+	   linker script, where it is not possible to declare them as
+	   Thumb (and thus are seen as Arm mode). We'll inform the
+	   user with a warning, though, in
+	   elf32_arm_final_link_relocate. */
+	branch_type = ST_BRANCH_TO_THUMB;
+      else
+	/* Otherwise do not silently build a stub, and let the users
+	   know they have to fix their code.  Indeed, we could decide
+	   to insert a stub involving Arm code and/or BLX, leading to
+	   a run-time crash.  */
+	return stub_type;
+    }
 
   /* For TLS call relocs, it is the caller's responsibility to provide
      the address of the appropriate trampoline.  */
@@ -10382,14 +10403,6 @@ elf32_arm_final_link_relocate (reloc_howto_type *	    howto,
   else
     addend = signed_addend = rel->r_addend;
 
-  /* ST_BRANCH_TO_ARM is nonsense to thumb-only targets when we
-     are resolving a function call relocation.  */
-  if (using_thumb_only (globals)
-      && (r_type == R_ARM_THM_CALL
-	  || r_type == R_ARM_THM_JUMP24)
-      && branch_type == ST_BRANCH_TO_ARM)
-    branch_type = ST_BRANCH_TO_THUMB;
-
   /* Record the symbol information that should be used in dynamic
      relocations.  */
   dynreloc_st_type = st_type;
@@ -10452,6 +10465,68 @@ elf32_arm_final_link_relocate (reloc_howto_type *	    howto,
       gotplt_offset = (bfd_vma) -1;
     }
 
+  /* ST_BRANCH_TO_ARM is nonsense to thumb-only targets when we are
+     resolving a function call relocation.  We want to inform the user
+     that something is wrong.  */
+  if (using_thumb_only (globals)
+      && (r_type == R_ARM_THM_CALL
+	  || r_type == R_ARM_THM_JUMP24)
+      && branch_type == ST_BRANCH_TO_ARM
+      /* Calls through a PLT are special: the assembly source code
+	 cannot be annotated with '.type foo(PLT), %function', and
+	 they handled specifically below anyway. */
+      && splt == NULL)
+    {
+      if (sym_sec == bfd_abs_section_ptr)
+	{
+	/* As an exception, assume that absolute symbols are of the
+	   right kind (Thumb).  They are presumably defined in the
+	   linker script, where it is not possible to declare them as
+	   Thumb (and thus are seen as Arm mode). Inform the user with
+	   a warning, though. */
+	  branch_type = ST_BRANCH_TO_THUMB;
+
+	  if (sym_sec->owner)
+	    _bfd_error_handler
+	      (_("warning: %pB(%s): Forcing bramch to absolute symbol in Thumb mode (Thumb-only CPU)"
+		 " in %pB"),
+	       sym_sec->owner, sym_name, input_bfd);
+	  else
+	    _bfd_error_handler
+	      (_("warning: (%s): Forcing branch to absolute symbol in Thumb mode (Thumb-only CPU)"
+		 " in %pB"),
+	       sym_name, input_bfd);
+	}
+      else
+	/* Otherwise do not silently build a stub, and let the users
+	   know they have to fix their code.  Indeed, we could decide
+	   to insert a stub involving Arm code and/or BLX, leading to
+	   a run-time crash.  */
+	branch_type = ST_BRANCH_UNKNOWN;
+    }
+
+  /* Fail early if branch_type is ST_BRANCH_UNKNOWN and we target a
+     Thumb-only CPU.  We could emit a warning on Arm-capable targets
+     too, but that would be too verbose (a lot of legacy code does not
+     use the .type foo, %function directive).  */
+  if (using_thumb_only (globals)
+      && (r_type == R_ARM_THM_CALL
+	  || r_type == R_ARM_THM_JUMP24)
+      && branch_type == ST_BRANCH_UNKNOWN)
+    {
+      if (sym_sec != NULL
+	  && sym_sec->owner != NULL)
+	_bfd_error_handler
+	  (_("%pB(%s): Unknown destination type (ARM/Thumb) in %pB"),
+	   sym_sec->owner, sym_name, input_bfd);
+      else
+	_bfd_error_handler
+	  (_("(%s): Unknown destination type (ARM/Thumb) in %pB"),
+	   sym_name, input_bfd);
+
+      return bfd_reloc_notsupported;
+    }
+
   resolved_to_zero = (h != NULL
 		      && UNDEFWEAK_NO_DYNAMIC_RELOC (info, h));
 
@@ -17838,7 +17913,7 @@ elf32_arm_output_map_sym (output_arch_syminfo *osi,
   sym.st_other = 0;
   sym.st_info = ELF_ST_INFO (STB_LOCAL, STT_NOTYPE);
   sym.st_shndx = osi->sec_shndx;
-  sym.st_target_internal = 0;
+  sym.st_target_internal = ST_BRANCH_TO_ARM;
   elf32_arm_section_map_add (osi->sec, names[type][1], offset);
   return osi->func (osi->flaginfo, names[type], &sym, osi->sec, NULL) == 1;
 }
@@ -17995,7 +18070,7 @@ elf32_arm_output_stub_sym (output_arch_syminfo *osi, const char *name,
   sym.st_other = 0;
   sym.st_info = ELF_ST_INFO (STB_LOCAL, STT_FUNC);
   sym.st_shndx = osi->sec_shndx;
-  sym.st_target_internal = 0;
+  sym.st_target_internal = ST_BRANCH_TO_ARM;
   return osi->func (osi->flaginfo, name, &sym, osi->sec, NULL) == 1;
 }
 
@@ -19743,7 +19818,7 @@ elf32_arm_swap_symbol_in (bfd * abfd,
 {
   if (!bfd_elf32_swap_symbol_in (abfd, psrc, pshn, dst))
     return false;
-  dst->st_target_internal = 0;
+  dst->st_target_internal = ST_BRANCH_TO_ARM;
 
   /* New EABI objects mark thumb function symbols by setting the low bit of
      the address.  */