Re: Long call support broke Thumb veneers

[Daniel Jacobowitz <drow at false dot org>]

On Mon, Jun 09, 2008 at 03:16:50PM +0200, Christophe LYON wrote:
>> 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.

R_ARM_JUMP24 is also an EABI relocation, used for tail calls.

>> 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?

There's probably going to be more people using embedded ARM targets
with long jumps than either of the above, since for Thumb the range is
so small.  So I think it's more important here.

By tables, I mean e.g. the ARM vector table (which contains
branches).  If it's built up by more than one section, we're going to
throw branch stubs in the middle of it - not cool.  Something like this:

  *(.text.header.reset-vector)
  *(.text.header.unaligned-vector)
  *(.text.header.interrupt-vector)

Does that make more sense?

> What about grouping your cs3 regions like this:

I don't understand.  Why would this help?  You've broken the one
output section into two, but I believe we create stubs in front of
input sections, not in front of output sections.

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

I've still got concerns about the implementation, but this is clearly
an improvement.  It's OK; I'll check it in for you, but you may want
to request a sourceware account.

> +	    if (globals->use_blx) {
> +	      if ( (stub_entry && !arm_stub_is_thumb(stub_entry->stub_type))
> +		   || (sym_flags != STT_ARM_TFUNC) )

I shuffled the formatting around a little.  The if should be:

	      if ((stub_entry && !arm_stub_is_thumb (stub_entry->stub_type))
		   || (sym_flags != STT_ARM_TFUNC))

And braces go on their own line.

-- 
Daniel Jacobowitz
CodeSourcery