diff mbox

[U-Boot] ARMv7: Fix linker errors across toolchain versions

Message ID 1291216634-7510-1-git-send-email-premi@ti.com
State Rejected
Headers show

Commit Message

Sanjeev Premi Dec. 1, 2010, 3:17 p.m. UTC
This patch fixes the linker problems noticed while
building the omap3_evm with Codesourcery toolchains
2009q1, 2009q3 and 2010q1.

The compilation was tested as success for both
omap3_evm and omap3_beagle with toolchain versions
2009q1 and 2010q1.

 [1] http://marc.info/?l=u-boot&m=129104332808386&w=2

Signed-off-by: Sanjeev Premi <premi@ti.com>
---
The patch touches all ARMv7 architectures, will need
to be reviewed thoroughly.

I am getting hang of relocation feature, but definitely
hands-on. Impact would have to be reviewd as well.
This is the reason for sending the patch early - before
i start testing on the evm.

 arch/arm/cpu/armv7/u-boot.lds |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

Comments

Albert ARIBAUD Dec. 1, 2010, 5:13 p.m. UTC | #1
Le 01/12/2010 16:17, Sanjeev Premi a écrit :
> This patch fixes the linker problems noticed while
> building the omap3_evm with Codesourcery toolchains
> 2009q1, 2009q3 and 2010q1.
>
> The compilation was tested as success for both
> omap3_evm and omap3_beagle with toolchain versions
> 2009q1 and 2010q1.
>
>   [1] http://marc.info/?l=u-boot&m=129104332808386&w=2
>
> Signed-off-by: Sanjeev Premi<premi@ti.com>
> ---
> The patch touches all ARMv7 architectures, will need
> to be reviewed thoroughly.
>
> I am getting hang of relocation feature, but definitely
> hands-on. Impact would have to be reviewd as well.
> This is the reason for sending the patch early - before
> i start testing on the evm.
>
>   arch/arm/cpu/armv7/u-boot.lds |   26 +++++++++++++++-----------
>   1 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/u-boot.lds b/arch/arm/cpu/armv7/u-boot.lds
> index 5725c30..faf6ad8 100644
> --- a/arch/arm/cpu/armv7/u-boot.lds
> +++ b/arch/arm/cpu/armv7/u-boot.lds
> @@ -55,22 +55,26 @@ SECTIONS
>
>   	. = ALIGN(4);
>
> -	.rel.dyn : {
> -		__rel_dyn_start = .;
> -		*(.rel*)
> -		__rel_dyn_end = .;
> -	}
> -
>   	.dynsym : {
>   		__dynsym_start = .;
>   		*(.dynsym)
>   	}
>
> -	.bss __rel_dyn_start (OVERLAY) : {
> -		__bss_start = .;
> -		*(.bss)
> -		 . = ALIGN(4);
> -		_end = .;
> +	OVERLAY : NOCROSSREFS
> +	{
> +		.rel.dyn {
> +			__rel_dyn_start = .;
> +			*(.rel*)
> +			__rel_dyn_end = .;
> +		}
> +
> +		.bss
> +		{
> +			__bss_start = .;
> +			*(.bss)
> +			 . = ALIGN(4);
> +			_end = .;
> +		}
>   	}
>
>   	/DISCARD/ : { *(.dynstr*) }

Nak -- what we want to overlay is .bss on one hand, and .rel.dyn *plus* 
.dynsym on the other hand; OVERLAY { ... } does not allow this.

Also, this change modifies the mapping, so if mi makes an obvious bug 
disappear, it may be only because the resulting u-boot corrupts 
relocation now in a less obvious way.

Amicalement,
Sanjeev Premi Dec. 1, 2010, 5:19 p.m. UTC | #2
> -----Original Message-----
> From: u-boot-bounces@lists.denx.de 
> [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert ARIBAUD
> Sent: Wednesday, December 01, 2010 10:43 PM
> To: u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across 
> toolchain versions
> 
> Le 01/12/2010 16:17, Sanjeev Premi a écrit :
> > This patch fixes the linker problems noticed while
> > building the omap3_evm with Codesourcery toolchains
> > 2009q1, 2009q3 and 2010q1.
> >
> > The compilation was tested as success for both
> > omap3_evm and omap3_beagle with toolchain versions
> > 2009q1 and 2010q1.
> >
> >   [1] http://marc.info/?l=u-boot&m=129104332808386&w=2
> >
> > Signed-off-by: Sanjeev Premi<premi@ti.com>
> > ---
> > The patch touches all ARMv7 architectures, will need
> > to be reviewed thoroughly.
> >
> > I am getting hang of relocation feature, but definitely
> > hands-on. Impact would have to be reviewd as well.
> > This is the reason for sending the patch early - before
> > i start testing on the evm.
> >
> >   arch/arm/cpu/armv7/u-boot.lds |   26 +++++++++++++++-----------
> >   1 files changed, 15 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv7/u-boot.lds 
> b/arch/arm/cpu/armv7/u-boot.lds
> > index 5725c30..faf6ad8 100644
> > --- a/arch/arm/cpu/armv7/u-boot.lds
> > +++ b/arch/arm/cpu/armv7/u-boot.lds
> > @@ -55,22 +55,26 @@ SECTIONS
> >
> >   	. = ALIGN(4);
> >
> > -	.rel.dyn : {
> > -		__rel_dyn_start = .;
> > -		*(.rel*)
> > -		__rel_dyn_end = .;
> > -	}
> > -
> >   	.dynsym : {
> >   		__dynsym_start = .;
> >   		*(.dynsym)
> >   	}
> >
> > -	.bss __rel_dyn_start (OVERLAY) : {
> > -		__bss_start = .;
> > -		*(.bss)
> > -		 . = ALIGN(4);
> > -		_end = .;
> > +	OVERLAY : NOCROSSREFS
> > +	{
> > +		.rel.dyn {
> > +			__rel_dyn_start = .;
> > +			*(.rel*)
> > +			__rel_dyn_end = .;
> > +		}
> > +
> > +		.bss
> > +		{
> > +			__bss_start = .;
> > +			*(.bss)
> > +			 . = ALIGN(4);
> > +			_end = .;
> > +		}
> >   	}
> >
> >   	/DISCARD/ : { *(.dynstr*) }
> 
> Nak -- what we want to overlay is .bss on one hand, and 
> .rel.dyn *plus* 
> .dynsym on the other hand; OVERLAY { ... } does not allow this.

[sp] From the earlier discussion, I inferred the overlay was supposed
     to be .rel.dyn and .bss.

     Let me get the ".rel.dyn + .dynsym" overlay with ".bss".

     If it works across compiler versions would that be okay?

~sanjeev

> 
> Also, this change modifies the mapping, so if mi makes an obvious bug 
> disappear, it may be only because the resulting u-boot corrupts 
> relocation now in a less obvious way.
> 
> Amicalement,
> -- 
> Albert.
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
Albert ARIBAUD Dec. 1, 2010, 5:32 p.m. UTC | #3
Le 01/12/2010 18:19, Premi, Sanjeev a écrit :

>> Nak -- what we want to overlay is .bss on one hand, and
>> .rel.dyn *plus*
>> .dynsym on the other hand; OVERLAY { ... } does not allow this.
>
> [sp] From the earlier discussion, I inferred the overlay was supposed
>       to be .rel.dyn and .bss.

That's because I avoid saying " .rel.dyn plus .dynsym" and just go for 
short ".rel.dyn" instead. Sorry for that.

>       Let me get the ".rel.dyn + .dynsym" overlay with ".bss".

>       If it works across compiler versions would that be okay?

Getting ".rel.dyn + .dynsym" overlay with ".bss" is exactly what the 
current linker file does, by emitting .rel.dyn, then .dynsym, then 
overlaying .bss back at the start of .rel.dyn. Look up a readelf -a of 
./u-boot and see where each section starts and ends.

If you find another way to do this overlay yet end up producing a 
different binary, I'll be interested in the result, but I honestly don't 
think you will find any.

> ~sanjeev

Amicalement,
Sanjeev Premi Dec. 1, 2010, 6:19 p.m. UTC | #4
> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud@free.fr] 
> Sent: Wednesday, December 01, 2010 11:02 PM
> To: Premi, Sanjeev
> Cc: u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across 
> toolchain versions
> 
> Le 01/12/2010 18:19, Premi, Sanjeev a écrit :
> 
> >> Nak -- what we want to overlay is .bss on one hand, and
> >> .rel.dyn *plus*
> >> .dynsym on the other hand; OVERLAY { ... } does not allow this.
> >
> > [sp] From the earlier discussion, I inferred the overlay 
> was supposed
> >       to be .rel.dyn and .bss.
> 
> That's because I avoid saying " .rel.dyn plus .dynsym" and 
> just go for 
> short ".rel.dyn" instead. Sorry for that.
>
> >       Let me get the ".rel.dyn + .dynsym" overlay with ".bss".
> 
> >       If it works across compiler versions would that be okay?
> 
> Getting ".rel.dyn + .dynsym" overlay with ".bss" is exactly what the 
> current linker file does, by emitting .rel.dyn, then .dynsym, then 
> overlaying .bss back at the start of .rel.dyn. Look up a 
> readelf -a of 
> ./u-boot and see where each section starts and ends.

[sp] I did the same - but focused only on .rel.dyn and .bss

> 
> If you find another way to do this overlay yet end up producing a 
> different binary, I'll be interested in the result, but I 
> honestly don't 
> think you will find any.

[sp] I am trying on this. Getting back to linker scripts after many
     years. But assuming that there is no way; I am still looking
     for an explanation on:

     1) why the current metod produces different errors across
        different toolchain versions.

     2) How does presence of one variable alone breaks the build?
        At least, compiler doesn't complain. Even moving it out
        of .bss by explicit initialization doesn't help.
        BTW, I verified that it is not used anywhere till end of
        board_init_f() - didn't go further. Should I?


    Can yo help me on these?

