Thumb interworking on untyped symbols

Thumb interworking on untyped symbols

[Paul Brook]

[-- Attachment #1: Type: Text/Plain, Size: 1587 bytes --]

The ARM EABI requires that linkers support interworking for function 
(STT_FUNC) symbols, either via veneers or by bl/blx conversion.  We already do 
this.

However for untyped symbols we blindly assume that these are ARM code.  At the 
time this was implemented it probably made sense - the EABI didn't exist and 
most of the world was ARM mode anyway.  With the advent of Thumb-2 and ARMv7-
M, likely as not this is wrong.

The EABI only requires interworking be performed for function symbols.  For 
other symbols the behavior is less clear, the EABI stating that "interworking 
for untyped symbols must be encoded directly in the object file".  By my 
reading that means we should assume the user wrote bl/blx as appropriate and 
leave well alone.

The attached patch implements this by effectively disabling veneer generation 
for symbols that do not have funciton type.

Tested on arm-none-eabi
Applied to CVS head

Paul

2011-05-31  Paul Brook  <paul@codesourcery.com>

	bfd/
	* elf32-arm.c (elf32_arm_final_link_relocate): Only do bl conversion
	for known functions.
	(elf32_arm_swap_symbol_in): Only set ST_BRANCH_TO_ARM for function
	symbols.

	include/elf/
	* arm.h (arm_st_branch_type): Add ST_BRANCH_UNKNOWN.

	ld/testsuite/
	* ld-arm/cortex-a8-far.d: Adjust expected output.
	* ld-arm/arm-call1.s: Give function symbol correct type.
	* ld-arm/arm-call2.s: Ditto.
	* ld-arm/farcall-group4.s: Ditto.
	* ld-arm/arm-elf.exp (cortex-a8-far): Define far symbols with correct
	type via assembly file.
	* ld-arm/cortex-a8-far-3.s: New file.
	* ld-arm/abs-call-1.s: Add Thumb tests

[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 4973 bytes --]

diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index 611e08e..58bb367 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -8304,7 +8304,7 @@ elf32_arm_final_link_relocate (reloc_howto_type *           howto,
 		     case, mode switching is performed by the stub.  */
 		  if (branch_type == ST_BRANCH_TO_THUMB && !stub_entry)
 		    value |= (1 << 28);
-		  else
+		  else if (stub_entry || branch_type != ST_BRANCH_UNKNOWN)
 		    {
 		      value &= ~(bfd_vma)(1 << 28);
 		      value |= (1 << 24);
@@ -15131,12 +15131,16 @@ elf32_arm_swap_symbol_in (bfd * abfd,
 
   /* New EABI objects mark thumb function symbols by setting the low bit of
      the address.  */
-  if ((ELF_ST_TYPE (dst->st_info) == STT_FUNC
-       || ELF_ST_TYPE (dst->st_info) == STT_GNU_IFUNC)
-      && (dst->st_value & 1))
+  if (ELF_ST_TYPE (dst->st_info) == STT_FUNC
+      || ELF_ST_TYPE (dst->st_info) == STT_GNU_IFUNC)
     {
-      dst->st_value &= ~(bfd_vma) 1;
-      dst->st_target_internal = ST_BRANCH_TO_THUMB;
+      if (dst->st_value & 1)
+	{
+	  dst->st_value &= ~(bfd_vma) 1;
+	  dst->st_target_internal = ST_BRANCH_TO_THUMB;
+	}
+      else
+	dst->st_target_internal = ST_BRANCH_TO_ARM;
     }
   else if (ELF_ST_TYPE (dst->st_info) == STT_ARM_TFUNC)
     {
@@ -15146,7 +15150,7 @@ elf32_arm_swap_symbol_in (bfd * abfd,
   else if (ELF_ST_TYPE (dst->st_info) == STT_SECTION)
     dst->st_target_internal = ST_BRANCH_LONG;
   else
-    dst->st_target_internal = ST_BRANCH_TO_ARM;
+    dst->st_target_internal = ST_BRANCH_UNKNOWN;
 
   return TRUE;
 }
diff --git a/include/elf/arm.h b/include/elf/arm.h
index 5b01835..860fdf7 100644
--- a/include/elf/arm.h
+++ b/include/elf/arm.h
@@ -328,7 +328,8 @@ enum
 enum arm_st_branch_type {
   ST_BRANCH_TO_ARM,
   ST_BRANCH_TO_THUMB,
-  ST_BRANCH_LONG
+  ST_BRANCH_LONG,
+  ST_BRANCH_UNKNOWN
 };
 
 #define ARM_SYM_BRANCH_TYPE(SYM) \
diff --git a/ld/testsuite/ld-arm/abs-call-1.d b/ld/testsuite/ld-arm/abs-call-1.d
index 4482beb..7214e3a 100644
--- a/ld/testsuite/ld-arm/abs-call-1.d
+++ b/ld/testsuite/ld-arm/abs-call-1.d
@@ -6,4 +6,9 @@ Disassembly of section .text:
 00008000 <arm>:
     8000:	eb03dffe 	bl	100000 <foo>
     8004:	ea03dffd 	b	100000 <foo>
-    8008:	eb03dffc 	bl	100000 <foo>
+    8008:	fa03dffc 	blx	100000 <foo>
+    800c:	eb03dffb 	bl	100000 <foo>
+00008010 <thumb>:
+    8010:	f0f7 fff6 	bl	100000 <foo>
+    8014:	f0f7 bff4 	b\.w	100000 <foo>
+    8018:	f0f7 eff2 	blx	100000 <foo>
diff --git a/ld/testsuite/ld-arm/abs-call-1.s b/ld/testsuite/ld-arm/abs-call-1.s
index c0a66b4..ab1ac3d 100644
--- a/ld/testsuite/ld-arm/abs-call-1.s
+++ b/ld/testsuite/ld-arm/abs-call-1.s
@@ -4,5 +4,12 @@
 
 arm:	bl	0x100000
 	b	0x100000
+	blx	0x100000
 	bl	foo
 
+	.syntax unified
+	.thumb
+thumb:	bl	0x100000
+	b	0x100000
+	blx	0x100000
+	@ bl foo is broken - gas fails to preserve the symbol reference
diff --git a/ld/testsuite/ld-arm/arm-call1.s b/ld/testsuite/ld-arm/arm-call1.s
index e6ea1f2..e4ab1c2 100644
--- a/ld/testsuite/ld-arm/arm-call1.s
+++ b/ld/testsuite/ld-arm/arm-call1.s
@@ -2,6 +2,7 @@
 	.text
 	.arch armv5t
 	.global _start
+	.type _start, %function
 _start:
 	bl arm
 	bl t1
diff --git a/ld/testsuite/ld-arm/arm-call2.s b/ld/testsuite/ld-arm/arm-call2.s
index 30ae349..02aa379 100644
--- a/ld/testsuite/ld-arm/arm-call2.s
+++ b/ld/testsuite/ld-arm/arm-call2.s
@@ -4,6 +4,7 @@
 	.global t1
 	.global t2
 	.global t5
+	.type arm, %function
 arm:
 	bx lr
 	.thumb
diff --git a/ld/testsuite/ld-arm/arm-elf.exp b/ld/testsuite/ld-arm/arm-elf.exp
index a8c51c2..8b8495e 100644
--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -276,8 +276,8 @@ set armelftests {
      {{objdump -dr cortex-a8-fix-blx-rel-thumb.d}}
      "cortex-a8-fix-blx-rel-thumb"}
     {"Cortex-A8 erratum fix, relocate bl.w and far call"
-     "-EL -Ttext=0x00 --fix-cortex-a8 --defsym far_fn1=0x80000000 --defsym far_fn2=0x80000004  --defsym far_fn=0x7fff0000 --defsym _start=0"
-     "-EL -mcpu=cortex-a8" {cortex-a8-far-1.s cortex-a8-far-2.s}
+     "-EL -Ttext=0x00 --fix-cortex-a8 --defsym _start=0"
+     "-EL -mcpu=cortex-a8" {cortex-a8-far-1.s cortex-a8-far-2.s cortex-a8-far-3.s}
      {{objdump -dr cortex-a8-far.d}}
      "cortex-a8-far"}
     {"Cortex-A8 erratum fix, headers"
diff --git a/ld/testsuite/ld-arm/cortex-a8-far-3.s b/ld/testsuite/ld-arm/cortex-a8-far-3.s
new file mode 100644
index 0000000..48241a5
--- /dev/null
+++ b/ld/testsuite/ld-arm/cortex-a8-far-3.s
@@ -0,0 +1,9 @@
+.globl far_fn
+.type far_fn, %function
+.set far_fn, 0x7fff0000
+.globl far_fn1
+.type far_fn1, %function
+.set far_fn1, 0x80000000
+.globl far_fn2
+.type far_fn2, %function
+.set far_fn2, 0x80000004
diff --git a/ld/testsuite/ld-arm/farcall-group4.s b/ld/testsuite/ld-arm/farcall-group4.s
index 17f503b..95ad035 100644
--- a/ld/testsuite/ld-arm/farcall-group4.s
+++ b/ld/testsuite/ld-arm/farcall-group4.s
@@ -8,6 +8,7 @@ myfunc:
 	bl bar
 
 	.section .far, "xa"
+	.type bar, %function
 	.global bar
 bar:
 	bx lr

Re: Thumb interworking on untyped symbols

[Richard Earnshaw]

On 31/05/11 15:10, Paul Brook wrote:
> The ARM EABI requires that linkers support interworking for function 
> (STT_FUNC) symbols, either via veneers or by bl/blx conversion.  We already do 
> this.
> 
> However for untyped symbols we blindly assume that these are ARM code.  At the 
> time this was implemented it probably made sense - the EABI didn't exist and 
> most of the world was ARM mode anyway.  With the advent of Thumb-2 and ARMv7-
> M, likely as not this is wrong.
> 
> The EABI only requires interworking be performed for function symbols.  For 
> other symbols the behavior is less clear, the EABI stating that "interworking 
> for untyped symbols must be encoded directly in the object file".  By my 
> reading that means we should assume the user wrote bl/blx as appropriate and 
> leave well alone.
> 
> The attached patch implements this by effectively disabling veneer generation 
> for symbols that do not have funciton type.
> 
Yes, that sounds sensible.  In particular a linker only has freedom to
insert a veneer that clobbers IP when it's a STT_FUNC symbol.

R.