public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
@ 2014-08-04  4:40 Tony Wang
  2014-08-04  9:14 ` Will Newton
  0 siblings, 1 reply; 11+ messages in thread
From: Tony Wang @ 2014-08-04  4:40 UTC (permalink / raw)
  To: binutils; +Cc: nickc

[-- Attachment #1: Type: text/plain, Size: 1401 bytes --]

Hi there,

According to AAPCS-ELF specification R_ARM_THM_JUMP19 should also be
veneered if the target is outside the addressable span of the branch
instruction or where interworking happened.

So the attached patch implements the veneer routine for R_ARM_THM_JUMP19,
and two new macros THM2_MAX_FWD_COND_BRANCH_OFFSET and
THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced. Also updated the conditional
branch for both cases mentioned above.

Tested with Binutils regression test, no new regressions. No regression on
gcc trunk with target cortex m4

Ok for the trunk?

bfd/ChangeLog:
2014-08-4  Tony Wang <tony.wang@arm.com>

     * elf32-arm.c (elf32_arm_final_link_relocate): Implement the veneer
routine for 
R_ARM_THM_JUMP19.
     (arm_type_of_stub): Add conditional clause for R_ARM_THM_JUMP19
     (elf32_arm_size_stub): Ditto.

ld/testsuite/ChangeLog:
2014-08-4  Tony Wang <tony.wang@arm.com>

     * ld-arm/jump-reloc-veneers-cond.s: New test.
     * ld-arm/farcall-cond-thumb-arm.s: Ditto.
     * ld-arm/jump-reloc-veneers-cond-short.d: Expected output for target
without 
a veneer generation.
     * ld-arm/jump-reloc-veneers-cond-long.d: Expected output for target
with a 
veneer generation.
     * ld-arm/farcall-cond-thumb-arm.d: Expected output for inter working
veneer 
generation.
     * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.

[-- Attachment #2: relocation_jump19.diff --]
[-- Type: application/octet-stream, Size: 11086 bytes --]

diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index e6f4a9f..a40f9f1 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -2283,6 +2283,8 @@ static const bfd_vma elf32_arm_nacl_plt_entry [] =
 #define THM_MAX_BWD_BRANCH_OFFSET  (-(1 << 22) + 4)
 #define THM2_MAX_FWD_BRANCH_OFFSET (((1 << 24) - 2) + 4)
 #define THM2_MAX_BWD_BRANCH_OFFSET (-(1 << 24) + 4)
+#define THM2_MAX_FWD_COND_BRANCH_OFFSET (((1 << 20) -2) + 4)
+#define THM2_MAX_BWD_COND_BRANCH_OFFSET (-(1 << 20) + 4)
 
 enum stub_insn_type
 {
@@ -3667,7 +3669,8 @@ arm_type_of_stub (struct bfd_link_info *info,
 
   /* 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)
+  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;
 
@@ -3711,7 +3714,7 @@ arm_type_of_stub (struct bfd_link_info *info,
   branch_offset = (bfd_signed_vma)(destination - location);
 
   if (r_type == R_ARM_THM_CALL || r_type == R_ARM_THM_JUMP24
-      || r_type == R_ARM_THM_TLS_CALL)
+      || r_type == R_ARM_THM_TLS_CALL || r_type == R_ARM_THM_JUMP19)
     {
       /* Handle cases where:
 	 - this call goes too far (different Thumb/Thumb2 max
@@ -3727,10 +3730,15 @@ arm_type_of_stub (struct bfd_link_info *info,
 	  || (thumb2
 	      && (branch_offset > THM2_MAX_FWD_BRANCH_OFFSET
 		  || (branch_offset < THM2_MAX_BWD_BRANCH_OFFSET)))
+	  || (thumb2
+	      && (branch_offset > THM2_MAX_FWD_COND_BRANCH_OFFSET
+		  || (branch_offset < THM2_MAX_BWD_COND_BRANCH_OFFSET))
+	      && (r_type == R_ARM_THM_JUMP19))
 	  || (branch_type == ST_BRANCH_TO_ARM
 	      && (((r_type == R_ARM_THM_CALL
 		    || r_type == R_ARM_THM_TLS_CALL) && !globals->use_blx)
-		  || (r_type == R_ARM_THM_JUMP24))
+		  || (r_type == R_ARM_THM_JUMP24)
+                  || (r_type == R_ARM_THM_JUMP19))
 	      && !use_plt))
 	{
 	  if (branch_type == ST_BRANCH_TO_THUMB)
@@ -5347,7 +5355,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
 		      /* For historical reasons, use the existing names for
 			 ARM-to-Thumb and Thumb-to-ARM stubs.  */
 		      if ((r_type == (unsigned int) R_ARM_THM_CALL
-			   || r_type == (unsigned int) R_ARM_THM_JUMP24)
+			   || r_type == (unsigned int) R_ARM_THM_JUMP24
+                           || r_type == (unsigned int) R_ARM_THM_JUMP19)
 			  && branch_type == ST_BRANCH_TO_ARM)
 			sprintf (stub_entry->output_name,
 				 THUMB2ARM_GLUE_ENTRY_NAME, sym_name);
@@ -9125,6 +9134,9 @@ elf32_arm_final_link_relocate (reloc_howto_type *           howto,
 	bfd_signed_vma reloc_signed_max = 0xffffe;
 	bfd_signed_vma reloc_signed_min = -0x100000;
 	bfd_signed_vma signed_check;
+        enum elf32_arm_stub_type stub_type = arm_stub_none;
+	struct elf32_arm_stub_hash_entry *stub_entry;
+	struct elf32_arm_link_hash_entry *hash;
 
 	/* Need to refetch the addend, reconstruct the top three bits,
 	   and squish the two 11 bit pieces together.  */
@@ -9156,8 +9168,25 @@ elf32_arm_final_link_relocate (reloc_howto_type *           howto,
 	    *unresolved_reloc_p = FALSE;
 	  }
 
-	/* ??? Should handle interworking?  GCC might someday try to
-	   use this for tail calls.  */
+	hash = (struct elf32_arm_link_hash_entry *)h;
+
+	stub_type = arm_type_of_stub (info, input_section, rel,
+		                      st_type, &branch_type,
+		                      hash, value, sym_sec,
+		                      input_bfd, sym_name);
+	if (stub_type != arm_stub_none)
+	  {
+	    stub_entry = elf32_arm_get_stub_entry (input_section,
+				                   sym_sec, h,
+				                   rel, globals,
+				                   stub_type);
+	    if (stub_entry != NULL)
+	      {
+	        value = (stub_entry->stub_offset
+                        + stub_entry->stub_sec->output_offset
+                        + stub_entry->stub_sec->output_section->vma);
+	      }
+	  }
 
 	relocation = value + signed_addend;
 	relocation -= (input_section->output_section->vma
diff --git a/ld/testsuite/ld-arm/arm-elf.exp b/ld/testsuite/ld-arm/arm-elf.exp
index 0576847..3289dfd 100644
--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -449,6 +449,16 @@ set armeabitests_nonacl {
      {{objdump -d farcall-thumb-arm-pic-veneer.d}}
      "farcall-thumb-arm-pic-veneer"}
 
+    {"Thumb-ARM farcall cond" "-Ttext 0x8000 --section-start .foo=0x118000" "" "-W" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm"}
+    {"Thumb-ARM farcall cond (BE8)" "-Ttext 0x8000 --section-start .foo=0x118000 -EB --be8" "" "-W -EB" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm-be8"}
+    {"Thumb-ARM farcall cond (BE)" "-Ttext 0x8000 --section-start .foo=0x118000 -EB" "" "-W -EB" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm-be"}
+
     {"Multiple farcalls" "-Ttext 0x1000 --section-start .foo=0x2002020" "" "" {farcall-mix.s}
      {{objdump -d farcall-mix.d}}
      "farcall-mix"}
@@ -547,6 +557,31 @@ set armeabitests_nonacl {
      {{objdump -d jump-reloc-veneers-long.d}}
      "jump-reloc-veneers-long"}
 
+    {"R_ARM_THM_JUMP19 Relocation veneers: Short"
+     "--section-start destsect=0x000108002 --section-start .text=0x8000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-short.d}}
+     "jump-reloc-veneers-cond-short"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Long"
+     "--section-start destsect=0x00108004 --section-start .text=0x8000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-long.d}}
+     "jump-reloc-veneers-cond-long"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Short backward"
+     "--section-start destsect=0x8004 --section-start .text=0x108000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-short-backward.d}}
+     "jump-reloc-veneers-cond-short-backward"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Long backward"
+     "--section-start destsect=0x8002 --section-start .text=0x108000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-long-backward.d}}
+     "jump-reloc-veneers-cond-long-backward"}
+
     {"Default group size" "-Ttext 0x1000 --section-start .foo=0x2003020" "" "" {farcall-group.s farcall-group2.s}
      {{objdump -d farcall-group.d}}
      "farcall-group-default"}
diff --git a/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d
new file mode 100644
index 0000000..0b0172b
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d
@@ -0,0 +1,18 @@
+.*:     file format .*
+
+Disassembly of section .text:
+
+00008000 <_start>:
+    8000:	f050 a002 	bne.w	58008 <__bar_from_thumb>
+	\.\.\.
+   58004:	f040 8000 	bne.w	58008 <__bar_from_thumb>
+
+00058008 <__bar_from_thumb>:
+   58008:	4778      	bx	pc
+   5800a:	46c0      	nop			; \(mov r8, r8\)
+   5800c:	ea02fffb 	b	118000 <bar>
+
+Disassembly of section .foo:
+
+00118000 <bar>:
+  118000:	e12fff1e 	bx	lr
diff --git a/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s
new file mode 100644
index 0000000..809f2fc
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s
@@ -0,0 +1,27 @@
+@ Test to ensure that a Thumb to ARM call exceeding 4Mb generates a stub.
+@ Check that we can generate two types of stub in the same section.
+
+	.global _start
+	.syntax unified
+
+@ We will place the section .text at 0x1c01010.
+
+	.text
+	.thumb_func
+_start:
+	.global bar
+	bne bar
+@ This call is close enough to generate a "short branch" stub
+@ or no stub if blx is available.
+	.space 0x050000
+	bne bar
+
+@ We will place the section .foo at 0x2001014.
+
+	.section .foo, "xa"
+
+	.arm
+	.type bar, %function
+bar:
+	bx lr
+
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d
new file mode 100644
index 0000000..ee0709a
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d
@@ -0,0 +1,24 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00008002 <[^>]*>:
+    8002:	f7ff fffe 	bl	8002 <dest>
+
+Disassembly of section .text:
+
+001080.. <[^>]*>:
+  1080..:	f040 8002 	bne.w	108008 <__dest_veneer>
+  1080..:	0000      	movs	r0, r0
+	...
+
+001080.. <[^>]*>:
+  1080..:	b401      	push	{r0}
+  1080..:	4802      	ldr	r0, \[pc, #8\]	; \(108014 <__dest_veneer\+0xc>\)
+  1080..:	4684      	mov	ip, r0
+  1080..:	bc01      	pop	{r0}
+  1080..:	4760      	bx	ip
+  1080..:	bf00      	nop
+  1080..:	00008003 	.word	0x00008003
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d
new file mode 100644
index 0000000..061999e
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d
@@ -0,0 +1,24 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00108004 <[^>]*>:
+  108004:	f7ff fffe 	bl	108004 <dest>
+
+Disassembly of section .text:
+
+000080.. <[^>]*>:
+    80..:	8002f040 	.word	0x8002f040
+    80..:	0000      	movs	r0, r0
+	...
+
+000080.. <__dest_veneer>:
+    80..:	b401      	push	{r0}
+    80..:	4802      	ldr	r0, \[pc, #8\]	; \(80.. <__dest_veneer\+0xc>\)
+    80..:	4684      	mov	ip, r0
+    80..:	bc01      	pop	{r0}
+    80..:	4760      	bx	ip
+    80..:	bf00      	nop
+    80..:	00108005 	.word	0x00108005
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d
new file mode 100644
index 0000000..d05425b
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d
@@ -0,0 +1,13 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00008004 <[^>]*>:
+    8004:	f7ff fffe 	bl	8004 <dest>
+
+Disassembly of section .text:
+
+001080.. <_start>:
+  1080..:	f440 8000 	bne.w	8004 <dest>
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d
new file mode 100644
index 0000000..08c2212
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d
@@ -0,0 +1,13 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00108002 <[^>]*>:
+  108002:	f7ff fffe 	bl	108002 <dest>
+
+Disassembly of section .text:
+
+000080.. <[^>]*>:
+    80..:	f07f afff 	bne.w	108002 <dest>
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s b/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s
new file mode 100644
index 0000000..83f969c
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s
@@ -0,0 +1,12 @@
+	.text
+	.syntax unified
+	.thumb_func
+	.global _start
+	.type _start,%function
+_start:
+	bne dest
+
+	.section destsect, "x"
+	.thumb_func
+dest:
+	bl dest

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

* Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
  2014-08-04  4:40 [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19 Tony Wang
@ 2014-08-04  9:14 ` Will Newton
  2014-08-04  9:41   ` Tony Wang
  0 siblings, 1 reply; 11+ messages in thread
From: Will Newton @ 2014-08-04  9:14 UTC (permalink / raw)
  To: Tony Wang; +Cc: binutils, nick clifton

On 4 August 2014 05:40, Tony Wang <tony.wang@arm.com> wrote:
> Hi there,
>
> According to AAPCS-ELF specification R_ARM_THM_JUMP19 should also be
> veneered if the target is outside the addressable span of the branch
> instruction or where interworking happened.
>
> So the attached patch implements the veneer routine for R_ARM_THM_JUMP19,
> and two new macros THM2_MAX_FWD_COND_BRANCH_OFFSET and
> THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced. Also updated the conditional
> branch for both cases mentioned above.
>
> Tested with Binutils regression test, no new regressions. No regression on
> gcc trunk with target cortex m4
>
> Ok for the trunk?
>
> bfd/ChangeLog:
> 2014-08-4  Tony Wang <tony.wang@arm.com>
>
>      * elf32-arm.c (elf32_arm_final_link_relocate): Implement the veneer
> routine for
> R_ARM_THM_JUMP19.
>      (arm_type_of_stub): Add conditional clause for R_ARM_THM_JUMP19
>      (elf32_arm_size_stub): Ditto.
>
> ld/testsuite/ChangeLog:
> 2014-08-4  Tony Wang <tony.wang@arm.com>
>
>      * ld-arm/jump-reloc-veneers-cond.s: New test.
>      * ld-arm/farcall-cond-thumb-arm.s: Ditto.
>      * ld-arm/jump-reloc-veneers-cond-short.d: Expected output for target
> without
> a veneer generation.
>      * ld-arm/jump-reloc-veneers-cond-long.d: Expected output for target
> with a
> veneer generation.
>      * ld-arm/farcall-cond-thumb-arm.d: Expected output for inter working
> veneer
> generation.
>      * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.

Looks ok to me (although I am not a maintainer). Small details but
logical operations bind more loosely than almost every other operator
so in some cases you can drop the brackets around assignments in
conditionals and make them a bit easier to read. Also the casts to
unsigned int in elf32_arm_size_stubs seem unnecessary.

-- 
Will Newton
Toolchain Working Group, Linaro

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

* RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
  2014-08-04  9:14 ` Will Newton