~sanjeev
> 
> > ~sanjeev
> 
> Amicalement,
> -- 
> Albert.
>
Sanjeev Premi Dec. 1, 2010, 6:23 p.m. UTC | #5
> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud@free.fr] 
> Sent: Wednesday, December 01, 2010 11:02 PM
> To: Premi, Sanjeev
> Cc: u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across 
> toolchain versions
> 
> Le 01/12/2010 18:19, Premi, Sanjeev a écrit :
> 
> >> Nak -- what we want to overlay is .bss on one hand, and
> >> .rel.dyn *plus*
> >> .dynsym on the other hand; OVERLAY { ... } does not allow this.
> >
> > [sp] From the earlier discussion, I inferred the overlay 
> was supposed
> >       to be .rel.dyn and .bss.
> 
> That's because I avoid saying " .rel.dyn plus .dynsym" and 
> just go for 
> short ".rel.dyn" instead. Sorry for that.
> 
> >       Let me get the ".rel.dyn + .dynsym" overlay with ".bss".
> 
> >       If it works across compiler versions would that be okay?
> 
> Getting ".rel.dyn + .dynsym" overlay with ".bss" is exactly what the 
> current linker file does, by emitting .rel.dyn, then .dynsym, then 
> overlaying .bss back at the start of .rel.dyn. Look up a 
> readelf -a of 
> ./u-boot and see where each section starts and ends.
> 
> If you find another way to do this overlay yet end up producing a 
> different binary, I'll be interested in the result, but I 
> honestly don't 
> think you will find any.

