Problems with branch-to-arm-from-thumb for typeless symbol

Hans-Peter Nilsson hans-peter.nilsson@axis.com
Mon Mar 4 03:41:00 GMT 2013


> 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



More information about the Binutils mailing list