From patchwork Sat Feb 18 16:34:56 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Aneesh V X-Patchwork-Id: 142038 X-Patchwork-Delegate: albert.aribaud@free.fr Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 3771DB6F9B for ; Sun, 19 Feb 2012 03:35:18 +1100 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 372B0280A5; Sat, 18 Feb 2012 17:35:16 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id V0SEJKvn9Zsl; Sat, 18 Feb 2012 17:35:15 +0100 (CET) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 89D0C2809D; Sat, 18 Feb 2012 17:35:11 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 395902809D for ; Sat, 18 Feb 2012 17:35:09 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id jye4tANGYhct for ; Sat, 18 Feb 2012 17:35:07 +0100 (CET) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from na3sys009aog119.obsmtp.com (na3sys009aog119.obsmtp.com [74.125.149.246]) by theia.denx.de (Postfix) with ESMTPS id 0BBE82809C for ; Sat, 18 Feb 2012 17:35:05 +0100 (CET) Received: from mail-tul01m020-f171.google.com ([209.85.214.171]) (using TLSv1) by na3sys009aob119.postini.com ([74.125.148.12]) with SMTP ID DSNKTz/TNpBwetLd0JNexgpRcu/12Gmhzvzl@postini.com; Sat, 18 Feb 2012 08:35:07 PST Received: by mail-tul01m020-f171.google.com with SMTP id uy19so7333506obc.2 for ; Sat, 18 Feb 2012 08:35:02 -0800 (PST) Received-SPF: pass (google.com: domain of aneesh@ti.com designates 10.60.12.103 as permitted sender) client-ip=10.60.12.103; Authentication-Results: mr.google.com; spf=pass (google.com: domain of aneesh@ti.com designates 10.60.12.103 as permitted sender) smtp.mail=aneesh@ti.com Received: from mr.google.com ([10.60.12.103]) by 10.60.12.103 with SMTP id x7mr5413562oeb.51.1329582902603 (num_hops = 1); Sat, 18 Feb 2012 08:35:02 -0800 (PST) Received: by 10.60.12.103 with SMTP id x7mr4612058oeb.51.1329582902522; Sat, 18 Feb 2012 08:35:02 -0800 (PST) Received: from [172.24.137.145] (dragon.ti.com. [192.94.94.33]) by mx.google.com with ESMTPS id h9sm1771212obr.20.2012.02.18.08.34.59 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 18 Feb 2012 08:35:01 -0800 (PST) Message-ID: <4F3FD330.7000204@ti.com> Date: Sat, 18 Feb 2012 22:04:56 +0530 From: Aneesh V User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.21) Gecko/20110831 Thunderbird/3.1.13 MIME-Version: 1.0 To: Albert ARIBAUD 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> In-Reply-To: <4F3FA67B.8060603@ti.com> X-Gm-Message-State: ALoCoQkoP+YpWsTXQAaZqsBPwKzzv4ldecloVnu8nggcJfWTQaAEO/fGVlxzlQadjNzvPscy7dFB Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de 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: Will you take a patch to make -fdata-sections optional, that is, having it under something like CONFIG_SYS_SPL_NO_FDATA_SECTIONS? br, Aneesh ==== 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