[sp] Had a quick question - hence separate mail.

     Do we really need to preserve section ".dynsym" in the final
     binary. OR are we okay with single section that contains
     contents from both?

~sanjeev

> 
> > ~sanjeev
> 
> Amicalement,
> -- 
> Albert.
>
Albert ARIBAUD Dec. 1, 2010, 6:28 p.m. UTC | #6
Le 01/12/2010 19:23, Premi, Sanjeev a écrit :

> [sp] Had a quick question - hence separate mail.
>
>       Do we really need to preserve section ".dynsym" in the final
>       binary. OR are we okay with single section that contains
>       contents from both?

A single section should be OK.

Amicalement,
Albert ARIBAUD Dec. 1, 2010, 6:36 p.m. UTC | #7
Le 01/12/2010 19:19, Premi, Sanjeev a écrit :

>> If you find another way to do this overlay yet end up producing a
>> different binary, I'll be interested in the result, but I
>> honestly don't
>> think you will find any.
>
> [sp] I am trying on this. Getting back to linker scripts after many
>       years. But assuming that there is no way; I am still looking
>       for an explanation on:
>
>       1) why the current metod produces different errors across
>          different toolchain versions.

Good question. I know this linker file is exactly the same as for 
arm926ejs, and I don't get the error with edminiv2 which is arm926ejs 
based; so it's not only different toolchains, it is also different 
boards or even different build environments.

>       2) How does presence of one variable alone breaks the build?

Good question again.

>          At least, compiler doesn't complain. Even moving it out
>          of .bss by explicit initialization doesn't help.
>          BTW, I verified that it is not used anywhere till end of
>          board_init_f() - didn't go further. Should I?

For the moment we should focus on what happens when we use the current 
.lds -- it causes a linker error, right? Once we get this sorted out, 
we'll see if the build actually runs.

>      Can yo help me on these?

As for the linker error, I am sorry I got a bit lost: I know which 
toolchains you tried (2010q1 and 2009q1) but not exactly which codebase 
you build (is it v2010.12-rc2 ? master ? specific patches?) or which 
board you build. Please refresh me on this, so that I can reproduce the 
issue locally (I've got 2010q1 already installed).

> ~sanjeev

Amicalement,
Sanjeev Premi Dec. 1, 2010, 6:54 p.m. UTC | #8
> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud@free.fr] 
> Sent: Thursday, December 02, 2010 12:07 AM
> To: Premi, Sanjeev
> Cc: u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across 
> toolchain versions

[snip]...[snip]

> 
> For the moment we should focus on what happens when we use 
> the current 
> .lds -- it causes a linker error, right? Once we get this sorted out, 
> we'll see if the build actually runs.
> 
> >      Can yo help me on these?
> 
> As for the linker error, I am sorry I got a bit lost: I know which 
> toolchains you tried (2010q1 and 2009q1) but not exactly 
> which codebase 
> you build (is it v2010.12-rc2 ? master ? specific patches?) or which 
> board you build. Please refresh me on this, so that I can 
> reproduce the 
> issue locally (I've got 2010q1 already installed).

I tried with 2009q1-203, 2009q3-68 and 2010q1-202.

But results with 2009q3 and 2010q1 were same - till I tried all three;
so, currently using 2009q1 and 2010q1.

Yes. I was on latest master when I sent patch. Had included the git log
as reference for same reason when I got it running.

> 
> > ~sanjeev
> 
> Amicalement,
> -- 
> Albert.
>
Wolfgang Denk Dec. 1, 2010, 8:08 p.m. UTC | #9
Dear "Premi, Sanjeev",

In message <B85A65D85D7EB246BE421B3FB0FBB5930247AFDACE@dbde02.ent.ti.com> you wrote:
>
>      1) why the current metod produces different errors across
>         different toolchain versions.

