public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: "Andreas Bießmann" <andreas.devel@googlemail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] avr32: fix relocation address calculation
Date: Mon, 13 May 2013 10:35:12 +0200	[thread overview]
Message-ID: <5190A5C0.4050207@gmail.com> (raw)
In-Reply-To: <20130510170924.5d03c9b5@lilith>

Hi Albert,

On 05/10/2013 05:09 PM, Albert ARIBAUD wrote:
> Hi Andreas,
> 
> On Fri, 10 May 2013 12:57:21 +0200, "Andreas Bie?mann"
> <andreas.devel@googlemail.com> wrote:
> 
>> Hi Albert,
>>
>> On 05/10/2013 11:24 AM, Albert ARIBAUD wrote:
>>> Hi Andreas,
>>>
>>> On Wed,  8 May 2013 11:25:17 +0200, Andreas Bie?mann
>>> <andreas.devel@googlemail.com> wrote:
>>>
>>>> Commit 1865286466a5d0c7f2e3c37632da56556c838e9e (Introduce generic link
>>>> section.h symbol files) changed the __bss_end symbol type from char[] to
>>>> ulong. This led to wrong relocation parameters which ended up in a not working
>>>> u-boot. Unfortunately this is not clear to see cause due to RAM aliasing we
>>>> may get a 'half-working' u-boot then.
>>>>
>>>> Fix this by dereferencing the __bss_end symbol where needed.
>>>
>>> (cc:ing Simon and Tom)
>>>
>>> The dereferencing is correct, so this patch seems good per se (it could
>>> actually have applied when __bss_end was still a char[]).
>>
>> well, as I understood this the __bss_end being a char[] did implicitly
>> take the address when accessing __bss_end (as we do when we have a
>> definition of char foo[2] and we take just 'foo'). But you say here we
>> should reference the address of __bss_end while it was still of type
>> char[]. Sorry, I do not understand that, can you please clarify?
> 
> There are several concepts here, some pertaining to the compiler, some
> to the linker.
> 
> From the linker viewpoint, a symbol is *always* and *only* an address,
> the first address of the object corresponding to the symbol, and an
> object is just some area in the addressable space.
> 
> From the compiler viewpoint, an object has a C type, possibly with an
> initial value, and a name, which is the symbol. The compiler considers
> the name/symbol to be value, not the address of the corresponding
> object... at least most of the time: as you indicate, when the symbol
> denotes a C array, then the C compiler understand the symbol as the
> address of the array.
> 
> The __bss_end symbol does not actually correspond to an object in the
> usual sense, since the BSS contains all sorts of data: any C global,
> either uninitialized or initialized with zeroes, whatever its type,
> ends up in BSS. The most general way one can assign a type to BSS
> itself is by considering it as a shapeless array of bytes -- hence the
> char[] definition.
> 
> Thus, the C compiler considered the name __bss_end to denote the
> address of the BSS "object", and the C code for AVR32 was correct as it
> was actually referring to the BSS "object"'s address.
> 
> When the __bss_end symbol's C type was changed to 'ulong', this changed
> the way the compiler understood the symbol: it now thinks __bss_end is
> the BSS' "value", which has no true sense, and certainly does not mean
> 'the first 4 bytes of BSS considered as a 32-bit value'.
>
> To compensate this, the AVR32 code has to add an & to find the address
> of __bss_end, but the original error is to have changed the "type" of
> the BSS.
> 
> IOW, we should *always* take the address of __bss_end, since this is
> the only thing it was defined for. We should never give it a chance to
> even *have* a value at the C level, because we don't want to read, or
> worse, write, the BSS itself; we only want to access C globals in the
> BSS.

thank you for your detailed explanation. So now its clear why referring
the address of an object of type char[] will also work.
Another question, wouldn't it make sense to declare these C globals as
const then?

Best regards

Andreas Bie?mann

  parent reply	other threads:[~2013-05-13  8:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-08  9:25 [U-Boot] [PATCH] avr32: fix relocation address calculation Andreas Bießmann
2013-05-10  9:24 ` Albert ARIBAUD
2013-05-10 10:57   ` Andreas Bießmann
2013-05-10 15:09     ` Albert ARIBAUD
2013-05-10 17:02       ` Michael Cashwell
2013-05-10 17:15         ` Albert ARIBAUD
2013-05-13  8:35       ` Andreas Bießmann [this message]
2013-05-13 11:34         ` Albert ARIBAUD
2013-05-13 11:43   ` Albert ARIBAUD
2013-05-13  8:38 ` [U-Boot] " Andreas Bießmann
2013-05-13  8:42   ` Andreas Bießmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5190A5C0.4050207@gmail.com \
    --to=andreas.devel@googlemail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox