Re: [ARM] global Thumb to ARM branches broken

[Ramana Radhakrishnan <ramana dot radhakrishnan at arm dot com>]

I'll need to take  a slightly more detailed look later and get back to you but here's something 
about the intent of the patch.

On Wed, 2010-02-03 at 16:15 +0100, Christophe LYON wrote:
> Hello,
> 
> It seems to me that the patch discussed in
> http://sourceware.org/ml/binutils/2009-03/msg00511.html
> broke the global Thumb branches by encoding the PC inside the instruction.
> 
> For instance:
> 	.global _start
> 	.syntax unified
> 	.text
> 	.thumb_func
> _start:
> 	.global bar
> 	.space 0x300000
> 	bl bar
> 
> 	.section .foo, "xa"
> 	.arm
> 	.type bar, %function
> bar:
> 	bx lr
> 
> is assembled into (using -march=armv5t):
> 00000000 <_start>:
>          ...
>    300000:       f4ff fffe       bl      ffd00000 <_start+0xffd00000>
>                          300000: R_ARM_THM_CALL  bar
> Disassembly of section .foo:
> 00000000 <bar>:
>     0:   e12fff1e        bx      lr
> 
> In turn, if one links, the resulting code is:
> 00008000 <_start>:
>          ...
>    308000:       f500 e800       blx     8004 <_start+0x4>
> Disassembly of section .foo:
> 00308004 <bar>:
>    308004:       e12fff1e        bx      lr


At the time I did this, I don't think I considered cases across sections
and that doesn't seem to be right. So it looks like this is a bit broken. 

> 
> This is because Ramana's fix in md_pcrel_from_section() does not check 
> whether both sides of the branch are in the same section, or if the 
> destination is external.
> 
> I am working on a patch, but some inputs/comments could help.
> Indeed, I am not sure what "in line with what happens with global 
> functions for both ARM and Thumb mode" really means (as appears in the 
> original post).
> 
> As I am not sure about the actual intent of that patch, I tried to 
> remove some parts, which did not result in regressions in the testsuite, 
> so some tests may be missing.

The original intent of the patch [1] was to transform any form of
branches and / or calls to local functions in the assembler from ARM
state to Thumb state and vice versa automatically. Also pushing out the
appropriate relocations for the linker to veneer for conditional
branches (and architectural variants) was the intent of this patch.

The reason for having this logic in the assembler is for the case where
you have function calls / branches to transform from one state to the
other and push the rest out to the linker to handle by veneering. For
arch versions that didn't have the equivalent {x} variant we would want
to push out the relocations. 


> Finally, I am not sure about the new behaviour of 
> arm_force_relocation(): it now returns 1 in several new occurrences, but 
> the corresponding relocations do not appear in the object file (see 
> blx-local test)

I'll have to take another look at this and get back to you.

> 
> I am trying to fix gas, but maybe a better fix would be in the linker 
> side, because the involved relocations are declared as *not* partial 
> inplace, but ld handles them as if they were (eg when it reads the 
> addend from upper_insn).

I would still try and fix this in GAS by converting bl's to blx's at the
right times. and pushing out relocs where GAS can't handle it for the
linker to handle.

cheers
Ramana


[1]  This was a part of a project that I was doing to get GCC to
automatically generate ARM and Thumb state functions within the same
compilation unit. That is still on the horizon, but having the support
in the assembler was necessary to allow cases where you had function
calls to static functions working correctly.