[U-Boot] [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*) }
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
On Friday, April 5, 2013 1:05:53 AM, Benoît Thébaudeau wrote:
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?)?
I meant __bss_base, not __image_copy_end.
Best regards, Benoît
Hi Benoît,
On Fri, 5 Apr 2013 01:13:51 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
On Friday, April 5, 2013 1:05:53 AM, Benoît Thébaudeau wrote:
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?)?
I meant __bss_base, not __image_copy_end.
Part of SPL can use BSS, once board_init_f() has handed things over to board_init_r(), and this test is meant to ensure that .text+.data+.bss fits in the RAM available to SPL. This ASSERT() compares the upper limit of this RAM to the upper limit of the SPL needs, i.e. __bss_end.
IOW, this ASSERT is about how much memory SPL will use when it runs, while OTOH, image_copy_end is about how much code from SPL need to becopied from e.g. NAND to RAM to make it run.
As for bss_limit, as indicated, it is only for linker convenience, so that overlaid sections are mapped properly; it should not be used to signify something about the image itself)
Hope this clarifies. If it doesn't, don't hesitate to ask more questions.
Best regards, Benoît
Amicalement,
Hi Albert,
On Friday, April 5, 2013 1:54:38 AM, Albert ARIBAUD wrote:
Hi Benoît,
On Fri, 5 Apr 2013 01:13:51 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
On Friday, April 5, 2013 1:05:53 AM, Benoît Thébaudeau wrote:
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?)?
I meant __bss_base, not __image_copy_end.
Part of SPL can use BSS, once board_init_f() has handed things over to board_init_r(),
I agree with that.
and this test is meant to ensure that .text+.data+.bss fits in the RAM available to SPL. This ASSERT() compares the upper limit of this RAM to the upper limit of the SPL needs, i.e. __bss_end.
IOW, this ASSERT is about how much memory SPL will use when it runs,
This is where there might be an issue with the definition / usage of CONFIG_SPL_MAX_SIZE.
I had understood that this is the purpose of this assert, but this usage of CONFIG_SPL_MAX_SIZE, while frequent on arm, conflicts with its definition in README, as well as with some arm and other usages, e.g. in include/configs/am335x_evm.h or include/configs/p1_p2_rdb_pc.h.
The README's definition is also how I understood this config when discussing it with Scott about my 16/30, which led to the #error test that was added at some point to this patch.
According to README, CONFIG_SPL_MAX_SIZE is for text + data + rodata (i.e. for the size of the binary image file to be programmed on the target device), while CONFIG_SPL_BSS_MAX_SIZE is dedicated to the BSS. Hence, in order to comply with this definition, that assert should either use __bss_base/__bss_start, or compare against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, depending on whether it's supposed to test the binary image size or the memory footprint. There could even be asserts testing both. This would anyway be a new patch separate from this one.
If you decided not to change that, CONFIG_SPL_MAX_SIZE would have different meanings depending on the board, and CONFIG_SPL_PAD_TO could be smaller than it in some cases, so the #error test in my 16/30 could be removed, but no such case exists so far, making this 16/30 change not strictly necessary for now.
while OTOH, image_copy_end is about how much code from SPL need to becopied from e.g. NAND to RAM to make it run.
Right.
As for bss_limit, as indicated, it is only for linker convenience, so that overlaid sections are mapped properly; it should not be used to signify something about the image itself)
OK, I'm fine with that. They have the same value, so it's only a matter of taste. I spotted that just because you said __bss_limit is for linker stuff, and it is an assert for the linker, but __bss_end is just fine too, all the more the resulting assert is then identical to the one in arch/arm/cpu/u-boot-spl.lds.
Best regards, Benoît
Hi Benoît,
CC:ing Stephen Warren who wrote commit 2b7818d4 which git blame tells me added the ASSERT() to arch/arm/cpu/u-boot.lds, and Tom Rini to help decide what to do.
On Fri, 5 Apr 2013 05:44:55 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Hi Albert,
On Friday, April 5, 2013 1:54:38 AM, Albert ARIBAUD wrote:
Hi Benoît,
On Fri, 5 Apr 2013 01:13:51 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
On Friday, April 5, 2013 1:05:53 AM, Benoît Thébaudeau wrote:
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?)?
I meant __bss_base, not __image_copy_end.
Part of SPL can use BSS, once board_init_f() has handed things over to board_init_r(),
I agree with that.
and this test is meant to ensure that .text+.data+.bss fits in the RAM available to SPL. This ASSERT() compares the upper limit of this RAM to the upper limit of the SPL needs, i.e. __bss_end.
IOW, this ASSERT is about how much memory SPL will use when it runs,
This is where there might be an issue with the definition / usage of CONFIG_SPL_MAX_SIZE.
I had understood that this is the purpose of this assert, but this usage of CONFIG_SPL_MAX_SIZE, while frequent on arm, conflicts with its definition in README, as well as with some arm and other usages, e.g. in include/configs/am335x_evm.h or include/configs/p1_p2_rdb_pc.h.
The README's definition is also how I understood this config when discussing it with Scott about my 16/30, which led to the #error test that was added at some point to this patch.
Sorry, I'd missed that. Adding Stephen for any comment regarding the ASSERT() and CONFIG_SPL_MAX_SIZE semantics.
According to README, CONFIG_SPL_MAX_SIZE is for text + data + rodata (i.e. for the size of the binary image file to be programmed on the target device), while CONFIG_SPL_BSS_MAX_SIZE is dedicated to the BSS. Hence, in order to comply with this definition, that assert should either use __bss_base/__bss_start, or compare against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, depending on whether it's supposed to test the binary image size or the memory footprint. There could even be asserts testing both. This would anyway be a new patch separate from this one.
It would indeed be a different patch.
IIUC, this future patch would increase the limit for SPL run-time size, as the constant against which the ASS tests __bss_end for would necessarily be greater than it is now. Correct? If so, this future patch should not break any target, as it would loosen the constraint, not tighten it.
Tom, would such a patch, if posted today, be acceptable for 2013.04?
If you decided not to change that, CONFIG_SPL_MAX_SIZE would have different meanings depending on the board, and CONFIG_SPL_PAD_TO could be smaller than it in some cases, so the #error test in my 16/30 could be removed, but no such case exists so far, making this 16/30 change not strictly necessary for now.
IF a patch is posted, it should be done above your series, and take care of fixing 16/30 as necessary.
Now, pending Tom's approval, the question is: who does this future patch?
Benoît, You obviously have most of the necessary inputs, but I might be more available to prepare a patch now. Your pick -- and in any case, you earn a second Reported-By: badge. :)
Best regards, Benoît
Amicalement,
On Fri, Apr 05, 2013 at 08:00:43AM +0200, Albert ARIBAUD wrote:
Hi Beno??t,
CC:ing Stephen Warren who wrote commit 2b7818d4 which git blame tells me added the ASSERT() to arch/arm/cpu/u-boot.lds, and Tom Rini to help decide what to do.
[snip]
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?)?
I meant __bss_base, not __image_copy_end.
Part of SPL can use BSS, once board_init_f() has handed things over to board_init_r(),
I agree with that.
and this test is meant to ensure that .text+.data+.bss fits in the RAM available to SPL. This ASSERT() compares the upper limit of this RAM to the upper limit of the SPL needs, i.e. __bss_end.
IOW, this ASSERT is about how much memory SPL will use when it runs,
This is where there might be an issue with the definition / usage of CONFIG_SPL_MAX_SIZE.
I had understood that this is the purpose of this assert, but this usage of CONFIG_SPL_MAX_SIZE, while frequent on arm, conflicts with its definition in README, as well as with some arm and other usages, e.g. in include/configs/am335x_evm.h or include/configs/p1_p2_rdb_pc.h.
The README's definition is also how I understood this config when discussing it with Scott about my 16/30, which led to the #error test that was added at some point to this patch.
Sorry, I'd missed that. Adding Stephen for any comment regarding the ASSERT() and CONFIG_SPL_MAX_SIZE semantics.
Yes. I'm a little confused about this one as well now.
According to README, CONFIG_SPL_MAX_SIZE is for text + data + rodata (i.e. for the size of the binary image file to be programmed on the target device), while CONFIG_SPL_BSS_MAX_SIZE is dedicated to the BSS. Hence, in order to comply with this definition, that assert should either use __bss_base/__bss_start, or compare against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, depending on whether it's supposed to test the binary image size or the memory footprint. There could even be asserts testing both. This would anyway be a new patch separate from this one.
It would indeed be a different patch.
IIUC, this future patch would increase the limit for SPL run-time size, as the constant against which the ASS tests __bss_end for would necessarily be greater than it is now. Correct? If so, this future patch should not break any target, as it would loosen the constraint, not tighten it.
Tom, would such a patch, if posted today, be acceptable for 2013.04?
So, lets talk things out. On (setting aside davinci) TI platforms, SPL's text/data/rodata is sitting in SRAM (starting at 0x4...) and we say BSS shall live in DDR (starting at 0x8...) and we configure DDR before clearing and using BSS. The "max" we set is mostly about trying to not possibly step on ourselves later on (SPL malloc, full U-Boot, etc).
If I read all of the tegra files correctly, the way they work is that SPL and U-Boot are both loaded into memory, and thus they're ensuring that the BSS won't overlap with the full U-Boot that's right behind.
So yes, a patch to correct the behavior is fine, and we can build test that easily enough as well.
On 04/05/2013 07:53 AM, Tom Rini wrote:
On Fri, Apr 05, 2013 at 08:00:43AM +0200, Albert ARIBAUD wrote:
Hi Beno??t,
CC:ing Stephen Warren who wrote commit 2b7818d4 which git blame tells me added the ASSERT() to arch/arm/cpu/u-boot.lds, and Tom Rini to help decide what to do.
[snip]
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?)?
I meant __bss_base, not __image_copy_end.
Part of SPL can use BSS, once board_init_f() has handed things over to board_init_r(),
I agree with that.
and this test is meant to ensure that .text+.data+.bss fits in the RAM available to SPL. This ASSERT() compares the upper limit of this RAM to the upper limit of the SPL needs, i.e. __bss_end.
IOW, this ASSERT is about how much memory SPL will use when it runs,
This is where there might be an issue with the definition / usage of CONFIG_SPL_MAX_SIZE.
I had understood that this is the purpose of this assert, but this usage of CONFIG_SPL_MAX_SIZE, while frequent on arm, conflicts with its definition in README, as well as with some arm and other usages, e.g. in include/configs/am335x_evm.h or include/configs/p1_p2_rdb_pc.h.
The README's definition is also how I understood this config when discussing it with Scott about my 16/30, which led to the #error test that was added at some point to this patch.
Sorry, I'd missed that. Adding Stephen for any comment regarding the ASSERT() and CONFIG_SPL_MAX_SIZE semantics.
Yes. I'm a little confused about this one as well now.
...
If I read all of the tegra files correctly, the way they work is that SPL and U-Boot are both loaded into memory, and thus they're ensuring that the BSS won't overlap with the full U-Boot that's right behind.
Yes exactly.
I'll explain the whole situation just so there's something to refer to in the archives.
Tegra has two different types of CPU. As far as boot process goes, the AVP (Audio/Video Processor) is what is released from reset at system boot time, and what runs the built-in Tegra boot ROM and the U-Boot SPL. The AVP is an ARM7TDMI. The SPL initializes the main CPU complex (A9/A15), and causes it to execute the main U-Boot.
The boot process is:
* Entire U-Boot binary (SPL+pad+U-Boot) sits in boot flash. * Boot ROM code running on AVP: ** Initializes DRAM controller. ** Copies entire SPL+pad+U-Boot to DRAM byte-for-byte. ** Jumps to U-Boot SPL. * U-Boot SPL running on AVP: ** Initializes clocks/... required to boot A9 CPUs. ** Set up the A9 reset vector to point at the main U-Boot code, and releases A9 CPUs from reset. * Main U-Boot running on A9: ** Runs in the typical fashion, including regular relocation.
Thus, the in-flash layout of SPL+U-Boot must match the in-DRAM layout, since the boot ROM just copies it byte-for-byte, and the SPL does nothing to move the appended main U-Boot out-of-the-way before executing.
The gap between SPL and concatenated U-Boot must be large enough to hold the BSS (since SPL will use BSS without moving U-boot), but is also larger so that we can hard-code the TEXT_BASE of the main U-Boot to some stable value, without having to move it about often if SPL changes size.
So the U-Boot binary layout in flash/DRAM is:
+---------------------+ | SPL | +---------------------+ | Pad, including | | space for BSS | +---------------------+ | Main U-Boot | +---------------------+
The assert in question ensures that the start of the "Main U-Boot" in the diagram above is at a large enough address that it doesn't overlap the SPL's BSS usage in RAM.
It's possible that I chose the wrong defines to validate when writing that assert. As long as the assert continues to ensure the above image layout in the generated u-boot*.bin, I have no problems with it being changed.
Albert,
-----Original Message----- From: Stephen Warren [mailto:swarren@wwwdotorg.org] Sent: Friday, April 05, 2013 8:59 AM To: Tom Rini Cc: Albert ARIBAUD; Scott Wood; u-boot@lists.denx.de; Stephen@theia.denx.de; Tom Warren; Simon Glass; Allen Martin Subject: Re: [U-Boot] [PATCH] ARM: Fix __bss_start and __bss_end in linker scripts
I just fetched & merged TOT u-boot-arm (getting ready for a PR from u-boot-tegra -> u-boot-arm), and did a test build of all Tegra boards.
I get the following linker (ld) error on all builds due to this commit (abbecf4c8, ARM: Fix __bss_start and __bss_end in linker scripts):
arm-linux-gnueabi-ld.bfd:u-boot.lds:44: syntax error
This is pointing to the 'HIDDEN(__bss_base = .);' lines added in this change (in the generated u-boot.lds in the root dir).
I'm seeing this with both gcc 4.4.1/ld 2.19.51 and gcc 4.6.2/ld 2.22 tools.
What compiler/binutils did you use to build/test this?
Tom
On 04/05/2013 07:53 AM, Tom Rini wrote:
On Fri, Apr 05, 2013 at 08:00:43AM +0200, Albert ARIBAUD wrote:
Hi Beno??t,
CC:ing Stephen Warren who wrote commit 2b7818d4 which git blame tells me added the ASSERT() to arch/arm/cpu/u-boot.lds, and Tom Rini to help decide what to do.
[snip]
> 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?)?
I meant __bss_base, not __image_copy_end.
Part of SPL can use BSS, once board_init_f() has handed things over to board_init_r(),
I agree with that.
and this test is meant to ensure that .text+.data+.bss fits in the RAM available to SPL. This ASSERT() compares the upper limit of this RAM to the upper limit of the SPL needs, i.e. __bss_end.
IOW, this ASSERT is about how much memory SPL will use when it runs,
This is where there might be an issue with the definition / usage of CONFIG_SPL_MAX_SIZE.
I had understood that this is the purpose of this assert, but this usage of CONFIG_SPL_MAX_SIZE, while frequent on arm, conflicts with its definition in README, as well as with some arm and other usages, e.g. in include/configs/am335x_evm.h or include/configs/p1_p2_rdb_pc.h.
The README's definition is also how I understood this config when discussing it with Scott about my 16/30, which led to the #error test that was added at some point to this patch.
Sorry, I'd missed that. Adding Stephen for any comment regarding the ASSERT() and CONFIG_SPL_MAX_SIZE semantics.
Yes. I'm a little confused about this one as well now.
...
If I read all of the tegra files correctly, the way they work is that SPL and U-Boot are both loaded into memory, and thus they're ensuring that the BSS won't overlap with the full U-Boot that's right behind.
Yes exactly.
I'll explain the whole situation just so there's something to refer to in the archives.
Tegra has two different types of CPU. As far as boot process goes, the AVP (Audio/Video Processor) is what is released from reset at system boot time, and what runs the built-in Tegra boot ROM and the U-Boot SPL. The AVP is an ARM7TDMI. The SPL initializes the main CPU complex (A9/A15), and causes it to execute the main U-Boot.
The boot process is:
- Entire U-Boot binary (SPL+pad+U-Boot) sits in boot flash.
- Boot ROM code running on AVP:
** Initializes DRAM controller. ** Copies entire SPL+pad+U-Boot to DRAM byte-for-byte. ** Jumps to U-Boot SPL.
- U-Boot SPL running on AVP:
** Initializes clocks/... required to boot A9 CPUs. ** Set up the A9 reset vector to point at the main U-Boot code, and releases A9 CPUs from reset.
- Main U-Boot running on A9:
** Runs in the typical fashion, including regular relocation.
Thus, the in-flash layout of SPL+U-Boot must match the in-DRAM layout, since the boot ROM just copies it byte-for-byte, and the SPL does nothing to move the appended main U-Boot out-of-the-way before executing.
The gap between SPL and concatenated U-Boot must be large enough to hold the BSS (since SPL will use BSS without moving U-boot), but is also larger so that we can hard-code the TEXT_BASE of the main U-Boot to some stable value, without having to move it about often if SPL changes size.
So the U-Boot binary layout in flash/DRAM is:
+---------------------+ | SPL | +---------------------+ | Pad, including | | space for BSS | +---------------------+ | Main U-Boot | +---------------------+
The assert in question ensures that the start of the "Main U-Boot" in the diagram above is at a large enough address that it doesn't overlap the SPL's BSS usage in RAM.
It's possible that I chose the wrong defines to validate when writing that assert. As long as the assert continues to ensure the above image layout in the generated u-boot*.bin, I have no problems with it being changed.
-- nvpublic
Hi Tom,
On Thu, 11 Apr 2013 10:52:08 -0700, Tom Warren TWarren@nvidia.com wrote:
Albert,
-----Original Message----- From: Stephen Warren [mailto:swarren@wwwdotorg.org] Sent: Friday, April 05, 2013 8:59 AM To: Tom Rini Cc: Albert ARIBAUD; Scott Wood; u-boot@lists.denx.de; Stephen@theia.denx.de; Tom Warren; Simon Glass; Allen Martin Subject: Re: [U-Boot] [PATCH] ARM: Fix __bss_start and __bss_end in linker scripts
I just fetched & merged TOT u-boot-arm (getting ready for a PR from u-boot-tegra -> u-boot-arm), and did a test build of all Tegra boards.
I get the following linker (ld) error on all builds due to this commit (abbecf4c8, ARM: Fix __bss_start and __bss_end in linker scripts):
arm-linux-gnueabi-ld.bfd:u-boot.lds:44: syntax error
This is pointing to the 'HIDDEN(__bss_base = .);' lines added in this change (in the generated u-boot.lds in the root dir).
I'm seeing this with both gcc 4.4.1/ld 2.19.51 and gcc 4.6.2/ld 2.22 tools.
What compiler/binutils did you use to build/test this?
This has been detected already, and one of the reasons why Tom and I decided to roll back ARM. Normally, you should have u-boot-arm/master = u-boot-arm/master = dd2445ec1b839a5ca61ff8438a5b7aebb21b7986.
Tom
Amicalement,
Albert,
On Thu, Apr 11, 2013 at 10:59 AM, Albert ARIBAUD albert.u.boot@aribaud.netwrote:
Hi Tom,
On Thu, 11 Apr 2013 10:52:08 -0700, Tom Warren TWarren@nvidia.com wrote:
Albert,
-----Original Message----- From: Stephen Warren [mailto:swarren@wwwdotorg.org] Sent: Friday, April 05, 2013 8:59 AM To: Tom Rini Cc: Albert ARIBAUD; Scott Wood; u-boot@lists.denx.de; Stephen@theia.denx.de; Tom Warren; Simon Glass; Allen Martin Subject: Re: [U-Boot] [PATCH] ARM: Fix __bss_start and __bss_end in
linker
scripts
I just fetched & merged TOT u-boot-arm (getting ready for a PR from
u-boot-tegra -> u-boot-arm), and did a test build of all Tegra boards.
I get the following linker (ld) error on all builds due to this commit
(abbecf4c8, ARM: Fix __bss_start and __bss_end in linker scripts):
arm-linux-gnueabi-ld.bfd:u-boot.lds:44: syntax error
This is pointing to the 'HIDDEN(__bss_base = .);' lines added in this
change (in the generated u-boot.lds in the root dir).
I'm seeing this with both gcc 4.4.1/ld 2.19.51 and gcc 4.6.2/ld 2.22
tools.
What compiler/binutils did you use to build/test this?
This has been detected already, and one of the reasons why Tom and I decided to roll back ARM. Normally, you should have u-boot-arm/master = u-boot-arm/master = dd2445ec1b839a5ca61ff8438a5b7aebb21b7986.
you should have u-boot-arm/master = u-boot-arm/master =
dd2445ec1b839a5ca61ff8438a5b7aebb21b7986.
I would hope that u-boot-arm/master would always == u-boot-arm/master ;)
That commit is not 5 patches back (from our other thread 'Rollback *again* -- 5 patches), but more like 45.
I'll wait to do a fetch/rebase/PR for u-boot-tegra until the Tegra builds are fixed.
Thanks,
Tom
Tom
Amicalement,
Albert.
Hi Albert,
On Friday, April 5, 2013 8:00:43 AM, Albert ARIBAUD wrote:
Hi Benoît,
CC:ing Stephen Warren who wrote commit 2b7818d4 which git blame tells me added the ASSERT() to arch/arm/cpu/u-boot.lds, and Tom Rini to help decide what to do.
On Fri, 5 Apr 2013 05:44:55 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Hi Albert,
On Friday, April 5, 2013 1:54:38 AM, Albert ARIBAUD wrote:
Hi Benoît,
On Fri, 5 Apr 2013 01:13:51 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
On Friday, April 5, 2013 1:05:53 AM, Benoît Thébaudeau wrote:
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?)?
I meant __bss_base, not __image_copy_end.
Part of SPL can use BSS, once board_init_f() has handed things over to board_init_r(),
I agree with that.
and this test is meant to ensure that .text+.data+.bss fits in the RAM available to SPL. This ASSERT() compares the upper limit of this RAM to the upper limit of the SPL needs, i.e. __bss_end.
IOW, this ASSERT is about how much memory SPL will use when it runs,
This is where there might be an issue with the definition / usage of CONFIG_SPL_MAX_SIZE.
I had understood that this is the purpose of this assert, but this usage of CONFIG_SPL_MAX_SIZE, while frequent on arm, conflicts with its definition in README, as well as with some arm and other usages, e.g. in include/configs/am335x_evm.h or include/configs/p1_p2_rdb_pc.h.
It's not even that frequent on ARM. I've looked at a few other ARM linker scripts (arm1136, omap), and they don't count bss in CONFIG_SPL_MAX_SIZE.
The README's definition is also how I understood this config when discussing it with Scott about my 16/30, which led to the #error test that was added at some point to this patch.
Sorry, I'd missed that. Adding Stephen for any comment regarding the ASSERT() and CONFIG_SPL_MAX_SIZE semantics.
According to README, CONFIG_SPL_MAX_SIZE is for text + data + rodata (i.e. for the size of the binary image file to be programmed on the target device), while CONFIG_SPL_BSS_MAX_SIZE is dedicated to the BSS. Hence, in order to comply with this definition, that assert should either use __bss_base/__bss_start, or compare against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, depending on whether it's supposed to test the binary image size or the memory footprint. There could even be asserts testing both. This would anyway be a new patch separate from this one.
It would indeed be a different patch.
IIUC, this future patch would increase the limit for SPL run-time size, as the constant against which the ASS tests __bss_end for would necessarily be greater than it is now. Correct? If so, this future patch should not break any target, as it would loosen the constraint, not tighten it.
Yes, it would either be the same or relaxed a bit, depending on the chosen option: - Define CONFIG_SPL_BSS_MAX_SIZE and test against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, the sum remaining the same as or being larger than currently, depending on the new values for CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE. - Define a new config meaning text + data + rodata + bss (e.g. CONFIG_SPL_MAX_RAM_SIZE or CONFIG_SPL_MAX_MEM_FOOTPRINT), and just replace CONFIG_SPL_MAX_SIZE with it for the users of arch/arm/cpu/u-boot*.lds, taking care that this was the only meaning those users were giving to CONFIG_SPL_MAX_SIZE.
The first option would probably be preferable, using the same value for CONFIG_SPL_MAX_SIZE, and a non-zero value for CONFIG_SPL_BSS_MAX_SIZE.
Tom, would such a patch, if posted today, be acceptable for 2013.04?
If you decided not to change that, CONFIG_SPL_MAX_SIZE would have different meanings depending on the board, and CONFIG_SPL_PAD_TO could be smaller than it in some cases, so the #error test in my 16/30 could be removed, but no such case exists so far, making this 16/30 change not strictly necessary for now.
IF a patch is posted, it should be done above your series, and take care of fixing 16/30 as necessary.
If this assert is fixed according to the README definition, nothing needs to be changed in 16/30.
Now, pending Tom's approval, the question is: who does this future patch?
Benoît, You obviously have most of the necessary inputs, but I might be more available to prepare a patch now. Your pick -- and in any case, you earn a second Reported-By: badge. :)
I let you handle this, and approved for my RB badge. :)
Best regards, Benoît
On Fri, Apr 05, 2013 at 03:56:46PM +0200, Beno??t Th??baudeau wrote:
Hi Albert,
On Friday, April 5, 2013 8:00:43 AM, Albert ARIBAUD wrote:
Hi Beno??t,
[snip]
IIUC, this future patch would increase the limit for SPL run-time size, as the constant against which the ASS tests __bss_end for would necessarily be greater than it is now. Correct? If so, this future patch should not break any target, as it would loosen the constraint, not tighten it.
Yes, it would either be the same or relaxed a bit, depending on the chosen option:
- Define CONFIG_SPL_BSS_MAX_SIZE and test against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, the sum remaining the same as or being larger than currently, depending on the new values for CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
- Define a new config meaning text + data + rodata + bss (e.g. CONFIG_SPL_MAX_RAM_SIZE or CONFIG_SPL_MAX_MEM_FOOTPRINT), and just replace CONFIG_SPL_MAX_SIZE with it for the users of arch/arm/cpu/u-boot*.lds, taking care that this was the only meaning those users were giving to CONFIG_SPL_MAX_SIZE.
The first option would probably be preferable, using the same value for CONFIG_SPL_MAX_SIZE, and a non-zero value for CONFIG_SPL_BSS_MAX_SIZE.
I think the problem is that Tegra really needs the second case as their constraint is "must fit below next part of payload". We can assume the users of that linker script today care about footprint and update their define I believe. da850evm and the rest of the davinci platforms would also be a case to convert to this, but the omap*/am3* platforms would not.
Hi Tom,
On Friday, April 5, 2013 6:00:30 PM, Tom Rini wrote:
On Fri, Apr 05, 2013 at 03:56:46PM +0200, Beno??t Th??baudeau wrote:
Hi Albert,
On Friday, April 5, 2013 8:00:43 AM, Albert ARIBAUD wrote:
Hi Beno??t,
[snip]
IIUC, this future patch would increase the limit for SPL run-time size, as the constant against which the ASS tests __bss_end for would necessarily be greater than it is now. Correct? If so, this future patch should not break any target, as it would loosen the constraint, not tighten it.
Yes, it would either be the same or relaxed a bit, depending on the chosen option:
- Define CONFIG_SPL_BSS_MAX_SIZE and test against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, the sum remaining the same as or being larger than currently, depending on the new values for CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
- Define a new config meaning text + data + rodata + bss (e.g. CONFIG_SPL_MAX_RAM_SIZE or CONFIG_SPL_MAX_MEM_FOOTPRINT), and just replace CONFIG_SPL_MAX_SIZE with it for the users of arch/arm/cpu/u-boot*.lds, taking care that this was the only meaning those users were giving to CONFIG_SPL_MAX_SIZE.
The first option would probably be preferable, using the same value for CONFIG_SPL_MAX_SIZE, and a non-zero value for CONFIG_SPL_BSS_MAX_SIZE.
I think the problem is that Tegra really needs the second case as their constraint is "must fit below next part of payload". We can assume the users of that linker script today care about footprint and update their define I believe. da850evm and the rest of the davinci platforms would also be a case to convert to this, but the omap*/am3* platforms would not.
Yes, then let's have an assert in arch/arm/cpu/u-boot*.lds with a different config name (as in option 2 above) just for Tegra, and another assert for CONFIG_SPL_MAX_SIZE against __bss_start.
And all users of CONFIG_SPL_MAX_SIZE should be checked to make sure that there is not another special case somewhere.
Best regards, Benoît
On Fri, Apr 05, 2013 at 07:32:54PM +0200, Beno??t Th??baudeau wrote:
Hi Tom,
On Friday, April 5, 2013 6:00:30 PM, Tom Rini wrote:
On Fri, Apr 05, 2013 at 03:56:46PM +0200, Beno??t Th??baudeau wrote:
Hi Albert,
On Friday, April 5, 2013 8:00:43 AM, Albert ARIBAUD wrote:
Hi Beno??t,
[snip]
IIUC, this future patch would increase the limit for SPL run-time size, as the constant against which the ASS tests __bss_end for would necessarily be greater than it is now. Correct? If so, this future patch should not break any target, as it would loosen the constraint, not tighten it.
Yes, it would either be the same or relaxed a bit, depending on the chosen option:
- Define CONFIG_SPL_BSS_MAX_SIZE and test against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, the sum remaining the same as or being larger than currently, depending on the new values for CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
- Define a new config meaning text + data + rodata + bss (e.g. CONFIG_SPL_MAX_RAM_SIZE or CONFIG_SPL_MAX_MEM_FOOTPRINT), and just replace CONFIG_SPL_MAX_SIZE with it for the users of arch/arm/cpu/u-boot*.lds, taking care that this was the only meaning those users were giving to CONFIG_SPL_MAX_SIZE.
The first option would probably be preferable, using the same value for CONFIG_SPL_MAX_SIZE, and a non-zero value for CONFIG_SPL_BSS_MAX_SIZE.
I think the problem is that Tegra really needs the second case as their constraint is "must fit below next part of payload". We can assume the users of that linker script today care about footprint and update their define I believe. da850evm and the rest of the davinci platforms would also be a case to convert to this, but the omap*/am3* platforms would not.
Yes, then let's have an assert in arch/arm/cpu/u-boot*.lds with a different config name (as in option 2 above) just for Tegra, and another assert for CONFIG_SPL_MAX_SIZE against __bss_start.
And all users of CONFIG_SPL_MAX_SIZE should be checked to make sure that there is not another special case somewhere.
I didn't audit the PowerPC targets, but on ARM we have, roughly: - Tegra (covered in Stephen's email, and in short, must include BSS in size check) which uses SPL_MAX_SIZE to include BSS - OMAP*/AM3* which does not constrain BSS to SPL_MAX_SIZE - DaVinci which must also constrain BSS to the initial RAM, but for different reasons. - iMX which also uses SPL_BSS_MAX to cover the BSS separate from the rest of the program.
Hi Tom,
On Fri, 5 Apr 2013 13:55:21 -0400, Tom Rini trini@ti.com wrote:
On Fri, Apr 05, 2013 at 07:32:54PM +0200, Beno??t Th??baudeau wrote:
Hi Tom,
On Friday, April 5, 2013 6:00:30 PM, Tom Rini wrote:
On Fri, Apr 05, 2013 at 03:56:46PM +0200, Beno??t Th??baudeau wrote:
Hi Albert,
On Friday, April 5, 2013 8:00:43 AM, Albert ARIBAUD wrote:
Hi Beno??t,
[snip]
IIUC, this future patch would increase the limit for SPL run-time size, as the constant against which the ASS tests __bss_end for would necessarily be greater than it is now. Correct? If so, this future patch should not break any target, as it would loosen the constraint, not tighten it.
Yes, it would either be the same or relaxed a bit, depending on the chosen option:
- Define CONFIG_SPL_BSS_MAX_SIZE and test against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, the sum remaining the same as or being larger than currently, depending on the new values for CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
- Define a new config meaning text + data + rodata + bss (e.g. CONFIG_SPL_MAX_RAM_SIZE or CONFIG_SPL_MAX_MEM_FOOTPRINT), and just replace CONFIG_SPL_MAX_SIZE with it for the users of arch/arm/cpu/u-boot*.lds, taking care that this was the only meaning those users were giving to CONFIG_SPL_MAX_SIZE.
The first option would probably be preferable, using the same value for CONFIG_SPL_MAX_SIZE, and a non-zero value for CONFIG_SPL_BSS_MAX_SIZE.
I think the problem is that Tegra really needs the second case as their constraint is "must fit below next part of payload". We can assume the users of that linker script today care about footprint and update their define I believe. da850evm and the rest of the davinci platforms would also be a case to convert to this, but the omap*/am3* platforms would not.
Yes, then let's have an assert in arch/arm/cpu/u-boot*.lds with a different config name (as in option 2 above) just for Tegra, and another assert for CONFIG_SPL_MAX_SIZE against __bss_start.
And all users of CONFIG_SPL_MAX_SIZE should be checked to make sure that there is not another special case somewhere.
I didn't audit the PowerPC targets, but on ARM we have, roughly:
- Tegra (covered in Stephen's email, and in short, must include BSS in size check) which uses SPL_MAX_SIZE to include BSS
- OMAP*/AM3* which does not constrain BSS to SPL_MAX_SIZE
- DaVinci which must also constrain BSS to the initial RAM, but for different reasons.
- iMX which also uses SPL_BSS_MAX to cover the BSS separate from the rest of the program.
How about this?
1. In the u-boot*.lds files, doing separate asserts for SPL and SPL BSS max size, with the SPL assert being further divided in two cases depending on BSS max size being defined or not:
#if defined(CONFIG_SPL_MAX_SIZE) #if defined(CONFIG_SPL_BSS_MAX_SIZE) ASSERT( __bss_end - __image_copy_start < (CONFIG_SPL_TEXT_BASE + \ CONFIG_SPL_MAX_SIZE), "SPL image code+BSS too big"); #else ASSERT( __bss_end - __image_copy_start < CONFIG_SPL_TEXT_BASE, \ CONFIG_SPL_MAX_SIZE), "SPL image code too big"); #endif
#if defined(CONFIG_SPL_BSS_MAX_SIZE) ASSERT( __bss_end - __bss_start < CONFIG_SPL_TEXT_BASE, \ CONFIG_SPL_BSS_MAX_SIZE), "SPL image BSS too big"); #endif
2. Defining CONFIG_SPL_BSS_MAX_SIZE only for Tegra, Davinci, IMX (where CONFIG_SPL_BSS_MAX_SIZE is actually the gap size)
3. *Not* defining CONFIG_SPL_MAX_SIZE or CONFIG_SPL_BSS_MAX_SIZE for OMAP*/AM3*
4. Adjusting README descriptions of CONFIG_SPL_[BSS_]MAX_SIZE and ensuring Makefile uses the right size for --pad-to, as well as the few other files which use CONFIG_SPL_MAX_SIZE.
Amicalement,
On Fri, 5 Apr 2013 21:17:40 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Tom,
On Fri, 5 Apr 2013 13:55:21 -0400, Tom Rini trini@ti.com wrote:
On Fri, Apr 05, 2013 at 07:32:54PM +0200, Beno??t Th??baudeau wrote:
Hi Tom,
On Friday, April 5, 2013 6:00:30 PM, Tom Rini wrote:
On Fri, Apr 05, 2013 at 03:56:46PM +0200, Beno??t Th??baudeau wrote:
Hi Albert,
On Friday, April 5, 2013 8:00:43 AM, Albert ARIBAUD wrote:
Hi Beno??t,
[snip]
IIUC, this future patch would increase the limit for SPL run-time size, as the constant against which the ASS tests __bss_end for would necessarily be greater than it is now. Correct? If so, this future patch should not break any target, as it would loosen the constraint, not tighten it.
Yes, it would either be the same or relaxed a bit, depending on the chosen option:
- Define CONFIG_SPL_BSS_MAX_SIZE and test against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, the sum remaining the same as or being larger than currently, depending on the new values for CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
- Define a new config meaning text + data + rodata + bss (e.g. CONFIG_SPL_MAX_RAM_SIZE or CONFIG_SPL_MAX_MEM_FOOTPRINT), and just replace CONFIG_SPL_MAX_SIZE with it for the users of arch/arm/cpu/u-boot*.lds, taking care that this was the only meaning those users were giving to CONFIG_SPL_MAX_SIZE.
The first option would probably be preferable, using the same value for CONFIG_SPL_MAX_SIZE, and a non-zero value for CONFIG_SPL_BSS_MAX_SIZE.
I think the problem is that Tegra really needs the second case as their constraint is "must fit below next part of payload". We can assume the users of that linker script today care about footprint and update their define I believe. da850evm and the rest of the davinci platforms would also be a case to convert to this, but the omap*/am3* platforms would not.
Yes, then let's have an assert in arch/arm/cpu/u-boot*.lds with a different config name (as in option 2 above) just for Tegra, and another assert for CONFIG_SPL_MAX_SIZE against __bss_start.
And all users of CONFIG_SPL_MAX_SIZE should be checked to make sure that there is not another special case somewhere.
I didn't audit the PowerPC targets, but on ARM we have, roughly:
- Tegra (covered in Stephen's email, and in short, must include BSS in size check) which uses SPL_MAX_SIZE to include BSS
- OMAP*/AM3* which does not constrain BSS to SPL_MAX_SIZE
- DaVinci which must also constrain BSS to the initial RAM, but for different reasons.
- iMX which also uses SPL_BSS_MAX to cover the BSS separate from the rest of the program.
How about this?
In the u-boot*.lds files, doing separate asserts for SPL and SPL BSS max size, with the SPL assert being further divided in two cases depending on BSS max size being defined or not:
#if defined(CONFIG_SPL_MAX_SIZE) #if defined(CONFIG_SPL_BSS_MAX_SIZE) ASSERT( __bss_end - __image_copy_start < (CONFIG_SPL_TEXT_BASE + \ CONFIG_SPL_MAX_SIZE), "SPL image code+BSS too big"); #else ASSERT( __bss_end - __image_copy_start < CONFIG_SPL_TEXT_BASE, \ CONFIG_SPL_MAX_SIZE), "SPL image code too big"); #endif
#endif
(sorry)
#if defined(CONFIG_SPL_BSS_MAX_SIZE) ASSERT( __bss_end - __bss_start < CONFIG_SPL_TEXT_BASE, \ CONFIG_SPL_BSS_MAX_SIZE), "SPL image BSS too big"); #endif
Defining CONFIG_SPL_BSS_MAX_SIZE only for Tegra, Davinci, IMX (where CONFIG_SPL_BSS_MAX_SIZE is actually the gap size)
*Not* defining CONFIG_SPL_MAX_SIZE or CONFIG_SPL_BSS_MAX_SIZE for OMAP*/AM3*
Adjusting README descriptions of CONFIG_SPL_[BSS_]MAX_SIZE and ensuring Makefile uses the right size for --pad-to, as well as the few other files which use CONFIG_SPL_MAX_SIZE.
Amicalement,
Amicalement,
On Fri, Apr 05, 2013 at 09:17:40PM +0200, Albert ARIBAUD wrote:
Hi Tom,
On Fri, 5 Apr 2013 13:55:21 -0400, Tom Rini trini@ti.com wrote:
On Fri, Apr 05, 2013 at 07:32:54PM +0200, Beno??t Th??baudeau wrote:
Hi Tom,
On Friday, April 5, 2013 6:00:30 PM, Tom Rini wrote:
On Fri, Apr 05, 2013 at 03:56:46PM +0200, Beno??t Th??baudeau wrote:
Hi Albert,
On Friday, April 5, 2013 8:00:43 AM, Albert ARIBAUD wrote:
Hi Beno??t,
[snip]
IIUC, this future patch would increase the limit for SPL run-time size, as the constant against which the ASS tests __bss_end for would necessarily be greater than it is now. Correct? If so, this future patch should not break any target, as it would loosen the constraint, not tighten it.
Yes, it would either be the same or relaxed a bit, depending on the chosen option:
- Define CONFIG_SPL_BSS_MAX_SIZE and test against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, the sum remaining the same as or being larger than currently, depending on the new values for CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
- Define a new config meaning text + data + rodata + bss (e.g. CONFIG_SPL_MAX_RAM_SIZE or CONFIG_SPL_MAX_MEM_FOOTPRINT), and just replace CONFIG_SPL_MAX_SIZE with it for the users of arch/arm/cpu/u-boot*.lds, taking care that this was the only meaning those users were giving to CONFIG_SPL_MAX_SIZE.
The first option would probably be preferable, using the same value for CONFIG_SPL_MAX_SIZE, and a non-zero value for CONFIG_SPL_BSS_MAX_SIZE.
I think the problem is that Tegra really needs the second case as their constraint is "must fit below next part of payload". We can assume the users of that linker script today care about footprint and update their define I believe. da850evm and the rest of the davinci platforms would also be a case to convert to this, but the omap*/am3* platforms would not.
Yes, then let's have an assert in arch/arm/cpu/u-boot*.lds with a different config name (as in option 2 above) just for Tegra, and another assert for CONFIG_SPL_MAX_SIZE against __bss_start.
And all users of CONFIG_SPL_MAX_SIZE should be checked to make sure that there is not another special case somewhere.
I didn't audit the PowerPC targets, but on ARM we have, roughly:
- Tegra (covered in Stephen's email, and in short, must include BSS in size check) which uses SPL_MAX_SIZE to include BSS
- OMAP*/AM3* which does not constrain BSS to SPL_MAX_SIZE
- DaVinci which must also constrain BSS to the initial RAM, but for different reasons.
- iMX which also uses SPL_BSS_MAX to cover the BSS separate from the rest of the program.
How about this?
In the u-boot*.lds files, doing separate asserts for SPL and SPL BSS max size, with the SPL assert being further divided in two cases depending on BSS max size being defined or not:
#if defined(CONFIG_SPL_MAX_SIZE) #if defined(CONFIG_SPL_BSS_MAX_SIZE) ASSERT( __bss_end - __image_copy_start < (CONFIG_SPL_TEXT_BASE + \ CONFIG_SPL_MAX_SIZE), "SPL image code+BSS too big"); #else ASSERT( __bss_end - __image_copy_start < CONFIG_SPL_TEXT_BASE, \ CONFIG_SPL_MAX_SIZE), "SPL image code too big"); #endif
#if defined(CONFIG_SPL_BSS_MAX_SIZE) ASSERT( __bss_end - __bss_start < CONFIG_SPL_TEXT_BASE, \ CONFIG_SPL_BSS_MAX_SIZE), "SPL image BSS too big"); #endif #endif
I think this is too complicated. Our cases are: 1. text/data/rodata/bss MUST fit within $size for $location 2. text/data/rodata MUST fit within $sizeA for $locationA and BSS must start at $locationB (which is at least $sizeB big)
The problem is that CONFIG_SPL_MAX_SIZE is defined to mean #2 but the generic ARM SPL linker script was using it for #1. We should correct the generic ARM SPL linker script to test #2, and add in a new option to cover #1 and convert tegra (as they're the user of the generic script) to this new option.
- Defining CONFIG_SPL_BSS_MAX_SIZE only for Tegra, Davinci, IMX (where CONFIG_SPL_BSS_MAX_SIZE is actually the gap size)
No, this is wrong. These do not care about the BSS size, they care about the text/data/rodata/bss size.
- *Not* defining CONFIG_SPL_MAX_SIZE or CONFIG_SPL_BSS_MAX_SIZE for OMAP*/AM3*
This is wrong. The main reason for SPL_BSS_MAX_SIZE here is that we have the BSS at a very different location from the rest of the binary (text/data/rodata at 0x4... BSS at 0x8...) and use MEMORY constructs to place things correctly. So we must(?) define a start and end, and that's what BSS_MAX_SIZE is for.
- Adjusting README descriptions of CONFIG_SPL_[BSS_]MAX_SIZE and ensuring Makefile uses the right size for --pad-to, as well as the few other files which use CONFIG_SPL_MAX_SIZE.
Yes, we need to audit README and users after all of this.
Hi Tom,
On Fri, 5 Apr 2013 15:44:08 -0400, Tom Rini trini@ti.com wrote:
On Fri, Apr 05, 2013 at 09:17:40PM +0200, Albert ARIBAUD wrote:
Hi Tom,
On Fri, 5 Apr 2013 13:55:21 -0400, Tom Rini trini@ti.com wrote:
On Fri, Apr 05, 2013 at 07:32:54PM +0200, Beno??t Th??baudeau wrote:
Hi Tom,
On Friday, April 5, 2013 6:00:30 PM, Tom Rini wrote:
On Fri, Apr 05, 2013 at 03:56:46PM +0200, Beno??t Th??baudeau wrote:
Hi Albert,
On Friday, April 5, 2013 8:00:43 AM, Albert ARIBAUD wrote: > Hi Beno??t,
[snip]
> IIUC, this future patch would increase the limit for SPL run-time size, > as the constant against which the ASS tests __bss_end for would > necessarily be greater than it is now. Correct? If so, this future > patch should not break any target, as it would loosen the constraint, > not tighten it.
Yes, it would either be the same or relaxed a bit, depending on the chosen option:
- Define CONFIG_SPL_BSS_MAX_SIZE and test against CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE, the sum remaining the same as or being larger than currently, depending on the new values for CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
- Define a new config meaning text + data + rodata + bss (e.g. CONFIG_SPL_MAX_RAM_SIZE or CONFIG_SPL_MAX_MEM_FOOTPRINT), and just replace CONFIG_SPL_MAX_SIZE with it for the users of arch/arm/cpu/u-boot*.lds, taking care that this was the only meaning those users were giving to CONFIG_SPL_MAX_SIZE.
The first option would probably be preferable, using the same value for CONFIG_SPL_MAX_SIZE, and a non-zero value for CONFIG_SPL_BSS_MAX_SIZE.
I think the problem is that Tegra really needs the second case as their constraint is "must fit below next part of payload". We can assume the users of that linker script today care about footprint and update their define I believe. da850evm and the rest of the davinci platforms would also be a case to convert to this, but the omap*/am3* platforms would not.
Yes, then let's have an assert in arch/arm/cpu/u-boot*.lds with a different config name (as in option 2 above) just for Tegra, and another assert for CONFIG_SPL_MAX_SIZE against __bss_start.
And all users of CONFIG_SPL_MAX_SIZE should be checked to make sure that there is not another special case somewhere.
I didn't audit the PowerPC targets, but on ARM we have, roughly:
- Tegra (covered in Stephen's email, and in short, must include BSS in size check) which uses SPL_MAX_SIZE to include BSS
- OMAP*/AM3* which does not constrain BSS to SPL_MAX_SIZE
- DaVinci which must also constrain BSS to the initial RAM, but for different reasons.
- iMX which also uses SPL_BSS_MAX to cover the BSS separate from the rest of the program.
How about this?
In the u-boot*.lds files, doing separate asserts for SPL and SPL BSS max size, with the SPL assert being further divided in two cases depending on BSS max size being defined or not:
#if defined(CONFIG_SPL_MAX_SIZE) #if defined(CONFIG_SPL_BSS_MAX_SIZE) ASSERT( __bss_end - __image_copy_start < (CONFIG_SPL_TEXT_BASE + \ CONFIG_SPL_MAX_SIZE), "SPL image code+BSS too big"); #else ASSERT( __bss_end - __image_copy_start < CONFIG_SPL_TEXT_BASE, \ CONFIG_SPL_MAX_SIZE), "SPL image code too big"); #endif
#if defined(CONFIG_SPL_BSS_MAX_SIZE) ASSERT( __bss_end - __bss_start < CONFIG_SPL_TEXT_BASE, \ CONFIG_SPL_BSS_MAX_SIZE), "SPL image BSS too big"); #endif #endif
I think this is too complicated. Our cases are:
- text/data/rodata/bss MUST fit within $size for $location
- text/data/rodata MUST fit within $sizeA for $locationA and BSS must start at $locationB (which is at least $sizeB big)
The problem is that CONFIG_SPL_MAX_SIZE is defined to mean #2 but the generic ARM SPL linker script was using it for #1. We should correct the generic ARM SPL linker script to test #2, and add in a new option to cover #1 and convert tegra (as they're the user of the generic script) to this new option.
- Defining CONFIG_SPL_BSS_MAX_SIZE only for Tegra, Davinci, IMX (where CONFIG_SPL_BSS_MAX_SIZE is actually the gap size)
No, this is wrong. These do not care about the BSS size, they care about the text/data/rodata/bss size.
Maybe I wasn't clear. I did not mean to "define only BSS max size for those [and not image max size]", but to "define, only for those, BSS max size [in addition to image max size being defined for most of the boards including these]" -- IOW, while most if not all boards want to check image size, only some boards seem to want to check BSS size in addition, and we can differentiate with CONFIG_SPL_BSS_MAX_SIZE.
- *Not* defining CONFIG_SPL_MAX_SIZE or CONFIG_SPL_BSS_MAX_SIZE for OMAP*/AM3*
This is wrong. The main reason for SPL_BSS_MAX_SIZE here is that we have the BSS at a very different location from the rest of the binary (text/data/rodata at 0x4... BSS at 0x8...) and use MEMORY constructs to place things correctly. So we must(?) define a start and end, and that's what BSS_MAX_SIZE is for.
Then I misunderstood you -- you seemed to be saying OMAP*/AM3* did not constrain BSS or SPL max size. If they do constrain image size, then they must define CONFIG_SPL_MAX_SIZE; if they do constrain BSS, then they must define CONFIG_SPL_BSS_MAX_SIZE.
The two general ideas of my proposal are:
1) to separate testing the image (text,data,rodata,lists) size on the one hand and the image BSS size on the other hand, and
2) to consider that if a target defines an image max size and a BSS max size, then the image max size does not include the BSS size ; and if it defined an image max size but no BSS max size, then the image max size includes the BSS.
The first idea allows boards with disjoint image and BSS to still check eahc part's size, a thing not feasible with the current code; the second idea allows fewer changes, but if one wants CONFIG_SPL_MAX_SIZE to have a strict meaning, we can drop idea #2 and still keep idea #1.
- Adjusting README descriptions of CONFIG_SPL_[BSS_]MAX_SIZE and ensuring Makefile uses the right size for --pad-to, as well as the few other files which use CONFIG_SPL_MAX_SIZE.
Yes, we need to audit README and users after all of this.
Amicalement,
On Fri, Apr 05, 2013 at 10:04:02PM +0200, Albert ARIBAUD wrote:
The two general ideas of my proposal are:
to separate testing the image (text,data,rodata,lists) size on the one hand and the image BSS size on the other hand, and
to consider that if a target defines an image max size and a BSS
max size, then the image max size does not include the BSS size ; and if it defined an image max size but no BSS max size, then the image max size includes the BSS.
The first idea allows boards with disjoint image and BSS to still check eahc part's size, a thing not feasible with the current code; the second idea allows fewer changes, but if one wants CONFIG_SPL_MAX_SIZE to have a strict meaning, we can drop idea #2 and still keep idea #1.
Make it so, thanks!
Hi Tom,
On Fri, 5 Apr 2013 16:23:30 -0400, Tom Rini trini@ti.com wrote:
On Fri, Apr 05, 2013 at 10:04:02PM +0200, Albert ARIBAUD wrote:
The two general ideas of my proposal are:
to separate testing the image (text,data,rodata,lists) size on the one hand and the image BSS size on the other hand, and
to consider that if a target defines an image max size and a BSS
max size, then the image max size does not include the BSS size ; and if it defined an image max size but no BSS max size, then the image max size includes the BSS.
The first idea allows boards with disjoint image and BSS to still check eahc part's size, a thing not feasible with the current code; the second idea allows fewer changes, but if one wants CONFIG_SPL_MAX_SIZE to have a strict meaning, we can drop idea #2 and still keep idea #1.
Make it so, thanks!
Just a quick heads up: in the little time I could spare this week-end, I analyzed the SPL image vs BSS max sizes issue, and the following appeared:
1. File arch/arm/cpu/u-boot.lds has an ASSERT() regarding SPL even though it is never used for building SPL (tested by replacing the ASSERT() condition by 0 and building all of ARM).
2. Several boards use arch/arm/cpu/u-boot-spl.lds, which has an ASSERT() wrongly comparing image+BSS size to image-only CONFIG_SPL_MAX_SIZE. Fixing the linker file will fix all tegra boards as well as the exynos origen and smdkv310.
Note: the tegra-common.h config file states a non-existent .lds file for SPL.
3. Boards cam_enc_4xx, da850evm, smdk5250 and snow all define maximum SPL size to include image and BSS. These must be split arbitrarily; I have chosen values that fit sizes from current build results.
4. Boards MPC8313ERDB_[NAND_]{33,66} hardcode their image+BSS limit. I have left this untouched.
5. Board a3m071 and its linker file define CONFIG_SYS_SPL_MAX_LEN instead of CONFIG_SPL_MAX_SIZE, but the semantics are correct. Left unchanged.
6. Generic p1_p2_rdb_pc board header defines CONFIG_SPL_MAX_SIZE but linker files do not seem to test it anyway. Left unchanged.
7. Tegra configs specify a non-existent lds file name. Build defaults to generic ARM lds file. Left unchanged.
Patch series to follow in a couple of hours.
Amicalement,
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 04/08/2013 03:03 PM, Albert ARIBAUD wrote:
Hi Tom,
On Fri, 5 Apr 2013 16:23:30 -0400, Tom Rini trini@ti.com wrote:
On Fri, Apr 05, 2013 at 10:04:02PM +0200, Albert ARIBAUD wrote:
The two general ideas of my proposal are:
- to separate testing the image (text,data,rodata,lists) size
on the one hand and the image BSS size on the other hand, and
- to consider that if a target defines an image max size and a
BSS max size, then the image max size does not include the BSS size ; and if it defined an image max size but no BSS max size, then the image max size includes the BSS.
The first idea allows boards with disjoint image and BSS to still check eahc part's size, a thing not feasible with the current code; the second idea allows fewer changes, but if one wants CONFIG_SPL_MAX_SIZE to have a strict meaning, we can drop idea #2 and still keep idea #1.
Make it so, thanks!
Just a quick heads up: in the little time I could spare this week-end, I analyzed the SPL image vs BSS max sizes issue, and the following appeared:
- File arch/arm/cpu/u-boot.lds has an ASSERT() regarding SPL even
though it is never used for building SPL (tested by replacing the ASSERT() condition by 0 and building all of ARM).
- Several boards use arch/arm/cpu/u-boot-spl.lds, which has an
ASSERT() wrongly comparing image+BSS size to image-only CONFIG_SPL_MAX_SIZE. Fixing the linker file will fix all tegra boards as well as the exynos origen and smdkv310.
Note: the tegra-common.h config file states a non-existent .lds file for SPL.
I was wondering about #1 and the tegra bit too, but thought there must be magic I'm missing.
- Boards cam_enc_4xx, da850evm, smdk5250 and snow all define
maximum SPL size to include image and BSS. These must be split arbitrarily; I have chosen values that fit sizes from current build results.
In all of the first two, it's limit on overall size (text/data/rodata/bss).
- -- Tom
Hi Tom,
On Mon, 8 Apr 2013 15:56:12 -0400, Tom Rini trini@ti.com wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 04/08/2013 03:03 PM, Albert ARIBAUD wrote:
Hi Tom,
On Fri, 5 Apr 2013 16:23:30 -0400, Tom Rini trini@ti.com wrote:
On Fri, Apr 05, 2013 at 10:04:02PM +0200, Albert ARIBAUD wrote:
The two general ideas of my proposal are:
- to separate testing the image (text,data,rodata,lists) size
on the one hand and the image BSS size on the other hand, and
- to consider that if a target defines an image max size and a
BSS max size, then the image max size does not include the BSS size ; and if it defined an image max size but no BSS max size, then the image max size includes the BSS.
The first idea allows boards with disjoint image and BSS to still check eahc part's size, a thing not feasible with the current code; the second idea allows fewer changes, but if one wants CONFIG_SPL_MAX_SIZE to have a strict meaning, we can drop idea #2 and still keep idea #1.
Make it so, thanks!
Just a quick heads up: in the little time I could spare this week-end, I analyzed the SPL image vs BSS max sizes issue, and the following appeared:
- File arch/arm/cpu/u-boot.lds has an ASSERT() regarding SPL even
though it is never used for building SPL (tested by replacing the ASSERT() condition by 0 and building all of ARM).
- Several boards use arch/arm/cpu/u-boot-spl.lds, which has an
ASSERT() wrongly comparing image+BSS size to image-only CONFIG_SPL_MAX_SIZE. Fixing the linker file will fix all tegra boards as well as the exynos origen and smdkv310.
Note: the tegra-common.h config file states a non-existent .lds file for SPL.
I was wondering about #1 and the tegra bit too, but thought there must be magic I'm missing.
The tegra ones got me triple-checking, but yes, the config-specified lds file is ignored and the build defaults to the generic ARM lds file.
- Boards cam_enc_4xx, da850evm, smdk5250 and snow all define
maximum SPL size to include image and BSS. These must be split arbitrarily; I have chosen values that fit sizes from current build results.
In all of the first two, it's limit on overall size (text/data/rodata/bss).
Not sure I understand you there. Anyway the patches are out, so best is you comment directly on the changes.
Amicalement,
Hi Albert,
On Fri, 5 Apr 2013 00:13:53 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
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 that "generated". If there is no request for changes, I'll fix this typo directly when applying the patch.
Amicalement,
On Fri, 5 Apr 2013 00:13:53 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
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*) }
Officially, HIDDEN() is known only from binutils 2.23 up, even though Ubuntu/Linaro ARM binutils 2.22.90.20120919 knows about it. HIDDEN() fails for ELDK 4.2 and 5.2.1 as well as Ubuntu/Linaro 2.23.52.20120713.
Amicalement,
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.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net Reported-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com --- Changes in v2: - remove HIDDEN(), not supported by binutils < 2.23
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 388a67f..553589c 100644 --- a/arch/arm/cpu/ixp/u-boot.lds +++ b/arch/arm/cpu/ixp/u-boot.lds @@ -69,17 +69,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)); + __bss_base = .; }
- .bss __bss_start (OVERLAY) : { + .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4); - __bss_end = .; + __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..d5e42d3a 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)); + __bss_base = .; }
- .bss __bss_start (OVERLAY) : { + .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4); - __bss_end = .; + __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 52fc9fc..ef4a25b 100644 --- a/board/actux1/u-boot.lds +++ b/board/actux1/u-boot.lds @@ -77,17 +77,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)); + __bss_base = .; }
- .bss __bss_start (OVERLAY) : { + .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4); - __bss_end = .; + __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 cafd3d8..00ad8b7 100644 --- a/board/actux2/u-boot.lds +++ b/board/actux2/u-boot.lds @@ -77,17 +77,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)); + __bss_base = .; }
- .bss __bss_start (OVERLAY) : { + .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4); - __bss_end = .; + __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 168fe17..44b990e 100644 --- a/board/actux3/u-boot.lds +++ b/board/actux3/u-boot.lds @@ -77,17 +77,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)); + __bss_base = .; }
- .bss __bss_start (OVERLAY) : { + .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4); - __bss_end = .; + __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 dd50bdb..6d4b187 100644 --- a/board/dvlhost/u-boot.lds +++ b/board/dvlhost/u-boot.lds @@ -77,17 +77,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)); + __bss_base = .; }
- .bss __bss_start (OVERLAY) : { + .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4); - __bss_end = .; + __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 f48fda1..4969960 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)); + __bss_base = .; }
- .bss __bss_start (OVERLAY) : { + .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4); - __bss_end = .; + __bss_limit = .; } - .bss_end __bss_end (OVERLAY) : { - KEEP(*(__bss_end)); + .bss_end __bss_limit (OVERLAY) : { + KEEP(*(.__bss_end)); }
/DISCARD/ : { *(.bss*) }
Hi Albert,
On Thu, 11 Apr 2013 17:43:21 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
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.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net Reported-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com
Changes in v2:
- remove HIDDEN(), not supported by binutils < 2.23
Any comments on this one? If not, I'll apply it to u-boot-arm/master within a few hours.
Amicalement,
On Thu, 11 Apr 2013 17:43:21 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
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.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net Reported-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com
Changes in v2:
- remove HIDDEN(), not supported by binutils < 2.23
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 388a67f..553589c 100644 --- a/arch/arm/cpu/ixp/u-boot.lds +++ b/arch/arm/cpu/ixp/u-boot.lds @@ -69,17 +69,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));
__bss_base = .;
- .bss __bss_start (OVERLAY) : {
- .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4);
__bss_end = .;
__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..d5e42d3a 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));
__bss_base = .;
- .bss __bss_start (OVERLAY) : {
- .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4);
__bss_end = .;
__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 52fc9fc..ef4a25b 100644 --- a/board/actux1/u-boot.lds +++ b/board/actux1/u-boot.lds @@ -77,17 +77,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));
__bss_base = .;
- .bss __bss_start (OVERLAY) : {
- .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4);
__bss_end = .;
__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 cafd3d8..00ad8b7 100644 --- a/board/actux2/u-boot.lds +++ b/board/actux2/u-boot.lds @@ -77,17 +77,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));
__bss_base = .;
- .bss __bss_start (OVERLAY) : {
- .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4);
__bss_end = .;
__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 168fe17..44b990e 100644 --- a/board/actux3/u-boot.lds +++ b/board/actux3/u-boot.lds @@ -77,17 +77,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));
__bss_base = .;
- .bss __bss_start (OVERLAY) : {
- .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4);
__bss_end = .;
__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 dd50bdb..6d4b187 100644 --- a/board/dvlhost/u-boot.lds +++ b/board/dvlhost/u-boot.lds @@ -77,17 +77,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));
__bss_base = .;
- .bss __bss_start (OVERLAY) : {
- .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4);
__bss_end = .;
__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 f48fda1..4969960 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));
__bss_base = .;
- .bss __bss_start (OVERLAY) : {
- .bss __bss_base (OVERLAY) : { *(.bss*) . = ALIGN(4);
__bss_end = .;
__bss_limit = .;
- .bss_end __bss_end (OVERLAY) : {
KEEP(*(__bss_end));
.bss_end __bss_limit (OVERLAY) : {
KEEP(*(.__bss_end));}
/DISCARD/ : { *(.bss*) }
Applied to u-boot-arm/master.
Amicalement,
participants (6)
-
Albert ARIBAUD -
Benoît Thébaudeau -
Stephen Warren -
Tom Rini -
Tom Warren -
Tom Warren