Long call support broke Thumb veneers

[Christophe LYON]

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.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ld.changelog
URL: <https://sourceware.org/pipermail/binutils/attachments/20080609/f1fc8eaa/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ld.patch
URL: <https://sourceware.org/pipermail/binutils/attachments/20080609/f1fc8eaa/attachment-0001.ksh>