[U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions

[Aneesh V]

On Monday 20 February 2012 09:38 PM, Aneesh V wrote:
> On Saturday 18 February 2012 10:18 PM, Albert ARIBAUD wrote:
>> Hi Aneesh,
>>
>
> [...]
>
>>>> 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?
>
> I tried investigating a bit. As I mentioned earlier it doesn't happen
> with some other files that use the same compiler and linker commands.
> So, I don't know what's going on. Also, I couldn't reproduce it with a
> simple program unlike in the other cases. Anyway, I have notified
> tool-chain folks at Linaro:
>
> http://article.gmane.org/gmane.linux.linaro.toolchain/2096

Ulrich Weigand has identified the root-cause [1]. The problem was due
to __attribute__ ((packed)) used in a structure. Let me quote him:

**********************************************************************
"I can reproduce the odd addresses of .rodata symbols.  However, this
occurs simply because the linker put *no* alignment requirement whatsoever
on those sections:

Section Headers:
   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk
Inf Al
[snip]
   [11] .rodata.wkup_padc PROGBITS        00000000 000100 000004 00   A  0
0  1
   [12] .rodata.wkup_padc PROGBITS        00000000 000104 000048 00   A  0
0  1
   [13] .rodata.wkup_padc PROGBITS        00000000 00014c 00000c 00   A  0
0  1
   [14] .rodata.wkup_padc PROGBITS        00000000 000158 000004 00   A  0
0  1

Note the "Al" column values of 1.  In the final executable, those sections
happen to end up immediately following a .rodata.str string section with
odd
lenght, and since they don't have any alignment requirement, they start out
at an odd address.

The reason for the lack of alignment requirement is actually in the source:

const struct pad_conf_entry core_padconf_array_essential[] = {

where

struct pad_conf_entry {

         u16 offset;

         u16 val;

} __attribute__ ((packed));

The "packed" attribute specifies that all struct elements ought to be
considered to have alignment requirement 1 instead of their default
alignment.  Thus the whole struct ends up having alignment requirement 1;
and since the section contains only a single variable of such struct
type, this is then also the alignment requirement of the section."
**********************************************************************


I tried removing "packed" and it works. I will send an updated series
with this fix.

br,
Aneesh


[1] http://article.gmane.org/gmane.linux.linaro.toolchain/2099