@ 2014-08-04  9:41   ` Tony Wang
  2014-08-04  9:50     ` Will Newton
  0 siblings, 1 reply; 11+ messages in thread
From: Tony Wang @ 2014-08-04  9:41 UTC (permalink / raw)
  To: 'Will Newton'; +Cc: binutils, nickc

> -----Original Message-----
> From: Will Newton [mailto:will.newton@linaro.org]
> Sent: Monday, August 04, 2014 5:14 PM
> To: Tony Wang
> Cc: binutils@sourceware.org; nickc@redhat.com
> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> 
> On 4 August 2014 05:40, Tony Wang <tony.wang@arm.com> wrote:
> > Hi there,
> >
> > According to AAPCS-ELF specification R_ARM_THM_JUMP19 should also be
> > veneered if the target is outside the addressable span of the branch
> > instruction or where interworking happened.
> >
> > So the attached patch implements the veneer routine for
> > R_ARM_THM_JUMP19, and two new macros
> THM2_MAX_FWD_COND_BRANCH_OFFSET
> > and THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced. Also
> updated the
> > conditional branch for both cases mentioned above.
> >
> > Tested with Binutils regression test, no new regressions. No
> > regression on gcc trunk with target cortex m4
> >
> > Ok for the trunk?
> >
> > bfd/ChangeLog:
> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> >
> >      * elf32-arm.c (elf32_arm_final_link_relocate): Implement the
> > veneer routine for R_ARM_THM_JUMP19.
> >      (arm_type_of_stub): Add conditional clause for R_ARM_THM_JUMP19
> >      (elf32_arm_size_stub): Ditto.
> >
> > ld/testsuite/ChangeLog:
> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> >
> >      * ld-arm/jump-reloc-veneers-cond.s: New test.
> >      * ld-arm/farcall-cond-thumb-arm.s: Ditto.
> >      * ld-arm/jump-reloc-veneers-cond-short.d: Expected output for
> > target without a veneer generation.
> >      * ld-arm/jump-reloc-veneers-cond-long.d: Expected output for
> > target with a veneer generation.
> >      * ld-arm/farcall-cond-thumb-arm.d: Expected output for inter
> > working veneer generation.
> >      * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.
> 
> Looks ok to me (although I am not a maintainer). Small details but logical
> operations bind more loosely than almost every other operator so in some
> cases you can drop the brackets around assignments in conditionals and
> make them a bit easier to read. 

I agreed with your point.

>Also the casts to unsigned int in
> elf32_arm_size_stubs seem unnecessary.

From my point of view, it might still needed here, as the enum type not always equals to unsigned int.  

> 
> --
> Will Newton
> Toolchain Working Group, Linaro



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

* Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
  2014-08-04  9:41   ` Tony Wang
@ 2014-08-04  9:50     ` Will Newton
  2014-08-04 10:27       ` Tony Wang
  0 siblings, 1 reply; 11+ messages in thread
From: Will Newton @ 2014-08-04  9:50 UTC (permalink / raw)
  To: Tony Wang; +Cc: binutils, nick clifton

On 4 August 2014 10:40, Tony Wang <tony.wang@arm.com> wrote:
>> -----Original Message-----
>> From: Will Newton [mailto:will.newton@linaro.org]
>> Sent: Monday, August 04, 2014 5:14 PM
>> To: Tony Wang
>> Cc: binutils@sourceware.org; nickc@redhat.com
>> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
>>
>> On 4 August 2014 05:40, Tony Wang <tony.wang@arm.com> wrote:
>> > Hi there,
>> >
>> > According to AAPCS-ELF specification R_ARM_THM_JUMP19 should also be
>> > veneered if the target is outside the addressable span of the branch
>> > instruction or where interworking happened.
>> >
>> > So the attached patch implements the veneer routine for
>> > R_ARM_THM_JUMP19, and two new macros
>> THM2_MAX_FWD_COND_BRANCH_OFFSET
>> > and THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced. Also
>> updated the
>> > conditional branch for both cases mentioned above.
>> >
>> > Tested with Binutils regression test, no new regressions. No
>> > regression on gcc trunk with target cortex m4
>> >
>> > Ok for the trunk?
>> >
>> > bfd/ChangeLog:
>> > 2014-08-4  Tony Wang <tony.wang@arm.com>
>> >
>> >      * elf32-arm.c (elf32_arm_final_link_relocate): Implement the
>> > veneer routine for R_ARM_THM_JUMP19.
>> >      (arm_type_of_stub): Add conditional clause for R_ARM_THM_JUMP19
>> >      (elf32_arm_size_stub): Ditto.
>> >
>> > ld/testsuite/ChangeLog:
>> > 2014-08-4  Tony Wang <tony.wang@arm.com>
>> >
>> >      * ld-arm/jump-reloc-veneers-cond.s: New test.
>> >      * ld-arm/farcall-cond-thumb-arm.s: Ditto.
>> >      * ld-arm/jump-reloc-veneers-cond-short.d: Expected output for
>> > target without a veneer generation.
>> >      * ld-arm/jump-reloc-veneers-cond-long.d: Expected output for
>> > target with a veneer generation.
>> >      * ld-arm/farcall-cond-thumb-arm.d: Expected output for inter
>> > working veneer generation.
>> >      * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.
>>
>> Looks ok to me (although I am not a maintainer). Small details but logical
>> operations bind more loosely than almost every other operator so in some
>> cases you can drop the brackets around assignments in conditionals and
>> make them a bit easier to read.
>
> I agreed with your point.
>
>>Also the casts to unsigned int in
>> elf32_arm_size_stubs seem unnecessary.
>
> From my point of view, it might still needed here, as the enum type not always equals to unsigned int.

Fair enough, its not a big issue.

I also noticed a testsuite failure for armeb:

/home/will/linaro/binutils-arm/ld/../binutils/objdump -d
tmpdir/jump-reloc-veneers-cond-long
regexp_diff match failure
regexp "^    80..:      8002f040        .word   0x8002f040$"
line   "    8000:       f0408002        .word   0xf0408002"
FAIL: R_ARM_THM_JUMP19 Relocation veneers: Long

-- 
Will Newton
Toolchain Working Group, Linaro

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

* RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
  2014-08-04  9:50     ` Will Newton
@ 2014-08-04 10:27       ` Tony Wang
  2014-08-04 10:32         ` Will Newton
  0 siblings, 1 reply; 11+ messages in thread
From: Tony Wang @ 2014-08-04 10:27 UTC (permalink / raw)
  To: 'Will Newton'; +Cc: binutils, nickc

