Re: Candidate fix for arm-coff testsuite failures

[Nick Clifton <nickc at redhat dot com>]

Hi Zack,

> 1) In a COFF configuration, local 'fb' labels (e.g. "1:") are not
>    recognized as local by find_real_start.  This is because
>    find_real_start expects all local labels to begin with '.', but
>    LOCAL_LABEL_PREFIX is only defined to '.' for OBJ_ELF.  It seems
>    safe to make that definition for all ARM targets.

Hmm - this might adversely affect hand-written assembler that is already 
out there.

>  (Perhaps
>    find_real_start should be invoking LOCAL_LABEL, or S_IS_LOCAL, or
>    both?  I didn't mess with that.)

Actually I think that this would be a better way to solve the problem. 
It should not be too hard to do I think.  You already have the symbol 
structure to hand and a simple S_IS_LOCAL() test ought to work.  eg:

  /* Names that start with '.' are local labels, not function entry
     points.  The compiler may generate BL instructions to these labels
     because it needs to perform a branch to a far away location.
     GAS generated local labels should also be ignored, but we cannot
     assume that they will start with a '.', so we must used
     S_IS_LOCAL.  */
  if (name[0] == '.' || S_IS_LOCAL (symbolP))
    return symbolP;

> 2) COFF and ELF disagree on whether certain relocations should be
>    optimized out by the assembler.  In private discussion with Richard
>    Earnshaw, we concluded that COFF was right, so this patch
>    removes the OBJ_ELF special case from arm_force_relocation.  

OK.

> 3) COFF and ELF also disagree on the names of some relocations (as
>    printed by objdump).  I avoided this problem by modifying the
>    affected test cases so that no relocations appear in the object
>    file.

Does that mean that you have removed all tests for the generation of 
relocs ?  ie do we now have no way of checking that GAS (for ARM) is 
actually generating relocs properly ?

> 4) The fix to (2) broke some other test cases for ELF.  This is
>    because md_apply_fix3 has OBJ_ELF #ifdefs that assume the special
>    case that was removed from arm_force_relocation.  I poked at them
>    until I got clean test results again, but I'm not confident in this
>    part of the patch.  (In an ideal world I'd like to see all those
>    #ifdefs gone from md_apply_fix3 - I don't understand why ELF, and
>    only ELF, needs to override *valP with fixP->fx_offset, but only
>    for some relocations...)

Agreed - it is a mess.  Another thing to fix one fine day.

> With this patch applied, I see only the following failures:
>
> arm-vxworks:  FAIL: TLS
> arm-wince-pe: FAIL: ARM basic instructions (WinCE version)
> arm-wince-pe: FAIL: ARM arm7t (WinCE version)
> arm-wince-pe: FAIL: ARM architecture 4t instructions
> arm-wince-pe: FAIL: immediate expressions
> arm-wince-pe: FAIL: OFFSET_IMM regression

Have you investigated the reasons for these failures ?

The WinCE tests for example are using customised expected disassembly 
ooutputs.  Maybe these need to be updated ?

> in this set of targets:
>
>     arm-elf
>     arm-coff
>     arm-aout
>     arm-epoc-pe
>     arm-nto
>     arm-vxworks
>     arm-wince-pe
>     arm-symbianelf
>     arm-none-eabi
>     xscale-coff
>     xscale-elf

Excellent - all the targets I test as well.

The patch is not approved yet, as I would like to hear your 
thoughts/test results on issue 1.

Cheers
  Nick