* Fix assembly of Thumb pcrel LDRs against global symbols
@ 2011-02-28 12:59 Richard Sandiford
2011-03-01 11:26 ` Dave Martin
2011-03-02 15:18 ` Richard Earnshaw
0 siblings, 2 replies; 8+ messages in thread
From: Richard Sandiford @ 2011-02-28 12:59 UTC (permalink / raw)
To: binutils; +Cc: patches
The PC-relative LDR instructions have no associated relocation,
so even LDRs for global symbols should be resolved by the assembler.
We currently handle this correctly for single-register ARM loads,
but we're missing the associated relocation types for LDRD and Thumb.
This leads to errors like:
Error: invalid offset, value too big (0xFFFFFFFFFFFFFFFC)
or:
Error: cannot represent T32_OFFSET_IMM relocation in this object file format
Tested on arm-linux-gnueabi. OK to install?
Rihard
gas/
* config/tc-arm.c (arm_force_relocation): Return 0 for
BFD_RELOC_ARM_OFFSET_IMM8, BFD_RELOC_ARM_THUMB_OFFSET and
BFD_RELOC_ARM_T32_OFFSET_IMM.
gas/testsuite/
* gas/arm/ldr-pcrel-1.s, gas/arm/ldr-pcrel-1.d: New test.
Index: gas/config/tc-arm.c
===================================================================
--- gas/config/tc-arm.c 2011-02-28 12:05:55.000000000 +0000
+++ gas/config/tc-arm.c 2011-02-28 12:17:54.000000000 +0000
@@ -21735,11 +21735,14 @@ arm_force_relocation (struct fix * fixp)
/* Resolve these relocations even if the symbol is extern or weak. */
if (fixp->fx_r_type == BFD_RELOC_ARM_IMMEDIATE
|| fixp->fx_r_type == BFD_RELOC_ARM_OFFSET_IMM
+ || fixp->fx_r_type == BFD_RELOC_ARM_OFFSET_IMM8
|| fixp->fx_r_type == BFD_RELOC_ARM_ADRL_IMMEDIATE
|| fixp->fx_r_type == BFD_RELOC_ARM_T32_ADD_IMM
|| fixp->fx_r_type == BFD_RELOC_ARM_T32_IMMEDIATE
|| fixp->fx_r_type == BFD_RELOC_ARM_T32_IMM12
- || fixp->fx_r_type == BFD_RELOC_ARM_T32_ADD_PC12)
+ || fixp->fx_r_type == BFD_RELOC_ARM_T32_ADD_PC12
+ || fixp->fx_r_type == BFD_RELOC_ARM_THUMB_OFFSET
+ || fixp->fx_r_type == BFD_RELOC_ARM_T32_OFFSET_IMM)
return 0;
/* Always leave these relocations for the linker. */
Index: gas/testsuite/gas/arm/ldr-pcrel-1.s
===================================================================
--- /dev/null 2011-02-21 12:47:04.267827113 +0000
+++ gas/testsuite/gas/arm/ldr-pcrel-1.s 2011-02-28 12:17:54.000000000 +0000
@@ -0,0 +1,45 @@
+ .macro defsym,sym
+ .globl g\sym
+g\sym:
+ .word 0
+ .word 0
+ .globl \sym
+\sym:
+ .word 0
+ .word 0
+ .endm
+
+ .macro ldrtest,ldr,ureg,sym
+ \ldr r0,g\sym
+ \ldr \ureg,\sym
+ \ldr r0,g\sym
+ \ldr \ureg,\sym
+ .endm
+
+ .macro symtest,suffix,ureg,sym
+ ldrtest ldrd, \ureg, \sym
+ ldrtest ldr\suffix, \ureg, \sym
+ ldrtest ldrh, \ureg, \sym
+ ldrtest ldrsh, \ureg, \sym
+ ldrtest ldrb, \ureg, \sym
+ ldrtest ldrsb, \ureg, \sym
+ .endm
+
+ .syntax unified
+ defsym begin
+
+arm:
+ symtest ,r12,begin
+ symtest ,r12,middle
+
+ defsym middle
+
+ .thumb_func
+thumb:
+ symtest ,r12,middle
+ symtest ,r12,end
+ symtest .w,r12,middle
+ symtest .w,r12,end
+ ldrtest ldr.n,r7,end
+
+ defsym end
Index: gas/testsuite/gas/arm/ldr-pcrel-1.d
===================================================================
--- /dev/null 2011-02-21 12:47:04.267827113 +0000
+++ gas/testsuite/gas/arm/ldr-pcrel-1.d 2011-02-28 12:17:54.000000000 +0000
@@ -0,0 +1,177 @@
+#objdump: -dr
+#name: PC-relative LDR 1
+
+.*
+
+
+Disassembly of section \.text:
+
+00000000 <gbegin>:
+ \.\.\.
+
+00000008 <begin>:
+ \.\.\.
+
+00000010 <arm>:
+ 10: e14f01d8 ldrd r0, \[pc, #-24\] ; 0 <gbegin>
+ 14: e14fc1d4 ldrd ip, \[pc, #-20\] ; 8 <begin>
+ 18: e14f02d0 ldrd r0, \[pc, #-32\] ; 0 <gbegin>
+ 1c: e14fc1dc ldrd ip, \[pc, #-28\] ; 8 <begin>
+ 20: e51f0028 ldr r0, \[pc, #-40\] ; 0 <gbegin>
+ 24: e51fc024 ldr ip, \[pc, #-36\] ; 8 <begin>
+ 28: e51f0030 ldr r0, \[pc, #-48\] ; 0 <gbegin>
+ 2c: e51fc02c ldr ip, \[pc, #-44\] ; 8 <begin>
+ 30: e15f03b8 ldrh r0, \[pc, #-56\] ; 0 <gbegin>
+ 34: e15fc3b4 ldrh ip, \[pc, #-52\] ; 8 <begin>
+ 38: e15f04b0 ldrh r0, \[pc, #-64\] ; 0 <gbegin>
+ 3c: e15fc3bc ldrh ip, \[pc, #-60\] ; 8 <begin>
+ 40: e15f04f8 ldrsh r0, \[pc, #-72\] ; 0 <gbegin>
+ 44: e15fc4f4 ldrsh ip, \[pc, #-68\] ; 8 <begin>
+ 48: e15f05f0 ldrsh r0, \[pc, #-80\] ; 0 <gbegin>
+ 4c: e15fc4fc ldrsh ip, \[pc, #-76\] ; 8 <begin>
+ 50: e55f0058 ldrb r0, \[pc, #-88\] ; 0 <gbegin>
+ 54: e55fc054 ldrb ip, \[pc, #-84\] ; 8 <begin>
+ 58: e55f0060 ldrb r0, \[pc, #-96\] ; 0 <gbegin>
+ 5c: e55fc05c ldrb ip, \[pc, #-92\] ; 8 <begin>
+ 60: e15f06d8 ldrsb r0, \[pc, #-104\] ; 0 <gbegin>
+ 64: e15fc6d4 ldrsb ip, \[pc, #-100\] ; 8 <begin>
+ 68: e15f07d0 ldrsb r0, \[pc, #-112\] ; 0 <gbegin>
+ 6c: e15fc6dc ldrsb ip, \[pc, #-108\] ; 8 <begin>
+ 70: e1cf05d8 ldrd r0, \[pc, #88\] ; d0 <gmiddle>
+ 74: e1cfc5dc ldrd ip, \[pc, #92\] ; d8 <middle>
+ 78: e1cf05d0 ldrd r0, \[pc, #80\] ; d0 <gmiddle>
+ 7c: e1cfc5d4 ldrd ip, \[pc, #84\] ; d8 <middle>
+ 80: e59f0048 ldr r0, \[pc, #72\] ; d0 <gmiddle>
+ 84: e59fc04c ldr ip, \[pc, #76\] ; d8 <middle>
+ 88: e59f0040 ldr r0, \[pc, #64\] ; d0 <gmiddle>
+ 8c: e59fc044 ldr ip, \[pc, #68\] ; d8 <middle>
+ 90: e1df03b8 ldrh r0, \[pc, #56\] ; d0 <gmiddle>
+ 94: e1dfc3bc ldrh ip, \[pc, #60\] ; d8 <middle>
+ 98: e1df03b0 ldrh r0, \[pc, #48\] ; d0 <gmiddle>
+ 9c: e1dfc3b4 ldrh ip, \[pc, #52\] ; d8 <middle>
+ a0: e1df02f8 ldrsh r0, \[pc, #40\] ; d0 <gmiddle>
+ a4: e1dfc2fc ldrsh ip, \[pc, #44\] ; d8 <middle>
+ a8: e1df02f0 ldrsh r0, \[pc, #32\] ; d0 <gmiddle>
+ ac: e1dfc2f4 ldrsh ip, \[pc, #36\] ; d8 <middle>
+ b0: e5df0018 ldrb r0, \[pc, #24\] ; d0 <gmiddle>
+ b4: e5dfc01c ldrb ip, \[pc, #28\] ; d8 <middle>
+ b8: e5df0010 ldrb r0, \[pc, #16\] ; d0 <gmiddle>
+ bc: e5dfc014 ldrb ip, \[pc, #20\] ; d8 <middle>
+ c0: e1df00d8 ldrsb r0, \[pc, #8\] ; d0 <gmiddle>
+ c4: e1dfc0dc ldrsb ip, \[pc, #12\] ; d8 <middle>
+ c8: e1df00d0 ldrsb r0, \[pc\] ; d0 <gmiddle>
+ cc: e1dfc0d4 ldrsb ip, \[pc, #4\] ; d8 <middle>
+
+000000d0 <gmiddle>:
+ \.\.\.
+
+000000d8 <middle>:
+ \.\.\.
+
+000000e0 <thumb>:
+ e0: e95f 0105 ldrd r0, r1, \[pc, #-20\]
+ e4: e95f cd04 ldrd ip, sp, \[pc, #-16\]
+ e8: e95f 0107 ldrd r0, r1, \[pc, #-28\]
+ ec: e95f cd06 ldrd ip, sp, \[pc, #-24\]
+ f0: f85f 0024 ldr\.w r0, \[pc, #-36\] ; d0 <gmiddle>
+ f4: f85f c020 ldr\.w ip, \[pc, #-32\] ; d8 <middle>
+ f8: f85f 002c ldr\.w r0, \[pc, #-44\] ; d0 <gmiddle>
+ fc: f85f c028 ldr\.w ip, \[pc, #-40\] ; d8 <middle>
+ 100: f83f 0034 ldrh\.w r0, \[pc, #-52\] ; d0 <gmiddle>
+ 104: f83f c030 ldrh\.w ip, \[pc, #-48\] ; d8 <middle>
+ 108: f83f 003c ldrh\.w r0, \[pc, #-60\] ; d0 <gmiddle>
+ 10c: f83f c038 ldrh\.w ip, \[pc, #-56\] ; d8 <middle>
+ 110: f93f 0044 ldrsh\.w r0, \[pc, #-68\] ; d0 <gmiddle>
+ 114: f93f c040 ldrsh\.w ip, \[pc, #-64\] ; d8 <middle>
+ 118: f93f 004c ldrsh\.w r0, \[pc, #-76\] ; d0 <gmiddle>
+ 11c: f93f c048 ldrsh\.w ip, \[pc, #-72\] ; d8 <middle>
+ 120: f81f 0054 ldrb\.w r0, \[pc, #-84\] ; d0 <gmiddle>
+ 124: f81f c050 ldrb\.w ip, \[pc, #-80\] ; d8 <middle>
+ 128: f81f 005c ldrb\.w r0, \[pc, #-92\] ; d0 <gmiddle>
+ 12c: f81f c058 ldrb\.w ip, \[pc, #-88\] ; d8 <middle>
+ 130: f91f 0064 ldrsb\.w r0, \[pc, #-100\] ; d0 <gmiddle>
+ 134: f91f c060 ldrsb\.w ip, \[pc, #-96\] ; d8 <middle>
+ 138: f91f 006c ldrsb\.w r0, \[pc, #-108\] ; d0 <gmiddle>
+ 13c: f91f c068 ldrsb\.w ip, \[pc, #-104\] ; d8 <middle>
+ 140: e9df 0148 ldrd r0, r1, \[pc, #288\] ; 0x120
+ 144: e9df cd49 ldrd ip, sp, \[pc, #292\] ; 0x124
+ 148: e9df 0146 ldrd r0, r1, \[pc, #280\] ; 0x118
+ 14c: e9df cd47 ldrd ip, sp, \[pc, #284\] ; 0x11c
+ 150: 4844 ldr r0, \[pc, #272\] ; \(264 <gend>\)
+ 152: f8df c118 ldr\.w ip, \[pc, #280\] ; 26c <end>
+ 156: 4843 ldr r0, \[pc, #268\] ; \(264 <gend>\)
+ 158: f8df c110 ldr\.w ip, \[pc, #272\] ; 26c <end>
+ 15c: f8bf 0104 ldrh\.w r0, \[pc, #260\] ; 264 <gend>
+ 160: f8bf c108 ldrh\.w ip, \[pc, #264\] ; 26c <end>
+ 164: f8bf 00fc ldrh\.w r0, \[pc, #252\] ; 264 <gend>
+ 168: f8bf c100 ldrh\.w ip, \[pc, #256\] ; 26c <end>
+ 16c: f9bf 00f4 ldrsh\.w r0, \[pc, #244\] ; 264 <gend>
+ 170: f9bf c0f8 ldrsh\.w ip, \[pc, #248\] ; 26c <end>
+ 174: f9bf 00ec ldrsh\.w r0, \[pc, #236\] ; 264 <gend>
+ 178: f9bf c0f0 ldrsh\.w ip, \[pc, #240\] ; 26c <end>
+ 17c: f89f 00e4 ldrb\.w r0, \[pc, #228\] ; 264 <gend>
+ 180: f89f c0e8 ldrb\.w ip, \[pc, #232\] ; 26c <end>
+ 184: f89f 00dc ldrb\.w r0, \[pc, #220\] ; 264 <gend>
+ 188: f89f c0e0 ldrb\.w ip, \[pc, #224\] ; 26c <end>
+ 18c: f99f 00d4 ldrsb\.w r0, \[pc, #212\] ; 264 <gend>
+ 190: f99f c0d8 ldrsb\.w ip, \[pc, #216\] ; 26c <end>
+ 194: f99f 00cc ldrsb\.w r0, \[pc, #204\] ; 264 <gend>
+ 198: f99f c0d0 ldrsb\.w ip, \[pc, #208\] ; 26c <end>
+ 19c: e95f 0134 ldrd r0, r1, \[pc, #-208\] ; 0xd0
+ 1a0: e95f cd33 ldrd ip, sp, \[pc, #-204\] ; 0xcc
+ 1a4: e95f 0136 ldrd r0, r1, \[pc, #-216\] ; 0xd8
+ 1a8: e95f cd35 ldrd ip, sp, \[pc, #-212\] ; 0xd4
+ 1ac: f85f 00e0 ldr\.w r0, \[pc, #-224\] ; d0 <gmiddle>
+ 1b0: f85f c0dc ldr\.w ip, \[pc, #-220\] ; d8 <middle>
+ 1b4: f85f 00e8 ldr\.w r0, \[pc, #-232\] ; d0 <gmiddle>
+ 1b8: f85f c0e4 ldr\.w ip, \[pc, #-228\] ; d8 <middle>
+ 1bc: f83f 00f0 ldrh\.w r0, \[pc, #-240\] ; d0 <gmiddle>
+ 1c0: f83f c0ec ldrh\.w ip, \[pc, #-236\] ; d8 <middle>
+ 1c4: f83f 00f8 ldrh\.w r0, \[pc, #-248\] ; d0 <gmiddle>
+ 1c8: f83f c0f4 ldrh\.w ip, \[pc, #-244\] ; d8 <middle>
+ 1cc: f93f 0100 ldrsh\.w r0, \[pc, #-256\] ; d0 <gmiddle>
+ 1d0: f93f c0fc ldrsh\.w ip, \[pc, #-252\] ; d8 <middle>
+ 1d4: f93f 0108 ldrsh\.w r0, \[pc, #-264\] ; d0 <gmiddle>
+ 1d8: f93f c104 ldrsh\.w ip, \[pc, #-260\] ; d8 <middle>
+ 1dc: f81f 0110 ldrb\.w r0, \[pc, #-272\] ; d0 <gmiddle>
+ 1e0: f81f c10c ldrb\.w ip, \[pc, #-268\] ; d8 <middle>
+ 1e4: f81f 0118 ldrb\.w r0, \[pc, #-280\] ; d0 <gmiddle>
+ 1e8: f81f c114 ldrb\.w ip, \[pc, #-276\] ; d8 <middle>
+ 1ec: f91f 0120 ldrsb\.w r0, \[pc, #-288\] ; d0 <gmiddle>
+ 1f0: f91f c11c ldrsb\.w ip, \[pc, #-284\] ; d8 <middle>
+ 1f4: f91f 0128 ldrsb\.w r0, \[pc, #-296\] ; d0 <gmiddle>
+ 1f8: f91f c124 ldrsb\.w ip, \[pc, #-292\] ; d8 <middle>
+ 1fc: e9df 0119 ldrd r0, r1, \[pc, #100\] ; 0x64
+ 200: e9df cd1a ldrd ip, sp, \[pc, #104\] ; 0x68
+ 204: e9df 0117 ldrd r0, r1, \[pc, #92\] ; 0x5c
+ 208: e9df cd18 ldrd ip, sp, \[pc, #96\] ; 0x60
+ 20c: f8df 0054 ldr\.w r0, \[pc, #84\] ; 264 <gend>
+ 210: f8df c058 ldr\.w ip, \[pc, #88\] ; 26c <end>
+ 214: f8df 004c ldr\.w r0, \[pc, #76\] ; 264 <gend>
+ 218: f8df c050 ldr\.w ip, \[pc, #80\] ; 26c <end>
+ 21c: f8bf 0044 ldrh\.w r0, \[pc, #68\] ; 264 <gend>
+ 220: f8bf c048 ldrh\.w ip, \[pc, #72\] ; 26c <end>
+ 224: f8bf 003c ldrh\.w r0, \[pc, #60\] ; 264 <gend>
+ 228: f8bf c040 ldrh\.w ip, \[pc, #64\] ; 26c <end>
+ 22c: f9bf 0034 ldrsh\.w r0, \[pc, #52\] ; 264 <gend>
+ 230: f9bf c038 ldrsh\.w ip, \[pc, #56\] ; 26c <end>
+ 234: f9bf 002c ldrsh\.w r0, \[pc, #44\] ; 264 <gend>
+ 238: f9bf c030 ldrsh\.w ip, \[pc, #48\] ; 26c <end>
+ 23c: f89f 0024 ldrb\.w r0, \[pc, #36\] ; 264 <gend>
+ 240: f89f c028 ldrb\.w ip, \[pc, #40\] ; 26c <end>
+ 244: f89f 001c ldrb\.w r0, \[pc, #28\] ; 264 <gend>
+ 248: f89f c020 ldrb\.w ip, \[pc, #32\] ; 26c <end>
+ 24c: f99f 0014 ldrsb\.w r0, \[pc, #20\] ; 264 <gend>
+ 250: f99f c018 ldrsb\.w ip, \[pc, #24\] ; 26c <end>
+ 254: f99f 000c ldrsb\.w r0, \[pc, #12\] ; 264 <gend>
+ 258: f99f c010 ldrsb\.w ip, \[pc, #16\] ; 26c <end>
+ 25c: 4801 ldr r0, \[pc, #4\] ; \(264 <gend>\)
+ 25e: 4f03 ldr r7, \[pc, #12\] ; \(26c <end>\)
+ 260: 4800 ldr r0, \[pc, #0\] ; \(264 <gend>\)
+ 262: 4f02 ldr r7, \[pc, #8\] ; \(26c <end>\)
+
+00000264 <gend>:
+ \.\.\.
+
+0000026c <end>:
+ \.\.\.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix assembly of Thumb pcrel LDRs against global symbols
2011-02-28 12:59 Fix assembly of Thumb pcrel LDRs against global symbols Richard Sandiford
@ 2011-03-01 11:26 ` Dave Martin
2011-03-01 13:50 ` Richard Sandiford
2011-03-02 15:18 ` Richard Earnshaw
1 sibling, 1 reply; 8+ messages in thread
From: Dave Martin @ 2011-03-01 11:26 UTC (permalink / raw)
To: binutils, patches, richard.sandiford
Hi,
On Mon, Feb 28, 2011 at 12:59:04PM +0000, Richard Sandiford wrote:
> The PC-relative LDR instructions have no associated relocation,
> so even LDRs for global symbols should be resolved by the assembler.
> We currently handle this correctly for single-register ARM loads,
> but we're missing the associated relocation types for LDRD and Thumb.
> This leads to errors like:
>
> Error: invalid offset, value too big (0xFFFFFFFFFFFFFFFC)
>
> or:
>
> Error: cannot represent T32_OFFSET_IMM relocation in this object file format
>
> Tested on arm-linux-gnueabi. OK to install?
This seems to work for me for some simple ldrs on locally-
defined global symbols in Thumb.
Note that the annotation of ldrd instructions in Thumb during
disassembly appears incomplete: the target label is not
annotated, whereas for ldr, or for ldr or ldrd in ARM, the
instruction is annotated with the target label in the
disassembly.
I believe the generated instruction encoding is right in the case
shown below, but it might be a good idea to double-check.
Cheers
---Dave
binutils$ cat <<EOF >ldr-tst.s
.syntax unified
.globl d
d: .quad 0
.type f, %function
f: ldrd r0, r1, d
nop
ldrd r0, r1, d
ldr r0, d
nop
ldr r0, d
EOF
binutils$ binutils/objdump -mthumb -o ldr-tst.o ldr-tst.s && arm-linux-gnueabi-objdump -dr ldr-tst.o
[...]
Disassembly of section .text:
00000000 <d>:
...
00000008 <f>:
8: e95f 0103 ldrd r0, r1, [pc, #-12]
c: 46c0 nop ; (mov r8, r8)
e: e95f 0104 ldrd r0, r1, [pc, #-16]
12: f85f 0014 ldr.w r0, [pc, #-20] ; 0 <d>
16: 46c0 nop ; (mov r8, r8)
18: f85f 001c ldr.w r0, [pc, #-28] ; 0 <d>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix assembly of Thumb pcrel LDRs against global symbols
2011-03-01 11:26 ` Dave Martin
@ 2011-03-01 13:50 ` Richard Sandiford
0 siblings, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2011-03-01 13:50 UTC (permalink / raw)
To: Dave Martin; +Cc: binutils, patches
Dave Martin <dave.martin@linaro.org> writes:
> Note that the annotation of ldrd instructions in Thumb during
> disassembly appears incomplete: the target label is not
> annotated, whereas for ldr, or for ldr or ldrd in ARM, the
> instruction is annotated with the target label in the
> disassembly.
OK, thanks for the report. I don't have nay plans to work on that just now,
so it might be worth filing it in bugzilla.
Richard
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix assembly of Thumb pcrel LDRs against global symbols
2011-02-28 12:59 Fix assembly of Thumb pcrel LDRs against global symbols Richard Sandiford
2011-03-01 11:26 ` Dave Martin
@ 2011-03-02 15:18 ` Richard Earnshaw
2011-03-02 16:36 ` Richard Sandiford
2011-03-02 16:37 ` Dave Martin
1 sibling, 2 replies; 8+ messages in thread
From: Richard Earnshaw @ 2011-03-02 15:18 UTC (permalink / raw)
To: Richard Sandiford; +Cc: binutils, patches
On Mon, 2011-02-28 at 12:59 +0000, Richard Sandiford wrote:
> The PC-relative LDR instructions have no associated relocation,
> so even LDRs for global symbols should be resolved by the assembler.
> We currently handle this correctly for single-register ARM loads,
> but we're missing the associated relocation types for LDRD and Thumb.
> This leads to errors like:
I'm not sure I agree with this. If I write
.global foo
...
ldr r0, foo
...
foo:
...
but then at link/load time pre-empt foo with some other definition, that
will silently leave me with the wrong answer.
It's clearly OK to do this if foo is protected or private, but not
otherwise.
The existing error message is not very informative as to what the
problem is, but I'm not sure I agree with your solution...
R.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix assembly of Thumb pcrel LDRs against global symbols
2011-03-02 15:18 ` Richard Earnshaw
@ 2011-03-02 16:36 ` Richard Sandiford
2011-03-02 16:37 ` Dave Martin
1 sibling, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2011-03-02 16:36 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: binutils, patches
Richard Earnshaw <rearnsha@arm.com> writes:
> On Mon, 2011-02-28 at 12:59 +0000, Richard Sandiford wrote:
>> The PC-relative LDR instructions have no associated relocation,
>> so even LDRs for global symbols should be resolved by the assembler.
>> We currently handle this correctly for single-register ARM loads,
>> but we're missing the associated relocation types for LDRD and Thumb.
>> This leads to errors like:
>
> I'm not sure I agree with this. If I write
>
> .global foo
>
> ...
> ldr r0, foo
>
> ...
>
> foo:
> ...
>
> but then at link/load time pre-empt foo with some other definition, that
> will silently leave me with the wrong answer.
Yeah, I realise we normally try to do that. The point was that we
already resolve these references at assembly time for ARM LDRs --
.syntax unified
ldr r3,foo
nop
.globl foo
foo:
.word 0x1234
-- just not for Thumb LDRs or for (any) LDRDs. I think I'd wrongly assumed
that the current ARM LDR behaviour was by design, and that we should make
the other cases match. However, looking at the ABI again, I see there is
a reloc (R_ARM_THM_PC12) that we can use here, but that we aren't using.
So if the current ARM behaviour is wrong, then I suppose there are
three bugs:
- We're using a generic ARM_OFFSET_IMM fixup for ARM LDRs, which is
always getting resolved by the assembler. We should generate a
reloc instead for this case (but not for some other ARM_OFFSET_IMM cases).
- We're using a generic T32_OFFSET_IMM fixup for Thumb LDRs, which is
always triggering an error (one of the two in my message). We should
generate a reloc instead (but not for some other T32_OFFSET_IMM cases).
- We generate the internal error for things like:
.syntax unified
.thumb_func
ldr r3,[r12,#foo-.]
nop
.globl foo
foo:
.word 0x1234
which AFAICS couldn't be handled by relocations. (The patch fixes
this too.)
That's a bit more work than I'd anticipated, and the first change might
annoy some people, so I think I'd better put it to one side for now. :-)
Richard
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix assembly of Thumb pcrel LDRs against global symbols
2011-03-02 15:18 ` Richard Earnshaw
2011-03-02 16:36 ` Richard Sandiford
@ 2011-03-02 16:37 ` Dave Martin
2011-03-02 16:45 ` Richard Earnshaw
1 sibling, 1 reply; 8+ messages in thread
From: Dave Martin @ 2011-03-02 16:37 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: Richard Sandiford, binutils, patches
On Wed, Mar 2, 2011 at 3:18 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>
> On Mon, 2011-02-28 at 12:59 +0000, Richard Sandiford wrote:
>> The PC-relative LDR instructions have no associated relocation,
>> so even LDRs for global symbols should be resolved by the assembler.
>> We currently handle this correctly for single-register ARM loads,
>> but we're missing the associated relocation types for LDRD and Thumb.
>> This leads to errors like:
>
> I'm not sure I agree with this. If I write
>
> .global foo
>
> ...
> ldr r0, foo
>
> ...
>
> foo:
> ...
>
> but then at link/load time pre-empt foo with some other definition, that
> will silently leave me with the wrong answer.
>
> It's clearly OK to do this if foo is protected or private, but not
> otherwise.
>
> The existing error message is not very informative as to what the
> problem is, but I'm not sure I agree with your solution...
>
> R.
That's a reasonable argument, but similarly to the ADR case, such code
referencing global symbols is already allowed through for ARM _with no
relocation emitted_ - the Thumb treatment is inconsistent and causes
existing code which builds for ARM (but where symbol preemption may
silently fail to work). This raises non-trivial questions about which
behaviour should be considered correct.
Unless there are some tricks I haven't thought of, it seems that ADR
and PC-relative LDR and friends simply can't be generally preemptible,
period -- because the relocation ranges these instructions support
just aren't large enough to guarantee that a relocation on these
instructions can ever be resolved except in a trivial subset of cases.
Solving this inconsistency requires a conclusion regarding whether the
tools should trust the programmer to write correct code (i.e., fix up
pc-relative LDR and ADR in the assembler and emit no relocation) or
not (i.e., treat these instructions as an error when they reference
global symbols).
I would regard the latter option as incorrect, since it's quite
legitimate to use these instructions in this way in static
environments, and the assembler doesn't have knowledge of whether this
is what's intended. There is also a possible middle way of allowing
the instructions but always emitting a relocation, which allows a tool
further down the line to check whether the preemption is needed and
throw an error if so. Since the Linux kernel doesn't do symbol
preemption it can safely ignore these relocations with a tiny amount
of extra code in the module loader.
In general, we can't expect to prevent absolutely the programmer from
doing something that generates the wrong answer, such as writing
non-PIC code and the linking it into a PIC environment; though any
diagnostics which help avoid this happening by accident are still
useful.
To give the flipside to this, there is a case when the assembler knows
that preemption absolutely must be supported, and that is when a
symbol is declared weak. So:
.type g, %function
g: ldr r0, d
.weak d
d: .long 0
$ arm-linux-gnueabi-as --version
GNU assembler (GNU Binutils for Ubuntu) 2.20.51.20100908
[...]
$ arm-linux-gnueabi-as -o tst.o tst.s && arm-linux-gnueabi-objdump -drt tst.o
tst.s: Assembler messages:
tst.s:2: Error: internal_relocation (type: OFFSET_IMM) not fixed up
$ arm-linux-gnueabi-as -mthumb -o tst.o tst.s &&
arm-linux-gnueabi-objdump -drt tst.o
tst.s: Assembler messages:
tst.s:2: Error: invalid offset, value too big (0xFFFFFFFFFFFFFFFC)
i.e., cryptic error messages aside, the assembler fails sensibly for
ARM and Thumb alike. This is appropriate, because ".weak" declares
explicitly the need for preemptibility, and the assembler knows that
can't be guaranteed with these instruction forms. ".globl" does not,
because where the symbol is not weak it's impossible for the assembler
to know what the programmer intends, and so throwing an error for such
cases seems too zealous from my point of view.
For comparison, changing ".weak" to ".globl" in the above example gives:
$ arm-linux-gnueabi-as -o tst.o tst.s && arm-linux-gnueabi-objdump -drt tst.o
[...]
00000004 g .text 00000000 d
[...]
00000000 <f>:
0: e51f0004 ldr r0, [pc, #-4] ; 4 <d>
00000004 <d>:
4: 00000000 .word 0x00000000
$ arm-linux-gnueabi-as -mthumb -o tst.o tst.s &&
arm-linux-gnueabi-objdump -drt tst.o
tst.s: Assembler messages:
tst.s:2: Error: invalid offset, value too big (0xFFFFFFFFFFFFFFFC)
Notice that the ARM code refernce to d is not preemptible.
Cheers
---Dave
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix assembly of Thumb pcrel LDRs against global symbols
2011-03-02 16:37 ` Dave Martin
@ 2011-03-02 16:45 ` Richard Earnshaw
2011-03-02 17:04 ` Dave Martin
0 siblings, 1 reply; 8+ messages in thread
From: Richard Earnshaw @ 2011-03-02 16:45 UTC (permalink / raw)
To: Dave Martin; +Cc: Richard Sandiford, binutils, patches
On Wed, 2011-03-02 at 16:37 +0000, Dave Martin wrote:
> On Wed, Mar 2, 2011 at 3:18 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
> >
> > On Mon, 2011-02-28 at 12:59 +0000, Richard Sandiford wrote:
> >> The PC-relative LDR instructions have no associated relocation,
> >> so even LDRs for global symbols should be resolved by the assembler.
> >> We currently handle this correctly for single-register ARM loads,
> >> but we're missing the associated relocation types for LDRD and Thumb.
> >> This leads to errors like:
> >
> > I'm not sure I agree with this. If I write
> >
> > .global foo
> >
> > ...
> > ldr r0, foo
> >
> > ...
> >
> > foo:
> > ...
> >
> > but then at link/load time pre-empt foo with some other definition, that
> > will silently leave me with the wrong answer.
> >
> > It's clearly OK to do this if foo is protected or private, but not
> > otherwise.
> >
> > The existing error message is not very informative as to what the
> > problem is, but I'm not sure I agree with your solution...
> >
> > R.
>
> That's a reasonable argument, but similarly to the ADR case, such code
> referencing global symbols is already allowed through for ARM _with no
> relocation emitted_ - the Thumb treatment is inconsistent and causes
> existing code which builds for ARM (but where symbol preemption may
> silently fail to work). This raises non-trivial questions about which
> behaviour should be considered correct.
>
> Unless there are some tricks I haven't thought of, it seems that ADR
> and PC-relative LDR and friends simply can't be generally preemptible,
> period -- because the relocation ranges these instructions support
> just aren't large enough to guarantee that a relocation on these
> instructions can ever be resolved except in a trivial subset of cases.
>
> Solving this inconsistency requires a conclusion regarding whether the
> tools should trust the programmer to write correct code (i.e., fix up
> pc-relative LDR and ADR in the assembler and emit no relocation) or
> not (i.e., treat these instructions as an error when they reference
> global symbols).
>
> I would regard the latter option as incorrect, since it's quite
> legitimate to use these instructions in this way in static
> environments, and the assembler doesn't have knowledge of whether this
> is what's intended. There is also a possible middle way of allowing
> the instructions but always emitting a relocation, which allows a tool
> further down the line to check whether the preemption is needed and
> throw an error if so. Since the Linux kernel doesn't do symbol
> preemption it can safely ignore these relocations with a tiny amount
> of extra code in the module loader.
>
> In general, we can't expect to prevent absolutely the programmer from
> doing something that generates the wrong answer, such as writing
> non-PIC code and the linking it into a PIC environment; though any
> diagnostics which help avoid this happening by accident are still
> useful.
>
>
> To give the flipside to this, there is a case when the assembler knows
> that preemption absolutely must be supported, and that is when a
> symbol is declared weak. So:
>
> .type g, %function
> g: ldr r0, d
>
> .weak d
> d: .long 0
>
> $ arm-linux-gnueabi-as --version
> GNU assembler (GNU Binutils for Ubuntu) 2.20.51.20100908
> [...]
> $ arm-linux-gnueabi-as -o tst.o tst.s && arm-linux-gnueabi-objdump -drt tst.o
> tst.s: Assembler messages:
> tst.s:2: Error: internal_relocation (type: OFFSET_IMM) not fixed up
> $ arm-linux-gnueabi-as -mthumb -o tst.o tst.s &&
> arm-linux-gnueabi-objdump -drt tst.o
> tst.s: Assembler messages:
> tst.s:2: Error: invalid offset, value too big (0xFFFFFFFFFFFFFFFC)
>
> i.e., cryptic error messages aside, the assembler fails sensibly for
> ARM and Thumb alike. This is appropriate, because ".weak" declares
> explicitly the need for preemptibility, and the assembler knows that
> can't be guaranteed with these instruction forms. ".globl" does not,
> because where the symbol is not weak it's impossible for the assembler
> to know what the programmer intends, and so throwing an error for such
> cases seems too zealous from my point of view.
>
> For comparison, changing ".weak" to ".globl" in the above example gives:
> $ arm-linux-gnueabi-as -o tst.o tst.s && arm-linux-gnueabi-objdump -drt tst.o
> [...]
> 00000004 g .text 00000000 d
> [...]
> 00000000 <f>:
> 0: e51f0004 ldr r0, [pc, #-4] ; 4 <d>
> 00000004 <d>:
> 4: 00000000 .word 0x00000000
>
> $ arm-linux-gnueabi-as -mthumb -o tst.o tst.s &&
> arm-linux-gnueabi-objdump -drt tst.o
> tst.s: Assembler messages:
> tst.s:2: Error: invalid offset, value too big (0xFFFFFFFFFFFFFFFC)
>
> Notice that the ARM code refernce to d is not preemptible.
>
It's easy for the user to express their intention, simply use
a .visibility (protected/hidden) directive (at least in ELF, and that's
what I really care about...).
Then the linker will (can be safely made to) permit the early resolution
of the symbol do the right thing, and will also do the right thing if
the user then accidentally tries to override the symbol later on.
The tools *can* correctly support the behaviours being asked for. The
implementation is currently wrong. The fact that there is broken code
out there that relies on the currently broken implementation is not
really an excuse for keeping the status quo.
R.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix assembly of Thumb pcrel LDRs against global symbols
2011-03-02 16:45 ` Richard Earnshaw
@ 2011-03-02 17:04 ` Dave Martin
0 siblings, 0 replies; 8+ messages in thread
From: Dave Martin @ 2011-03-02 17:04 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: Richard Sandiford, binutils, patches
On Wed, Mar 2, 2011 at 4:45 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>
> On Wed, 2011-03-02 at 16:37 +0000, Dave Martin wrote:
>> On Wed, Mar 2, 2011 at 3:18 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>> >
>> > On Mon, 2011-02-28 at 12:59 +0000, Richard Sandiford wrote:
>> >> The PC-relative LDR instructions have no associated relocation,
>> >> so even LDRs for global symbols should be resolved by the assembler.
>> >> We currently handle this correctly for single-register ARM loads,
>> >> but we're missing the associated relocation types for LDRD and Thumb.
>> >> This leads to errors like:
>> >
>> > I'm not sure I agree with this. If I write
>> >
>> > .global foo
>> >
>> > ...
>> > ldr r0, foo
>> >
>> > ...
>> >
>> > foo:
>> > ...
>> >
>> > but then at link/load time pre-empt foo with some other definition, that
>> > will silently leave me with the wrong answer.
>> >
>> > It's clearly OK to do this if foo is protected or private, but not
>> > otherwise.
>> >
>> > The existing error message is not very informative as to what the
>> > problem is, but I'm not sure I agree with your solution...
>> >
>> > R.
>>
>> That's a reasonable argument, but similarly to the ADR case, such code
>> referencing global symbols is already allowed through for ARM _with no
>> relocation emitted_ - the Thumb treatment is inconsistent and causes
>> existing code which builds for ARM (but where symbol preemption may
>> silently fail to work). This raises non-trivial questions about which
>> behaviour should be considered correct.
>>
>> Unless there are some tricks I haven't thought of, it seems that ADR
>> and PC-relative LDR and friends simply can't be generally preemptible,
>> period -- because the relocation ranges these instructions support
>> just aren't large enough to guarantee that a relocation on these
>> instructions can ever be resolved except in a trivial subset of cases.
>>
>> Solving this inconsistency requires a conclusion regarding whether the
>> tools should trust the programmer to write correct code (i.e., fix up
>> pc-relative LDR and ADR in the assembler and emit no relocation) or
>> not (i.e., treat these instructions as an error when they reference
>> global symbols).
>>
>> I would regard the latter option as incorrect, since it's quite
>> legitimate to use these instructions in this way in static
>> environments, and the assembler doesn't have knowledge of whether this
>> is what's intended. There is also a possible middle way of allowing
>> the instructions but always emitting a relocation, which allows a tool
>> further down the line to check whether the preemption is needed and
>> throw an error if so. Since the Linux kernel doesn't do symbol
>> preemption it can safely ignore these relocations with a tiny amount
>> of extra code in the module loader.
>>
>> In general, we can't expect to prevent absolutely the programmer from
>> doing something that generates the wrong answer, such as writing
>> non-PIC code and the linking it into a PIC environment; though any
>> diagnostics which help avoid this happening by accident are still
>> useful.
>>
>>
>> To give the flipside to this, there is a case when the assembler knows
>> that preemption absolutely must be supported, and that is when a
>> symbol is declared weak. So:
>>
>> .type g, %function
>> g: ldr r0, d
>>
>> .weak d
>> d: .long 0
>>
>> $ arm-linux-gnueabi-as --version
>> GNU assembler (GNU Binutils for Ubuntu) 2.20.51.20100908
>> [...]
>> $ arm-linux-gnueabi-as -o tst.o tst.s && arm-linux-gnueabi-objdump -drt tst.o
>> tst.s: Assembler messages:
>> tst.s:2: Error: internal_relocation (type: OFFSET_IMM) not fixed up
>> $ arm-linux-gnueabi-as -mthumb -o tst.o tst.s &&
>> arm-linux-gnueabi-objdump -drt tst.o
>> tst.s: Assembler messages:
>> tst.s:2: Error: invalid offset, value too big (0xFFFFFFFFFFFFFFFC)
>>
>> i.e., cryptic error messages aside, the assembler fails sensibly for
>> ARM and Thumb alike. This is appropriate, because ".weak" declares
>> explicitly the need for preemptibility, and the assembler knows that
>> can't be guaranteed with these instruction forms. ".globl" does not,
>> because where the symbol is not weak it's impossible for the assembler
>> to know what the programmer intends, and so throwing an error for such
>> cases seems too zealous from my point of view.
>>
>> For comparison, changing ".weak" to ".globl" in the above example gives:
>> $ arm-linux-gnueabi-as -o tst.o tst.s && arm-linux-gnueabi-objdump -drt tst.o
>> [...]
>> 00000004 g .text 00000000 d
>> [...]
>> 00000000 <f>:
>> 0: e51f0004 ldr r0, [pc, #-4] ; 4 <d>
>> 00000004 <d>:
>> 4: 00000000 .word 0x00000000
>>
>> $ arm-linux-gnueabi-as -mthumb -o tst.o tst.s &&
>> arm-linux-gnueabi-objdump -drt tst.o
>> tst.s: Assembler messages:
>> tst.s:2: Error: invalid offset, value too big (0xFFFFFFFFFFFFFFFC)
>>
>> Notice that the ARM code refernce to d is not preemptible.
>>
>
> It's easy for the user to express their intention, simply use
> a .visibility (protected/hidden) directive (at least in ELF, and that's
> what I really care about...).
Can non-weak symbol references which resolve within the static partion
of the link (i.e., the main program in dynamic environments) be
preempted? I thought the answer was "no", hence the need for things
like R_ARM_COPY. If so that suggests that making all symbols in the
static portion hidden shouldn't be needed, and using the affected
instruction forms on global symbols should be legitimate -- but the
suggested behaviour contradicts this.
My assumption may be wrong here, though.
>
> Then the linker will (can be safely made to) permit the early resolution
> of the symbol do the right thing, and will also do the right thing if
> the user then accidentally tries to override the symbol later on.
>
> The tools *can* correctly support the behaviours being asked for. The
> implementation is currently wrong. The fact that there is broken code
> out there that relies on the currently broken implementation is not
> really an excuse for keeping the status quo.
Fair point. Just to be clear, the current implementation may
therefore be considered broken in either one or two ways: take your
pick from:
a) For ARM code, non-preemptible ADR and LDRs to preemptible symbols
are fixed up locally and emitted as-is, with no relocation.
b) For Thumb code, ADR and LDR to preemptible symbols both cause the
assembler to throw an error, even if the symbol is explicitly declared
non-preemptible (e.g., .hidden or similar).
Cheers
---Dave
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-03-02 17:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-28 12:59 Fix assembly of Thumb pcrel LDRs against global symbols Richard Sandiford
2011-03-01 11:26 ` Dave Martin
2011-03-01 13:50 ` Richard Sandiford
2011-03-02 15:18 ` Richard Earnshaw
2011-03-02 16:36 ` Richard Sandiford
2011-03-02 16:37 ` Dave Martin
2011-03-02 16:45 ` Richard Earnshaw
2011-03-02 17:04 ` Dave Martin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).