This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: Candidate fix for arm-coff testsuite failures
- From: Zack Weinberg <zack at codesourcery dot com>
- To: Nick Clifton <nickc at redhat dot com>
- Cc: binutils <binutils at sourceware dot org>, Richard Earnshaw <rearnsha at arm dot com>, Alan Modra <amodra at bigpond dot net dot au>
- Date: Wed, 08 Jun 2005 11:11:34 -0700
- Subject: Re: Candidate fix for arm-coff testsuite failures
- References: <87k6l85qrd.fsf@codesourcery.com> <42A6F6CD.4030806@redhat.com>
Nick Clifton <nickc@redhat.com> writes:
> 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.
I don't believe it can. The only effect of LOCAL_LABEL_PREFIX in gas/
(do not confuse this with the unrelated macro of the same name in
bfd/coff-arm.c) is to prefix the specified character to the internal
names of fb labels and dollar labels. Since those names are invisible
to source code, it shouldn't be a problem.
However, I observe that only three targets (arm, crx, h8300) define
this macro, so perhaps one should be working toward getting rid of it.
>> (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.
I didn't originally do this because I couldn't see where the flags
that S_IS_LOCAL checks were getting set for fb labels. However, I've
now run another test cycle with LOCAL_LABEL_PREFIX not defined for any
ARM target, and find_real_start checking S_IS_LOCAL, and this works
just fine. (The leading-dot check stays for compatibility, of
course.)
It occurs to me, however, that this might cause problems if GCC is
expecting GAS to redirect a Thumb->Thumb call to the .real_start_of
label even when the targeted function was declared 'static'. I don't
have an easy way to check this.
>> 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 ?
The ld testsuite contains at least some such tests. It is in a better
position to do so, as its dump files discriminate on both architecture
and object format. I don't know how comprehensive it is, though.
>> 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 ?
No, but they appear without my patch as well, and I intend to look at
them next.
Revised patch follows - the only difference is S_IS_LOCAL in
find_real_start, and LOCAL_LABEL_PREFIX completely removed from
tc-arm.h.
zw
gas:
* config/tc-arm.c (find_real_start): Check S_IS_LOCAL on
symbolP as well as for names with a leading dot. Use ACONCAT.
(md_apply_fix): For branch relocations, only replace value
with fixP->fx_offset (under #ifdef OBJ_ELF) when !fixP->fx_done.
(arm_force_relocation): Remove #ifdef OBJ_ELF case.
* config/tc-arm.h (LOCAL_LABEL): Remove unnecessary parentheses.
(LOCAL_LABEL_PREFIX): Don't define.
gas/testsuite:
* gas/arm/thumb.s: Only branch to labels defined in this file.
* gas/arm/thumb.d, gas/arm/thumb32.d: Adjust expected output.
===================================================================
Index: gas/config/tc-arm.c
--- gas/config/tc-arm.c 7 Jun 2005 18:03:16 -0000 1.209
+++ gas/config/tc-arm.c 8 Jun 2005 18:06:44 -0000
@@ -1414,15 +1414,15 @@ find_real_start (symbolS * symbolP)
if (name == NULL)
abort ();
- /* 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. */
- if (name[0] == '.')
+ /* The compiler may generate BL instructions to local labels because
+ it needs to perform a branch to a far away location. These labels
+ do not have a corresponding ".real_start_of" label. We check
+ both for S_IS_LOCAL and for a leading dot, to give a way to bypass
+ the ".real_start_of" convention for nonlocal branches. */
+ if (S_IS_LOCAL (symbolP) || name[0] == '.')
return symbolP;
- real_start = malloc (strlen (name) + strlen (STUB_NAME) + 1);
- sprintf (real_start, "%s%s", STUB_NAME, name);
-
+ real_start = ACONCAT ((STUB_NAME, name, NULL));
new_target = symbol_find (real_start);
if (new_target == NULL)
@@ -1431,8 +1431,6 @@ find_real_start (symbolS * symbolP)
new_target = symbolP;
}
- free (real_start);
-
return new_target;
}
@@ -10513,7 +10511,8 @@ md_apply_fix (fixS * fixP,
#define SEXT24(x) ((((x) & 0xffffff) ^ (~ 0x7fffff)) + 0x800000)
#ifdef OBJ_ELF
- value = fixP->fx_offset;
+ if (!fixP->fx_done)
+ value = fixP->fx_offset;
#endif
/* We are going to store value (shifted right by two) in the
@@ -10583,7 +10582,8 @@ md_apply_fix (fixS * fixP,
newval = md_chars_to_number (buf, INSN_SIZE);
#ifdef OBJ_ELF
- value = fixP->fx_offset;
+ if (!fixP->fx_done)
+ value = fixP->fx_offset;
#endif
hbit = (value >> 1) & 1;
value = (value >> 2) & 0x00ffffff;
@@ -10742,7 +10742,8 @@ md_apply_fix (fixS * fixP,
if (diff & 0x400000)
diff |= ~0x3fffff;
#ifdef OBJ_ELF
- value = fixP->fx_offset;
+ if (!fixP->fx_done)
+ value = fixP->fx_offset;
#endif
value += diff;
@@ -11353,13 +11354,6 @@ arm_force_relocation (struct fix * fixp)
if (fixp->fx_r_type == BFD_RELOC_RVA)
return 1;
#endif
-#ifdef OBJ_ELF
- if (fixp->fx_r_type == BFD_RELOC_ARM_PCREL_BRANCH
- || fixp->fx_r_type == BFD_RELOC_ARM_PCREL_BLX
- || fixp->fx_r_type == BFD_RELOC_THUMB_PCREL_BLX
- || fixp->fx_r_type == BFD_RELOC_THUMB_PCREL_BRANCH23)
- return 1;
-#endif
/* Resolve these relocations even if the symbol is extern or weak. */
if (fixp->fx_r_type == BFD_RELOC_ARM_IMMEDIATE
===================================================================
Index: gas/config/tc-arm.h
--- gas/config/tc-arm.h 7 Jun 2005 18:03:17 -0000 1.31
+++ gas/config/tc-arm.h 8 Jun 2005 18:06:44 -0000
@@ -127,8 +127,8 @@ struct fix;
#define OPTIONAL_REGISTER_PREFIX '%'
-#define LOCAL_LABEL(name) (name[0] == '.' && (name[1] == 'L'))
-#define LOCAL_LABELS_FB 1
+#define LOCAL_LABEL(name) (name[0] == '.' && name[1] == 'L')
+#define LOCAL_LABELS_FB 1
/* This expression evaluates to true if the relocation is for a local
object for which we still want to do the relocation at runtime.
@@ -168,7 +168,6 @@ struct fix;
# define md_elf_section_change_hook() arm_elf_change_section ()
# define md_elf_section_type(str, len) arm_elf_section_type (str, len)
# define GLOBAL_OFFSET_TABLE_NAME "_GLOBAL_OFFSET_TABLE_"
-# define LOCAL_LABEL_PREFIX '.'
# define TC_SEGMENT_INFO_TYPE struct arm_segment_info_type
enum mstate
===================================================================
Index: gas/testsuite/gas/arm/thumb.d
--- gas/testsuite/gas/arm/thumb.d 7 Jun 2005 18:03:17 -0000 1.4
+++ gas/testsuite/gas/arm/thumb.d 8 Jun 2005 18:06:44 -0000
@@ -119,68 +119,41 @@ Disassembly of section \.text:
0+0de <[^>]+> 2f68 cmp r7, #104
0+0e0 <[^>]+> 46c0 nop \(mov r8, r8\)
0+0e2 <[^>]+> 46c0 nop \(mov r8, r8\)
-0+0e4 <[^>]+> ea000037 b 0+0e4 <[^>]+>
- e4: R_ARM_PC24 \.text
-0+0e8 <[^>]+> eafffffe b 0+000 <[^>]+>
- e8: R_ARM_PC24 \.wombat
-0+0ec <[^>]+> eb000037 bl 0+0e4 <[^>]+>
- ec: R_ARM_PC24 \.text
-0+0f0 <[^>]+> ebfffffe bl 0+000 <[^>]+>
- f0: R_ARM_PC24 \.wombat
+0+0e4 <[^>]+> eafffffe b 0+0e4 <[^>]+>
+0+0e8 <[^>]+> ea000011 b 0+134 <[^>]+>
+0+0ec <[^>]+> ebfffffc bl 0+0e4 <[^>]+>
+0+0f0 <[^>]+> eb00000f bl 0+134 <[^>]+>
0+0f4 <[^>]+> e12fff10 bx r0
0+0f8 <[^>]+> ef123456 swi 0x00123456
0+0fc <[^>]+> a004 add r0, pc, #16 \(adr r0,0+110 <[^>]+>\)
0+0fe <[^>]+> e77f b.n 0+000 <[^>]+>
-0+100 <[^>]+> e7fe b.n 0+000 <[^>]+>
- 100: R_ARM_THM_JUMP11 \.wombat
-0+102 <[^>]+> f7ff fffe bl 0+000 <[^>]+>
- 102: R_ARM_THM_CALL \.text
-0+106 <[^>]+> f7ff fffe bl 0+000 <[^>]+>
- 106: R_ARM_THM_CALL \.wombat
+0+100 <[^>]+> e018 b.n 0+134 <[^>]+>
+0+102 <[^>]+> f7ff ff7d bl 0+000 <[^>]+>
+0+106 <[^>]+> f000 f815 bl 0+134 <[^>]+>
0+10a <[^>]+> 4700 bx r0
0+10c <[^>]+> dfff swi 255
\.\.\.
-0+110 <[^>]+> d0fe beq.n 0+000 <[^>]+>
- 110: R_ARM_THM_JUMP8 \.wombat
-0+112 <[^>]+> d1fe bne.n 0+000 <[^>]+>
- 112: R_ARM_THM_JUMP8 \.wombat
-0+114 <[^>]+> d2fe bcs.n 0+000 <[^>]+>
- 114: R_ARM_THM_JUMP8 \.wombat
-0+116 <[^>]+> d3fe bcc.n 0+000 <[^>]+>
- 116: R_ARM_THM_JUMP8 \.wombat
-0+118 <[^>]+> d4fe bmi.n 0+000 <[^>]+>
- 118: R_ARM_THM_JUMP8 \.wombat
-0+11a <[^>]+> d5fe bpl.n 0+000 <[^>]+>
- 11a: R_ARM_THM_JUMP8 \.wombat
-0+11c <[^>]+> d6fe bvs.n 0+000 <[^>]+>
- 11c: R_ARM_THM_JUMP8 \.wombat
-0+11e <[^>]+> d7fe bvc.n 0+000 <[^>]+>
- 11e: R_ARM_THM_JUMP8 \.wombat
-0+120 <[^>]+> d8fe bhi.n 0+000 <[^>]+>
- 120: R_ARM_THM_JUMP8 \.wombat
-0+122 <[^>]+> d9fe bls.n 0+000 <[^>]+>
- 122: R_ARM_THM_JUMP8 \.wombat
-0+124 <[^>]+> dafe bge.n 0+000 <[^>]+>
- 124: R_ARM_THM_JUMP8 \.wombat
-0+126 <[^>]+> dcfe bgt.n 0+000 <[^>]+>
- 126: R_ARM_THM_JUMP8 \.wombat
-0+128 <[^>]+> dbfe blt.n 0+000 <[^>]+>
- 128: R_ARM_THM_JUMP8 \.wombat
-0+12a <[^>]+> dcfe bgt.n 0+000 <[^>]+>
- 12a: R_ARM_THM_JUMP8 \.wombat
-0+12c <[^>]+> ddfe ble.n 0+000 <[^>]+>
- 12c: R_ARM_THM_JUMP8 \.wombat
-0+12e <[^>]+> d8fe bhi.n 0+000 <[^>]+>
- 12e: R_ARM_THM_JUMP8 \.wombat
-0+130 <[^>]+> d3fe bcc.n 0+000 <[^>]+>
- 130: R_ARM_THM_JUMP8 \.wombat
-0+132 <[^>]+> d3fe bcc.n 0+000 <[^>]+>
- 132: R_ARM_THM_JUMP8 \.wombat
-0+134 <[^>]+> f000 fc9a bl 0+938 <[^>]+>
- 134: R_ARM_THM_CALL \.text
+0+110 <[^>]+> d010 beq.n 0+134 <[^>]+>
+0+112 <[^>]+> d10f bne.n 0+134 <[^>]+>
+0+114 <[^>]+> d20e bcs.n 0+134 <[^>]+>
+0+116 <[^>]+> d30d bcc.n 0+134 <[^>]+>
+0+118 <[^>]+> d40c bmi.n 0+134 <[^>]+>
+0+11a <[^>]+> d50b bpl.n 0+134 <[^>]+>
+0+11c <[^>]+> d60a bvs.n 0+134 <[^>]+>
+0+11e <[^>]+> d709 bvc.n 0+134 <[^>]+>
+0+120 <[^>]+> d808 bhi.n 0+134 <[^>]+>
+0+122 <[^>]+> d907 bls.n 0+134 <[^>]+>
+0+124 <[^>]+> da06 bge.n 0+134 <[^>]+>
+0+126 <[^>]+> dc05 bgt.n 0+134 <[^>]+>
+0+128 <[^>]+> db04 blt.n 0+134 <[^>]+>
+0+12a <[^>]+> dc03 bgt.n 0+134 <[^>]+>
+0+12c <[^>]+> dd02 ble.n 0+134 <[^>]+>
+0+12e <[^>]+> d801 bhi.n 0+134 <[^>]+>
+0+130 <[^>]+> d300 bcc.n 0+134 <[^>]+>
+0+132 <[^>]+> d3ff bcc.n 0+134 <[^>]+>
+0+134 <[^>]+> f000 fc00 bl 0+938 <[^>]+>
\.\.\.
-0+938 <[^>]+> f000 f898 bl 0+134 <[^>]+>
- 938: R_ARM_THM_CALL \.text
+0+938 <[^>]+> f7ff fbfc bl 0+134 <[^>]+>
0+93c <[^>]+> 4801 ldr r0, \[pc, #4\] \(0+944 <[^>]+>\)
0+93e <[^>]+> 4801 ldr r0, \[pc, #4\] \(0+944 <[^>]+>\)
0+940 <[^>]+> 4801 ldr r0, \[pc, #4\] \(0+948 <[^>]+>\)
===================================================================
Index: gas/testsuite/gas/arm/thumb.s
--- gas/testsuite/gas/arm/thumb.s 7 Jun 2005 18:03:17 -0000 1.5
+++ gas/testsuite/gas/arm/thumb.s 8 Jun 2005 18:06:44 -0000
@@ -145,9 +145,9 @@ near:
.arm
.localbar:
b .localbar
- b .wombat
+ b .back
bl .localbar
- bl .wombat
+ bl .back
bx r0
swi 0x123456
@@ -159,33 +159,33 @@ morethumb:
adr r0, forwardonly
b .foo
- b .wombat
+ b .back
bl .foo
- bl .wombat
+ bl .back
bx r0
swi 0xff
.align 0
forwardonly:
- beq .wombat
- bne .wombat
- bcs .wombat
- bcc .wombat
- bmi .wombat
- bpl .wombat
- bvs .wombat
- bvc .wombat
- bhi .wombat
- bls .wombat
- bge .wombat
- bgt .wombat
- blt .wombat
- bgt .wombat
- ble .wombat
- bhi .wombat
- blo .wombat
- bul .wombat
+ beq .back
+ bne .back
+ bcs .back
+ bcc .back
+ bmi .back
+ bpl .back
+ bvs .back
+ bvc .back
+ bhi .back
+ bls .back
+ bge .back
+ bgt .back
+ blt .back
+ bgt .back
+ ble .back
+ bhi .back
+ blo .back
+ bul .back
.back:
bl .local
===================================================================
Index: gas/testsuite/gas/arm/thumb32.d
--- gas/testsuite/gas/arm/thumb32.d 7 Jun 2005 18:03:17 -0000 1.4
+++ gas/testsuite/gas/arm/thumb32.d 8 Jun 2005 18:06:44 -0000
@@ -295,14 +295,10 @@ Disassembly of section .text:
0+3cc <[^>]+> f340 800c ble\.w 0+3e8 <[^>]+>
0+3d0 <[^>]+> f7ff bfae b\.w 0+330 <[^>]+>
0+3d4 <[^>]+> f000 b808 b\.w 0+3e8 <[^>]+>
-0+3d8 <[^>]+> f000 f996 bl 0+330 <[^>]+>
- 3d8: R_ARM_THM_CALL \.text
-0+3dc <[^>]+> f000 f9f2 bl 0+3e8 <[^>]+>
- 3dc: R_ARM_THM_CALL \.text
-0+3e0 <[^>]+> f000 e996 blx 0+330 <[^>]+>
- 3e0: R_ARM_THM_XPC22 \.text
-0+3e4 <[^>]+> f000 e9f2 blx 0+3e8 <[^>]+>
- 3e4: R_ARM_THM_XPC22 \.text
+0+3d8 <[^>]+> f7ff ffaa bl 0+330 <[^>]+>
+0+3dc <[^>]+> f000 f804 bl 0+3e8 <[^>]+>
+0+3e0 <[^>]+> f7ff efa6 blx 0+330 <[^>]+>
+0+3e4 <[^>]+> f000 e800 blx 0+3e8 <[^>]+>
0+3e8 <[^>]+> 4748 bx r9
0+3ea <[^>]+> 4780 blx r0
0+3ec <[^>]+> 47c8 blx r9