Different tool chains may prvide different quality of optimizations
(and bugs), resulting in differnt variable locations. With one memory
map you notice immediately if a value gets overwritten, with another
you may not notice it.

>      2) How does presence of one variable alone breaks the build?

It changes the memory map, and thus the location which may get
overwritten.

>         At least, compiler doesn't complain. Even moving it out

How should it? From the tool chain's point of view everything is fine.
It's the programmer who did things that lead to trouble; we only get
what we deserve.

>         of .bss by explicit initialization doesn't help.

How exactly did you initialize the variable? Like "int foo = 0;"?
Note that this will still go to BSS. 


Best regards,

Wolfgang Denk
Albert ARIBAUD Dec. 1, 2010, 9:39 p.m. UTC | #10
This one is a conundrum.

Using 2010q1, building omap3_evm causes a linker warning 
"arm-none-linux-gnueabi-ld: u-boot: section .bss vma 0x8003e8f0 overlaps 
previous sections" while building omap3_beagle does not cause any linker 
warning.

Both boards use the same armv7 u-boot.lds and have a .bss which is way 
bigger than the .rel.dyn plus .dynsym sections that it does overlay. 
IOW, they have a similar layout for .rel.dyn, .dynsym and .bss, but one 
gets the warning and one does not.

The one difference a readelf shows is that for beagle, there is only one 
segment:

00  .text .rodata .hash .data .got.plt .u_boot_cmd .rel.dyn .dynsym

While for evm there is

00  .text .rodata .hash .data .got.plt .u_boot_cmd .rel.dyn .bss
01  .dynsym

Note that .bss has appeared in segment 00 for evm, whereas it was absent 
for beagle, and that .dynsym was rejected to a second segment -- why? I 
don't know.

Note: I've tried with putting input sections .rel.dyn and .dynsym into a 
single output section .rel.dyn: this makes the second segment disappear, 
but for evm the warning remains and .bss remains in the segment.

Amicalement,
Albert ARIBAUD Dec. 2, 2010, 6:59 a.m. UTC | #11
Le 01/12/2010 22:39, Albert ARIBAUD a écrit :
> This one is a conundrum.
>
> Using 2010q1, building omap3_evm causes a linker warning
> "arm-none-linux-gnueabi-ld: u-boot: section .bss vma 0x8003e8f0 overlaps
> previous sections" while building omap3_beagle does not cause any linker
> warning.
>
> Both boards use the same armv7 u-boot.lds and have a .bss which is way
> bigger than the .rel.dyn plus .dynsym sections that it does overlay.
> IOW, they have a similar layout for .rel.dyn, .dynsym and .bss, but one
> gets the warning and one does not.
>
> The one difference a readelf shows is that for beagle, there is only one
> segment:
>
> 00  .text .rodata .hash .data .got.plt .u_boot_cmd .rel.dyn .dynsym
>
> While for evm there is
>
> 00  .text .rodata .hash .data .got.plt .u_boot_cmd .rel.dyn .bss
> 01  .dynsym
>
> Note that .bss has appeared in segment 00 for evm, whereas it was absent
> for beagle, and that .dynsym was rejected to a second segment -- why? I
> don't know.
>
> Note: I've tried with putting input sections .rel.dyn and .dynsym into a
> single output section .rel.dyn: this makes the second segment disappear,
> but for evm the warning remains and .bss remains in the segment.

I have a tiny clue.

Starting with the fact that the linker issue is only for one board, 
omap3_evm, I looked up the board-specific code. First thing that I 
noticed was

	static u8 omap3_evm_version;

I changed this to

	static u8 omap3_evm_version = 1;

so that the static was moved out of BSS and the linker warning 
disappeared (reminder: v2010.12-rc2, omap3_evm, arm-2010q1).

Now this is not the first static BSS variable we use in U-Boot, and the 
others did not cause linker warnings (not *all* the others, at least), 
so the real cause is yet unknown to me. But that's at least a lead we 
can follow.

