From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Sat, 18 Feb 2012 17:48:57 +0100 Subject: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions In-Reply-To: <4F3FD330.7000204@ti.com> 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> <4F3FA67B.8060603@ti.com> <4F3FD330.7000204@ti.com> Message-ID: <4F3FD679.9030509@aribaud.net> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Aneesh, Le 18/02/2012 17:34, Aneesh V a ?crit : > On Saturday 18 February 2012 06:54 PM, Aneesh V wrote: >> 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) > > Linaro GCC 2012.01 has the same problem when assembly function(ARM is > called from C (Thumb). I can reproduce it using this program: > > a.c: > ==== > int main (void) > { > 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 > > .global __aeabi_unwind_cpp_pr0 > __aeabi_unwind_cpp_pr0: > bx lr > > arm-linux-gnueabi-gcc -mthumb -mthumb-interwork -c a.c > arm-linux-gnueabi-gcc -mthumb -mthumb-interwork -c b.S > arm-linux-gnueabi-ld -r a.o -o alib.o > arm-linux-gnueabi-ld -r b.o -o blib.o > arm-linux-gnueabi-ld --start-group alib.o blib.o --end-group -o a.out > arm-linux-gnueabi-objdump -S --reloc a.out > > gives: > 8076: af00 add r7, sp, #0 > 8078: f000 f802 bl 8080 > 807c: 4618 mov r0, r3 > > It should have been "blx foo" > > Again, %function solves it. Conclusion: %function is necessary with > both old and new tool-chains when building for Thumb. > > > It should have been "blx 8080 ", isn't it? Again, %function > solves it. > > Conclusion: %function is necessary with both old and new tool-chains > when building for Thumb. > >> 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. > > I meant "the Linaro GCC 2012.01 tool-chain problem" > > This is a different problem. Some of the .rodata symbols are given an > odd address although they should be aligned to at least 2-byte boundary > ). In fact the data is actually put at the even address but the symbol's > value is +1 of the actual address. This is the ARM convention for Thumb > functions, but they have applied it here for data too. That's the > problem. I see that this doesn't happen to all the .rodata in SPL. > Neither could I reproduce it with a small program. But the workaround > for this problem is to avoid -fdata-sections. The following patch works > around it. > > diff --git a/config.mk b/config.mk > index ddaa477..723286a 100644 > --- a/config.mk > +++ b/config.mk > @@ -190,7 +190,7 @@ CPPFLAGS := $(DBGFLAGS) $(OPTFLAGS) $(RELFLAGS) \ > > # Enable garbage collection of un-used sections for SPL > ifeq ($(CONFIG_SPL_BUILD),y) > -CPPFLAGS += -ffunction-sections -fdata-sections > +CPPFLAGS += -ffunction-sections > LDFLAGS_FINAL += --gc-sections > endif > > Will you take a patch to make -fdata-sections optional, that is, having > it under something like CONFIG_SYS_SPL_NO_FDATA_SECTIONS? Hmm... considering you're seeing the issue in a fairly new toolchain release, I prefer notifying the toolchain makers, rather than removing the -fdata-sections from SPL even for specific boards. Can you go and see why the Linaro toolchain generated odd "thumb data" at all and get this fixed? > br, > Aneesh Amicalement, -- Albert.