> -----Original Message-----
> From: Will Newton [mailto:will.newton@linaro.org]
> Sent: Monday, August 04, 2014 5:50 PM
> To: Tony Wang
> Cc: binutils@sourceware.org; nickc@redhat.com
> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> 
> On 4 August 2014 10:40, Tony Wang <tony.wang@arm.com> wrote:
> >> -----Original Message-----
> >> From: Will Newton [mailto:will.newton@linaro.org]
> >> Sent: Monday, August 04, 2014 5:14 PM
> >> To: Tony Wang
> >> Cc: binutils@sourceware.org; nickc@redhat.com
> >> Subject: Re: [Patch, ARM]Enable veneer routine for
> R_ARM_THM_JUMP19
> >>
> >> On 4 August 2014 05:40, Tony Wang <tony.wang@arm.com> wrote:
> >> > Hi there,
> >> >
> >> > According to AAPCS-ELF specification R_ARM_THM_JUMP19 should also
> >> > be veneered if the target is outside the addressable span of the
> >> > branch instruction or where interworking happened.
> >> >
> >> > So the attached patch implements the veneer routine for
> >> > R_ARM_THM_JUMP19, and two new macros
> >> THM2_MAX_FWD_COND_BRANCH_OFFSET
> >> > and THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced. Also
> >> updated the
> >> > conditional branch for both cases mentioned above.
> >> >
> >> > Tested with Binutils regression test, no new regressions. No
> >> > regression on gcc trunk with target cortex m4
> >> >
> >> > Ok for the trunk?
> >> >
> >> > bfd/ChangeLog:
> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> >> >
> >> >      * elf32-arm.c (elf32_arm_final_link_relocate): Implement the
> >> > veneer routine for R_ARM_THM_JUMP19.
> >> >      (arm_type_of_stub): Add conditional clause for
> R_ARM_THM_JUMP19
> >> >      (elf32_arm_size_stub): Ditto.
> >> >
> >> > ld/testsuite/ChangeLog:
> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> >> >
> >> >      * ld-arm/jump-reloc-veneers-cond.s: New test.
> >> >      * ld-arm/farcall-cond-thumb-arm.s: Ditto.
> >> >      * ld-arm/jump-reloc-veneers-cond-short.d: Expected output for
> >> > target without a veneer generation.
> >> >      * ld-arm/jump-reloc-veneers-cond-long.d: Expected output for
> >> > target with a veneer generation.
> >> >      * ld-arm/farcall-cond-thumb-arm.d: Expected output for inter
> >> > working veneer generation.
> >> >      * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.
> >>
> >> Looks ok to me (although I am not a maintainer). Small details but
> >> logical operations bind more loosely than almost every other operator
> >> so in some cases you can drop the brackets around assignments in
> >> conditionals and make them a bit easier to read.
> >
> > I agreed with your point.
> >
> >>Also the casts to unsigned int in
> >> elf32_arm_size_stubs seem unnecessary.
> >
> > From my point of view, it might still needed here, as the enum type not
> always equals to unsigned int.
> 
> Fair enough, its not a big issue.
> 
> I also noticed a testsuite failure for armeb:
> 
> /home/will/linaro/binutils-arm/ld/../binutils/objdump -d tmpdir/jump-reloc-
> veneers-cond-long
> regexp_diff match failure
> regexp "^    80..:      8002f040        .word   0x8002f040$"
> line   "    8000:       f0408002        .word   0xf0408002"
> FAIL: R_ARM_THM_JUMP19 Relocation veneers: Long
> 

Hi Will,
Thanks a lot for your report, and are you using the big endian toolchain? It looks like an endian issue. I only tested it with little endian toolchain. 

> --
> Will Newton
> Toolchain Working Group, Linaro



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

* Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
  2014-08-04 10:27       ` Tony Wang
@ 2014-08-04 10:32         ` Will Newton
  2014-08-05  1:46           ` Tony Wang
                             ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Will Newton @ 2014-08-04 10:32 UTC (permalink / raw)
  To: Tony Wang; +Cc: binutils, nick clifton

On 4 August 2014 11:27, Tony Wang <tony.wang@arm.com> wrote:
>> -----Original Message-----
>> From: Will Newton [mailto:will.newton@linaro.org]
>> Sent: Monday, August 04, 2014 5:50 PM
>> To: Tony Wang
>> Cc: binutils@sourceware.org; nickc@redhat.com
>> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
>>
>> On 4 August 2014 10:40, Tony Wang <tony.wang@arm.com> wrote:
>> >> -----Original Message-----
>> >> From: Will Newton [mailto:will.newton@linaro.org]
>> >> Sent: Monday, August 04, 2014 5:14 PM
>> >> To: Tony Wang
>> >> Cc: binutils@sourceware.org; nickc@redhat.com
>> >> Subject: Re: [Patch, ARM]Enable veneer routine for
>> R_ARM_THM_JUMP19
>> >>
>> >> On 4 August 2014 05:40, Tony Wang <tony.wang@arm.com> wrote:
>> >> > Hi there,
>> >> >
>> >> > According to AAPCS-ELF specification R_ARM_THM_JUMP19 should also
>> >> > be veneered if the target is outside the addressable span of the
>> >> > branch instruction or where interworking happened.
>> >> >
>> >> > So the attached patch implements the veneer routine for
>> >> > R_ARM_THM_JUMP19, and two new macros
>> >> THM2_MAX_FWD_COND_BRANCH_OFFSET
>> >> > and THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced. Also
>> >> updated the
>> >> > conditional branch for both cases mentioned above.
>> >> >
>> >> > Tested with Binutils regression test, no new regressions. No
>> >> > regression on gcc trunk with target cortex m4
>> >> >
>> >> > Ok for the trunk?
>> >> >
>> >> > bfd/ChangeLog:
>> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
>> >> >
>> >> >      * elf32-arm.c (elf32_arm_final_link_relocate): Implement the
>> >> > veneer routine for R_ARM_THM_JUMP19.
>> >> >      (arm_type_of_stub): Add conditional clause for
>> R_ARM_THM_JUMP19
>> >> >      (elf32_arm_size_stub): Ditto.
>> >> >
>> >> > ld/testsuite/ChangeLog:
>> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
>> >> >
>> >> >      * ld-arm/jump-reloc-veneers-cond.s: New test.
>> >> >      * ld-arm/farcall-cond-thumb-arm.s: Ditto.
>> >> >      * ld-arm/jump-reloc-veneers-cond-short.d: Expected output for
>> >> > target without a veneer generation.
>> >> >      * ld-arm/jump-reloc-veneers-cond-long.d: Expected output for
>> >> > target with a veneer generation.
>> >> >      * ld-arm/farcall-cond-thumb-arm.d: Expected output for inter
>> >> > working veneer generation.
>> >> >      * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.
>> >>
>> >> Looks ok to me (although I am not a maintainer). Small details but
>> >> logical operations bind more loosely than almost every other operator
>> >> so in some cases you can drop the brackets around assignments in
>> >> conditionals and make them a bit easier to read.
>> >
>> > I agreed with your point.
>> >
>> >>Also the casts to unsigned int in
>> >> elf32_arm_size_stubs seem unnecessary.
>> >
>> > From my point of view, it might still needed here, as the enum type not
>> always equals to unsigned int.
>>
>> Fair enough, its not a big issue.
>>
>> I also noticed a testsuite failure for armeb:
>>
>> /home/will/linaro/binutils-arm/ld/../binutils/objdump -d tmpdir/jump-reloc-
>> veneers-cond-long
>> regexp_diff match failure
>> regexp "^    80..:      8002f040        .word   0x8002f040$"
>> line   "    8000:       f0408002        .word   0xf0408002"
>> FAIL: R_ARM_THM_JUMP19 Relocation veneers: Long
>>
>
> Hi Will,
> Thanks a lot for your report, and are you using the big endian toolchain? It looks like an endian issue. I only tested it with little endian toolchain.

I configured binutils with --target=armeb-none-eabi, although I also
found that armeb EABI targets seem to be broken in general so I
submitted a patch to fix that.

-- 
Will Newton
Toolchain Working Group, Linaro

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

* RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
  2014-08-04 10:32         ` Will Newton