If this BSS variable is used before relocation (I haven't checked this), 
then anyway it'll have to move; in that case I'll keep an eye on this 
linker warning and try to sort it out if I get time.

Amicalement,
Wolfgang Denk Dec. 2, 2010, 7:34 a.m. UTC | #12
Dear Albert ARIBAUD,

In message <4CF743E6.60706@free.fr> you wrote:
>
> Starting with the fact that the linker issue is only for one board, 
> omap3_evm, I looked up the board-specific code. First thing that I 
> noticed was
> 
> 	static u8 omap3_evm_version;
> 
> I changed this to
> 
> 	static u8 omap3_evm_version = 1;
> 
> so that the static was moved out of BSS and the linker warning 
> disappeared (reminder: v2010.12-rc2, omap3_evm, arm-2010q1).
> 
> Now this is not the first static BSS variable we use in U-Boot, and the 
> others did not cause linker warnings (not *all* the others, at least), 
> so the real cause is yet unknown to me. But that's at least a lead we 
> can follow.

Write access is only in omap3_evm_get_revision() which in turn only
gets called in misc_init_r(), i. e. after relocation.

Read access is only in get_omap3_evm_rev() [which is not called
outside this file and thus should be made static!] which gets called
only in omap3_evm_need_extvbus() which gets acalled only in
musb_platform_init(), i. e. during USB init.

This should be safe.


You could try out what happens if you make get_omap3_evm_rev() static...

Best regards,

Wolfgang Denk
Albert ARIBAUD Dec. 2, 2010, 7:51 a.m. UTC | #13
Hi Wolfgang,

Le 02/12/2010 08:34, Wolfgang Denk a écrit :
> Dear Albert ARIBAUD,
>
> In message<4CF743E6.60706@free.fr>  you wrote:
>>
>> Starting with the fact that the linker issue is only for one board,
>> omap3_evm, I looked up the board-specific code. First thing that I
>> noticed was
>>
>> 	static u8 omap3_evm_version;
>>
>> I changed this to
>>
>> 	static u8 omap3_evm_version = 1;
>>
>> so that the static was moved out of BSS and the linker warning
>> disappeared (reminder: v2010.12-rc2, omap3_evm, arm-2010q1).
>>
>> Now this is not the first static BSS variable we use in U-Boot, and the
>> others did not cause linker warnings (not *all* the others, at least),
>> so the real cause is yet unknown to me. But that's at least a lead we
>> can follow.
>
> Write access is only in omap3_evm_get_revision() which in turn only
> gets called in misc_init_r(), i. e. after relocation.
>
> Read access is only in get_omap3_evm_rev() [which is not called
> outside this file and thus should be made static!] which gets called
> only in omap3_evm_need_extvbus() which gets acalled only in
> musb_platform_init(), i. e. during USB init.
>
> This should be safe.

Alright.

> You could try out what happens if you make get_omap3_evm_rev() static...

No change: the warning remains, and so do the two segments.

Maybe I should subscribe the binutils list and ask there.

Meanwhile, this variable could be initialized to some safe value such as 
OMAP3EVM_BOARD_UNKNOWN.

BTW... Why on Earth is it an uint8? Probably a space saving measure, but 
one I think is not really fruitful, because of probable paddings and 
alignments; making it an int would allow using smsc_id directly as 
values for the OMAP3EVM_BOARD_GEN_1 and OMAP3EVM_BOARD_GEN_2...

... plus it removes the linker warning!

I thus suggest turning omap3_evm_version from uint8 to int and get on 
with debugging the board.

> Best regards,
>
> Wolfgang Denk

Amicalement,
Wolfgang Denk Dec. 2, 2010, 8:13 a.m. UTC | #14
Dear Albert ARIBAUD,

In message <4CF74FED.2030102@free.fr> you wrote:
> 
> BTW... Why on Earth is it an uint8? Probably a space saving measure, but 
> one I think is not really fruitful, because of probable paddings and 
> alignments; making it an int would allow using smsc_id directly as 
> values for the OMAP3EVM_BOARD_GEN_1 and OMAP3EVM_BOARD_GEN_2...
> 
> ... plus it removes the linker warning!

Oops. Why would this make a difference?

> I thus suggest turning omap3_evm_version from uint8 to int and get on 
> with debugging the board.

No, I don't think this is a good idea as it just papers over an
existing problem.

Does it help when you change the "*(.bss)" in
"arch/arm/cpu/armv7/u-boot.lds"  into "*(.*bss)"
(so it also includes any .sbss objects) ?

Best regards,

Wolfgang Denk
Sanjeev Premi Dec. 2, 2010, 8:14 a.m. UTC | #15
> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud@free.fr] 
> Sent: Thursday, December 02, 2010 1:21 PM
> To: Wolfgang Denk
> Cc: u-boot@lists.denx.de; Premi, Sanjeev
> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across 
> toolchain versions
> 
> Hi Wolfgang,
> 
> Le 02/12/2010 08:34, Wolfgang Denk a écrit :
> > Dear Albert ARIBAUD,
> >
> > In message<4CF743E6.60706@free.fr>  you wrote:
> >>
> >> Starting with the fact that the linker issue is only for one board,
> >> omap3_evm, I looked up the board-specific code. First thing that I
> >> noticed was
> >>
> >> 	static u8 omap3_evm_version;
> >>
> >> I changed this to
> >>
> >> 	static u8 omap3_evm_version = 1;
> >>
> >> so that the static was moved out of BSS and the linker warning
> >> disappeared (reminder: v2010.12-rc2, omap3_evm, arm-2010q1).
> >>
> >> Now this is not the first static BSS variable we use in 
> U-Boot, and the
> >> others did not cause linker warnings (not *all* the 
> others, at least),
> >> so the real cause is yet unknown to me. But that's at 
> least a lead we
> >> can follow.
> >
> > Write access is only in omap3_evm_get_revision() which in turn only
> > gets called in misc_init_r(), i. e. after relocation.
> >
> > Read access is only in get_omap3_evm_rev() [which is not called
> > outside this file and thus should be made static!] which gets called
> > only in omap3_evm_need_extvbus() which gets acalled only in
> > musb_platform_init(), i. e. during USB init.
> >
> > This should be safe.
> 
> Alright.
> 
> > You could try out what happens if you make 
> get_omap3_evm_rev() static...
> 
> No change: the warning remains, and so do the two segments.
> 
> Maybe I should subscribe the binutils list and ask there.
> 
> Meanwhile, this variable could be initialized to some safe 
> value such as 
> OMAP3EVM_BOARD_UNKNOWN.

