All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Benoît Thébaudeau" <benoit.thebaudeau@advansee.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ARM: Fix __bss_start and __bss_end in linker scripts
Date: Fri, 5 Apr 2013 01:05:53 +0200 (CEST)	[thread overview]
Message-ID: <436405570.1175811.1365116753714.JavaMail.root@advansee.com> (raw)
In-Reply-To: <1365113633-31281-1-git-send-email-albert.u.boot@aribaud.net>

Hi Albert,

On Friday, April 5, 2013 12:13:53 AM, Albert ARIBAUD wrote:
> Subject: [PATCH] ARM: Fix __bss_start and __bss_end in linker scripts
> 
> Commit 3ebd1cbc introduced compiler-generated __bss_start
> and __bss_end__ and commit c23561e7 rewrote all __bss_end__
> as __bss_end. Their merge caused silent and harmless but
> potentially bug-inducing clashes between compiler- and linker-
> enerated __bss_end symbols.
> 
> Make __bss_end and __bss_start compiler-only, and create
> __bss_base and __bss_limit for linker-only use.
> 
> Reported-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> ---
>  arch/arm/cpu/ixp/u-boot.lds        |   14 ++++++++++----
>  arch/arm/cpu/u-boot.lds            |   14 ++++++++++----
>  board/actux1/u-boot.lds            |   14 ++++++++++----
>  board/actux2/u-boot.lds            |   14 ++++++++++----
>  board/actux3/u-boot.lds            |   14 ++++++++++----
>  board/dvlhost/u-boot.lds           |   14 ++++++++++----
>  board/freescale/mx31ads/u-boot.lds |   14 ++++++++++----
>  7 files changed, 70 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm/cpu/ixp/u-boot.lds b/arch/arm/cpu/ixp/u-boot.lds
> index 8345b55..c999829 100644
> --- a/arch/arm/cpu/ixp/u-boot.lds
> +++ b/arch/arm/cpu/ixp/u-boot.lds
> @@ -67,17 +67,23 @@ SECTIONS
>  
>  	_end = .;
>  
> +/*
> + * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
> + * __bss_base and __bss_limit are for linker only (overlay ordering)
> + */
> +
>  	.bss_start __rel_dyn_start (OVERLAY) : {
>  		KEEP(*(.__bss_start));
> +		HIDDEN(__bss_base = .);
>  	}
>  
> -	.bss __bss_start (OVERLAY) : {
> +	.bss __bss_base (OVERLAY) : {
>  		*(.bss*)
>  		 . = ALIGN(4);
> -		 __bss_end = .;
> +		 HIDDEN(__bss_limit = .);
>  	}
> -	.bss_end __bss_end (OVERLAY) : {
> -		KEEP(*(__bss_end));
> +	.bss_end __bss_limit (OVERLAY) : {
> +		KEEP(*(.__bss_end));
>  	}
>  
>  	/DISCARD/ : { *(.dynstr*) }
> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> index 3a1083d..0543b06 100644
> --- a/arch/arm/cpu/u-boot.lds
> +++ b/arch/arm/cpu/u-boot.lds
> @@ -81,18 +81,24 @@ SECTIONS
>  		*(.mmutable)
>  	}
>  
> +/*
> + * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
> + * __bss_base and __bss_limit are for linker only (overlay ordering)
> + */
> +
>  	.bss_start __rel_dyn_start (OVERLAY) : {
>  		KEEP(*(.__bss_start));
> +		HIDDEN(__bss_base = .);
>  	}
>  
> -	.bss __bss_start (OVERLAY) : {
> +	.bss __bss_base (OVERLAY) : {
>  		*(.bss*)
>  		 . = ALIGN(4);
> -		 __bss_end = .;
> +		 HIDDEN(__bss_limit = .);
>  	}
>  
> -	.bss_end __bss_end (OVERLAY) : {
> -		KEEP(*(__bss_end));
> +	.bss_end __bss_limit (OVERLAY) : {
> +		KEEP(*(.__bss_end));
>  	}
>  
>  	/DISCARD/ : { *(.dynstr*) }
> diff --git a/board/actux1/u-boot.lds b/board/actux1/u-boot.lds
> index c76728a..7e5c4d8 100644
> --- a/board/actux1/u-boot.lds
> +++ b/board/actux1/u-boot.lds
> @@ -74,17 +74,23 @@ SECTIONS
>  
>  	_end = .;
>  
> +/*
> + * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
> + * __bss_base and __bss_limit are for linker only (overlay ordering)
> + */
> +
>  	.bss_start __rel_dyn_start (OVERLAY) : {
>  		KEEP(*(.__bss_start));
> +		HIDDEN(__bss_base = .);
>  	}
>  
> -	.bss __bss_start (OVERLAY) : {
> +	.bss __bss_base (OVERLAY) : {
>  		*(.bss*)
>  		 . = ALIGN(4);
> -		 __bss_end = .;
> +		 HIDDEN(__bss_limit = .);
>  	}
> -	.bss_end __bss_end (OVERLAY) : {
> -		KEEP(*(__bss_end));
> +	.bss_end __bss_limit (OVERLAY) : {
> +		KEEP(*(.__bss_end));
>  	}
>  
>  	/DISCARD/ : { *(.dynstr*) }
> diff --git a/board/actux2/u-boot.lds b/board/actux2/u-boot.lds
> index 984f70e..ce1b7c9 100644
> --- a/board/actux2/u-boot.lds
> +++ b/board/actux2/u-boot.lds
> @@ -74,17 +74,23 @@ SECTIONS
>  
>  	_end = .;
>  
> +/*
> + * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
> + * __bss_base and __bss_limit are for linker only (overlay ordering)
> + */
> +
>  	.bss_start __rel_dyn_start (OVERLAY) : {
>  		KEEP(*(.__bss_start));
> +		HIDDEN(__bss_base = .);
>  	}
>  
> -	.bss __bss_start (OVERLAY) : {
> +	.bss __bss_base (OVERLAY) : {
>  		*(.bss*)
>  		 . = ALIGN(4);
> -		 __bss_end = .;
> +		 HIDDEN(__bss_limit = .);
>  	}
> -	.bss_end __bss_end (OVERLAY) : {
> -		KEEP(*(__bss_end));
> +	.bss_end __bss_limit (OVERLAY) : {
> +		KEEP(*(.__bss_end));
>  	}
>  
>  	/DISCARD/ : { *(.dynstr*) }
> diff --git a/board/actux3/u-boot.lds b/board/actux3/u-boot.lds
> index fc48cf0..3e091dd 100644
> --- a/board/actux3/u-boot.lds
> +++ b/board/actux3/u-boot.lds
> @@ -74,17 +74,23 @@ SECTIONS
>  
>  	_end = .;
>  
> +/*
> + * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
> + * __bss_base and __bss_limit are for linker only (overlay ordering)
> + */
> +
>  	.bss_start __rel_dyn_start (OVERLAY) : {
>  		KEEP(*(.__bss_start));
> +		HIDDEN(__bss_base = .);
>  	}
>  
> -	.bss __bss_start (OVERLAY) : {
> +	.bss __bss_base (OVERLAY) : {
>  		*(.bss*)
>  		 . = ALIGN(4);
> -		 __bss_end = .;
> +		HIDDEN(__bss_limit = .);
>  	}
> -	.bss_end __bss_end (OVERLAY) : {
> -		KEEP(*(__bss_end));
> +	.bss_end __bss_limit (OVERLAY) : {
> +		KEEP(*(.__bss_end));
>  	}
>  
>  	/DISCARD/ : { *(.dynstr*) }
> diff --git a/board/dvlhost/u-boot.lds b/board/dvlhost/u-boot.lds
> index b13d3e1..fe3c21b 100644
> --- a/board/dvlhost/u-boot.lds
> +++ b/board/dvlhost/u-boot.lds
> @@ -74,17 +74,23 @@ SECTIONS
>  
>  	_end = .;
>  
> +/*
> + * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
> + * __bss_base and __bss_limit are for linker only (overlay ordering)
> + */
> +
>  	.bss_start __rel_dyn_start (OVERLAY) : {
>  		KEEP(*(.__bss_start));
> +		HIDDEN(__bss_base = .);
>  	}
>  
> -	.bss __bss_start (OVERLAY) : {
> +	.bss __bss_base (OVERLAY) : {
>  		*(.bss*)
>  		 . = ALIGN(4);
> -		 __bss_end = .;
> +		 HIDDEN(__bss_limit = .);
>  	}
> -	.bss_end __bss_end (OVERLAY) : {
> -		KEEP(*(__bss_end));
> +	.bss_end __bss_limit (OVERLAY) : {
> +		KEEP(*(.__bss_end));
>  	}
>  
>  	/DISCARD/ : { *(.dynstr*) }
> diff --git a/board/freescale/mx31ads/u-boot.lds
> b/board/freescale/mx31ads/u-boot.lds
> index 264c4e8..e6885a5 100644
> --- a/board/freescale/mx31ads/u-boot.lds
> +++ b/board/freescale/mx31ads/u-boot.lds
> @@ -80,17 +80,23 @@ SECTIONS
>  
>  	_end = .;
>  
> +/*
> + * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
> + * __bss_base and __bss_limit are for linker only (overlay ordering)
> + */
> +
>  	.bss_start __rel_dyn_start (OVERLAY) : {
>  		KEEP(*(.__bss_start));
> +		HIDDEN(__bss_base = .);
>  	}
>  
> -	.bss __bss_start (OVERLAY) : {
> +	.bss __bss_base (OVERLAY) : {
>  		*(.bss*)
>  		 . = ALIGN(4);
> -		 __bss_end = .;
> +		 HIDDEN(__bss_limit = .);
>  	}
> -	.bss_end __bss_end (OVERLAY) : {
> -		KEEP(*(__bss_end));
> +	.bss_end __bss_limit (OVERLAY) : {
> +		KEEP(*(.__bss_end));
>  	}
>  
>  	/DISCARD/ : { *(.bss*) }
> --
> 1.7.10.4
> 
> 

Looks good, but what about the __bss_end in
ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big");
?

Shouldn't it be replaced with __bss_limit, or even better, with __image_copy_end
(why should BSS be taken into account for SPL image size?)?

Best regards,
Beno?t

  reply	other threads:[~2013-04-04 23:05 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-04 22:13 [U-Boot] [PATCH] ARM: Fix __bss_start and __bss_end in linker scripts Albert ARIBAUD
2013-04-04 23:05 ` Benoît Thébaudeau [this message]
2013-04-04 23:13   ` Benoît Thébaudeau
2013-04-04 23:54     ` Albert ARIBAUD
2013-04-05  3:44       ` Benoît Thébaudeau
2013-04-05  6:00         ` Albert ARIBAUD
2013-04-05 13:53           ` Tom Rini
2013-04-05 15:59             ` Stephen Warren
2013-04-11 17:52               ` Tom Warren
2013-04-11 17:59                 ` Albert ARIBAUD
2013-04-11 18:13                   ` Tom Warren
2013-04-05 13:56           ` Benoît Thébaudeau
2013-04-05 16:00             ` Tom Rini
2013-04-05 17:32               ` Benoît Thébaudeau
2013-04-05 17:55                 ` Tom Rini
2013-04-05 19:17                   ` Albert ARIBAUD
2013-04-05 19:28                     ` Albert ARIBAUD
2013-04-05 19:44                     ` Tom Rini
2013-04-05 20:04                       ` Albert ARIBAUD
2013-04-05 20:23                         ` Tom Rini
2013-04-08 19:03                           ` Albert ARIBAUD
2013-04-08 19:56                             ` Tom Rini
2013-04-08 20:05                               ` Albert ARIBAUD
2013-04-04 23:12 ` Albert ARIBAUD
2013-04-11 15:30 ` Albert ARIBAUD
2013-04-11 15:43 ` [U-Boot] [PATCH V2] " Albert ARIBAUD
2013-04-12 16:34   ` Albert ARIBAUD
2013-04-13 21:16   ` Albert ARIBAUD

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=436405570.1175811.1365116753714.JavaMail.root@advansee.com \
    --to=benoit.thebaudeau@advansee.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.