@ 2014-08-05  1:46           ` Tony Wang
  2014-08-12  2:25           ` Tony Wang
  2014-08-19  2:38           ` Tony Wang
  2 siblings, 0 replies; 11+ messages in thread
From: Tony Wang @ 2014-08-05  1:46 UTC (permalink / raw)
  To: 'Will Newton', binutils; +Cc: nickc

[-- Attachment #1: Type: text/plain, Size: 4670 bytes --]

New patch updated, fix Will's failure.

And it looks strange to me why binutils change the first branch instruction into a 32bit data, which introduced endian difference. Maybe anyone can help on me to understand why the first branch instruction is optimized out in some situation? However, as the jump24 is just doing the same thing, in this patch, I just keep conform with the old test case.

BR,
Tony
> -----Original Message-----
> From: Will Newton [mailto:will.newton@linaro.org]
> Sent: Monday, August 04, 2014 6:33 PM
> To: Tony Wang
> Cc: binutils@sourceware.org; nickc@redhat.com
> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> 
> On 4 August 2014 11:27, Tony Wang <tony.wang@arm.com> wrote:
> >> -----Original Message-----
> >> From: Will Newton [mailto:will.newton@linaro.org]
> >> Sent: Monday, August 04, 2014 5:50 PM
> >> To: Tony Wang
> >> Cc: binutils@sourceware.org; nickc@redhat.com
> >> Subject: Re: [Patch, ARM]Enable veneer routine for
> R_ARM_THM_JUMP19
> >>
> >> On 4 August 2014 10:40, Tony Wang <tony.wang@arm.com> wrote:
> >> >> -----Original Message-----
> >> >> From: Will Newton [mailto:will.newton@linaro.org]
> >> >> Sent: Monday, August 04, 2014 5:14 PM
> >> >> To: Tony Wang
> >> >> Cc: binutils@sourceware.org; nickc@redhat.com
> >> >> Subject: Re: [Patch, ARM]Enable veneer routine for
> >> R_ARM_THM_JUMP19
> >> >>
> >> >> On 4 August 2014 05:40, Tony Wang <tony.wang@arm.com> wrote:
> >> >> > Hi there,
> >> >> >
> >> >> > According to AAPCS-ELF specification R_ARM_THM_JUMP19 should
> >> >> > also be veneered if the target is outside the addressable span
> >> >> > of the branch instruction or where interworking happened.
> >> >> >
> >> >> > So the attached patch implements the veneer routine for
> >> >> > R_ARM_THM_JUMP19, and two new macros
> >> >> THM2_MAX_FWD_COND_BRANCH_OFFSET
> >> >> > and THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced.
> Also
> >> >> updated the
> >> >> > conditional branch for both cases mentioned above.
> >> >> >
> >> >> > Tested with Binutils regression test, no new regressions. No
> >> >> > regression on gcc trunk with target cortex m4
> >> >> >
> >> >> > Ok for the trunk?
> >> >> >
> >> >> > bfd/ChangeLog:
> >> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> >> >> >
> >> >> >      * elf32-arm.c (elf32_arm_final_link_relocate): Implement
> >> >> > the veneer routine for R_ARM_THM_JUMP19.
> >> >> >      (arm_type_of_stub): Add conditional clause for
> >> R_ARM_THM_JUMP19
> >> >> >      (elf32_arm_size_stub): Ditto.
> >> >> >
> >> >> > ld/testsuite/ChangeLog:
> >> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> >> >> >
> >> >> >      * ld-arm/jump-reloc-veneers-cond.s: New test.
> >> >> >      * ld-arm/farcall-cond-thumb-arm.s: Ditto.
> >> >> >      * ld-arm/jump-reloc-veneers-cond-short.d: Expected output
> >> >> > for target without a veneer generation.
> >> >> >      * ld-arm/jump-reloc-veneers-cond-long.d: Expected output
> >> >> > for target with a veneer generation.
> >> >> >      * ld-arm/farcall-cond-thumb-arm.d: Expected output for
> >> >> > inter working veneer generation.
> >> >> >      * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.
> >> >>
> >> >> Looks ok to me (although I am not a maintainer). Small details but
> >> >> logical operations bind more loosely than almost every other
> >> >> operator so in some cases you can drop the brackets around
> >> >> assignments in conditionals and make them a bit easier to read.
> >> >
> >> > I agreed with your point.
> >> >
> >> >>Also the casts to unsigned int in
> >> >> elf32_arm_size_stubs seem unnecessary.
> >> >
> >> > From my point of view, it might still needed here, as the enum type
> >> > not
> >> always equals to unsigned int.
> >>
> >> Fair enough, its not a big issue.
> >>
> >> I also noticed a testsuite failure for armeb:
> >>
> >> /home/will/linaro/binutils-arm/ld/../binutils/objdump -d
> >> tmpdir/jump-reloc- veneers-cond-long regexp_diff match failure
> >> regexp "^    80..:      8002f040        .word   0x8002f040$"
> >> line   "    8000:       f0408002        .word   0xf0408002"
> >> FAIL: R_ARM_THM_JUMP19 Relocation veneers: Long
> >>
> >
> > Hi Will,
> > Thanks a lot for your report, and are you using the big endian toolchain? It
> looks like an endian issue. I only tested it with little endian toolchain.
> 
> I configured binutils with --target=armeb-none-eabi, although I also
> found that armeb EABI targets seem to be broken in general so I
> submitted a patch to fix that.
> 
> --
> Will Newton
> Toolchain Working Group, Linaro

[-- Attachment #2: relocation_jump19.diff --]
[-- Type: application/octet-stream, Size: 11100 bytes --]

diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index e6f4a9f..a40f9f1 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -2283,6 +2283,8 @@ static const bfd_vma elf32_arm_nacl_plt_entry [] =
 #define THM_MAX_BWD_BRANCH_OFFSET  (-(1 << 22) + 4)
 #define THM2_MAX_FWD_BRANCH_OFFSET (((1 << 24) - 2) + 4)
 #define THM2_MAX_BWD_BRANCH_OFFSET (-(1 << 24) + 4)
+#define THM2_MAX_FWD_COND_BRANCH_OFFSET (((1 << 20) -2) + 4)
+#define THM2_MAX_BWD_COND_BRANCH_OFFSET (-(1 << 20) + 4)
 
 enum stub_insn_type
 {
@@ -3667,7 +3669,8 @@ arm_type_of_stub (struct bfd_link_info *info,
 
   /* 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)
+  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;
 
@@ -3711,7 +3714,7 @@ arm_type_of_stub (struct bfd_link_info *info,
   branch_offset = (bfd_signed_vma)(destination - location);
 
   if (r_type == R_ARM_THM_CALL || r_type == R_ARM_THM_JUMP24
-      || r_type == R_ARM_THM_TLS_CALL)
+      || r_type == R_ARM_THM_TLS_CALL || r_type == R_ARM_THM_JUMP19)
     {
       /* Handle cases where:
 	 - this call goes too far (different Thumb/Thumb2 max
@@ -3727,10 +3730,15 @@ arm_type_of_stub (struct bfd_link_info *info,
 	  || (thumb2
 	      && (branch_offset > THM2_MAX_FWD_BRANCH_OFFSET
 		  || (branch_offset < THM2_MAX_BWD_BRANCH_OFFSET)))
+	  || (thumb2
+	      && (branch_offset > THM2_MAX_FWD_COND_BRANCH_OFFSET
+		  || (branch_offset < THM2_MAX_BWD_COND_BRANCH_OFFSET))
+	      && (r_type == R_ARM_THM_JUMP19))
 	  || (branch_type == ST_BRANCH_TO_ARM
 	      && (((r_type == R_ARM_THM_CALL
 		    || r_type == R_ARM_THM_TLS_CALL) && !globals->use_blx)
-		  || (r_type == R_ARM_THM_JUMP24))
+		  || (r_type == R_ARM_THM_JUMP24)
+                  || (r_type == R_ARM_THM_JUMP19))
 	      && !use_plt))
 	{
 	  if (branch_type == ST_BRANCH_TO_THUMB)
@@ -5347,7 +5355,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
 		      /* For historical reasons, use the existing names for
 			 ARM-to-Thumb and Thumb-to-ARM stubs.  */
 		      if ((r_type == (unsigned int) R_ARM_THM_CALL
-			   || r_type == (unsigned int) R_ARM_THM_JUMP24)
+			   || r_type == (unsigned int) R_ARM_THM_JUMP24
+                           || r_type == (unsigned int) R_ARM_THM_JUMP19)
 			  && branch_type == ST_BRANCH_TO_ARM)
 			sprintf (stub_entry->output_name,
 				 THUMB2ARM_GLUE_ENTRY_NAME, sym_name);
@@ -9125,6 +9134,9 @@ elf32_arm_final_link_relocate (reloc_howto_type *           howto,
 	bfd_signed_vma reloc_signed_max = 0xffffe;
 	bfd_signed_vma reloc_signed_min = -0x100000;
 	bfd_signed_vma signed_check;
+        enum elf32_arm_stub_type stub_type = arm_stub_none;
+	struct elf32_arm_stub_hash_entry *stub_entry;
+	struct elf32_arm_link_hash_entry *hash;
 
 	/* Need to refetch the addend, reconstruct the top three bits,
 	   and squish the two 11 bit pieces together.  */
@@ -9156,8 +9168,25 @@ elf32_arm_final_link_relocate (reloc_howto_type *           howto,
 	    *unresolved_reloc_p = FALSE;
 	  }
 
-	/* ??? Should handle interworking?  GCC might someday try to
-	   use this for tail calls.  */
+	hash = (struct elf32_arm_link_hash_entry *)h;
+
+	stub_type = arm_type_of_stub (info, input_section, rel,
+		                      st_type, &branch_type,
+		                      hash, value, sym_sec,
+		                      input_bfd, sym_name);
+	if (stub_type != arm_stub_none)
+	  {
+	    stub_entry = elf32_arm_get_stub_entry (input_section,
+				                   sym_sec, h,
+				                   rel, globals,
+				                   stub_type);
+	    if (stub_entry != NULL)
+	      {
+	        value = (stub_entry->stub_offset
+                        + stub_entry->stub_sec->output_offset
+                        + stub_entry->stub_sec->output_section->vma);
+	      }
+	  }
 
 	relocation = value + signed_addend;
 	relocation -= (input_section->output_section->vma
diff --git a/ld/testsuite/ld-arm/arm-elf.exp b/ld/testsuite/ld-arm/arm-elf.exp
index 0576847..3289dfd 100644
--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -449,6 +449,16 @@ set armeabitests_nonacl {
      {{objdump -d farcall-thumb-arm-pic-veneer.d}}
      "farcall-thumb-arm-pic-veneer"}
 
+    {"Thumb-ARM farcall cond" "-Ttext 0x8000 --section-start .foo=0x118000" "" "-W" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm"}
+    {"Thumb-ARM farcall cond (BE8)" "-Ttext 0x8000 --section-start .foo=0x118000 -EB --be8" "" "-W -EB" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm-be8"}
+    {"Thumb-ARM farcall cond (BE)" "-Ttext 0x8000 --section-start .foo=0x118000 -EB" "" "-W -EB" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm-be"}
+
     {"Multiple farcalls" "-Ttext 0x1000 --section-start .foo=0x2002020" "" "" {farcall-mix.s}
      {{objdump -d farcall-mix.d}}
      "farcall-mix"}
@@ -547,6 +557,31 @@ set armeabitests_nonacl {
      {{objdump -d jump-reloc-veneers-long.d}}
      "jump-reloc-veneers-long"}
 
+    {"R_ARM_THM_JUMP19 Relocation veneers: Short"
+     "--section-start destsect=0x000108002 --section-start .text=0x8000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-short.d}}
+     "jump-reloc-veneers-cond-short"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Long"
+     "--section-start destsect=0x00108004 --section-start .text=0x8000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-long.d}}
+     "jump-reloc-veneers-cond-long"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Short backward"
+     "--section-start destsect=0x8004 --section-start .text=0x108000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-short-backward.d}}
+     "jump-reloc-veneers-cond-short-backward"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Long backward"
+     "--section-start destsect=0x8002 --section-start .text=0x108000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-long-backward.d}}
+     "jump-reloc-veneers-cond-long-backward"}
+
     {"Default group size" "-Ttext 0x1000 --section-start .foo=0x2003020" "" "" {farcall-group.s farcall-group2.s}
      {{objdump -d farcall-group.d}}
      "farcall-group-default"}
diff --git a/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d
new file mode 100644
index 0000000..0b0172b
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d
@@ -0,0 +1,18 @@
+.*:     file format .*
+
+Disassembly of section .text:
+
+00008000 <_start>:
+    8000:	f050 a002 	bne.w	58008 <__bar_from_thumb>
+	\.\.\.
+   58004:	f040 8000 	bne.w	58008 <__bar_from_thumb>
+
+00058008 <__bar_from_thumb>:
+   58008:	4778      	bx	pc
+   5800a:	46c0      	nop			; \(mov r8, r8\)
+   5800c:	ea02fffb 	b	118000 <bar>
+
+Disassembly of section .foo:
+
+00118000 <bar>:
+  118000:	e12fff1e 	bx	lr
diff --git a/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s
new file mode 100644
index 0000000..809f2fc
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s
@@ -0,0 +1,27 @@
+@ Test to ensure that a Thumb to ARM call exceeding 4Mb generates a stub.
+@ Check that we can generate two types of stub in the same section.
+
+	.global _start
+	.syntax unified
+
+@ We will place the section .text at 0x1c01010.
+
+	.text
+	.thumb_func
+_start:
+	.global bar
+	bne bar
+@ This call is close enough to generate a "short branch" stub
+@ or no stub if blx is available.
+	.space 0x050000
+	bne bar
+
+@ We will place the section .foo at 0x2001014.
+
+	.section .foo, "xa"
+
+	.arm
+	.type bar, %function
+bar:
+	bx lr
+
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d
new file mode 100644
index 0000000..ee0709a
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d
@@ -0,0 +1,24 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00008002 <[^>]*>:
+    8002:	f7ff fffe 	bl	8002 <dest>
+
+Disassembly of section .text:
+
+001080.. <[^>]*>:
+  1080..:	f040 8002 	bne.w	108008 <__dest_veneer>
+  1080..:	0000      	movs	r0, r0
+	...
+
+001080.. <[^>]*>:
+  1080..:	b401      	push	{r0}
+  1080..:	4802      	ldr	r0, \[pc, #8\]	; \(108014 <__dest_veneer\+0xc>\)
+  1080..:	4684      	mov	ip, r0
+  1080..:	bc01      	pop	{r0}
+  1080..:	4760      	bx	ip
+  1080..:	bf00      	nop
+  1080..:	00008003 	.word	0x00008003
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d
new file mode 100644
index 0000000..276a24e
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d
@@ -0,0 +1,24 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00108004 <[^>]*>:
+  108004:	f7ff fffe 	bl	108004 <dest>
+
+Disassembly of section .text:
+
+000080.. <[^>]*>:
+    80..:	(8002f040|f0408002) 	.word	0x(8002f040|f0408002)
+    80..:	0000      	movs	r0, r0
+	...
+
+000080.. <[^>]*>:
+    80..:	b401      	push	{r0}
+    80..:	4802      	ldr	r0, \[pc, #8\]	; \(80.. <__dest_veneer\+0xc>\)
+    80..:	4684      	mov	ip, r0
+    80..:	bc01      	pop	{r0}
+    80..:	4760      	bx	ip
+    80..:	bf00      	nop
+    80..:	00108005 	.word	0x00108005
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d
new file mode 100644
index 0000000..d05425b
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d
@@ -0,0 +1,13 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00008004 <[^>]*>:
+    8004:	f7ff fffe 	bl	8004 <dest>
+
+Disassembly of section .text:
+
+001080.. <_start>:
+  1080..:	f440 8000 	bne.w	8004 <dest>
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d
new file mode 100644
index 0000000..08c2212
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d
@@ -0,0 +1,13 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00108002 <[^>]*>:
+  108002:	f7ff fffe 	bl	108002 <dest>
+
+Disassembly of section .text:
+
+000080.. <[^>]*>:
+    80..:	f07f afff 	bne.w	108002 <dest>
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s b/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s
new file mode 100644
index 0000000..83f969c
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s
@@ -0,0 +1,12 @@
+	.text
+	.syntax unified
+	.thumb_func
+	.global _start
+	.type _start,%function
+_start:
+	bne dest
+
+	.section destsect, "x"
+	.thumb_func
+dest:
+	bl dest

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

* RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
  2014-08-04 10:32         ` Will Newton
  2014-08-05  1:46           ` Tony Wang
@ 2014-08-12  2:25           ` Tony Wang
  2014-08-19  2:38           ` Tony Wang
  2 siblings, 0 replies; 11+ messages in thread
From: Tony Wang @ 2014-08-12  2:25 UTC (permalink / raw)
  To: binutils; +Cc: nickc

[-- Attachment #1: Type: text/plain, Size: 5170 bytes --]

Ping? Is it ok for trunk?

BR,
Tony
> -----Original Message-----
> From: Tony Wang [mailto:tony.wang@arm.com]
> Sent: Tuesday, August 05, 2014 9:46 AM
> To: 'Will Newton'; binutils@sourceware.org
> Cc: nickc@redhat.com
> Subject: RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> 
> New patch updated, fix Will's failure.
> 
> And it looks strange to me why binutils change the first branch instruction into a 32bit data, which introduced
> endian difference. Maybe anyone can help on me to understand why the first branch instruction is optimized
> out in some situation? However, as the jump24 is just doing the same thing, in this patch, I just keep conform
> with the old test case.
> 
> BR,
> Tony
> > -----Original Message-----
> > From: Will Newton [mailto:will.newton@linaro.org]
> > Sent: Monday, August 04, 2014 6:33 PM
> > To: Tony Wang
> > Cc: binutils@sourceware.org; nickc@redhat.com
> > Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> >
> > On 4 August 2014 11:27, Tony Wang <tony.wang@arm.com> wrote:
> > >> -----Original Message-----
> > >> From: Will Newton [mailto:will.newton@linaro.org]
> > >> Sent: Monday, August 04, 2014 5:50 PM
> > >> To: Tony Wang
> > >> Cc: binutils@sourceware.org; nickc@redhat.com
> > >> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> > >>
> > >> On 4 August 2014 10:40, Tony Wang <tony.wang@arm.com> wrote:
> > >> >> -----Original Message-----
> > >> >> From: Will Newton [mailto:will.newton@linaro.org]
> > >> >> Sent: Monday, August 04, 2014 5:14 PM
> > >> >> To: Tony Wang
> > >> >> Cc: binutils@sourceware.org; nickc@redhat.com
> > >> >> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> > >> >>
> > >> >> On 4 August 2014 05:40, Tony Wang <tony.wang@arm.com> wrote:
> > >> >> > Hi there,
> > >> >> >
> > >> >> > According to AAPCS-ELF specification R_ARM_THM_JUMP19 should
> > >> >> > also be veneered if the target is outside the addressable span
> > >> >> > of the branch instruction or where interworking happened.
> > >> >> >
> > >> >> > So the attached patch implements the veneer routine for
> > >> >> > R_ARM_THM_JUMP19, and two new macros
> > >> >> > THM2_MAX_FWD_COND_BRANCH_OFFSET
> > >> >> > and THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced.
 > > >> >> > Also updated the
> > >> >> > conditional branch for both cases mentioned above.
> > >> >> >
> > >> >> > Tested with Binutils regression test, no new regressions. No
> > >> >> > regression on gcc trunk with target cortex m4
> > >> >> >
> > >> >> > Ok for the trunk?
> > >> >> >
> > >> >> > bfd/ChangeLog:
> > >> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> > >> >> >
> > >> >> >      * elf32-arm.c (elf32_arm_final_link_relocate): Implement
> > >> >> > the veneer routine for R_ARM_THM_JUMP19.
> > >> >> >      (arm_type_of_stub): Add conditional clause for R_ARM_THM_JUMP19
> > >> >> >      (elf32_arm_size_stub): Ditto.
> > >> >> >
> > >> >> > ld/testsuite/ChangeLog:
> > >> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> > >> >> >
> > >> >> >      * ld-arm/jump-reloc-veneers-cond.s: New test.
> > >> >> >      * ld-arm/farcall-cond-thumb-arm.s: Ditto.
> > >> >> >      * ld-arm/jump-reloc-veneers-cond-short.d: Expected output
> > >> >> > for target without a veneer generation.
> > >> >> >      * ld-arm/jump-reloc-veneers-cond-long.d: Expected output
> > >> >> > for target with a veneer generation.
> > >> >> >      * ld-arm/farcall-cond-thumb-arm.d: Expected output for
> > >> >> > inter working veneer generation.
> > >> >> >      * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.
> > >> >>
> > >> >> Looks ok to me (although I am not a maintainer). Small details but
> > >> >> logical operations bind more loosely than almost every other
> > >> >> operator so in some cases you can drop the brackets around
> > >> >> assignments in conditionals and make them a bit easier to read.
> > >> >
> > >> > I agreed with your point.
> > >> >
> > >> >>Also the casts to unsigned int in
> > >> >> elf32_arm_size_stubs seem unnecessary.
> > >> >
> > >> > From my point of view, it might still needed here, as the enum type
> > >> > not always equals to unsigned int.
> > >>
> > >> Fair enough, its not a big issue.
> > >>
> > >> I also noticed a testsuite failure for armeb:
> > >>
> > >> /home/will/linaro/binutils-arm/ld/../binutils/objdump -d
> > >> tmpdir/jump-reloc- veneers-cond-long regexp_diff match failure
> > >> regexp "^    80..:      8002f040        .word   0x8002f040$"
> > >> line   "    8000:       f0408002        .word   0xf0408002"
> > >> FAIL: R_ARM_THM_JUMP19 Relocation veneers: Long
> > >>
> > >
> > > Hi Will,
> > > Thanks a lot for your report, and are you using the big endian toolchain? It
> > > looks like an endian issue. I only tested it with little endian toolchain.
> >
> > I configured binutils with --target=armeb-none-eabi, although I also
> > found that armeb EABI targets seem to be broken in general so I
> > submitted a patch to fix that.
> >
> > --
> > Will Newton
> > Toolchain Working Group, Linaro

[-- Attachment #2: relocation_jump19.diff --]
[-- Type: application/octet-stream, Size: 11100 bytes --]

diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index e6f4a9f..a40f9f1 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -2283,6 +2283,8 @@ static const bfd_vma elf32_arm_nacl_plt_entry [] =
 #define THM_MAX_BWD_BRANCH_OFFSET  (-(1 << 22) + 4)
 #define THM2_MAX_FWD_BRANCH_OFFSET (((1 << 24) - 2) + 4)
 #define THM2_MAX_BWD_BRANCH_OFFSET (-(1 << 24) + 4)
+#define THM2_MAX_FWD_COND_BRANCH_OFFSET (((1 << 20) -2) + 4)
+#define THM2_MAX_BWD_COND_BRANCH_OFFSET (-(1 << 20) + 4)
 
 enum stub_insn_type
 {
@@ -3667,7 +3669,8 @@ arm_type_of_stub (struct bfd_link_info *info,
 
   /* 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)
+  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;
 
@@ -3711,7 +3714,7 @@ arm_type_of_stub (struct bfd_link_info *info,
   branch_offset = (bfd_signed_vma)(destination - location);
 
   if (r_type == R_ARM_THM_CALL || r_type == R_ARM_THM_JUMP24
-      || r_type == R_ARM_THM_TLS_CALL)
+      || r_type == R_ARM_THM_TLS_CALL || r_type == R_ARM_THM_JUMP19)
     {
       /* Handle cases where:
 	 - this call goes too far (different Thumb/Thumb2 max
@@ -3727,10 +3730,15 @@ arm_type_of_stub (struct bfd_link_info *info,
 	  || (thumb2
 	      && (branch_offset > THM2_MAX_FWD_BRANCH_OFFSET
 		  || (branch_offset < THM2_MAX_BWD_BRANCH_OFFSET)))
+	  || (thumb2
+	      && (branch_offset > THM2_MAX_FWD_COND_BRANCH_OFFSET
+		  || (branch_offset < THM2_MAX_BWD_COND_BRANCH_OFFSET))
+	      && (r_type == R_ARM_THM_JUMP19))
 	  || (branch_type == ST_BRANCH_TO_ARM
 	      && (((r_type == R_ARM_THM_CALL
 		    || r_type == R_ARM_THM_TLS_CALL) && !globals->use_blx)
-		  || (r_type == R_ARM_THM_JUMP24))
+		  || (r_type == R_ARM_THM_JUMP24)
+                  || (r_type == R_ARM_THM_JUMP19))
 	      && !use_plt))
 	{
 	  if (branch_type == ST_BRANCH_TO_THUMB)
@@ -5347,7 +5355,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
 		      /* For historical reasons, use the existing names for
 			 ARM-to-Thumb and Thumb-to-ARM stubs.  */
 		      if ((r_type == (unsigned int) R_ARM_THM_CALL
-			   || r_type == (unsigned int) R_ARM_THM_JUMP24)
+			   || r_type == (unsigned int) R_ARM_THM_JUMP24
+                           || r_type == (unsigned int) R_ARM_THM_JUMP19)
 			  && branch_type == ST_BRANCH_TO_ARM)
 			sprintf (stub_entry->output_name,
 				 THUMB2ARM_GLUE_ENTRY_NAME, sym_name);
@@ -9125,6 +9134,9 @@ elf32_arm_final_link_relocate (reloc_howto_type *           howto,
 	bfd_signed_vma reloc_signed_max = 0xffffe;
 	bfd_signed_vma reloc_signed_min = -0x100000;
 	bfd_signed_vma signed_check;
+        enum elf32_arm_stub_type stub_type = arm_stub_none;
+	struct elf32_arm_stub_hash_entry *stub_entry;
+	struct elf32_arm_link_hash_entry *hash;
 
 	/* Need to refetch the addend, reconstruct the top three bits,
 	   and squish the two 11 bit pieces together.  */
@@ -9156,8 +9168,25 @@ elf32_arm_final_link_relocate (reloc_howto_type *           howto,
 	    *unresolved_reloc_p = FALSE;
 	  }
 
-	/* ??? Should handle interworking?  GCC might someday try to
-	   use this for tail calls.  */
+	hash = (struct elf32_arm_link_hash_entry *)h;
+
+	stub_type = arm_type_of_stub (info, input_section, rel,
+		                      st_type, &branch_type,
+		                      hash, value, sym_sec,
+		                      input_bfd, sym_name);
+	if (stub_type != arm_stub_none)
+	  {
+	    stub_entry = elf32_arm_get_stub_entry (input_section,
+				                   sym_sec, h,
+				                   rel, globals,
+				                   stub_type);
+	    if (stub_entry != NULL)
+	      {
+	        value = (stub_entry->stub_offset
+                        + stub_entry->stub_sec->output_offset
+                        + stub_entry->stub_sec->output_section->vma);
+	      }
+	  }
 
 	relocation = value + signed_addend;
 	relocation -= (input_section->output_section->vma
diff --git a/ld/testsuite/ld-arm/arm-elf.exp b/ld/testsuite/ld-arm/arm-elf.exp
index 0576847..3289dfd 100644
--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -449,6 +449,16 @@ set armeabitests_nonacl {
      {{objdump -d farcall-thumb-arm-pic-veneer.d}}
      "farcall-thumb-arm-pic-veneer"}
 
+    {"Thumb-ARM farcall cond" "-Ttext 0x8000 --section-start .foo=0x118000" "" "-W" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm"}
+    {"Thumb-ARM farcall cond (BE8)" "-Ttext 0x8000 --section-start .foo=0x118000 -EB --be8" "" "-W -EB" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm-be8"}
+    {"Thumb-ARM farcall cond (BE)" "-Ttext 0x8000 --section-start .foo=0x118000 -EB" "" "-W -EB" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm-be"}
+
     {"Multiple farcalls" "-Ttext 0x1000 --section-start .foo=0x2002020" "" "" {farcall-mix.s}
      {{objdump -d farcall-mix.d}}
      "farcall-mix"}
@@ -547,6 +557,31 @@ set armeabitests_nonacl {
      {{objdump -d jump-reloc-veneers-long.d}}
      "jump-reloc-veneers-long"}
 
+    {"R_ARM_THM_JUMP19 Relocation veneers: Short"
+     "--section-start destsect=0x000108002 --section-start .text=0x8000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-short.d}}
+     "jump-reloc-veneers-cond-short"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Long"
+     "--section-start destsect=0x00108004 --section-start .text=0x8000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-long.d}}
+     "jump-reloc-veneers-cond-long"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Short backward"
+     "--section-start destsect=0x8004 --section-start .text=0x108000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-short-backward.d}}
+     "jump-reloc-veneers-cond-short-backward"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Long backward"
+     "--section-start destsect=0x8002 --section-start .text=0x108000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-long-backward.d}}
+     "jump-reloc-veneers-cond-long-backward"}
+
     {"Default group size" "-Ttext 0x1000 --section-start .foo=0x2003020" "" "" {farcall-group.s farcall-group2.s}
      {{objdump -d farcall-group.d}}
      "farcall-group-default"}
diff --git a/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d
new file mode 100644
index 0000000..0b0172b
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d
@@ -0,0 +1,18 @@
+.*:     file format .*
+
+Disassembly of section .text:
+
+00008000 <_start>:
+    8000:	f050 a002 	bne.w	58008 <__bar_from_thumb>
+	\.\.\.
+   58004:	f040 8000 	bne.w	58008 <__bar_from_thumb>
+
+00058008 <__bar_from_thumb>:
+   58008:	4778      	bx	pc
+   5800a:	46c0      	nop			; \(mov r8, r8\)
+   5800c:	ea02fffb 	b	118000 <bar>
+
+Disassembly of section .foo:
+
+00118000 <bar>:
+  118000:	e12fff1e 	bx	lr
diff --git a/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s
new file mode 100644
index 0000000..809f2fc
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s
@@ -0,0 +1,27 @@
+@ Test to ensure that a Thumb to ARM call exceeding 4Mb generates a stub.
+@ Check that we can generate two types of stub in the same section.
+
+	.global _start
+	.syntax unified
+
+@ We will place the section .text at 0x1c01010.
+
+	.text
+	.thumb_func
+_start:
+	.global bar
+	bne bar
+@ This call is close enough to generate a "short branch" stub
+@ or no stub if blx is available.
+	.space 0x050000
+	bne bar
+
+@ We will place the section .foo at 0x2001014.
+
+	.section .foo, "xa"
+
+	.arm
+	.type bar, %function
+bar:
+	bx lr
+
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d
new file mode 100644
index 0000000..ee0709a
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d
@@ -0,0 +1,24 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00008002 <[^>]*>:
+    8002:	f7ff fffe 	bl	8002 <dest>
+
+Disassembly of section .text:
+
+001080.. <[^>]*>:
+  1080..:	f040 8002 	bne.w	108008 <__dest_veneer>
+  1080..:	0000      	movs	r0, r0
+	...
+
+001080.. <[^>]*>:
+  1080..:	b401      	push	{r0}
+  1080..:	4802      	ldr	r0, \[pc, #8\]	; \(108014 <__dest_veneer\+0xc>\)
+  1080..:	4684      	mov	ip, r0
+  1080..:	bc01      	pop	{r0}
+  1080..:	4760      	bx	ip
+  1080..:	bf00      	nop
+  1080..:	00008003 	.word	0x00008003
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d
new file mode 100644
index 0000000..276a24e
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d
@@ -0,0 +1,24 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00108004 <[^>]*>:
+  108004:	f7ff fffe 	bl	108004 <dest>
+
+Disassembly of section .text:
+
+000080.. <[^>]*>:
+    80..:	(8002f040|f0408002) 	.word	0x(8002f040|f0408002)
+    80..:	0000      	movs	r0, r0
+	...
+
+000080.. <[^>]*>:
+    80..:	b401      	push	{r0}
+    80..:	4802      	ldr	r0, \[pc, #8\]	; \(80.. <__dest_veneer\+0xc>\)
+    80..:	4684      	mov	ip, r0
+    80..:	bc01      	pop	{r0}
+    80..:	4760      	bx	ip
+    80..:	bf00      	nop
+    80..:	00108005 	.word	0x00108005
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d
new file mode 100644
index 0000000..d05425b
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d
@@ -0,0 +1,13 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00008004 <[^>]*>:
+    8004:	f7ff fffe 	bl	8004 <dest>
+
+Disassembly of section .text:
+
+001080.. <_start>:
+  1080..:	f440 8000 	bne.w	8004 <dest>
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d
new file mode 100644
index 0000000..08c2212
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d
@@ -0,0 +1,13 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00108002 <[^>]*>:
+  108002:	f7ff fffe 	bl	108002 <dest>
+
+Disassembly of section .text:
+
+000080.. <[^>]*>:
+    80..:	f07f afff 	bne.w	108002 <dest>
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s b/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s
new file mode 100644
index 0000000..83f969c
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s
@@ -0,0 +1,12 @@
+	.text
+	.syntax unified
+	.thumb_func
+	.global _start
+	.type _start,%function
+_start:
+	bne dest
+
+	.section destsect, "x"
+	.thumb_func
+dest:
+	bl dest

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

* RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
  2014-08-04 10:32         ` Will Newton
  2014-08-05  1:46           ` Tony Wang
  2014-08-12  2:25           ` Tony Wang
