From: Richard Earnshaw <rearnsha@arm.com>
Date: Fri, 1 Mar 2013 15:16:39 +0100
On 01/03/13 05:23, Hans-Peter Nilsson wrote:
See <http://sourceware.org/bugzilla/show_bug.cgi?id=15217>. Are
target symbols required to have a type specified in order for
arm/thumb stubs to work?
Yes.
That's definitely the hard-and-fast rule for well-behaving
AAELF-conformant code. But, the current linker behavior is both
inconsistent and apparently an unintended change, so it should
change back. At the very least, I hope you agree the linker
behavior should be consistent and shouldn't be allowed to
oscillate.
I did a little digging. You know all this, but for the record,
AAELF (ELF for the ARM Architecture; the BPAPI for ARM in ELF
parlance) as per
<http://infocenter.arm.com/help/topic/com.arm.doc.ihi0044e/IHI0044E_aaelf.pdf>
says in "4.5.2 Symbol Types": "All code symbols exported from an
object file (symbols with binding STB_GLOBAL) shall have type
STT_FUNC", but I'm not sure whether that implies there's a bug
in the assembler or if it just means to restrict where AAELF
applies. Maybe best leave current behavior as-is there. AAELF
seems to open up for the existence of untyped code symbols when
it later states "The linker is only required to provide
interworking support for symbols of type STT_FUNC (interworking
for untyped symbols must be encoded directly in the object
file)." Note "only required to", not "required only to",
leaving whether it also does it for untyped symbols is optional.
(Regarding splitting hairs, there's a "must" there, but as
noted, it already accepted an exception to a "shall".)
The issue would not have arisen if behavior had been consistent
across versions. Indeed, a mode-changing stub *used* to be
generated for the test-case; it was changed in binutils-1.21
(apparently CVS 1.261 of elf32-arm.c), but the behavioral change
seems to have been unintended; it wasn't mentioned as such in
the patch, the post or covered by the test-suite. This would
IMHO have been a newsworthy item. See
<http://sourceware.org/ml/binutils/2011-03/msg00055.html>, where
apparently this change was introduced (arm_type_of_stub as
below) when doing a rewrite as part of ARM support for IFUNC,
but the specific behavioral change is not noted in that thread,
references or test-suite changes, while being a newsworthy
change, silently causing such existing code to "fail".
Also, the current linker behavior is inconsistent; if the branch
is out-of-range, a mode-changing-stub *is* generated. Note the
lack of symbol type specification for the global symbol
func_to_branch_to in e.g. ld/testsuite/ld-arm/fix-arm1176.s that
is used to trigger a stub or change to blx!
Regarding the impact of this unintended change for borderline
code, I'm guessing a low count of thousands of sites with code
that assumes stubs are generated for calls from thumb to arm for
untyped symbols, developed using binutils-2.21 or before back to
at least 2.18. Ok, that was toungue-in-cheek as I'm including
the linker test-suite as above but there are at least
more-than-three; the chip vendor from whom we got the code where
this behavior "changed" and their customers, which may or may
not include the people at
<http://sourceware.org/ml/binutils/2012-03/msg00121.html>.
The assumption is that if you call an untyped symbol the code knows what
it is doing and that special processing for interworking is not
required. Furthermore, the guarantee that IP (r12) is free as an
interworking register is not available for untyped symbol interfaces.
I know I stated my question generally, but the issue is just
with stubs *from thumb to arm* so IIUC clobbering r12/ip isn't
an issue.
I'm suggesting we change back behavior to always generate
mode-changing stubs from thumb to arm as per below. Either way,
consistency is expected and I can't see how this could cause
harm for users, in contrast to the current situation.
Checked arm-linux-eabi and arm-eabi.
Ok to commit?
bfd:
PR ld/15217
* elf32-arm.c (arm_type_of_stub): Match calls to untyped
symbols as calls to ARM mode.
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index aba1814..9c3e5a2 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -3637,7 +3637,9 @@ arm_type_of_stub (struct bfd_link_info *info,
|| (thumb2
&& (branch_offset > THM2_MAX_FWD_BRANCH_OFFSET
|| (branch_offset < THM2_MAX_BWD_BRANCH_OFFSET)))
- || (branch_type == ST_BRANCH_TO_ARM
+ || ((branch_type == ST_BRANCH_TO_ARM
+ /* Match calls to untyped symbols as calls to ARM. */
+ || (branch_type == ST_BRANCH_UNKNOWN && hash != NULL))
&& (((r_type == R_ARM_THM_CALL
|| r_type == R_ARM_THM_TLS_CALL) && !globals->use_blx)
|| (r_type == R_ARM_THM_JUMP24))
ld/testsuite:
PR ld/15217
* ld-arm/app-notype.s, ld-arm/lib-notype.s,
ld-arm/interwork-notype.d: New test.
* ld-arm/arm-elf.exp: Run it.
diff --git a/ld/testsuite/ld-arm/app-notype.s b/ld/testsuite/ld-arm/app-notype.s
new file mode 100644
index 0000000..38cb3a5
--- /dev/null
+++ b/ld/testsuite/ld-arm/app-notype.s
@@ -0,0 +1,35 @@
+# Check that calls to untyped symbols get stubs.
+# The lack of symbol types below is intended; the commented-out
+# .type lines show proper symbol type designation.
+
+ .text
+ .p2align 4
+ .globl _start
+_start:
+ mov ip, sp
+ stmdb sp!, {r11, ip, lr, pc}
+ bl app_tfunc
+ ldmia sp, {r11, sp, lr}
+ bx lr
+
+ .p2align 4
+ .globl app_tfunc
+# .type app_tfunc,%function
+ .thumb_func
+ .code 16
+app_tfunc:
+ push {lr}
+ bl lib_func2
+ pop {pc}
+ bx lr
+
+ .p2align 4
+ .globl app_tfunc2
+# .type app_tfunc2,%function
+ .thumb_func
+ .code 16
+app_tfunc2:
+ push {lr}
+ bl app_tfunc
+ pop {pc}
+ bx lr
diff --git a/ld/testsuite/ld-arm/arm-elf.exp b/ld/testsuite/ld-arm/arm-elf.exp
index d9375ec..ea0beba 100644
--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -789,6 +789,7 @@ run_dump_test "attr-merge-vfp-7"
run_dump_test "attr-merge-vfp-7r"
run_dump_test "attr-merge-incompatible"
run_dump_test "unresolved-1"
+run_dump_test "interwork-notype"
if { ![istarget "arm*-*-nacl*"] } {
run_dump_test "unresolved-1-dyn"
}
diff --git a/ld/testsuite/ld-arm/interwork-notype.d b/ld/testsuite/ld-arm/interwork-notype.d
new file mode 100644
index 0000000..f36cb18
--- /dev/null
+++ b/ld/testsuite/ld-arm/interwork-notype.d
@@ -0,0 +1,63 @@
+# source: app-notype.s
+# source: lib-notype.s
+# ld: --fix-arm1176
+# objdump: -d
+
+.*: file format elf32-littlearm
+
+Disassembly of section .text:
+
+[0-9a-f]+ <_start>:
+ [0-9a-f]+: e1a0c00d mov ip, sp
+ [0-9a-f]+: e92dd800 push {fp, ip, lr, pc}
+ [0-9a-f]+: eb000016 bl [0-9a-f]+ <__app_tfunc_from_arm>
+ [0-9a-f]+: e89d6800 ldm sp, {fp, sp, lr}
+ [0-9a-f]+: e12fff1e bx lr
+ [0-9a-f]+: e1a00000 nop .*
+ [0-9a-f]+: e1a00000 nop .*
+ [0-9a-f]+: e1a00000 nop .*
+
+[0-9a-f]+ <app_tfunc>:
+ [0-9a-f]+: b500 push {lr}
+ [0-9a-f]+: f000 f81d bl [0-9a-f]+ <__lib_func2_(from_thumb|veneer)>
+ [0-9a-f]+: bd00 pop {pc}
+ [0-9a-f]+: 4770 bx lr
+ [0-9a-f]+: 46c0 nop .*
+ [0-9a-f]+: 46c0 nop .*
+ [0-9a-f]+: 46c0 nop .*
+
+[0-9a-f]+ <app_tfunc2>:
+ [0-9a-f]+: b500 push {lr}
+ [0-9a-f]+: f7ff fff5 bl [0-9a-f]+ <app_tfunc>
+ [0-9a-f]+: bd00 pop {pc}
+ [0-9a-f]+: 4770 bx lr
+ [0-9a-f]+: 46c0 nop .*
+ [0-9a-f]+: 46c0 nop .*
+ [0-9a-f]+: 46c0 nop .*
+
+[0-9a-f]+ <lib_func2>:
+ [0-9a-f]+: e1a0c00d mov ip, sp
+ [0-9a-f]+: e92dd800 push {fp, ip, lr, pc}
+ [0-9a-f]+: eb000009 bl [0-9a-f]+ <__app_tfunc2_from_arm>
+ [0-9a-f]+: e89d6800 ldm sp, {fp, sp, lr}
+ [0-9a-f]+: e12fff1e bx lr
+ [0-9a-f]+: e1a00000 nop .*
+ [0-9a-f]+: e1a00000 nop .*
+ [0-9a-f]+: e1a00000 nop .*
+
+[0-9a-f]+ <__lib_func2_(from_thumb|veneer)>:
+ [0-9a-f]+: 4778 bx pc
+ [0-9a-f]+: 46c0 nop .*
+#...
+ [0-9a-f]+: eafffff5 b [0-9a-f]+ <lib_func2>
+
+[0-9a-f]+ <__app_tfunc_from_arm>:
+ [0-9a-f]+: e59fc000 ldr ip, \[pc[^]]*\] ; [0-9a-f]+ <__app_tfunc_from_arm\+0x8>
+ [0-9a-f]+: e12fff1c bx ip
+ [0-9a-f]+: ........ .word 0x........
+
+[0-9a-f]+ <__app_tfunc2_from_arm>:
+ [0-9a-f]+: e59fc000 ldr ip, \[pc[^]]*\] ; [0-9a-f]+ <__app_tfunc2_from_arm\+0x8>
+ [0-9a-f]+: e12fff1c bx ip
+ [0-9a-f]+: ........ .word 0x........
+#...
diff --git a/ld/testsuite/ld-arm/lib-notype.s b/ld/testsuite/ld-arm/lib-notype.s
new file mode 100644
index 0000000..ba84183
--- /dev/null
+++ b/ld/testsuite/ld-arm/lib-notype.s
@@ -0,0 +1,14 @@
+ .text
+
+ .p2align 4
+ .globl lib_func2
+# The lack of symbol type is intended. Proper code would have a
+# symbol type specified, like:
+# .type lib_func2, %function
+lib_func2:
+ mov ip, sp
+ stmdb sp!, {r11, ip, lr, pc}
+ bl app_tfunc2
+ ldmia sp, {r11, sp, lr}
+ bx lr
+ .size lib_func2, . - lib_func2
brgds, H-P