From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aneesh V Date: Sat, 18 Feb 2012 18:54:11 +0530 Subject: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions In-Reply-To: <4F3F79B4.5060903@aribaud.net> References: <1328528248-20872-1-git-send-email-aneesh@ti.com> <1329314253-4596-3-git-send-email-aneesh@ti.com> <4F3E357E.8080607@ti.com> <4F3F79B4.5060903@aribaud.net> Message-ID: <4F3FA67B.8060603@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Albert, On Saturday 18 February 2012 03:43 PM, Albert ARIBAUD wrote: > Hi Aneesh, > > Le 17/02/2012 12:09, Aneesh V a ?crit : >> Hi Albert, >> >> On Wednesday 15 February 2012 07:27 PM, Aneesh V wrote: >>> This is done using the following directive preceding >>> each function definition: >>> >>> .type, %function >>> >>> This marks the symbol as a function in the object >>> header which in turn helps the linker in some cases. >>> >>> In particular this was found needed for resolving ARM/Thumb >>> calls correctly in a build with Thumb interworking enabled. >>> >>> This solves the following problem I had reported earlier: >>> >>> "When U-Boot/SPL is built using the Thumb instruction set the >>> toolchain has a potential issue with weakly linked symbols. >>> If a function has a weakly linked default implementation in C >>> and a real implementation in assembly GCC is confused about the >>> instruction set of the assembly implementation. As a result >>> the assembly function that is built in ARM is executed as >>> if it is Thumb. This results in a crash" >>> >>> Signed-off-by: Aneesh V >> >> Does this look good to you. I was a bit nervous about touching so many >> files. Please let me know if you would prefer to change only the OMAP >> function that was creating the ARM/Thumb problem. I did a "MAKEALL -a >> arm" and didn't see any new errors. >> >> Let me know if this is an acceptable solution to the problem. > > Regarding the solution: it is quite ok to me. I would just like to > understand the exact effect of the .function directive, what its options > are and if some of these should not be explicitly specified. > > Regarding touching many files: I won't be worried as long as you check > that the first three patches have no effect on existing boards. This can > be verified as follows -- if you haven't done so already: > > - build your OMAP target without the patch set and do a hex dump of > u-boot.bin; > > - apply the first three patches of your set, rebuild your OMAP target > without the patch set and do a hex dump of u-boot.bin; > > - compare both dumps. Normally you should only see one difference, in > the build version and date -- if .function does not actually alter the > assembly code, which I hope it indeed does not when building for ARM. > > If there are more changes than build version and date, then they might > be due to .function requiring some yet unknown additional option, or to > some change in patch 1 or 3 not being completely conditioned on > CONFIG_SYS_THUMB_BUILD. I can reproduce the problem with a simple test program. Note: I can reproduce this with Sourcery G++ Lite 2010q1-202 (GCC 4.4.1 - Binutils 2.19.51.20090709) But I *can not* reproduce reproduce this with Linaro GCC 2012.01 (GCC 4.6.3 , Binutils 2.22) So apparently the issue has been fixed recently. Unfortunately Linaro GCC 2012.01 creates a new Thumb problem that I am investigating now. Somehow I missed this when I tested earlier. So, my Thumb build is not working with Linaro GCC 2012.01. But this one is not reproduced on Sourcery G++ Lite 2010q1-202! Here is the program I used to reproduce the problem in Sourcery G++ Lite 2010q1-202 that this patch is addressing a.c: ==== extern void foo (void) __attribute__ ((weak, alias ("__foo"))); void __foo (void) { } extern void call_foo(void); int main (void) { call_foo (); } b.S: ==== .text .align 2 .global foo foo: push {r7} add r7, sp, #0 mov sp, r7 pop {r7} bx lr .size foo, .-foo c.S: ==== .text .align 2 .global call_foo call_foo: bl foo bx lr .global __aeabi_unwind_cpp_pr0 __aeabi_unwind_cpp_pr0: bx lr Now build it and take the assembly dump using the following commands: arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c a.c arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c b.S arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c c.S arm-none-linux-gnueabi-ld -r a.o -o alib.o arm-none-linux-gnueabi-ld -r b.o -o blib.o arm-none-linux-gnueabi-ld -r c.o -o clib.o arm-none-linux-gnueabi-ld --start-group clib.o alib.o blib.o --end-group -o a.out arm-none-linux-gnueabi-objdump -S --reloc a.out You will get something like this in the assembly dump: 00008094 : 8094: fa000006 blx 80b4 8098: e12fff1e bx lr The blx is wrong as we are jumping to an ARM function from ARM. Now if you change b.S like this: .text .align 2 +.type foo, %function .global foo foo: push {r7} And compile it again in the same way you will see: 00008094 : 8094: eb000006 bl 80b4 8098: e12fff1e bx lr Please note that the branch to foo is correct now. I hope this convinces you that %function indeed has an effect. I will get back with more details on the Linaro GCC 2012.01 later. br, Aneesh