@ 2014-08-19  2:38           ` Tony Wang
  2014-08-19  8:05             ` Will Newton
  2014-08-20 15:50             ` Nicholas Clifton
  2 siblings, 2 replies; 11+ messages in thread
From: Tony Wang @ 2014-08-19  2:38 UTC (permalink / raw)
  To: binutils; +Cc: nickc

[-- Attachment #1: Type: text/plain, Size: 5694 bytes --]

Ping 2? Is this patch ok?

BR,
Tony
> -----Original Message-----
> From: Tony Wang [mailto:tony.wang@arm.com]
> Sent: Tuesday, August 12, 2014 10:25 AM
> To: 'binutils@sourceware.org'
> Cc: nickc@redhat.com
> Subject: RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> 
> Ping? Is it ok for trunk?
> 
> BR,
> Tony
> > -----Original Message-----
> > From: Tony Wang [mailto:tony.wang@arm.com]
> > Sent: Tuesday, August 05, 2014 9:46 AM
> > To: 'Will Newton'; binutils@sourceware.org
> > Cc: nickc@redhat.com
> > Subject: RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> >
> > New patch updated, fix Will's failure.
> >
> > And it looks strange to me why binutils change the first branch instruction into a 32bit data, which introduced
> > endian difference. Maybe anyone can help on me to understand why the first branch instruction is optimized
> > out in some situation? However, as the jump24 is just doing the same thing, in this patch, I just keep conform
> > with the old test case.
> >
> > BR,
> > Tony
> > > -----Original Message-----
> > > From: Will Newton [mailto:will.newton@linaro.org]
> > > Sent: Monday, August 04, 2014 6:33 PM
> > > To: Tony Wang
> > > Cc: binutils@sourceware.org; nickc@redhat.com
> > > Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> > >
> > > On 4 August 2014 11:27, Tony Wang <tony.wang@arm.com> wrote:
> > > >> -----Original Message-----
> > > >> From: Will Newton [mailto:will.newton@linaro.org]
> > > >> Sent: Monday, August 04, 2014 5:50 PM
> > > >> To: Tony Wang
> > > >> Cc: binutils@sourceware.org; nickc@redhat.com
> > > >> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> > > >>
> > > >> On 4 August 2014 10:40, Tony Wang <tony.wang@arm.com> wrote:
> > > >> >> -----Original Message-----
> > > >> >> From: Will Newton [mailto:will.newton@linaro.org]
> > > >> >> Sent: Monday, August 04, 2014 5:14 PM
> > > >> >> To: Tony Wang
> > > >> >> Cc: binutils@sourceware.org; nickc@redhat.com
> > > >> >> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> > > >> >>
> > > >> >> On 4 August 2014 05:40, Tony Wang <tony.wang@arm.com> wrote:
> > > >> >> > Hi there,
> > > >> >> >
> > > >> >> > According to AAPCS-ELF specification R_ARM_THM_JUMP19 should
> > > >> >> > also be veneered if the target is outside the addressable span
> > > >> >> > of the branch instruction or where interworking happened.
> > > >> >> >
> > > >> >> > So the attached patch implements the veneer routine for
> > > >> >> > R_ARM_THM_JUMP19, and two new macros
> > > >> >> > THM2_MAX_FWD_COND_BRANCH_OFFSET
> > > >> >> > and THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced.
>  > > >> >> > Also updated the
> > > >> >> > conditional branch for both cases mentioned above.
> > > >> >> >
> > > >> >> > Tested with Binutils regression test, no new regressions. No
> > > >> >> > regression on gcc trunk with target cortex m4
> > > >> >> >
> > > >> >> > Ok for the trunk?
> > > >> >> >
> > > >> >> > bfd/ChangeLog:
> > > >> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> > > >> >> >
> > > >> >> >      * elf32-arm.c (elf32_arm_final_link_relocate): Implement
> > > >> >> > the veneer routine for R_ARM_THM_JUMP19.
> > > >> >> >      (arm_type_of_stub): Add conditional clause for R_ARM_THM_JUMP19
> > > >> >> >      (elf32_arm_size_stub): Ditto.
> > > >> >> >
> > > >> >> > ld/testsuite/ChangeLog:
> > > >> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> > > >> >> >
> > > >> >> >      * ld-arm/jump-reloc-veneers-cond.s: New test.
> > > >> >> >      * ld-arm/farcall-cond-thumb-arm.s: Ditto.
> > > >> >> >      * ld-arm/jump-reloc-veneers-cond-short.d: Expected output
> > > >> >> > for target without a veneer generation.
> > > >> >> >      * ld-arm/jump-reloc-veneers-cond-long.d: Expected output
> > > >> >> > for target with a veneer generation.
> > > >> >> >      * ld-arm/farcall-cond-thumb-arm.d: Expected output for
> > > >> >> > inter working veneer generation.
> > > >> >> >      * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.
> > > >> >>
> > > >> >> Looks ok to me (although I am not a maintainer). Small details but
> > > >> >> logical operations bind more loosely than almost every other
> > > >> >> operator so in some cases you can drop the brackets around
> > > >> >> assignments in conditionals and make them a bit easier to read.
> > > >> >
> > > >> > I agreed with your point.
> > > >> >
> > > >> >>Also the casts to unsigned int in
> > > >> >> elf32_arm_size_stubs seem unnecessary.
> > > >> >
> > > >> > From my point of view, it might still needed here, as the enum type
> > > >> > not always equals to unsigned int.
> > > >>
> > > >> Fair enough, its not a big issue.
> > > >>
> > > >> I also noticed a testsuite failure for armeb:
> > > >>
> > > >> /home/will/linaro/binutils-arm/ld/../binutils/objdump -d
> > > >> tmpdir/jump-reloc- veneers-cond-long regexp_diff match failure
> > > >> regexp "^    80..:      8002f040        .word   0x8002f040$"
> > > >> line   "    8000:       f0408002        .word   0xf0408002"
> > > >> FAIL: R_ARM_THM_JUMP19 Relocation veneers: Long
> > > >>
> > > >
> > > > Hi Will,
> > > > Thanks a lot for your report, and are you using the big endian toolchain? It
> > > > looks like an endian issue. I only tested it with little endian toolchain.
> > >
> > > I configured binutils with --target=armeb-none-eabi, although I also
> > > found that armeb EABI targets seem to be broken in general so I
> > > submitted a patch to fix that.
> > >
> > > --
> > > Will Newton
> > > Toolchain Working Group, Linaro

[-- Attachment #2: relocation_jump19.diff --]
[-- Type: application/octet-stream, Size: 11100 bytes --]

diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index e6f4a9f..a40f9f1 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -2283,6 +2283,8 @@ static const bfd_vma elf32_arm_nacl_plt_entry [] =
 #define THM_MAX_BWD_BRANCH_OFFSET  (-(1 << 22) + 4)
 #define THM2_MAX_FWD_BRANCH_OFFSET (((1 << 24) - 2) + 4)
 #define THM2_MAX_BWD_BRANCH_OFFSET (-(1 << 24) + 4)
+#define THM2_MAX_FWD_COND_BRANCH_OFFSET (((1 << 20) -2) + 4)
+#define THM2_MAX_BWD_COND_BRANCH_OFFSET (-(1 << 20) + 4)
 
 enum stub_insn_type
 {
@@ -3667,7 +3669,8 @@ arm_type_of_stub (struct bfd_link_info *info,
 
   /* 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)
+  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;
 
@@ -3711,7 +3714,7 @@ arm_type_of_stub (struct bfd_link_info *info,
   branch_offset = (bfd_signed_vma)(destination - location);
 
   if (r_type == R_ARM_THM_CALL || r_type == R_ARM_THM_JUMP24
-      || r_type == R_ARM_THM_TLS_CALL)
+      || r_type == R_ARM_THM_TLS_CALL || r_type == R_ARM_THM_JUMP19)
     {
       /* Handle cases where:
 	 - this call goes too far (different Thumb/Thumb2 max
@@ -3727,10 +3730,15 @@ arm_type_of_stub (struct bfd_link_info *info,
 	  || (thumb2
 	      && (branch_offset > THM2_MAX_FWD_BRANCH_OFFSET
 		  || (branch_offset < THM2_MAX_BWD_BRANCH_OFFSET)))
+	  || (thumb2
+	      && (branch_offset > THM2_MAX_FWD_COND_BRANCH_OFFSET
+		  || (branch_offset < THM2_MAX_BWD_COND_BRANCH_OFFSET))
+	      && (r_type == R_ARM_THM_JUMP19))
 	  || (branch_type == ST_BRANCH_TO_ARM
 	      && (((r_type == R_ARM_THM_CALL
 		    || r_type == R_ARM_THM_TLS_CALL) && !globals->use_blx)
-		  || (r_type == R_ARM_THM_JUMP24))
+		  || (r_type == R_ARM_THM_JUMP24)
+                  || (r_type == R_ARM_THM_JUMP19))
 	      && !use_plt))
 	{
 	  if (branch_type == ST_BRANCH_TO_THUMB)
@@ -5347,7 +5355,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
 		      /* For historical reasons, use the existing names for
 			 ARM-to-Thumb and Thumb-to-ARM stubs.  */
 		      if ((r_type == (unsigned int) R_ARM_THM_CALL
-			   || r_type == (unsigned int) R_ARM_THM_JUMP24)
+			   || r_type == (unsigned int) R_ARM_THM_JUMP24
+                           || r_type == (unsigned int) R_ARM_THM_JUMP19)
 			  && branch_type == ST_BRANCH_TO_ARM)
 			sprintf (stub_entry->output_name,
 				 THUMB2ARM_GLUE_ENTRY_NAME, sym_name);
@@ -9125,6 +9134,9 @@ elf32_arm_final_link_relocate (reloc_howto_type *           howto,
 	bfd_signed_vma reloc_signed_max = 0xffffe;
 	bfd_signed_vma reloc_signed_min = -0x100000;
 	bfd_signed_vma signed_check;
+        enum elf32_arm_stub_type stub_type = arm_stub_none;
+	struct elf32_arm_stub_hash_entry *stub_entry;
+	struct elf32_arm_link_hash_entry *hash;
 
 	/* Need to refetch the addend, reconstruct the top three bits,
 	   and squish the two 11 bit pieces together.  */
@@ -9156,8 +9168,25 @@ elf32_arm_final_link_relocate (reloc_howto_type *           howto,
 	    *unresolved_reloc_p = FALSE;
 	  }
 
-	/* ??? Should handle interworking?  GCC might someday try to
-	   use this for tail calls.  */
+	hash = (struct elf32_arm_link_hash_entry *)h;
+
+	stub_type = arm_type_of_stub (info, input_section, rel,
+		                      st_type, &branch_type,
+		                      hash, value, sym_sec,
+		                      input_bfd, sym_name);
+	if (stub_type != arm_stub_none)
+	  {
+	    stub_entry = elf32_arm_get_stub_entry (input_section,
+				                   sym_sec, h,
+				                   rel, globals,
+				                   stub_type);
+	    if (stub_entry != NULL)
+	      {
+	        value = (stub_entry->stub_offset
+                        + stub_entry->stub_sec->output_offset
+                        + stub_entry->stub_sec->output_section->vma);
+	      }
+	  }
 
 	relocation = value + signed_addend;
 	relocation -= (input_section->output_section->vma
diff --git a/ld/testsuite/ld-arm/arm-elf.exp b/ld/testsuite/ld-arm/arm-elf.exp
index 0576847..3289dfd 100644
--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -449,6 +449,16 @@ set armeabitests_nonacl {
      {{objdump -d farcall-thumb-arm-pic-veneer.d}}
      "farcall-thumb-arm-pic-veneer"}
 
+    {"Thumb-ARM farcall cond" "-Ttext 0x8000 --section-start .foo=0x118000" "" "-W" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm"}
+    {"Thumb-ARM farcall cond (BE8)" "-Ttext 0x8000 --section-start .foo=0x118000 -EB --be8" "" "-W -EB" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm-be8"}
+    {"Thumb-ARM farcall cond (BE)" "-Ttext 0x8000 --section-start .foo=0x118000 -EB" "" "-W -EB" {farcall-cond-thumb-arm.s}
+     {{objdump -d farcall-cond-thumb-arm.d}}
+     "farcall-cond-thumb-arm-be"}
+
     {"Multiple farcalls" "-Ttext 0x1000 --section-start .foo=0x2002020" "" "" {farcall-mix.s}
      {{objdump -d farcall-mix.d}}
      "farcall-mix"}
@@ -547,6 +557,31 @@ set armeabitests_nonacl {
      {{objdump -d jump-reloc-veneers-long.d}}
      "jump-reloc-veneers-long"}
 
+    {"R_ARM_THM_JUMP19 Relocation veneers: Short"
+     "--section-start destsect=0x000108002 --section-start .text=0x8000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-short.d}}
+     "jump-reloc-veneers-cond-short"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Long"
+     "--section-start destsect=0x00108004 --section-start .text=0x8000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-long.d}}
+     "jump-reloc-veneers-cond-long"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Short backward"
+     "--section-start destsect=0x8004 --section-start .text=0x108000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-short-backward.d}}
+     "jump-reloc-veneers-cond-short-backward"}
+    {"R_ARM_THM_JUMP19 Relocation veneers: Long backward"
+     "--section-start destsect=0x8002 --section-start .text=0x108000" ""
+     "-march=armv7-m -mthumb"
+     {jump-reloc-veneers-cond.s}
+     {{objdump -d jump-reloc-veneers-cond-long-backward.d}}
+     "jump-reloc-veneers-cond-long-backward"}
+
     {"Default group size" "-Ttext 0x1000 --section-start .foo=0x2003020" "" "" {farcall-group.s farcall-group2.s}
      {{objdump -d farcall-group.d}}
      "farcall-group-default"}
diff --git a/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d
new file mode 100644
index 0000000..0b0172b
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.d
@@ -0,0 +1,18 @@
+.*:     file format .*
+
+Disassembly of section .text:
+
+00008000 <_start>:
+    8000:	f050 a002 	bne.w	58008 <__bar_from_thumb>
+	\.\.\.
+   58004:	f040 8000 	bne.w	58008 <__bar_from_thumb>
+
+00058008 <__bar_from_thumb>:
+   58008:	4778      	bx	pc
+   5800a:	46c0      	nop			; \(mov r8, r8\)
+   5800c:	ea02fffb 	b	118000 <bar>
+
+Disassembly of section .foo:
+
+00118000 <bar>:
+  118000:	e12fff1e 	bx	lr
diff --git a/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s
new file mode 100644
index 0000000..809f2fc
--- /dev/null
+++ b/ld/testsuite/ld-arm/farcall-cond-thumb-arm.s
@@ -0,0 +1,27 @@
+@ Test to ensure that a Thumb to ARM call exceeding 4Mb generates a stub.
+@ Check that we can generate two types of stub in the same section.
+
+	.global _start
+	.syntax unified
+
+@ We will place the section .text at 0x1c01010.
+
+	.text
+	.thumb_func
+_start:
+	.global bar
+	bne bar
+@ This call is close enough to generate a "short branch" stub
+@ or no stub if blx is available.
+	.space 0x050000
+	bne bar
+
+@ We will place the section .foo at 0x2001014.
+
+	.section .foo, "xa"
+
+	.arm
+	.type bar, %function
+bar:
+	bx lr
+
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d
new file mode 100644
index 0000000..ee0709a
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long-backward.d
@@ -0,0 +1,24 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00008002 <[^>]*>:
+    8002:	f7ff fffe 	bl	8002 <dest>
+
+Disassembly of section .text:
+
+001080.. <[^>]*>:
+  1080..:	f040 8002 	bne.w	108008 <__dest_veneer>
+  1080..:	0000      	movs	r0, r0
+	...
+
+001080.. <[^>]*>:
+  1080..:	b401      	push	{r0}
+  1080..:	4802      	ldr	r0, \[pc, #8\]	; \(108014 <__dest_veneer\+0xc>\)
+  1080..:	4684      	mov	ip, r0
+  1080..:	bc01      	pop	{r0}
+  1080..:	4760      	bx	ip
+  1080..:	bf00      	nop
+  1080..:	00008003 	.word	0x00008003
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d
new file mode 100644
index 0000000..276a24e
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-long.d
@@ -0,0 +1,24 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00108004 <[^>]*>:
+  108004:	f7ff fffe 	bl	108004 <dest>
+
+Disassembly of section .text:
+
+000080.. <[^>]*>:
+    80..:	(8002f040|f0408002) 	.word	0x(8002f040|f0408002)
+    80..:	0000      	movs	r0, r0
+	...
+
+000080.. <[^>]*>:
+    80..:	b401      	push	{r0}
+    80..:	4802      	ldr	r0, \[pc, #8\]	; \(80.. <__dest_veneer\+0xc>\)
+    80..:	4684      	mov	ip, r0
+    80..:	bc01      	pop	{r0}
+    80..:	4760      	bx	ip
+    80..:	bf00      	nop
+    80..:	00108005 	.word	0x00108005
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d
new file mode 100644
index 0000000..d05425b
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short-backward.d
@@ -0,0 +1,13 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00008004 <[^>]*>:
+    8004:	f7ff fffe 	bl	8004 <dest>
+
+Disassembly of section .text:
+
+001080.. <_start>:
+  1080..:	f440 8000 	bne.w	8004 <dest>
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d
new file mode 100644
index 0000000..08c2212
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond-short.d
@@ -0,0 +1,13 @@
+
+.*:     file format.*
+
+
+Disassembly of section destsect:
+
+00108002 <[^>]*>:
+  108002:	f7ff fffe 	bl	108002 <dest>
+
+Disassembly of section .text:
+
+000080.. <[^>]*>:
+    80..:	f07f afff 	bne.w	108002 <dest>
diff --git a/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s b/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s
new file mode 100644
index 0000000..83f969c
--- /dev/null
+++ b/ld/testsuite/ld-arm/jump-reloc-veneers-cond.s
@@ -0,0 +1,12 @@
+	.text
+	.syntax unified
+	.thumb_func
+	.global _start
+	.type _start,%function
+_start:
+	bne dest
+
+	.section destsect, "x"
+	.thumb_func
+dest:
+	bl dest

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

* Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
  2014-08-19  2:38           ` Tony Wang
