diff mbox

[U-Boot,RFC] Avoid R_ARM_ABS32 for bss start and end references

Message ID 1360690047-26599-1-git-send-email-albert.u.boot@aribaud.net
State RFC
Delegated to: Tom Rini
Headers show

Commit Message

Albert ARIBAUD Feb. 12, 2013, 5:27 p.m. UTC 
This patch is intended to be patch 2/2 of my first patch to remove
R_ARM_ABS32 relocation types from ARM builds.

With this change, the type of ARM references to __bss_start
and __bss_end__ is changed from R_ARM_ABS32 to R_ARM_RELATIVE.
It should have no functional impact, as it only affects the
resolution of references to __bss_start and __bss_end__ before
relocation, and no code should ever perform such references...
so far. References performed after relocation are unchanged by
this patch.

This patch SHOULD NOT BE applied in any official U-boot tree! It is
submitted as an RFC and a request to test.

This patch can only work on ARM; it will not work on any ARM target
that uses another linker script than arch/arm/cpu/u-boot.lds, and it
will not work on any non-ARM target.

HOWEVER, if you can test it on one of these targets, then please do
so by manually patching the appropriate linker script the same way
arch/arm/cpu/u-boot.lds is patched here.

If you are keen on testing but don't know how to patch your linker
script, just let me know which target you intend to test on, and which
linker script you need patched, and I'll do a v2 / v3... of this RFC.

The goal here is to help me ensure the patch works well on enough
targets that I can safely start the tedious work of patching all 150+
linker scripts.
---
 arch/arm/cpu/u-boot.lds |   12 +++++++++---
 lib/Makefile            |    1 +
 lib/bss.c               |   35 +++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)
 create mode 100644 lib/bss.c

Comments

Luka Perkov Feb. 14, 2013, 12:17 a.m. UTC | #1 
On Tue, Feb 12, 2013 at 06:27:27PM +0100, Albert ARIBAUD wrote:
> This patch SHOULD NOT BE applied in any official U-boot tree! It is
> submitted as an RFC and a request to test.

I can confirm that this patch applied on top of 9c748e0 worked for my
two kirkwood boards. One is in mainline (ib62x0) and one is not.

Tested-by: Luka Perkov <luka@openwrt.org>
Albert ARIBAUD Feb. 14, 2013, 8:10 a.m. UTC | #2 
Hi Luka,

On Thu, 14 Feb 2013 01:17:24 +0100, Luka Perkov <luka@openwrt.org>
wrote:

> On Tue, Feb 12, 2013 at 06:27:27PM +0100, Albert ARIBAUD wrote:
> > This patch SHOULD NOT BE applied in any official U-boot tree! It is
> > submitted as an RFC and a request to test.
> 
> I can confirm that this patch applied on top of 9c748e0 worked for my
> two kirkwood boards. One is in mainline (ib62x0) and one is not.
> 
> Tested-by: Luka Perkov <luka@openwrt.org>

Thanks Luka!

Anyone available to test on a non-ARM architecture? I can provide the
patched linker script if testing on a mainline-supported target.

Amicalement,
Andreas Bießmann Feb. 14, 2013, 8:43 a.m. UTC | #3 
Hi Albert,

On 02/14/2013 09:10 AM, Albert ARIBAUD wrote:
> Hi Luka,
> 
> On Thu, 14 Feb 2013 01:17:24 +0100, Luka Perkov <luka@openwrt.org>
> wrote:
> 
>> On Tue, Feb 12, 2013 at 06:27:27PM +0100, Albert ARIBAUD wrote:
>>> This patch SHOULD NOT BE applied in any official U-boot tree! It is
>>> submitted as an RFC and a request to test.
>>
>> I can confirm that this patch applied on top of 9c748e0 worked for my
>> two kirkwood boards. One is in mainline (ib62x0) and one is not.
>>
>> Tested-by: Luka Perkov <luka@openwrt.org>
> 
> Thanks Luka!
> 
> Anyone available to test on a non-ARM architecture? I can provide the
> patched linker script if testing on a mainline-supported target.

I can on AVR32, would you provide the linker script (atstk1002
preferred, but AFAIR all AVR32 targets use the same linker script)?
I think we should apply this change on top of "Refactor linker-generated
arrays", am I right?

Best regards

Andreas Bießmann
Albert ARIBAUD Feb. 14, 2013, 10:31 a.m. UTC | #4 
Hi Andreas,

On Thu, 14 Feb 2013 09:43:06 +0100, "Andreas Bießmann"
<andreas.devel@googlemail.com> wrote:

