public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] arm: Do not insert stubs needing Arm code on Thumb-only cores.
@ 2024-06-19 14:35 Christophe Lyon
  2024-06-21 14:38 ` Richard Earnshaw (lists)
  0 siblings, 1 reply; 3+ messages in thread
From: Christophe Lyon @ 2024-06-19 14:35 UTC (permalink / raw)
  To: binutils, richard.earnshaw, andre.simoesdiasvieira; +Cc: Christophe Lyon

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.
---
 bfd/elf32-arm.c                               | 99 ++++++++++++++++---
 ld/testsuite/ld-arm/arm-elf.exp               |  5 +-
 ld/testsuite/ld-arm/attr-merge-arch-2b.s      |  1 +
 ld/testsuite/ld-arm/bfs-1.s                   |  1 +
 ld/testsuite/ld-arm/branch-futures.d          | 10 +-
 .../ld-arm/farcall-missing-type-bad.s         |  7 ++
 .../ld-arm/farcall-missing-type-main.s        |  9 ++
 ld/testsuite/ld-arm/farcall-missing-type.d    |  5 +
 ld/testsuite/ld-arm/farcall-missing-type.ld   | 23 +++++
 ld/testsuite/ld-arm/thumb-b-lks-sym.output    |  1 +
 ld/testsuite/ld-arm/thumb-bl-lks-sym.output   |  1 +
 11 files changed, 143 insertions(+), 19 deletions(-)
 create mode 100644 ld/testsuite/ld-arm/farcall-missing-type-bad.s
 create mode 100644 ld/testsuite/ld-arm/farcall-missing-type-main.s
 create mode 100644 ld/testsuite/ld-arm/farcall-missing-type.d
 create mode 100644 ld/testsuite/ld-arm/farcall-missing-type.ld
 create mode 100644 ld/testsuite/ld-arm/thumb-b-lks-sym.output
 create mode 100644 ld/testsuite/ld-arm/thumb-bl-lks-sym.output

diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index ca76bee6adc..7441ee2cc38 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.  */
diff --git a/ld/testsuite/ld-arm/arm-elf.exp b/ld/testsuite/ld-arm/arm-elf.exp
index 272d518c4d4..5f380e383d0 100644
--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -653,10 +653,10 @@ set armeabitests_nonacl {
      {{objdump -d thumb2-bl-bad.d}}
      "thumb2-bl-bad"}
     {"Branch to linker script symbol with BL for thumb-only target" "-T branch-lks-sym.ld" "" "" {thumb-bl-lks-sym.s}
-     {{objdump -d thumb-bl-lks-sym.d}}
+     {{ld thumb-bl-lks-sym.output} {objdump -d thumb-bl-lks-sym.d}}
      "thumb-bl-lks-sym"}
     {"Branch to linker script symbol with B for thumb-only target" "-T branch-lks-sym.ld" "" "" {thumb-b-lks-sym.s}
-     {{objdump -d thumb-b-lks-sym.d}}
+     {{ld thumb-b-lks-sym.output} {objdump -d thumb-b-lks-sym.d}}
      "thumb-b-lks-sym"}
 
     {"erratum 760522 fix (default for v6z)" "--section-start=.foo=0x2001014" ""
@@ -1207,6 +1207,7 @@ run_dump_test "attr-merge-wchar-40-nowarn"
 run_dump_test "attr-merge-wchar-42-nowarn"
 run_dump_test "attr-merge-wchar-44-nowarn"
 run_dump_test "farcall-section"
+run_dump_test "farcall-missing-type"
 run_dump_test "attr-merge-unknown-1"
 run_dump_test "attr-merge-unknown-2"
 run_dump_test "attr-merge-unknown-2r"
diff --git a/ld/testsuite/ld-arm/attr-merge-arch-2b.s b/ld/testsuite/ld-arm/attr-merge-arch-2b.s
index 57718354145..f20522f81d2 100644
--- a/ld/testsuite/ld-arm/attr-merge-arch-2b.s
+++ b/ld/testsuite/ld-arm/attr-merge-arch-2b.s
@@ -4,5 +4,6 @@
         .align  2
         .global foo
         .type   foo, %function
+	.thumb_func
 foo:
         bx      lr
diff --git a/ld/testsuite/ld-arm/bfs-1.s b/ld/testsuite/ld-arm/bfs-1.s
index 2b72819598e..1e31d440cc2 100644
--- a/ld/testsuite/ld-arm/bfs-1.s
+++ b/ld/testsuite/ld-arm/bfs-1.s
@@ -4,6 +4,7 @@
 .thumb
 .global _start
 .global target
+.type target, %function
 _start:
 target:
 	add	r0, r0, r1
diff --git a/ld/testsuite/ld-arm/branch-futures.d b/ld/testsuite/ld-arm/branch-futures.d
index 427ecce62a4..bc9ae8eddf7 100644
--- a/ld/testsuite/ld-arm/branch-futures.d
+++ b/ld/testsuite/ld-arm/branch-futures.d
@@ -5,13 +5,13 @@
 Disassembly of section .text:
 
 0[0-9a-f]+ <future>:
-    [0-9a-f]+:	f2c0 e807 	bf	a, 8012 <_start>
-    [0-9a-f]+:	f182 e805 	bfcsel	6, 8012 <_start>, a, eq
-    [0-9a-f]+:	f080 c803 	bfl	2, 8012 <_start>
+    [0-9a-f]+:	f2c0 e807 	bf	a, 8012 <target>
+    [0-9a-f]+:	f182 e805 	bfcsel	6, 8012 <target>, a, eq
+    [0-9a-f]+:	f080 c803 	bfl	2, 8012 <target>
     [0-9a-f]+:	4408      	add	r0, r1
 
 0[0-9a-f]+ <branch>:
-    [0-9a-f]+:	f000 b800 	b.w	8012 <_start>
+    [0-9a-f]+:	f000 b800 	b.w	8012 <target>
 
-0[0-9a-f]+ <_start>:
+0[0-9a-f]+ <target>:
     [0-9a-f]+:	4408      	add	r0, r1
diff --git a/ld/testsuite/ld-arm/farcall-missing-type-bad.s b/ld/testsuite/ld-arm/farcall-missing-type-bad.s
new file mode 100644
index 00000000000..e087992b33f
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-missing-type-bad.s
@@ -0,0 +1,7 @@
+	.thumb
+	.cpu cortex-m33
+	.syntax unified
+	.section .text.bar
+	.global bad @ untyped
+#	.type bad, function
+bad:	bx lr
diff --git a/ld/testsuite/ld-arm/farcall-missing-type-main.s b/ld/testsuite/ld-arm/farcall-missing-type-main.s
new file mode 100644
index 00000000000..c97df0cf459
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-missing-type-main.s
@@ -0,0 +1,9 @@
+	.thumb
+	.cpu cortex-m33
+	.syntax unified
+	.global __start
+	.type __start, function
+__start:
+	push	{r4, lr}
+	bl	bad
+	pop	{r4, pc}
diff --git a/ld/testsuite/ld-arm/farcall-missing-type.d b/ld/testsuite/ld-arm/farcall-missing-type.d
new file mode 100644
index 00000000000..9766bf1e10c
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-missing-type.d
@@ -0,0 +1,5 @@
+#source: farcall-missing-type-main.s
+#source: farcall-missing-type-bad.s
+#as:
+#ld:-T farcall-missing-type.ld
+#error: .* .*/farcall-missing-type-bad.o\(bad\): Unknown destination type \(ARM/Thumb\) in .*/farcall-missing-type-main.o
diff --git a/ld/testsuite/ld-arm/farcall-missing-type.ld b/ld/testsuite/ld-arm/farcall-missing-type.ld
new file mode 100644
index 00000000000..b1136de93ba
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-missing-type.ld
@@ -0,0 +1,23 @@
+/* Linker script to configure memory regions. */
+MEMORY
+{
+  FLASH (rx) : ORIGIN = 0x00000000, LENGTH = 0x40000   /* 256k */
+  FLASH2 (rx) : ORIGIN = 0x200001e0, LENGTH = 0x4000
+}
+
+ENTRY(__start)
+
+SECTIONS
+{
+	.far_away_section :
+	{
+	  *(.text.bar)
+	} > FLASH2
+
+	.text :
+	{
+		*(.text*)
+
+	} > FLASH
+
+}
diff --git a/ld/testsuite/ld-arm/thumb-b-lks-sym.output b/ld/testsuite/ld-arm/thumb-b-lks-sym.output
new file mode 100644
index 00000000000..49612042910
--- /dev/null
+++ b/ld/testsuite/ld-arm/thumb-b-lks-sym.output
@@ -0,0 +1 @@
+.* \(extFunc\): Forcing branch to absolute symbol in Thumb mode \(Thumb-only CPU\) in tmpdir/thumb-b-lks-sym.o
diff --git a/ld/testsuite/ld-arm/thumb-bl-lks-sym.output b/ld/testsuite/ld-arm/thumb-bl-lks-sym.output
new file mode 100644
index 00000000000..7c5e3543d84
--- /dev/null
+++ b/ld/testsuite/ld-arm/thumb-bl-lks-sym.output
@@ -0,0 +1 @@
+.* \(extFunc\): Forcing branch to absolute symbol in Thumb mode \(Thumb-only CPU\) in tmpdir/thumb-bl-lks-sym.o
-- 
2.34.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm: Do not insert stubs needing Arm code on Thumb-only cores.
  2024-06-19 14:35 [PATCH] arm: Do not insert stubs needing Arm code on Thumb-only cores Christophe Lyon
@ 2024-06-21 14:38 ` Richard Earnshaw (lists)
  2024-06-21 15:19   ` Christophe Lyon
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Earnshaw (lists) @ 2024-06-21 14:38 UTC (permalink / raw)
  To: Christophe Lyon, binutils, andre.simoesdiasvieira

On 19/06/2024 15:35, Christophe Lyon wrote:
> 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
> 

This is OK thanks, but I think that, given this might have some unexpected fallout, it might be wise to delay pushing it until after the 2.43 branch has been cut.  That way we get plenty of time to assess whether or not this is too agressive.

R.

> Tested on arm-eabi and arm-pe with no regression.
> ---
>  bfd/elf32-arm.c                               | 99 ++++++++++++++++---
>  ld/testsuite/ld-arm/arm-elf.exp               |  5 +-
>  ld/testsuite/ld-arm/attr-merge-arch-2b.s      |  1 +
>  ld/testsuite/ld-arm/bfs-1.s                   |  1 +
>  ld/testsuite/ld-arm/branch-futures.d          | 10 +-
>  .../ld-arm/farcall-missing-type-bad.s         |  7 ++
>  .../ld-arm/farcall-missing-type-main.s        |  9 ++
>  ld/testsuite/ld-arm/farcall-missing-type.d    |  5 +
>  ld/testsuite/ld-arm/farcall-missing-type.ld   | 23 +++++
>  ld/testsuite/ld-arm/thumb-b-lks-sym.output    |  1 +
>  ld/testsuite/ld-arm/thumb-bl-lks-sym.output   |  1 +
>  11 files changed, 143 insertions(+), 19 deletions(-)
>  create mode 100644 ld/testsuite/ld-arm/farcall-missing-type-bad.s
>  create mode 100644 ld/testsuite/ld-arm/farcall-missing-type-main.s
>  create mode 100644 ld/testsuite/ld-arm/farcall-missing-type.d
>  create mode 100644 ld/testsuite/ld-arm/farcall-missing-type.ld
>  create mode 100644 ld/testsuite/ld-arm/thumb-b-lks-sym.output
>  create mode 100644 ld/testsuite/ld-arm/thumb-bl-lks-sym.output
> 
> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> index ca76bee6adc..7441ee2cc38 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.  */
> diff --git a/ld/testsuite/ld-arm/arm-elf.exp b/ld/testsuite/ld-arm/arm-elf.exp
> index 272d518c4d4..5f380e383d0 100644
> --- a/ld/testsuite/ld-arm/arm-elf.exp
> +++ b/ld/testsuite/ld-arm/arm-elf.exp
> @@ -653,10 +653,10 @@ set armeabitests_nonacl {
>       {{objdump -d thumb2-bl-bad.d}}
>       "thumb2-bl-bad"}
>      {"Branch to linker script symbol with BL for thumb-only target" "-T branch-lks-sym.ld" "" "" {thumb-bl-lks-sym.s}
> -     {{objdump -d thumb-bl-lks-sym.d}}
> +     {{ld thumb-bl-lks-sym.output} {objdump -d thumb-bl-lks-sym.d}}
>       "thumb-bl-lks-sym"}
>      {"Branch to linker script symbol with B for thumb-only target" "-T branch-lks-sym.ld" "" "" {thumb-b-lks-sym.s}
> -     {{objdump -d thumb-b-lks-sym.d}}
> +     {{ld thumb-b-lks-sym.output} {objdump -d thumb-b-lks-sym.d}}
>       "thumb-b-lks-sym"}
>  
>      {"erratum 760522 fix (default for v6z)" "--section-start=.foo=0x2001014" ""
> @@ -1207,6 +1207,7 @@ run_dump_test "attr-merge-wchar-40-nowarn"
>  run_dump_test "attr-merge-wchar-42-nowarn"
>  run_dump_test "attr-merge-wchar-44-nowarn"
>  run_dump_test "farcall-section"
> +run_dump_test "farcall-missing-type"
>  run_dump_test "attr-merge-unknown-1"
>  run_dump_test "attr-merge-unknown-2"
>  run_dump_test "attr-merge-unknown-2r"
> diff --git a/ld/testsuite/ld-arm/attr-merge-arch-2b.s b/ld/testsuite/ld-arm/attr-merge-arch-2b.s
> index 57718354145..f20522f81d2 100644
> --- a/ld/testsuite/ld-arm/attr-merge-arch-2b.s
> +++ b/ld/testsuite/ld-arm/attr-merge-arch-2b.s
> @@ -4,5 +4,6 @@
>          .align  2
>          .global foo
>          .type   foo, %function
> +	.thumb_func
>  foo:
>          bx      lr
> diff --git a/ld/testsuite/ld-arm/bfs-1.s b/ld/testsuite/ld-arm/bfs-1.s
> index 2b72819598e..1e31d440cc2 100644
> --- a/ld/testsuite/ld-arm/bfs-1.s
> +++ b/ld/testsuite/ld-arm/bfs-1.s
> @@ -4,6 +4,7 @@
>  .thumb
>  .global _start
>  .global target
> +.type target, %function
>  _start:
>  target:
>  	add	r0, r0, r1
> diff --git a/ld/testsuite/ld-arm/branch-futures.d b/ld/testsuite/ld-arm/branch-futures.d
> index 427ecce62a4..bc9ae8eddf7 100644
> --- a/ld/testsuite/ld-arm/branch-futures.d
> +++ b/ld/testsuite/ld-arm/branch-futures.d
> @@ -5,13 +5,13 @@
>  Disassembly of section .text:
>  
>  0[0-9a-f]+ <future>:
> -    [0-9a-f]+:	f2c0 e807 	bf	a, 8012 <_start>
> -    [0-9a-f]+:	f182 e805 	bfcsel	6, 8012 <_start>, a, eq
> -    [0-9a-f]+:	f080 c803 	bfl	2, 8012 <_start>
> +    [0-9a-f]+:	f2c0 e807 	bf	a, 8012 <target>
> +    [0-9a-f]+:	f182 e805 	bfcsel	6, 8012 <target>, a, eq
> +    [0-9a-f]+:	f080 c803 	bfl	2, 8012 <target>
>      [0-9a-f]+:	4408      	add	r0, r1
>  
>  0[0-9a-f]+ <branch>:
> -    [0-9a-f]+:	f000 b800 	b.w	8012 <_start>
> +    [0-9a-f]+:	f000 b800 	b.w	8012 <target>
>  
> -0[0-9a-f]+ <_start>:
> +0[0-9a-f]+ <target>:
>      [0-9a-f]+:	4408      	add	r0, r1
> diff --git a/ld/testsuite/ld-arm/farcall-missing-type-bad.s b/ld/testsuite/ld-arm/farcall-missing-type-bad.s
> new file mode 100644
> index 00000000000..e087992b33f
> --- /dev/null
> +++ b/ld/testsuite/ld-arm/farcall-missing-type-bad.s
> @@ -0,0 +1,7 @@
> +	.thumb
> +	.cpu cortex-m33
> +	.syntax unified
> +	.section .text.bar
> +	.global bad @ untyped
> +#	.type bad, function
> +bad:	bx lr
> diff --git a/ld/testsuite/ld-arm/farcall-missing-type-main.s b/ld/testsuite/ld-arm/farcall-missing-type-main.s
> new file mode 100644
> index 00000000000..c97df0cf459
> --- /dev/null
> +++ b/ld/testsuite/ld-arm/farcall-missing-type-main.s
> @@ -0,0 +1,9 @@
> +	.thumb
> +	.cpu cortex-m33
> +	.syntax unified
> +	.global __start
> +	.type __start, function
> +__start:
> +	push	{r4, lr}
> +	bl	bad
> +	pop	{r4, pc}
> diff --git a/ld/testsuite/ld-arm/farcall-missing-type.d b/ld/testsuite/ld-arm/farcall-missing-type.d
> new file mode 100644
> index 00000000000..9766bf1e10c
> --- /dev/null
> +++ b/ld/testsuite/ld-arm/farcall-missing-type.d
> @@ -0,0 +1,5 @@
> +#source: farcall-missing-type-main.s
> +#source: farcall-missing-type-bad.s
> +#as:
> +#ld:-T farcall-missing-type.ld
> +#error: .* .*/farcall-missing-type-bad.o\(bad\): Unknown destination type \(ARM/Thumb\) in .*/farcall-missing-type-main.o
> diff --git a/ld/testsuite/ld-arm/farcall-missing-type.ld b/ld/testsuite/ld-arm/farcall-missing-type.ld
> new file mode 100644
> index 00000000000..b1136de93ba
> --- /dev/null
> +++ b/ld/testsuite/ld-arm/farcall-missing-type.ld
> @@ -0,0 +1,23 @@
> +/* Linker script to configure memory regions. */
> +MEMORY
> +{
> +  FLASH (rx) : ORIGIN = 0x00000000, LENGTH = 0x40000   /* 256k */
> +  FLASH2 (rx) : ORIGIN = 0x200001e0, LENGTH = 0x4000
> +}
> +
> +ENTRY(__start)
> +
> +SECTIONS
> +{
> +	.far_away_section :
> +	{
> +	  *(.text.bar)
> +	} > FLASH2
> +
> +	.text :
> +	{
> +		*(.text*)
> +
> +	} > FLASH
> +
> +}
> diff --git a/ld/testsuite/ld-arm/thumb-b-lks-sym.output b/ld/testsuite/ld-arm/thumb-b-lks-sym.output
> new file mode 100644
> index 00000000000..49612042910
> --- /dev/null
> +++ b/ld/testsuite/ld-arm/thumb-b-lks-sym.output
> @@ -0,0 +1 @@
> +.* \(extFunc\): Forcing branch to absolute symbol in Thumb mode \(Thumb-only CPU\) in tmpdir/thumb-b-lks-sym.o
> diff --git a/ld/testsuite/ld-arm/thumb-bl-lks-sym.output b/ld/testsuite/ld-arm/thumb-bl-lks-sym.output
> new file mode 100644
> index 00000000000..7c5e3543d84
> --- /dev/null
> +++ b/ld/testsuite/ld-arm/thumb-bl-lks-sym.output
> @@ -0,0 +1 @@
> +.* \(extFunc\): Forcing branch to absolute symbol in Thumb mode \(Thumb-only CPU\) in tmpdir/thumb-bl-lks-sym.o


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm: Do not insert stubs needing Arm code on Thumb-only cores.
  2024-06-21 14:38 ` Richard Earnshaw (lists)