@ 2014-08-19  8:05             ` Will Newton
  2014-08-20 15:50             ` Nicholas Clifton
  1 sibling, 0 replies; 11+ messages in thread
From: Will Newton @ 2014-08-19  8:05 UTC (permalink / raw)
  To: Tony Wang; +Cc: binutils, nick clifton

On 19 August 2014 03:37, Tony Wang <tony.wang@arm.com> wrote:
> Ping 2? Is this patch ok?

Looks ok to me (although I am not a maintainer).

> BR,
> Tony
>> -----Original Message-----
>> From: Tony Wang [mailto:tony.wang@arm.com]
>> Sent: Tuesday, August 12, 2014 10:25 AM
>> To: 'binutils@sourceware.org'
>> Cc: nickc@redhat.com
>> Subject: RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
>>
>> Ping? Is it ok for trunk?
>>
>> BR,
>> Tony
>> > -----Original Message-----
>> > From: Tony Wang [mailto:tony.wang@arm.com]
>> > Sent: Tuesday, August 05, 2014 9:46 AM
>> > To: 'Will Newton'; binutils@sourceware.org
>> > Cc: nickc@redhat.com
>> > Subject: RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
>> >
>> > New patch updated, fix Will's failure.
>> >
>> > And it looks strange to me why binutils change the first branch instruction into a 32bit data, which introduced
>> > endian difference. Maybe anyone can help on me to understand why the first branch instruction is optimized
>> > out in some situation? However, as the jump24 is just doing the same thing, in this patch, I just keep conform
>> > with the old test case.
>> >
>> > BR,
>> > Tony
>> > > -----Original Message-----
>> > > From: Will Newton [mailto:will.newton@linaro.org]
>> > > Sent: Monday, August 04, 2014 6:33 PM
>> > > To: Tony Wang
>> > > Cc: binutils@sourceware.org; nickc@redhat.com
>> > > Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
>> > >
>> > > On 4 August 2014 11:27, Tony Wang <tony.wang@arm.com> wrote:
>> > > >> -----Original Message-----
>> > > >> From: Will Newton [mailto:will.newton@linaro.org]
>> > > >> Sent: Monday, August 04, 2014 5:50 PM
>> > > >> To: Tony Wang
>> > > >> Cc: binutils@sourceware.org; nickc@redhat.com
>> > > >> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
>> > > >>
>> > > >> On 4 August 2014 10:40, Tony Wang <tony.wang@arm.com> wrote:
>> > > >> >> -----Original Message-----
>> > > >> >> From: Will Newton [mailto:will.newton@linaro.org]
>> > > >> >> Sent: Monday, August 04, 2014 5:14 PM
>> > > >> >> To: Tony Wang
>> > > >> >> Cc: binutils@sourceware.org; nickc@redhat.com
>> > > >> >> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
>> > > >> >>
>> > > >> >> On 4 August 2014 05:40, Tony Wang <tony.wang@arm.com> wrote:
>> > > >> >> > Hi there,
>> > > >> >> >
>> > > >> >> > According to AAPCS-ELF specification R_ARM_THM_JUMP19 should
>> > > >> >> > also be veneered if the target is outside the addressable span
>> > > >> >> > of the branch instruction or where interworking happened.
>> > > >> >> >
>> > > >> >> > So the attached patch implements the veneer routine for
>> > > >> >> > R_ARM_THM_JUMP19, and two new macros
>> > > >> >> > THM2_MAX_FWD_COND_BRANCH_OFFSET
>> > > >> >> > and THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced.
>>  > > >> >> > Also updated the
>> > > >> >> > conditional branch for both cases mentioned above.
>> > > >> >> >
>> > > >> >> > Tested with Binutils regression test, no new regressions. No
>> > > >> >> > regression on gcc trunk with target cortex m4
>> > > >> >> >
>> > > >> >> > Ok for the trunk?
>> > > >> >> >
>> > > >> >> > bfd/ChangeLog:
>> > > >> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
>> > > >> >> >
>> > > >> >> >      * elf32-arm.c (elf32_arm_final_link_relocate): Implement
>> > > >> >> > the veneer routine for R_ARM_THM_JUMP19.
>> > > >> >> >      (arm_type_of_stub): Add conditional clause for R_ARM_THM_JUMP19
>> > > >> >> >      (elf32_arm_size_stub): Ditto.
>> > > >> >> >
>> > > >> >> > ld/testsuite/ChangeLog:
>> > > >> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
>> > > >> >> >
>> > > >> >> >      * ld-arm/jump-reloc-veneers-cond.s: New test.
>> > > >> >> >      * ld-arm/farcall-cond-thumb-arm.s: Ditto.
>> > > >> >> >      * ld-arm/jump-reloc-veneers-cond-short.d: Expected output
>> > > >> >> > for target without a veneer generation.
>> > > >> >> >      * ld-arm/jump-reloc-veneers-cond-long.d: Expected output
>> > > >> >> > for target with a veneer generation.
>> > > >> >> >      * ld-arm/farcall-cond-thumb-arm.d: Expected output for
>> > > >> >> > inter working veneer generation.
>> > > >> >> >      * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.
>> > > >> >>
>> > > >> >> Looks ok to me (although I am not a maintainer). Small details but
>> > > >> >> logical operations bind more loosely than almost every other
>> > > >> >> operator so in some cases you can drop the brackets around
>> > > >> >> assignments in conditionals and make them a bit easier to read.
>> > > >> >
>> > > >> > I agreed with your point.
>> > > >> >
>> > > >> >>Also the casts to unsigned int in
>> > > >> >> elf32_arm_size_stubs seem unnecessary.
>> > > >> >
>> > > >> > From my point of view, it might still needed here, as the enum type
>> > > >> > not always equals to unsigned int.
>> > > >>
>> > > >> Fair enough, its not a big issue.
>> > > >>
>> > > >> I also noticed a testsuite failure for armeb:
>> > > >>
>> > > >> /home/will/linaro/binutils-arm/ld/../binutils/objdump -d
>> > > >> tmpdir/jump-reloc- veneers-cond-long regexp_diff match failure
>> > > >> regexp "^    80..:      8002f040        .word   0x8002f040$"
>> > > >> line   "    8000:       f0408002        .word   0xf0408002"
>> > > >> FAIL: R_ARM_THM_JUMP19 Relocation veneers: Long
>> > > >>
>> > > >
>> > > > Hi Will,
>> > > > Thanks a lot for your report, and are you using the big endian toolchain? It
>> > > > looks like an endian issue. I only tested it with little endian toolchain.
>> > >
>> > > I configured binutils with --target=armeb-none-eabi, although I also
>> > > found that armeb EABI targets seem to be broken in general so I
>> > > submitted a patch to fix that.
>> > >
>> > > --
>> > > Will Newton
>> > > Toolchain Working Group, Linaro



-- 
Will Newton
Toolchain Working Group, Linaro

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

* Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
  2014-08-19  2:38           ` Tony Wang
  2014-08-19  8:05             ` Will Newton
@ 2014-08-20 15:50             ` Nicholas Clifton
  1 sibling, 0 replies; 11+ messages in thread
From: Nicholas Clifton @ 2014-08-20 15:50 UTC (permalink / raw)
  To: Tony Wang, binutils

Hi Tony,

> Ping 2? Is this patch ok?

It is - please apply.

Cheers
   Nick


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

end of thread, other threads:[~2014-08-20 15:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-04  4:40 [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19 Tony Wang
2014-08-04  9:14 ` Will Newton
2014-08-04  9:41   ` Tony Wang
2014-08-04  9:50     ` Will Newton
2014-08-04 10:27       ` Tony Wang
2014-08-04 10:32         ` Will Newton
2014-08-05  1:46           ` Tony Wang
2014-08-12  2:25           ` Tony Wang
2014-08-19  2:38           ` Tony Wang
2014-08-19  8:05             ` Will Newton
2014-08-20 15:50             ` Nicholas Clifton

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).