Re: [ARM] global Thumb to ARM branches broken

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

I had a quick look at this - 

On Thu, 2010-02-04 at 17:46 +0100, Christophe LYON wrote: 
> > 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.
> 
> I don't understand why your patch to md_pcrel_from_section does not 
> involve S_IS_EXTERNAL, while your patch to md_apply_fix does. Could you 
> explain it to me?

IIRC because md_pcrel_from_section is never called for an external
unresolved reference or even if it is the base value should always be
0 ? 


> 
> >> 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.
> >
> Actually, it seems that I misunderstood the purpose of 
> arm_force_relocation. It just triggers a flag for gas internal use, 
> which is set appropriately in md_apply_fix.


I think the solution would be that base in md_pcrel_from_section should
not be reset when the 2 segments aren't the same i.e. each of the relocs
should make sure that `base' is reset only for internal . I tried this
patch and things worked with no regressions and it appeared to also fix
your case.

I don't have more time to work on this in the next week or so, but hope
this helps.

cheers
Ramana

Index: tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.428
diff -a -u -r1.428 tc-arm.c
--- tc-arm.c	29 Jan 2010 16:02:39 -0000	1.428
+++ tc-arm.c	4 Feb 2010 18:40:17 -0000
@@ -19241,17 +19241,19 @@
 
     case BFD_RELOC_THUMB_PCREL_BRANCH23:
        if (fixP->fx_addsy
-	  && ARM_IS_FUNC (fixP->fx_addsy)
- 	  && ARM_CPU_HAS_FEATURE (selected_cpu, arm_ext_v5t))
- 	base = fixP->fx_where + fixP->fx_frag->fr_address;
+	   && ARM_IS_FUNC (fixP->fx_addsy)
+	   && (fixP->fx_addsy && S_GET_SEGMENT (fixP->fx_addsy) == seg)
+	   && ARM_CPU_HAS_FEATURE (selected_cpu, arm_ext_v5t))
+	 base = fixP->fx_where + fixP->fx_frag->fr_address;
        return base + 4;
 
       /* BLX is like branches above, but forces the low two bits of PC to
 	 zero.  */
      case BFD_RELOC_THUMB_PCREL_BLX:
        if (fixP->fx_addsy
- 	  && THUMB_IS_FUNC (fixP->fx_addsy)
- 	  && ARM_CPU_HAS_FEATURE (selected_cpu, arm_ext_v5t))
+	   && THUMB_IS_FUNC (fixP->fx_addsy)
+	   && (fixP->fx_addsy && S_GET_SEGMENT (fixP->fx_addsy) == seg)
+	   && ARM_CPU_HAS_FEATURE (selected_cpu, arm_ext_v5t))
  	base = fixP->fx_where + fixP->fx_frag->fr_address;
       return (base + 4) & ~3;
 
@@ -19259,18 +19261,19 @@
 	 loader expects the relocation not to take this into account.  */
     case BFD_RELOC_ARM_PCREL_BLX:
        if (fixP->fx_addsy
- 	  && ARM_IS_FUNC (fixP->fx_addsy)
- 	  && ARM_CPU_HAS_FEATURE (selected_cpu, arm_ext_v5t))
+	   && ARM_IS_FUNC (fixP->fx_addsy)
+	   && ARM_CPU_HAS_FEATURE (selected_cpu, arm_ext_v5t))
  	base = fixP->fx_where + fixP->fx_frag->fr_address;
        return base + 8;
 
       case BFD_RELOC_ARM_PCREL_CALL:
        if (fixP->fx_addsy
- 	  && THUMB_IS_FUNC (fixP->fx_addsy)
- 	  && ARM_CPU_HAS_FEATURE (selected_cpu, arm_ext_v5t))
+	   && (fixP->fx_addsy && S_GET_SEGMENT (fixP->fx_addsy) == seg)
+	   && THUMB_IS_FUNC (fixP->fx_addsy)
+	   && ARM_CPU_HAS_FEATURE (selected_cpu, arm_ext_v5t))
  	base = fixP->fx_where + fixP->fx_frag->fr_address;
        return base + 8;
-
+       
     case BFD_RELOC_ARM_PCREL_BRANCH:
     case BFD_RELOC_ARM_PCREL_JUMP:
     case BFD_RELOC_ARM_PLT32: