On Mon, Feb 20, 2012 at 06:59:53PM +0100, Ulrich Weigand wrote:
"V, Aneesh" aneesh@ti.com wrote:
I agree that not marking the assembly functions ' %function' is a problem in the code, so it's not a critical bug. But I would've been happier if the linker refused to link it rather than branching with the wrong instruction. Isn't that a problem?
Well, if the target symbol of a branch is not marked as %function, the linker has no way of knowing whether that target is ARM or Thumb, so it cannot specifically error out if (and only if) the instruction is wrong.
The linker *could* in theory give an error *unconditionally* whenever it detects a branch to a non-%function symbol. The reason this is not done is probably for backwards compatibility with old hand-written code, say from an ARM-only era: branches to non-function symbols used to be allowed, and in fact work fine if all code is ARM-only. Adding an error would break such old code.
Problem No:2
Linaro GCC 2012.01 is giving a new problem w.r.to Thumb build that is not existing in Sourcery G++ Lite 2010q1-202. However, I couldn't reproduce this problem with a small program like above. So, let me give you reference to the original u-boot code that shows the problem and steps to reproduce it.
[snip]
Please note that the .rodata symbols have odd addresses. These arrays actually need to be aligned at least to half-word boundary. In fact, in the image I verified that they are put at even addresses. So, the symbols have been kept as real address +1.
This seems strange. How did you verify "that they are put at even addresses"? 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.
Is "packed" even wanted here?
Based on a very brief skim of the code, it looks like the packed attribute is an unnecessary attempt to save some space -- unnecessary because all ARM ABI variants I know of (actually, all arches I know of) will pack that structure into a word anyway as a result of natural alignment of the members. It doesn't look to me like the packed structure is used to map a memory-mapped peripheral directly or otherwise communicate with the outside world -- which would be the only situations in which a packed structure would normally make sense.
Of course, I may have missed something...
Cheers ---Dave