One of the things I did to move it away from .bss

> 
> BTW... Why on Earth is it an uint8? Probably a space saving 
> measure, but 
> one I think is not really fruitful, because of probable paddings and 
> alignments; making it an int would allow using smsc_id directly as 
> values for the OMAP3EVM_BOARD_GEN_1 and OMAP3EVM_BOARD_GEN_2...
> 
> ... plus it removes the linker warning!
> 
> I thus suggest turning omap3_evm_version from uint8 to int and get on 
> with debugging the board.

Albert,

I have already done this - may not be exactly same. while trying to debug
linker script problem; did not want to deviate from the current problem
so it is in my working code.

~sanjeev

> 
> > Best regards,
> >
> > Wolfgang Denk
> 
> Amicalement,
> -- 
> Albert.
>
Sanjeev Premi Dec. 2, 2010, 8:18 a.m. UTC | #16
> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud@free.fr] 
> Sent: Thursday, December 02, 2010 12:30 PM
> To: u-boot@lists.denx.de
> Cc: Premi, Sanjeev; Wolfgang Denk
> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across 
> toolchain versions
> 
> Le 01/12/2010 22:39, Albert ARIBAUD a écrit :
> > This one is a conundrum.
> >
> > Using 2010q1, building omap3_evm causes a linker warning
> > "arm-none-linux-gnueabi-ld: u-boot: section .bss vma 
> 0x8003e8f0 overlaps
> > previous sections" while building omap3_beagle does not 
> cause any linker
> > warning.
> >
> > Both boards use the same armv7 u-boot.lds and have a .bss 
> which is way
> > bigger than the .rel.dyn plus .dynsym sections that it does overlay.
> > IOW, they have a similar layout for .rel.dyn, .dynsym and 
> .bss, but one
> > gets the warning and one does not.
> >
> > The one difference a readelf shows is that for beagle, 
> there is only one
> > segment:
> >
> > 00  .text .rodata .hash .data .got.plt .u_boot_cmd .rel.dyn .dynsym
> >
> > While for evm there is
> >
> > 00  .text .rodata .hash .data .got.plt .u_boot_cmd .rel.dyn .bss
> > 01  .dynsym
> >
> > Note that .bss has appeared in segment 00 for evm, whereas 
> it was absent
> > for beagle, and that .dynsym was rejected to a second 
> segment -- why? I
> > don't know.
> >
> > Note: I've tried with putting input sections .rel.dyn and 
> .dynsym into a
> > single output section .rel.dyn: this makes the second 
> segment disappear,
> > but for evm the warning remains and .bss remains in the segment.
> 
> I have a tiny clue.
> 
> Starting with the fact that the linker issue is only for one board, 
> omap3_evm, I looked up the board-specific code. First thing that I 
> noticed was
> 
> 	static u8 omap3_evm_version;
> 
> I changed this to
> 
> 	static u8 omap3_evm_version = 1;
> 
> so that the static was moved out of BSS and the linker warning 
> disappeared (reminder: v2010.12-rc2, omap3_evm, arm-2010q1).
> 
> Now this is not the first static BSS variable we use in 
> U-Boot, and the 
> others did not cause linker warnings (not *all* the others, 
> at least), 
> so the real cause is yet unknown to me. But that's at least a lead we 
> can follow.
> 
> If this BSS variable is used before relocation (I haven't 
> checked this), 
> then anyway it'll have to move; in that case I'll keep an eye on this 
> linker warning and try to sort it out if I get time.