> Hi Albert,
> 
> On 02/14/2013 09:10 AM, Albert ARIBAUD wrote:
> > Hi Luka,
> > 
> > On Thu, 14 Feb 2013 01:17:24 +0100, Luka Perkov <luka@openwrt.org>
> > wrote:
> > 
> >> On Tue, Feb 12, 2013 at 06:27:27PM +0100, Albert ARIBAUD wrote:
> >>> This patch SHOULD NOT BE applied in any official U-boot tree! It is
> >>> submitted as an RFC and a request to test.
> >>
> >> I can confirm that this patch applied on top of 9c748e0 worked for my
> >> two kirkwood boards. One is in mainline (ib62x0) and one is not.
> >>
> >> Tested-by: Luka Perkov <luka@openwrt.org>
> > 
> > Thanks Luka!
> > 
> > Anyone available to test on a non-ARM architecture? I can provide the
> > patched linker script if testing on a mainline-supported target.
> 
> I can on AVR32, would you provide the linker script (atstk1002
> preferred, but AFAIR all AVR32 targets use the same linker script)?

Yes, there is a single linker script for AVR. Do you have a URL handy
for the AVR toolchain you're using?

> I think we should apply this change on top of "Refactor linker-generated
> arrays", am I right?

It uses the same methods as the linker lists one, and eventually it
will be part of the series, but it is autonomous -- no need to apply
anything else.

Thanks for your help!

> Best regards
> 
> Andreas Bießmann

Amicalement,
Andreas Bießmann Feb. 14, 2013, 10:59 a.m. UTC | #5 
Hi Albert,

On 02/14/2013 11:31 AM, Albert ARIBAUD wrote:
> Hi Andreas,
> 
> On Thu, 14 Feb 2013 09:43:06 +0100, "Andreas Bießmann"
> <andreas.devel@googlemail.com> wrote:
> 
>> Hi Albert,
>>
>> On 02/14/2013 09:10 AM, Albert ARIBAUD wrote:

>> I can on AVR32, would you provide the linker script (atstk1002
>> preferred, but AFAIR all AVR32 targets use the same linker script)?
> 
> Yes, there is a single linker script for AVR. Do you have a URL handy
> for the AVR toolchain you're using?

No, I use a self patched OSELAS toolchain, I like to push the changes
mainline but haven't managed yet.
I know buildroot and openwrt has toolchain support for avr32 and AFAIR
Mike Frysinger has a set of pre-compiled toolchains including a AVR32
one [1].

> 
>> I think we should apply this change on top of "Refactor linker-generated
>> arrays", am I right?
> 
> It uses the same methods as the linker lists one, and eventually it
> will be part of the series, but it is autonomous -- no need to apply
> anything else.

Ok.

Best regards

Andreas Bießmann

[1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/117404
diff mbox

Patch

diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index e6b202b..e1bc8e7 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -81,11 +81,17 @@  SECTIONS
 		*(.mmutable)
 	}
 
-	.bss __rel_dyn_start (OVERLAY) : {
-		__bss_start = .;
+	.bss_start __rel_dyn_start (OVERLAY) : {
+		KEEP(*(.__bss_start));
+	}
+
+	.bss __bss_start (OVERLAY) : {
 		*(.bss*)
 		 . = ALIGN(4);
-		__bss_end__ = .;
+		 ___bssend___ = .;
+	}
+	.bss_end ___bssend___ (OVERLAY) : {
+		KEEP(*(.__bss_end__));
 	}
 
 	/DISCARD/ : { *(.dynstr*) }
diff --git a/lib/Makefile b/lib/Makefile
index 86ca1a6..73ee160 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -25,6 +25,7 @@  include $(TOPDIR)/config.mk
 
 LIB	= $(obj)libgeneric.o
 
+COBJS-y += bss.o
 ifndef CONFIG_SPL_BUILD
 COBJS-$(CONFIG_ADDR_MAP) += addr_map.o
 COBJS-$(CONFIG_BCH) += bch.o
diff --git a/lib/bss.c b/lib/bss.c
new file mode 100644
index 0000000..5678f30
--- /dev/null
+++ b/lib/bss.c
@@ -0,0 +1,35 @@ 
+/*
+ * Copyright 2013 Albert ARIBAUD <albert.u.boot@aribaud.net>
+ * 
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+/**
+ * These two symbols are declared in a C file so that the linker
+ * uses R_ARM_RELATIVE relocation, rather than the R_ARM_ABS32 one
+ * it would use if the symbols were defined in the linker file.
+ * Using only R_ARM_RELATIVE relocation ensures that references to
+ * the symbols are correct after as well as before relocation.
+ * 
+ * As the symbols do not require any content, and as we cannot define
+ * them as 'void', we go for the next best thing, 'struct {}'.
+ */
+
+struct {} __bss_start __attribute__((used,section(".__bss_start")));
+struct {} __bss_end__ __attribute__((used,section(".__bss_end__")));