On Tuesday 21 February 2012 03:27 PM, Dave Martin wrote:
On Mon, Feb 20, 2012 at 06:59:53PM +0100, Ulrich Weigand wrote:
"V, Aneesh"<ane...@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...

No. I think packed is not needed here. Removing it didn't change the
size of the arrays. I will remove it.

Thanks,
Aneesh

_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Reply via email to