I did explain yesterday that variable is not used in relocation. If you
notice the code snippet I sent yesterday, I had removed every assignment
to the variable. And 2009q1 was still not happy!
> 
> Amicalement,
> -- 
> Albert.
>
Albert ARIBAUD Dec. 2, 2010, 8:26 a.m. UTC | #17
Le 02/12/2010 09:13, Wolfgang Denk a écrit :
> Dear Albert ARIBAUD,
>
> In message<4CF74FED.2030102@free.fr>  you wrote:
>>
>> BTW... Why on Earth is it an uint8? Probably a space saving measure, but
>> one I think is not really fruitful, because of probable paddings and
>> alignments; making it an int would allow using smsc_id directly as
>> values for the OMAP3EVM_BOARD_GEN_1 and OMAP3EVM_BOARD_GEN_2...
>>
>> ... plus it removes the linker warning!
>
> Oops. Why would this make a difference?

Alignement issues, possibly, although we do align the location pointer 
before the overlay point -- or some bug in the linker.

>> I thus suggest turning omap3_evm_version from uint8 to int and get on
>> with debugging the board.
>
> No, I don't think this is a good idea as it just papers over an
> existing problem.

Oh--I did not mean to forget the linker warning thing. Just separate the 
issues, because I think this particular linker warning is unrelated with 
getting the board working.

So Sanjeev would change the u8 to an int and proceed with board debug, 
and I would investigate this issue -- actually, try and find a minimal 
example that I could go to the binutils list with rather than a 
full-blown u-boot tree.

> Does it help when you change the "*(.bss)" in
> "arch/arm/cpu/armv7/u-boot.lds"  into "*(.*bss)"
> (so it also includes any .sbss objects) ?

No change.

> Best regards,
>
> Wolfgang Denk

Amicalement,
Sanjeev Premi Dec. 2, 2010, 8:30 a.m. UTC | #18
> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud@free.fr] 
> Sent: Thursday, December 02, 2010 1:57 PM
> To: Wolfgang Denk
> Cc: u-boot@lists.denx.de; Premi, Sanjeev
> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across 
> toolchain versions

[snip]

> 
> So Sanjeev would change the u8 to an int and proceed with 
> board debug, 

I will be away from desk for an hour. Will have the patch
posted immediately after...

~sanjeev
Albert ARIBAUD Dec. 2, 2010, 8:42 a.m. UTC | #19
Hi Sanjeev,

Le 02/12/2010 09:30, Premi, Sanjeev a écrit :
>> -----Original Message-----
>> From: Albert ARIBAUD [mailto:albert.aribaud@free.fr]
>> Sent: Thursday, December 02, 2010 1:57 PM
>> To: Wolfgang Denk
>> Cc: u-boot@lists.denx.de; Premi, Sanjeev
>> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across
>> toolchain versions
>
> [snip]
>
>>
>> So Sanjeev would change the u8 to an int and proceed with
>> board debug,
>
> I will be away from desk for an hour. Will have the patch
> posted immediately after...

Please let us know the current status of the board, i.e. whether it 
works or not, and if not, what the console output is.

Amicalement,
Wolfgang Denk Dec. 2, 2010, 8:56 a.m. UTC | #20
Dear Albert ARIBAUD,

In message <4CF7583C.7000001@free.fr> you wrote:
>
> > Does it help when you change the "*(.bss)" in
> > "arch/arm/cpu/armv7/u-boot.lds"  into "*(.*bss)"
> > (so it also includes any .sbss objects) ?
> 
> No change.

Hm... Maybe it is indeed not a good idea to mix short objects with
those requiring larger alignments - we might lose space at best, or
case misaligned variables?  Eventually it's better to explicitly write

	*(.bss*)
	*(.sbss*)

as we have it in many PowerPC linker scripts (but I have to admit that
I don't know if this is really needed - I don't see any .sbss anywhere
in this build (I do see them in some objects for PowerPC). For
example:

PowerPC:

-> readelf -S common/dlmalloc.o | grep bss
  [ 3] .bss              NOBITS          00000000 000034 000000 00  WA  0   0  1
  [36] .bss.n_mmaps_max  NOBITS          00000000 002728 000004 00  WA  0   0  4
  [37] .sbss.mem_malloc_ NOBITS          00000000 002728 000004 00  WA  0   0  4
  [38] .bss.max_total_me NOBITS          00000000 002728 000004 00  WA  0   0  4
  [40] .bss.max_sbrked_m NOBITS          00000000 00272c 000004 00  WA  0   0  4
  [43] .bss.top_pad      NOBITS          00000000 002734 000004 00  WA  0   0  4
  [44] .sbss.mem_malloc_ NOBITS          00000000 002734 000004 00  WA  0   0  4
  [45] .sbss.mem_malloc_ NOBITS          00000000 002734 000004 00  WA  0   0  4
  [46] .bss.current_mall NOBITS          00000000 002734 000028 00  WA  0   0  4

ARM:

-> readelf -S common/dlmalloc.o | grep bss
  [ 5] .bss              NOBITS          00000000 0015e0 000044 00  WA  0   0  4



Best regards,

Wolfgang Denk
Sanjeev Premi Dec. 2, 2010, 11:25 a.m. UTC | #21
> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud@free.fr] 
> Sent: Thursday, December 02, 2010 2:12 PM
> To: Premi, Sanjeev
> Cc: Wolfgang Denk; u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across 
> toolchain versions
> 
> Hi Sanjeev,
> 
> Le 02/12/2010 09:30, Premi, Sanjeev a écrit :
> >> -----Original Message-----
> >> From: Albert ARIBAUD [mailto:albert.aribaud@free.fr]
> >> Sent: Thursday, December 02, 2010 1:57 PM
> >> To: Wolfgang Denk
> >> Cc: u-boot@lists.denx.de; Premi, Sanjeev
> >> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across
> >> toolchain versions
> >
> > [snip]
> >
> >>
> >> So Sanjeev would change the u8 to an int and proceed with
> >> board debug,
> >
> > I will be away from desk for an hour. Will have the patch
> > posted immediately after...
> 
> Please let us know the current status of the board, i.e. whether it 
> works or not, and if not, what the console output is.

