This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Long call support broke Thumb veneers


Hi Daniel,

What about the non-call (or pre-EABI, which may be call or not)
branch relocations?

            case R_ARM_PC24:
            case R_ARM_PLT32:
            case R_ARM_JUMP24:
            case R_ARM_THM_JUMP24:

I don't think it's a good idea to have two completely different code
paths which produce similar stubs.  And if you handle R_ARM_CALL but
not R_ARM_JUMP24, then you'll insert islands for normal calls but not
for sibling calls, which is unfortunate.


I wanted to avoid breaking existing code as much as possible.


My initial need is only to support R_ARM_CALL and R_ARM_THM_CALL relocations (we use only EABI).
I also considered changing the code handling the relocs you mention, but as I would be unable to test it properly I prefered not to change it.


Minor formatting comments on your patch: in function definitions the
function name starts a new line, and there is almost always a space
before a left parenthesis (but not if the preceeding character is a
left parenthesis also).

OK. I attached it again taking your remarks into account.

Looks like you've put them before the section that requires them.
After would avoid this problem.  Would it cause other problems?
There is no strong reason for putting them before.

Should we document in the manual where to expect them - and is there a
way to avoid getting them in the middle of tables?
What do you mean in your last sentence?

I have checked the ld documentation for --stub-group-size for HPPA and PowerPC, (which is the same word for word), and they don't document this issue. I don't know these architectures, so maybe the problem cannot occur in their cases?

Here's the start of that linker script, as context for other readers.

  .text :
  {
    CREATE_OBJECT_SYMBOLS
    __cs3_region_start_ram = .;
    *(.cs3.region-head.ram)
    ASSERT (. == __cs3_region_start_ram, ".cs3.region-head.ram not permitted");
    __cs3_interrupt_vector = __cs3_interrupt_vector_arm;
    *(.cs3.interrupt_vector)
    /* Make sure we pulled in an interrupt vector.  */
    ASSERT (. != __cs3_interrupt_vector_arm, "No interrupt vector");

    _start = __cs3_reset_generic;
    __cs3_reset = __cs3_reset_generic;
    *(.cs3.reset)
    /* Make sure we pulled in some reset code.  */
    ASSERT (. != __cs3_reset, "No reset code");

    *(.text .text.* .gnu.linkonce.t.*)
    *(.plt)
    *(.gnu.warning)
    *(.glue_7t) *(.glue_7) *(.vfp11_veneer)


What about grouping your cs3 regions like this:
.cs3:
{
__cs3_region_start_ram = .;
*(.cs3.region-head.ram)
ASSERT (. == __cs3_region_start_ram, ".cs3.region-head.ram not permitted");
__cs3_interrupt_vector = __cs3_interrupt_vector_arm;
*(.cs3.interrupt_vector)
/* Make sure we pulled in an interrupt vector. */
ASSERT (. != __cs3_interrupt_vector_arm, "No interrupt vector");


    _start = __cs3_reset_generic;
    __cs3_reset = __cs3_reset_generic;
    *(.cs3.reset)
    /* Make sure we pulled in some reset code.  */
    ASSERT (. != __cs3_reset, "No reset code");
} >ram

.text:
{
    CREATE_OBJECT_SYMBOLS

    *(.text .text.* .gnu.linkonce.t.*)
    *(.plt)
    *(.gnu.warning)
    *(.glue_7t) *(.glue_7) *(.vfp11_veneer)
....

Christophe.
2008-06-03  Christophe Lyon  <christophe.lyon@st.com>

	bfd/
	* elf32-arm.c (arm_stub_is_thumb): Define.
	(elf32_arm_final_link_relocate): Handle near mode switching stubs.

	ld/testsuite/
	* ld-arm/farcall-thumb-thumb-m.d: Fix branch type.
	* ld-arm/farcall-thumb-arm.d: Likewise.
Index: ld/testsuite/ld-arm/farcall-thumb-thumb-m.d
===================================================================
--- ld/testsuite/ld-arm/farcall-thumb-thumb-m.d	(revision 680)
+++ ld/testsuite/ld-arm/farcall-thumb-thumb-m.d	(working copy)
@@ -12,7 +12,7 @@ Disassembly of section .text:
     100c:	02001015 	.word	0x02001015
 
 00001010 <_start>:
-    1010:	f7ff eff6 	blx	1000 <_start-0x10>
+    1010:	f7ff fff6 	bl	1000 <_start-0x10>
 Disassembly of section .foo:
 
 02001014 <bar>:
Index: ld/testsuite/ld-arm/farcall-thumb-arm.d
===================================================================
--- ld/testsuite/ld-arm/farcall-thumb-arm.d	(revision 747)
+++ ld/testsuite/ld-arm/farcall-thumb-arm.d	(working copy)
@@ -12,7 +12,7 @@ Disassembly of section .text:
 	...
 
 00001018 <_start>:
-    1018:	f7ff eff2 	blx	1000 <_start-0x18>
+    1018:	f7ff fff2 	bl	1000 <_start-0x18>
 Disassembly of section .foo:
 
 02001014 <bar>:
Index: bfd/elf32-arm.c
===================================================================
--- bfd/elf32-arm.c	(revision 747)
+++ bfd/elf32-arm.c	(working copy)
@@ -2680,6 +2680,22 @@ elf32_arm_hash_table_free (struct bfd_li
 static int using_thumb2 (struct elf32_arm_link_hash_table *globals);
 static int using_thumb_only (struct elf32_arm_link_hash_table *globals);
 
+static bfd_boolean
+arm_stub_is_thumb (enum elf32_arm_stub_type stub_type)
+{
+  switch (stub_type) {
+  case arm_thumb_thumb_stub_long_branch:
+  case arm_thumb_arm_v4t_stub_long_branch:
+    return TRUE;
+  case arm_stub_none:
+    BFD_FAIL ();
+    return FALSE;
+    break;
+  default:
+    return FALSE;
+  }
+}
+
 static enum elf32_arm_stub_type
 arm_type_of_stub (struct bfd_link_info *info,
 		  asection *input_sec,
@@ -6306,6 +6322,7 @@ elf32_arm_final_link_relocate (reloc_how
 	       (branch_offset > THM2_MAX_FWD_BRANCH_OFFSET
 		|| (branch_offset < THM2_MAX_BWD_BRANCH_OFFSET))
 	       )
+	     || ((sym_flags != STT_ARM_TFUNC) && !globals->use_blx)
 	    )
 	  {
 	    /* The target is out of reach or we are changing modes, so
@@ -6320,10 +6337,14 @@ elf32_arm_final_link_relocate (reloc_how
 		       + stub_entry->stub_sec->output_offset
 		       + stub_entry->stub_sec->output_section->vma);
 
-	    /* This call becomes a call to Arm for sure. Force BLX */
+	    /* If this call becomes a call to Arm, force BLX */
+	    if (globals->use_blx) {
+	      if ( (stub_entry && !arm_stub_is_thumb(stub_entry->stub_type))
+		   || (sym_flags != STT_ARM_TFUNC) )
 	    lower_insn = (lower_insn & ~0x1000) | 0x0800;
 	  }
 	}
+	}
 
 	relocation = value + signed_addend;
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]