@ 2024-06-21 15:19   ` Christophe Lyon
  0 siblings, 0 replies; 3+ messages in thread
From: Christophe Lyon @ 2024-06-21 15:19 UTC (permalink / raw)
  To: Richard Earnshaw (lists); +Cc: binutils, andre.simoesdiasvieira

On Fri, 21 Jun 2024 at 16:38, Richard Earnshaw (lists)
<Richard.Earnshaw@arm.com> wrote:
>
> On 19/06/2024 15:35, Christophe Lyon wrote:
> > 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
> >
>
> This is OK thanks, but I think that, given this might have some unexpected fallout, it might be wise to delay pushing it until after the 2.43 branch has been cut.  That way we get plenty of time to assess whether or not this is too agressive.
>

This makes sense, indeed.
I'll wait for a few weeks before committing this patch.

Thanks,

Christophe

> R.
>
> > Tested on arm-eabi and arm-pe with no regression.
> > ---
> >  bfd/elf32-arm.c                               | 99 ++++++++++++++++---
> >  ld/testsuite/ld-arm/arm-elf.exp               |  5 +-
> >  ld/testsuite/ld-arm/attr-merge-arch-2b.s      |  1 +
> >  ld/testsuite/ld-arm/bfs-1.s                   |  1 +
> >  ld/testsuite/ld-arm/branch-futures.d          | 10 +-
> >  .../ld-arm/farcall-missing-type-bad.s         |  7 ++
> >  .../ld-arm/farcall-missing-type-main.s        |  9 ++
> >  ld/testsuite/ld-arm/farcall-missing-type.d    |  5 +
> >  ld/testsuite/ld-arm/farcall-missing-type.ld   | 23 +++++
> >  ld/testsuite/ld-arm/thumb-b-lks-sym.output    |  1 +
> >  ld/testsuite/ld-arm/thumb-bl-lks-sym.output   |  1 +
> >  11 files changed, 143 insertions(+), 19 deletions(-)
> >  create mode 100644 ld/testsuite/ld-arm/farcall-missing-type-bad.s
> >  create mode 100644 ld/testsuite/ld-arm/farcall-missing-type-main.s
> >  create mode 100644 ld/testsuite/ld-arm/farcall-missing-type.d
> >  create mode 100644 ld/testsuite/ld-arm/farcall-missing-type.ld
> >  create mode 100644 ld/testsuite/ld-arm/thumb-b-lks-sym.output
> >  create mode 100644 ld/testsuite/ld-arm/thumb-bl-lks-sym.output
> >
> > diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> > index ca76bee6adc..7441ee2cc38 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.  */
> > diff --git a/ld/testsuite/ld-arm/arm-elf.exp b/ld/testsuite/ld-arm/arm-elf.exp
> > index 272d518c4d4..5f380e383d0 100644
> > --- a/ld/testsuite/ld-arm/arm-elf.exp
> > +++ b/ld/testsuite/ld-arm/arm-elf.exp
> > @@ -653,10 +653,10 @@ set armeabitests_nonacl {
> >       {{objdump -d thumb2-bl-bad.d}}
> >       "thumb2-bl-bad"}
> >      {"Branch to linker script symbol with BL for thumb-only target" "-T branch-lks-sym.ld" "" "" {thumb-bl-lks-sym.s}
> > -     {{objdump -d thumb-bl-lks-sym.d}}
> > +     {{ld thumb-bl-lks-sym.output} {objdump -d thumb-bl-lks-sym.d}}
> >       "thumb-bl-lks-sym"}
> >      {"Branch to linker script symbol with B for thumb-only target" "-T branch-lks-sym.ld" "" "" {thumb-b-lks-sym.s}
> > -     {{objdump -d thumb-b-lks-sym.d}}
> > +     {{ld thumb-b-lks-sym.output} {objdump -d thumb-b-lks-sym.d}}
> >       "thumb-b-lks-sym"}
> >
> >      {"erratum 760522 fix (default for v6z)" "--section-start=.foo=0x2001014" ""
> > @@ -1207,6 +1207,7 @@ run_dump_test "attr-merge-wchar-40-nowarn"
> >  run_dump_test "attr-merge-wchar-42-nowarn"
> >  run_dump_test "attr-merge-wchar-44-nowarn"
> >  run_dump_test "farcall-section"
> > +run_dump_test "farcall-missing-type"
> >  run_dump_test "attr-merge-unknown-1"
> >  run_dump_test "attr-merge-unknown-2"
> >  run_dump_test "attr-merge-unknown-2r"
> > diff --git a/ld/testsuite/ld-arm/attr-merge-arch-2b.s b/ld/testsuite/ld-arm/attr-merge-arch-2b.s
> > index 57718354145..f20522f81d2 100644
> > --- a/ld/testsuite/ld-arm/attr-merge-arch-2b.s
> > +++ b/ld/testsuite/ld-arm/attr-merge-arch-2b.s
> > @@ -4,5 +4,6 @@
> >          .align  2
> >          .global foo
> >          .type   foo, %function
> > +     .thumb_func
> >  foo:
> >          bx      lr
> > diff --git a/ld/testsuite/ld-arm/bfs-1.s b/ld/testsuite/ld-arm/bfs-1.s
> > index 2b72819598e..1e31d440cc2 100644
> > --- a/ld/testsuite/ld-arm/bfs-1.s
> > +++ b/ld/testsuite/ld-arm/bfs-1.s
> > @@ -4,6 +4,7 @@
> >  .thumb
> >  .global _start
> >  .global target
> > +.type target, %function
> >  _start:
> >  target:
> >       add     r0, r0, r1
> > diff --git a/ld/testsuite/ld-arm/branch-futures.d b/ld/testsuite/ld-arm/branch-futures.d
> > index 427ecce62a4..bc9ae8eddf7 100644
> > --- a/ld/testsuite/ld-arm/branch-futures.d
> > +++ b/ld/testsuite/ld-arm/branch-futures.d
> > @@ -5,13 +5,13 @@
> >  Disassembly of section .text:
> >
> >  0[0-9a-f]+ <future>:
> > -    [0-9a-f]+:       f2c0 e807       bf      a, 8012 <_start>
> > -    [0-9a-f]+:       f182 e805       bfcsel  6, 8012 <_start>, a, eq
> > -    [0-9a-f]+:       f080 c803       bfl     2, 8012 <_start>
> > +    [0-9a-f]+:       f2c0 e807       bf      a, 8012 <target>
> > +    [0-9a-f]+:       f182 e805       bfcsel  6, 8012 <target>, a, eq
> > +    [0-9a-f]+:       f080 c803       bfl     2, 8012 <target>
> >      [0-9a-f]+:       4408            add     r0, r1
> >
> >  0[0-9a-f]+ <branch>:
> > -    [0-9a-f]+:       f000 b800       b.w     8012 <_start>
> > +    [0-9a-f]+:       f000 b800       b.w     8012 <target>
> >
> > -0[0-9a-f]+ <_start>:
> > +0[0-9a-f]+ <target>:
> >      [0-9a-f]+:       4408            add     r0, r1
> > diff --git a/ld/testsuite/ld-arm/farcall-missing-type-bad.s b/ld/testsuite/ld-arm/farcall-missing-type-bad.s
> > new file mode 100644
> > index 00000000000..e087992b33f
> > --- /dev/null
> > +++ b/ld/testsuite/ld-arm/farcall-missing-type-bad.s
> > @@ -0,0 +1,7 @@
> > +     .thumb
> > +     .cpu cortex-m33
> > +     .syntax unified
> > +     .section .text.bar
> > +     .global bad @ untyped
> > +#    .type bad, function
> > +bad: bx lr
> > diff --git a/ld/testsuite/ld-arm/farcall-missing-type-main.s b/ld/testsuite/ld-arm/farcall-missing-type-main.s
> > new file mode 100644
> > index 00000000000..c97df0cf459
> > --- /dev/null
> > +++ b/ld/testsuite/ld-arm/farcall-missing-type-main.s
> > @@ -0,0 +1,9 @@
> > +     .thumb
> > +     .cpu cortex-m33
> > +     .syntax unified
> > +     .global __start
> > +     .type __start, function
> > +__start:
> > +     push    {r4, lr}
> > +     bl      bad
> > +     pop     {r4, pc}
> > diff --git a/ld/testsuite/ld-arm/farcall-missing-type.d b/ld/testsuite/ld-arm/farcall-missing-type.d
> > new file mode 100644
> > index 00000000000..9766bf1e10c
> > --- /dev/null
> > +++ b/ld/testsuite/ld-arm/farcall-missing-type.d
> > @@ -0,0 +1,5 @@
> > +#source: farcall-missing-type-main.s
> > +#source: farcall-missing-type-bad.s
> > +#as:
> > +#ld:-T farcall-missing-type.ld
> > +#error: .* .*/farcall-missing-type-bad.o\(bad\): Unknown destination type \(ARM/Thumb\) in .*/farcall-missing-type-main.o
> > diff --git a/ld/testsuite/ld-arm/farcall-missing-type.ld b/ld/testsuite/ld-arm/farcall-missing-type.ld
> > new file mode 100644
> > index 00000000000..b1136de93ba
> > --- /dev/null
> > +++ b/ld/testsuite/ld-arm/farcall-missing-type.ld
> > @@ -0,0 +1,23 @@
> > +/* Linker script to configure memory regions. */
> > +MEMORY
> > +{
> > +  FLASH (rx) : ORIGIN = 0x00000000, LENGTH = 0x40000   /* 256k */
> > +  FLASH2 (rx) : ORIGIN = 0x200001e0, LENGTH = 0x4000
> > +}
> > +
> > +ENTRY(__start)
> > +
> > +SECTIONS
> > +{
> > +     .far_away_section :
> > +     {
> > +       *(.text.bar)
> > +     } > FLASH2
> > +
> > +     .text :
> > +     {
> > +             *(.text*)
> > +
> > +     } > FLASH
> > +
> > +}
> > diff --git a/ld/testsuite/ld-arm/thumb-b-lks-sym.output b/ld/testsuite/ld-arm/thumb-b-lks-sym.output
> > new file mode 100644
> > index 00000000000..49612042910
> > --- /dev/null
> > +++ b/ld/testsuite/ld-arm/thumb-b-lks-sym.output
> > @@ -0,0 +1 @@
> > +.* \(extFunc\): Forcing branch to absolute symbol in Thumb mode \(Thumb-only CPU\) in tmpdir/thumb-b-lks-sym.o
> > diff --git a/ld/testsuite/ld-arm/thumb-bl-lks-sym.output b/ld/testsuite/ld-arm/thumb-bl-lks-sym.output
> > new file mode 100644
> > index 00000000000..7c5e3543d84
> > --- /dev/null
> > +++ b/ld/testsuite/ld-arm/thumb-bl-lks-sym.output
> > @@ -0,0 +1 @@
> > +.* \(extFunc\): Forcing branch to absolute symbol in Thumb mode \(Thumb-only CPU\) in tmpdir/thumb-bl-lks-sym.o
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-06-21 15:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-19 14:35 [PATCH] arm: Do not insert stubs needing Arm code on Thumb-only cores Christophe Lyon
2024-06-21 14:38 ` Richard Earnshaw (lists)
2024-06-21 15:19   ` Christophe Lyon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).