Just posted the patch. The u-boot comes up on the EVM - only after
sort related change in the Makefile. Haven't debuged it yet.

Also noticed a difference between the changes I made yesterday; and
your mail - maintaining 'u8' vs. 'int' for the omap3_evm_revision.

~sanjeev
> 
> Amicalement,
> -- 
> Albert.
>
Wolfgang Denk Dec. 2, 2010, 11:39 a.m. UTC | #22
Dear "Premi, Sanjeev",

In message <B85A65D85D7EB246BE421B3FB0FBB5930247B9A23B@dbde02.ent.ti.com> you wrote:
>
> Just posted the patch. The u-boot comes up on the EVM - only after
> sort related change in the Makefile. Haven't debuged it yet.

Could you ***please*** be a bit more precise?

What EXACTLY was that "sort related change"?

Do you still need to remove the $(sort) call for the LIBS?  That
should NOT be done.


Best regards,

Wolfgang Denk
Sanjeev Premi Dec. 2, 2010, 12:45 p.m. UTC | #23
> -----Original Message-----
> From: Wolfgang Denk [mailto:wd@denx.de] 
> Sent: Thursday, December 02, 2010 5:09 PM
> To: Premi, Sanjeev
> Cc: Albert ARIBAUD; u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] ARMv7: Fix linker errors across 
> toolchain versions
> 
> Dear "Premi, Sanjeev",
> 
> In message 
> <B85A65D85D7EB246BE421B3FB0FBB5930247B9A23B@dbde02.ent.ti.com>
>  you wrote:
> >
> > Just posted the patch. The u-boot comes up on the EVM - only after
> > sort related change in the Makefile. Haven't debuged it yet.
> 
> Could you ***please*** be a bit more precise?
> 
> What EXACTLY was that "sort related change"?
> 
> Do you still need to remove the $(sort) call for the LIBS?  That
> should NOT be done.

Wolfgang,

I know it shouldn't be done - but just trying to take problem one by
one. Unless you want me to report the problems onl after I have a fix
for them - and not seek any help.

~sanjeev

> 
> 
> Best regards,
> 
> Wolfgang Denk
> 
> -- 
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> You can't evaluate a man by logic alone.
> 	-- McCoy, "I, Mudd", stardate 4513.3
>
Wolfgang Denk Dec. 2, 2010, 2 p.m. UTC | #24
Dear "Premi, Sanjeev",

In message <B85A65D85D7EB246BE421B3FB0FBB5930247B9A2D5@dbde02.ent.ti.com> you wrote:
>
> > Do you still need to remove the $(sort) call for the LIBS?  That
> > should NOT be done.
...
> I know it shouldn't be done - but just trying to take problem one by
> one. Unless you want me to report the problems onl after I have a fix
> for them - and not seek any help.

Well, the thing is that I am not convinced that these are really two
separate issues.  Both seem to have somethign to do with what the
linker is doing.  

Best regards,

Wolfgang Denk
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/u-boot.lds b/arch/arm/cpu/armv7/u-boot.lds
index 5725c30..faf6ad8 100644
--- a/arch/arm/cpu/armv7/u-boot.lds
+++ b/arch/arm/cpu/armv7/u-boot.lds
@@ -55,22 +55,26 @@  SECTIONS
 
 	. = ALIGN(4);
 
-	.rel.dyn : {
-		__rel_dyn_start = .;
-		*(.rel*)
-		__rel_dyn_end = .;
-	}
-
 	.dynsym : {
 		__dynsym_start = .;
 		*(.dynsym)
 	}
 
-	.bss __rel_dyn_start (OVERLAY) : {
-		__bss_start = .;
-		*(.bss)
-		 . = ALIGN(4);
-		_end = .;
+	OVERLAY : NOCROSSREFS
+	{
+		.rel.dyn {
+			__rel_dyn_start = .;
+			*(.rel*)
+			__rel_dyn_end = .;
+		}
+
+		.bss
+		{
+			__bss_start = .;
+			*(.bss)
+			 . = ALIGN(4);
+			_end = .;
+		}
 	}
 
 	/DISCARD/ : { *(.dynstr*) }