[U-Boot] ARM: problems with elf relocation on arm920t / maybe toolchain related

[U-Boot] ARM: problems with elf relocation on arm920t / maybe toolchain related

[Andreas Bießmann]

Dear Albert Aribaud,

in reply to http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88014/focus=88022 I have to tell another issue. I try to get the elf relocation stuff working for arm920t and run into some compiler issues. Sorry, had no time to investigate that but maybe one has some pointers?

I have have adopted your changes to arm926ejs to arm920t and built that with my own toolchain (gcc 4.5.1, binutils 2.20.1.20100303) and codesourcery 2010q1-202. The codesourcery report is compareable so i send it here:

---8<---
...
cd /tmp/build_uboot-at91rm9200ek && arm-none-linux-gnueabi-ld -Bstatic -T /tmp/build_uboot-at91rm9200ek/u-boot.lds  -pie -Ttext 0x21000000 $UNDEF_SYM arch/arm/cpu/arm920t/start.o --start-group lib/libgeneric.a lib/lzma/liblzma.a lib/lzo/liblzo.a arch/arm/cpu/arm920t/libarm920t.a arch/arm/cpu/arm920t/at91/libat91.a arch/arm/lib/libarm.a fs/cramfs/libcramfs.a fs/fat/libfat.a fs/fdos/libfdos.a fs/jffs2/libjffs2.a fs/reiserfs/libreiserfs.a fs/ext2/libext2fs.a fs/yaffs2/libyaffs2.a fs/ubifs/libubifs.a net/libnet.a disk/libdisk.a drivers/bios_emulator/libatibiosemu.a drivers/block/libblock.a drivers/dma/libdma.a drivers/fpga/libfpga.a drivers/gpio/libgpio.a drivers/hwmon/libhwmon.a drivers/i2c/libi2c.a drivers/input/libinput.a drivers/misc/libmisc.a drivers/mmc/libmmc.a drivers/mtd/libmtd.a drivers/mtd/nand/libnand.a drivers/mtd/onenand/libonenand.a drivers/mtd/ubi/libubi.a drivers/mtd/spi/libspi_flash.a drivers/net/libnet.a drivers/net/phy/libphy.a drivers/pci/libpci.a drivers/pcmcia/libpcmcia.a drivers/power/libpower.a drivers/spi/libspi.a drivers/rtc/librtc.a drivers/serial/libserial.a drivers/twserial/libtws.a drivers/usb/gadget/libusb_gadget.a drivers/usb/host/libusb_host.a drivers/usb/musb/libusb_musb.a drivers/usb/phy/libusb_phy.a drivers/video/libvideo.a drivers/watchdog/libwatchdog.a common/libcommon.a lib/libfdt/libfdt.a api/libapi.a post/libpost.a board/atmel/at91rm9200ek/libat91rm9200ek.a --end-group /tmp/build_uboot-at91rm9200ek/arch/arm/lib/eabi_compat.o -L /home/andreas/src/arm-2010q1/bin/../lib/gcc/arm-none-linux-gnueabi/4.4.1 -lgcc -Map u-boot.map -o u-boot
arm-none-linux-gnueabi-ld: BFD (Sourcery G++ Lite 2010q1-202) 2.19.51.20090709 assertion fail /scratch/julian/2010q1-release-linux-lite/obj/binutils-src-2010q1-202-arm-none-linux-gnueabi-i686-pc-linux-gnu/bfd/elf32-arm.c:12338
arm-none-linux-gnueabi-ld: BFD (Sourcery G++ Lite 2010q1-202) 2.19.51.20090709 assertion fail /scratch/julian/2010q1-release-linux-lite/obj/binutils-src-2010q1-202-arm-none-linux-gnueabi-i686-pc-linux-gnu/bfd/elf32-arm.c:12572
/bin/sh: line 1: 12104 Speicherzugriffsfehler  arm-none-linux-gnueabi-ld -Bstatic -T /tmp/build_uboot-at91rm9200ek/u-boot.lds -pie -Ttext 0x21000000 $UNDEF_SYM arch/arm/cpu/arm920t/start.o --start-group lib/libgeneric.a lib/lzma/liblzma.a lib/lzo/liblzo.a arch/arm/cpu/arm920t/libarm920t.a arch/arm/cpu/arm920t/at91/libat91.a arch/arm/lib/libarm.a fs/cramfs/libcramfs.a fs/fat/libfat.a fs/fdos/libfdos.a fs/jffs2/libjffs2.a fs/reiserfs/libreiserfs.a fs/ext2/libext2fs.a fs/yaffs2/libyaffs2.a fs/ubifs/libubifs.a net/libnet.a disk/libdisk.a drivers/bios_emulator/libatibiosemu.a drivers/block/libblock.a drivers/dma/libdma.a drivers/fpga/libfpga.a drivers/gpio/libgpio.a drivers/hwmon/libhwmon.a drivers/i2c/libi2c.a drivers/input/libinput.a drivers/misc/libmisc.a drivers/mmc/libmmc.a drivers/mtd/libmtd.a drivers/mtd/nand/libnand.a drivers/mtd/onenand/libonenand.a drivers/mtd/ubi/libubi.a drivers/mtd/spi/libspi_flash.a drivers/net/libnet.a drivers/net/phy/libphy.a drivers/pci/libpci.a drivers/pcmcia/libpcmcia.a drivers/power/libpower.a drivers/spi/libspi.a drivers/rtc/librtc.a drivers/serial/libserial.a drivers/twserial/libtws.a drivers/usb/gadget/libusb_gadget.a drivers/usb/host/libusb_host.a drivers/usb/musb/libusb_musb.a drivers/usb/phy/libusb_phy.a drivers/video/libvideo.a drivers/watchdog/libwatchdog.a common/libcommon.a lib/libfdt/libfdt.a api/libapi.a post/libpost.a board/atmel/at91rm9200ek/libat91rm9200ek.a --end-group /tmp/build_uboot-at91rm9200ek/arch/arm/lib/eabi_compat.o -L /home/andreas/src/arm-2010q1/bin/../lib/gcc/arm-none-linux-gnueabi/4.4.1 -lgcc -Map u-boot.map -o u-boot
make: *** [/tmp/build_uboot-at91rm9200ek/u-boot] Fehler 139
...
--->8---

My self built toolchain runs into the same assertions, I think so at least. I wonder whether this is a pure toolchain problem/bug or if it have some connection to the input and maybe we are missing a essential thing? I had a short look into the binutils code and found it crashes when looking up .plt sections which are discarded in linker script. See the following snippet of binutils-2.20.1/bfd/elf32-arm.c near lines 12337

---8<---
static bfd_boolean
elf32_arm_finish_dynamic_sections (bfd * output_bfd, struct bfd_link_info * info)
...
  if (elf_hash_table (info)->dynamic_sections_created)
...
      for (; dyncon < dynconend; dyncon++)
...
          bfd_elf32_swap_dyn_in (dynobj, dyncon, &dyn);

          switch (dyn.d_tag)
...
            case DT_PLTRELSZ:
              s = bfd_get_section_by_name (output_bfd,
                                           RELOC_SECTION (htab, ".plt"));
              BFD_ASSERT (s != NULL);                                                 <- second assertion
              dyn.d_un.d_val = s->size;                                                   <- segfault
              bfd_elf32_swap_dyn_out (output_bfd, &dyn, dyncon);
              break;
...
--->8---

My changes in arm920t/start.S will follow. The board to test is at91rm9200ek. Maybe one can test my changes with ELDK toolchain?

regards

Andreas Bie?mann

[U-Boot] [PATCH RFC] arm920t: implement elf relocation

[Andreas Bießmann]

Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
---
This changes give segfault in at least codesourcery 2010q1-202 arm lite toolchain. To test this segfault use:
 # make at91rm9200ek_config
 # make

 arch/arm/cpu/arm920t/start.S    |  122 ++++++++++++++++++++-------------------
 arch/arm/cpu/arm920t/u-boot.lds |   26 +++++----
 2 files changed, 78 insertions(+), 70 deletions(-)

diff --git a/arch/arm/cpu/arm920t/start.S b/arch/arm/cpu/arm920t/start.S
index d4edde7..adaf193 100644
--- a/arch/arm/cpu/arm920t/start.S
+++ b/arch/arm/cpu/arm920t/start.S
@@ -77,14 +77,17 @@ _TEXT_BASE:
 
 /*
  * These are defined in the board-specific linker script.
+ * Subtracting _start from them lets the linker put their
+ * relative position in the executable instead of leaving
+ * them null.
  */
-.globl _bss_start
-_bss_start:
-	.word __bss_start
+.globl _bss_start_ofs
+_bss_start_ofs:
+	.word __bss_start - _start
 
-.globl _bss_end
-_bss_end:
-	.word _end
+.globl _bss_end_ofs
+_bss_end_ofs:
+	.word _end - _start
 
 #ifdef CONFIG_USE_IRQ
 /* IRQ stack memory (calculated at run-time) */
@@ -103,30 +106,6 @@ FIQ_STACK_START:
 IRQ_STACK_START_IN:
 	.word	0x0badc0de
 
-.globl _datarel_start
-_datarel_start:
-	.word __datarel_start
-
-.globl _datarelrolocal_start
-_datarelrolocal_start:
-	.word __datarelrolocal_start
-
-.globl _datarellocal_start
-_datarellocal_start:
-	.word __datarellocal_start
-
-.globl _datarelro_start
-_datarelro_start:
-	.word __datarelro_start
-
-.globl _got_start
-_got_start:
-	.word __got_start
-
-.globl _got_end
-_got_end:
-	.word __got_end
-
 /*
  * the actual start code
  */
@@ -230,9 +209,8 @@ stack_setup:
 
 	adr	r0, _start
 	ldr	r2, _TEXT_BASE
-	ldr	r3, _bss_start
-	sub	r2, r3, r2		/* r2 <- size of armboot	    */
-	add	r2, r0, r2		/* r2 <- source end address	    */
+	ldr	r3, _bss_start_ofs
+	add	r2, r0, r3		/* r2 <- source end address	    */
 	cmp	r0, r6
 	beq	clear_bss
 
@@ -243,35 +221,53 @@ copy_loop:
 	blo	copy_loop
 
 #ifndef CONFIG_PRELOADER
-	/* fix got entries */
-	ldr	r1, _TEXT_BASE		/* Text base */
-	mov	r0, r7			/* reloc addr */
-	ldr	r2, _got_start		/* addr in Flash */
-	ldr	r3, _got_end		/* addr in Flash */
-	sub	r3, r3, r1
-	add	r3, r3, r0
-	sub	r2, r2, r1
-	add	r2, r2, r0
-
+	/*
+	 * fix .rel.dyn relocations
+	 */
+	ldr	r0, _TEXT_BASE		/*  r0 <- Text base */
+	sub	r9, r7, r0		/*  r9 <- relocation offset */
+	ldr	r10, _dynsym_start_ofs	/* r10 <- sym table offset */
+	add	r10, r10, r0		/* r10 <- sym table in FLASH */
+	ldr	r2, _rel_dyn_start_ofs	/*  r2 <- rel dyn start offset */
+	add	r2, r2, r0		/*  r2 <- rel dyn start in FLASH */
+	ldr	r3, _rel_dyn_end_ofs	/*  r3 <- rel dyn end offset */
+	add	r3, r3, r0		/*  r3 <- rel dyn end in FLASH */
 fixloop:
-	ldr	r4, [r2]
-	sub	r4, r4, r1
-	add	r4, r4, r0
-	str	r4, [r2]
-	add	r2, r2, #4
+	ldr	r0, [r2]		/*  r0 <- location to fix up in FLASH */
+	add	r0, r0, r9		/*  r0 <- location to fix up in RAM */
+	ldr	r1, [r2, #4]
+	and	r8, r1, #0xff
+	cmp	r8, #23			/* relative fixup? */
+	beq	fixrel
+	cmp	r8, #2			/* absolute fixup? */
+	beq	fixabs
+	/* ignore unknown type of fixup */
+	b	fixnext
+fixabs:
+	/* absolute fix: set location to (offste) symbol value */
+	mov	r1, r1, LSR #4		/*  r1 <- symbol index in .dynsym */
+	add	r1, r10, r1		/*  r1 <- address of symbol in table */
+	ldr	r1, [r1, #4]		/*  r1 <- symbol value */
+	add	r1, r9			/*  r1 <- relocated sym address */
+	b	fixnext
+fixrel:
+	/* relative fix: increase location by offset */
+	ldr	r1, [r0]
+	add	r1, r1, r9
+fixnext:
+	str	r1, [r0]
+	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
 	cmp	r2, r3
 	blo	fixloop
 #endif
 
 clear_bss:
 #ifndef CONFIG_PRELOADER
-	ldr	r0, _bss_start
-	ldr	r1, _bss_end
+	ldr	r0, _bss_start_ofs
+	ldr	r1, _bss_end_ofs
 	ldr	r3, _TEXT_BASE		/* Text base */
 	mov	r4, r7			/* reloc addr */
-	sub	r0, r0, r3
 	add	r0, r0, r4
-	sub	r1, r1, r3
 	add	r1, r1, r4
 	mov	r2, #0x00000000		/* clear			    */
 
@@ -289,12 +285,14 @@ clbss_l:str	r2, [r0]		/* clear loop...		    */
  * initialization, now running from RAM.
  */
 #ifdef CONFIG_NAND_SPL
-	ldr     pc, _nand_boot
-
-_nand_boot: .word nand_boot
+	ldr	r0, _nand_boot_ofs
+	adr	r1, _start
+	add	pc, r0, r1
+_nand_boot_ofs:
+	.word nand_boot - _start
 #else
 	ldr	r0, _TEXT_BASE
-	ldr	r2, _board_init_r
+	ldr	r2, _board_init_r_ofs
 	sub	r2, r2, r0
 	add	r2, r2, r7	/* position from board_init_r in RAM */
 	/* setup parameters for board_init_r */
@@ -304,9 +302,17 @@ _nand_boot: .word nand_boot
 	mov	lr, r2
 	mov	pc, lr
 
-_board_init_r: .word board_init_r
+_board_init_r_ofs:
+	.word board_init_r - _start
 #endif
 
+_rel_dyn_start_ofs:
+	.word __rel_dyn_start - _start
+_rel_dyn_end_ofs:
+	.word __rel_dyn_end - _start
+_dynsym_start_ofs:
+	.word __dynsym_start - _start
+
 /*
  *************************************************************************
  *
@@ -317,8 +323,6 @@ _board_init_r: .word board_init_r
  *
  *************************************************************************
  */
-
-
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
 cpu_init_crit:
 	/*
diff --git a/arch/arm/cpu/arm920t/u-boot.lds b/arch/arm/cpu/arm920t/u-boot.lds
index 6985434..2a4ac8f 100644
--- a/arch/arm/cpu/arm920t/u-boot.lds
+++ b/arch/arm/cpu/arm920t/u-boot.lds
@@ -49,21 +49,19 @@ SECTIONS
 	. = ALIGN(4);
 	.data : {
 		*(.data)
-	__datarel_start = .;
-		*(.data.rel)
-	__datarelrolocal_start = .;
-		*(.data.rel.ro.local)
-	__datarellocal_start = .;
-		*(.data.rel.local)
-	__datarelro_start = .;
-		*(.data.rel.ro)
 	}
 
-	__got_start = .;
 	. = ALIGN(4);
-	.got : { *(.got) }
 
-	__got_end = .;
+	__rel_dyn_start = .;
+	.rel.dyn : { *(.rel.dyn) }
+	__rel_dyn_end = .;
+
+	__dynsym_start = .;
+	.dynsym : { *(.dynsym) }
+
+	. = ALIGN(4);
+
 	. = .;
 	__u_boot_cmd_start = .;
 	.u_boot_cmd : { *(.u_boot_cmd) }
@@ -73,4 +71,10 @@ SECTIONS
 	__bss_start = .;
 	.bss (NOLOAD) : { *(.bss) . = ALIGN(4); }
 	_end = .;
+
+	/DISCARD/ : { *(.dynstr*) }
+	/DISCARD/ : { *(.dynamic*) }
+	/DISCARD/ : { *(.plt*) }
+	/DISCARD/ : { *(.interp*) }
+	/DISCARD/ : { *(.gnu*) }
 }
-- 
1.7.3.2

[U-Boot] [PATCH RFC] arm920t: implement elf relocation

[Albert ARIBAUD]

Thanks Andreas for this helpful contribution; I'll look up the .plt 
section and see if it may lead to the root cause of the issue. However:

Le 31/10/2010 08:25, Andreas Bie?mann a ?crit :
> Signed-off-by: Andreas Bie?mann<andreas.devel@googlemail.com>
> ---
> This changes give segfault in at least codesourcery 2010q1-202 arm lite toolchain. To test this segfault use:
>   # make at91rm9200ek_config
>   # make
>
>   arch/arm/cpu/arm920t/start.S    |  122 ++++++++++++++++++++-------------------
>   arch/arm/cpu/arm920t/u-boot.lds |   26 +++++----
>   2 files changed, 78 insertions(+), 70 deletions(-)
>
> diff --git a/arch/arm/cpu/arm920t/start.S b/arch/arm/cpu/arm920t/start.S
> index d4edde7..adaf193 100644
> --- a/arch/arm/cpu/arm920t/start.S
> +++ b/arch/arm/cpu/arm920t/start.S
> @@ -77,14 +77,17 @@ _TEXT_BASE:
>
>   /*
>    * These are defined in the board-specific linker script.
> + * Subtracting _start from them lets the linker put their
> + * relative position in the executable instead of leaving
> + * them null.
>    */
> -.globl _bss_start
> -_bss_start:
> -	.word __bss_start
> +.globl _bss_start_ofs
> +_bss_start_ofs:
> +	.word __bss_start - _start

This patch seems to apply to u-boot before ELF relocation was 
introduced. Can you make it against the latest master?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH RFC] arm920t: implement elf relocation

[Andreas Bießmann]

Dear Albert Aribaud,

Am 31.10.2010 um 08:28 schrieb Albert ARIBAUD:

> Thanks Andreas for this helpful contribution; I'll look up the .plt section and see if it may lead to the root cause of the issue. However:
> 
> Le 31/10/2010 08:25, Andreas Bie?mann a ?crit :
>> Signed-off-by: Andreas Bie?mann<andreas.devel@googlemail.com>
>> ---
>> 
> 
> This patch seems to apply to u-boot before ELF relocation was introduced. Can you make it against the latest master?

It should be against the latest master:

---8<---
843e147 arm920t: implement elf relocation
0c0892b Merge branch 'master' of git://git.denx.de/u-boot-marvell
d75c2a3 Merge branch 'master' of git://git.denx.de/u-boot-imx
17dd883 Merge branch 'master' of git://git.denx.de/u-boot-samsung
3388db2 Merge branch 'master' of git://git.denx.de/u-boot-blackfin
1ecb758 Merge branch 'for-wd-master' of git://git.denx.de/u-boot-pxa
e03f316 Drop support for CONFIG_SKIP_RELOCATE_UBOOT
a9aa392 Drop support for CONFIG_SYS_ARM_WITHOUT_RELOC
...
--->8---
The latest merges where within 35 hours, maybe you have missed them?

regards

Andreas Bie?mann

[U-Boot] [PATCH RFC] arm920t: implement elf relocation

[Albert ARIBAUD]

Le 31/10/2010 08:39, Andreas Bie?mann a ?crit :
> Dear Albert Aribaud,
>
> Am 31.10.2010 um 08:28 schrieb Albert ARIBAUD:
>
>> Thanks Andreas for this helpful contribution; I'll look up the .plt section and see if it may lead to the root cause of the issue. However:
>>
>> Le 31/10/2010 08:25, Andreas Bie?mann a ?crit :
>>> Signed-off-by: Andreas Bie?mann<andreas.devel@googlemail.com>
>>> ---
>>>
>>
>> This patch seems to apply to u-boot before ELF relocation was introduced. Can you make it against the latest master?
>
> It should be against the latest master:
>
> ---8<---
> 843e147 arm920t: implement elf relocation
> 0c0892b Merge branch 'master' of git://git.denx.de/u-boot-marvell
> d75c2a3 Merge branch 'master' of git://git.denx.de/u-boot-imx
> 17dd883 Merge branch 'master' of git://git.denx.de/u-boot-samsung
> 3388db2 Merge branch 'master' of git://git.denx.de/u-boot-blackfin
> 1ecb758 Merge branch 'for-wd-master' of git://git.denx.de/u-boot-pxa
> e03f316 Drop support for CONFIG_SKIP_RELOCATE_UBOOT
> a9aa392 Drop support for CONFIG_SYS_ARM_WITHOUT_RELOC
> ...
> --->8---
> The latest merges where within 35 hours, maybe you have missed them?

No but I'd missed the mergeback of elf_reloc to master and thought all 
ARMs were converted to ELF relocation then. Obviously arm920t was not, 
(neither were sa1100, lh7a40x, s3c44b0, arm720t, ixp, arm_intcm, arm925t 
and arm946es).

> regards
>
> Andreas Bie?mann

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 1/2] arm920t/at91/reset.c: fix weak reset_board()

[Andreas Bießmann]

The arm920t compiler/linker dif not handle weak functions correctely.
Therefore the linker tried to link outside the ELF (isn't that lazy
linking?). This leads to segfault of linker in the end.

This patch adds a empty stub for weak function reset_board() to catch
that case.

Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
---
introduced in v2

 arch/arm/cpu/arm920t/at91/reset.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/arm920t/at91/reset.c b/arch/arm/cpu/arm920t/at91/reset.c
index ce9c156..8c81030 100644
--- a/arch/arm/cpu/arm920t/at91/reset.c
+++ b/arch/arm/cpu/arm920t/at91/reset.c
@@ -35,7 +35,14 @@
 #include <asm/arch/hardware.h>
 #include <asm/arch/at91_st.h>
 
-void board_reset(void) __attribute__((__weak__));
+void __attribute__((weak)) board_reset(void)
+{
+	/*
+	 * do absolute nothing here
+	 * but to have a empty stub for weak function to satisfy the linker
+	 */
+}
 
 void reset_cpu(ulong ignored)
 {
@@ -45,8 +52,7 @@ void reset_cpu(ulong ignored)
 	serial_exit();
 #endif
 
-	if (board_reset)
-		board_reset();
+	board_reset();
 
 	/* Reset the cpu by setting up the watchdog timer */
 	writel(AT91_ST_WDMR_RSTEN | AT91_ST_WDMR_EXTEN | AT91_ST_WDMR_WDV(2),
-- 
1.7.3.2

[U-Boot] [PATCH v2 2/2] arm920t: implement elf relocation

[Andreas Bießmann]

This is mostly a copy of arm926ejs relocation and known to _not_ work
correctly a.t.m!

Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
---
changes since v1:
 - adopted linker script as in patch series v3 "arm926ejs: fix linker file for newer ld support"
  * move rel.dyn and dynsym after bss
  * overlay bss with rel.dyn
 - double checked fixup loop in start.S
  -> there is still an error in relocation fixup loop breaks the boot,
     the board hangs after "relocation Offset is: 00f80000"

 Dear Albert Aribaud,
 can you please have a look for that?

 arch/arm/cpu/arm920t/start.S    |  122 ++++++++++++++++++++-------------------
 arch/arm/cpu/arm920t/u-boot.lds |   37 +++++++-----
 2 files changed, 86 insertions(+), 73 deletions(-)

diff --git a/arch/arm/cpu/arm920t/start.S b/arch/arm/cpu/arm920t/start.S
index d4edde7..adaf193 100644
--- a/arch/arm/cpu/arm920t/start.S
+++ b/arch/arm/cpu/arm920t/start.S
@@ -77,14 +77,17 @@ _TEXT_BASE:
 
 /*
  * These are defined in the board-specific linker script.
+ * Subtracting _start from them lets the linker put their
+ * relative position in the executable instead of leaving
+ * them null.
  */
-.globl _bss_start
-_bss_start:
-	.word __bss_start
+.globl _bss_start_ofs
+_bss_start_ofs:
+	.word __bss_start - _start
 
-.globl _bss_end
-_bss_end:
-	.word _end
+.globl _bss_end_ofs
+_bss_end_ofs:
+	.word _end - _start
 
 #ifdef CONFIG_USE_IRQ
 /* IRQ stack memory (calculated at run-time) */
@@ -103,30 +106,6 @@ FIQ_STACK_START:
 IRQ_STACK_START_IN:
 	.word	0x0badc0de
 
-.globl _datarel_start
-_datarel_start:
-	.word __datarel_start
-
-.globl _datarelrolocal_start
-_datarelrolocal_start:
-	.word __datarelrolocal_start
-
-.globl _datarellocal_start
-_datarellocal_start:
-	.word __datarellocal_start
-
-.globl _datarelro_start
-_datarelro_start:
-	.word __datarelro_start
-
-.globl _got_start
-_got_start:
-	.word __got_start
-
-.globl _got_end
-_got_end:
-	.word __got_end
-
 /*
  * the actual start code
  */
@@ -230,9 +209,8 @@ stack_setup:
 
 	adr	r0, _start
 	ldr	r2, _TEXT_BASE
-	ldr	r3, _bss_start
-	sub	r2, r3, r2		/* r2 <- size of armboot	    */
-	add	r2, r0, r2		/* r2 <- source end address	    */
+	ldr	r3, _bss_start_ofs
+	add	r2, r0, r3		/* r2 <- source end address	    */
 	cmp	r0, r6
 	beq	clear_bss
 
@@ -243,35 +221,53 @@ copy_loop:
 	blo	copy_loop
 
 #ifndef CONFIG_PRELOADER
-	/* fix got entries */
-	ldr	r1, _TEXT_BASE		/* Text base */
-	mov	r0, r7			/* reloc addr */
-	ldr	r2, _got_start		/* addr in Flash */
-	ldr	r3, _got_end		/* addr in Flash */
-	sub	r3, r3, r1
-	add	r3, r3, r0
-	sub	r2, r2, r1
-	add	r2, r2, r0
-
+	/*
+	 * fix .rel.dyn relocations
+	 */
+	ldr	r0, _TEXT_BASE		/*  r0 <- Text base */
+	sub	r9, r7, r0		/*  r9 <- relocation offset */
+	ldr	r10, _dynsym_start_ofs	/* r10 <- sym table offset */
+	add	r10, r10, r0		/* r10 <- sym table in FLASH */
+	ldr	r2, _rel_dyn_start_ofs	/*  r2 <- rel dyn start offset */
+	add	r2, r2, r0		/*  r2 <- rel dyn start in FLASH */
+	ldr	r3, _rel_dyn_end_ofs	/*  r3 <- rel dyn end offset */
+	add	r3, r3, r0		/*  r3 <- rel dyn end in FLASH */
 fixloop:
-	ldr	r4, [r2]
-	sub	r4, r4, r1
-	add	r4, r4, r0
-	str	r4, [r2]
-	add	r2, r2, #4
+	ldr	r0, [r2]		/*  r0 <- location to fix up in FLASH */
+	add	r0, r0, r9		/*  r0 <- location to fix up in RAM */
+	ldr	r1, [r2, #4]
+	and	r8, r1, #0xff
+	cmp	r8, #23			/* relative fixup? */
+	beq	fixrel
+	cmp	r8, #2			/* absolute fixup? */
+	beq	fixabs
+	/* ignore unknown type of fixup */
+	b	fixnext
+fixabs:
+	/* absolute fix: set location to (offste) symbol value */
+	mov	r1, r1, LSR #4		/*  r1 <- symbol index in .dynsym */
+	add	r1, r10, r1		/*  r1 <- address of symbol in table */
+	ldr	r1, [r1, #4]		/*  r1 <- symbol value */
+	add	r1, r9			/*  r1 <- relocated sym address */
+	b	fixnext
+fixrel:
+	/* relative fix: increase location by offset */
+	ldr	r1, [r0]
+	add	r1, r1, r9
+fixnext:
+	str	r1, [r0]
+	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
 	cmp	r2, r3
 	blo	fixloop
 #endif
 
 clear_bss:
 #ifndef CONFIG_PRELOADER
-	ldr	r0, _bss_start
-	ldr	r1, _bss_end
+	ldr	r0, _bss_start_ofs
+	ldr	r1, _bss_end_ofs
 	ldr	r3, _TEXT_BASE		/* Text base */
 	mov	r4, r7			/* reloc addr */
-	sub	r0, r0, r3
 	add	r0, r0, r4
-	sub	r1, r1, r3
 	add	r1, r1, r4
 	mov	r2, #0x00000000		/* clear			    */
 
@@ -289,12 +285,14 @@ clbss_l:str	r2, [r0]		/* clear loop...		    */
  * initialization, now running from RAM.
  */
 #ifdef CONFIG_NAND_SPL
-	ldr     pc, _nand_boot
-
-_nand_boot: .word nand_boot
+	ldr	r0, _nand_boot_ofs
+	adr	r1, _start
+	add	pc, r0, r1
+_nand_boot_ofs:
+	.word nand_boot - _start
 #else
 	ldr	r0, _TEXT_BASE
-	ldr	r2, _board_init_r
+	ldr	r2, _board_init_r_ofs
 	sub	r2, r2, r0
 	add	r2, r2, r7	/* position from board_init_r in RAM */
 	/* setup parameters for board_init_r */
@@ -304,9 +302,17 @@ _nand_boot: .word nand_boot
 	mov	lr, r2
 	mov	pc, lr
 
-_board_init_r: .word board_init_r
+_board_init_r_ofs:
+	.word board_init_r - _start
 #endif
 
+_rel_dyn_start_ofs:
+	.word __rel_dyn_start - _start
+_rel_dyn_end_ofs:
+	.word __rel_dyn_end - _start
+_dynsym_start_ofs:
+	.word __dynsym_start - _start
+
 /*
  *************************************************************************
  *
@@ -317,8 +323,6 @@ _board_init_r: .word board_init_r
  *
  *************************************************************************
  */
-
-
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
 cpu_init_crit:
 	/*
diff --git a/arch/arm/cpu/arm920t/u-boot.lds b/arch/arm/cpu/arm920t/u-boot.lds
index 6985434..d16e36d 100644
--- a/arch/arm/cpu/arm920t/u-boot.lds
+++ b/arch/arm/cpu/arm920t/u-boot.lds
@@ -49,28 +49,37 @@ SECTIONS
 	. = ALIGN(4);
 	.data : {
 		*(.data)
-	__datarel_start = .;
-		*(.data.rel)
-	__datarelrolocal_start = .;
-		*(.data.rel.ro.local)
-	__datarellocal_start = .;
-		*(.data.rel.local)
-	__datarelro_start = .;
-		*(.data.rel.ro)
 	}
 
-	__got_start = .;
 	. = ALIGN(4);
-	.got : { *(.got) }
 
-	__got_end = .;
 	. = .;
 	__u_boot_cmd_start = .;
 	.u_boot_cmd : { *(.u_boot_cmd) }
 	__u_boot_cmd_end = .;
 
 	. = ALIGN(4);
-	__bss_start = .;
-	.bss (NOLOAD) : { *(.bss) . = ALIGN(4); }
-	_end = .;
+	.bss (NOLOAD) : {
+		__bss_start = .;
+		*(.bss)
+		 . = ALIGN(4);
+		_end = .;
+	}
+
+	.rel.dyn __bss_start (OVERLAY) : {
+		__rel_dyn_start = .;
+		*(.rel*)
+		__rel_dyn_end = .;
+	}
+
+	.dynsym : {
+		__dynsym_start = .;
+		*(.dynsym)
+	}
+
+	/DISCARD/ : { *(.dynstr*) }
+	/DISCARD/ : { *(.dynamic*) }
+	/DISCARD/ : { *(.plt*) }
+	/DISCARD/ : { *(.interp*) }
+	/DISCARD/ : { *(.gnu*) }
 }
-- 
1.7.3.2

[U-Boot] [PATCH v3] arm920t: implement elf relocation

[Andreas Bießmann]

This is mostly a copy of arm926ejs relocation and known to _not_ work
correctly a.t.m!

Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
---
changes since v1:
 - adopted linker script as in patch series v3 "arm926ejs: fix linker file for newer ld support"
  * move rel.dyn and dynsym after bss
  * overlay bss with rel.dyn
 - double checked fixup loop in start.S
  -> there is still an error in relocation fixup loop breaks the boot,
     the board hangs after "relocation Offset is: 00f80000"

 Dear Albert Aribaud,
 can you please have a look for that?

changes since v2:
 - add missed parts in "branch board_init_r"

 Dear Albert Aribaud,
 still not working, do you have any advice?

 arch/arm/cpu/arm920t/start.S    |  129 ++++++++++++++++++++-------------------
 arch/arm/cpu/arm920t/u-boot.lds |   37 +++++++----
 2 files changed, 89 insertions(+), 77 deletions(-)

diff --git a/arch/arm/cpu/arm920t/start.S b/arch/arm/cpu/arm920t/start.S
index d4edde7..c547181 100644
--- a/arch/arm/cpu/arm920t/start.S
+++ b/arch/arm/cpu/arm920t/start.S
@@ -77,14 +77,17 @@ _TEXT_BASE:
 
 /*
  * These are defined in the board-specific linker script.
+ * Subtracting _start from them lets the linker put their
+ * relative position in the executable instead of leaving
+ * them null.
  */
-.globl _bss_start
-_bss_start:
-	.word __bss_start
+.globl _bss_start_ofs
+_bss_start_ofs:
+	.word __bss_start - _start
 
-.globl _bss_end
-_bss_end:
-	.word _end
+.globl _bss_end_ofs
+_bss_end_ofs:
+	.word _bss_end - _start
 
 #ifdef CONFIG_USE_IRQ
 /* IRQ stack memory (calculated at run-time) */
@@ -103,30 +106,6 @@ FIQ_STACK_START:
 IRQ_STACK_START_IN:
 	.word	0x0badc0de
 
-.globl _datarel_start
-_datarel_start:
-	.word __datarel_start
-
-.globl _datarelrolocal_start
-_datarelrolocal_start:
-	.word __datarelrolocal_start
-
-.globl _datarellocal_start
-_datarellocal_start:
-	.word __datarellocal_start
-
-.globl _datarelro_start
-_datarelro_start:
-	.word __datarelro_start
-
-.globl _got_start
-_got_start:
-	.word __got_start
-
-.globl _got_end
-_got_end:
-	.word __got_end
-
 /*
  * the actual start code
  */
@@ -164,7 +143,7 @@ copyex:
 #  define pWTCON	0x15300000
 #  define INTMSK	0x14400008	/* Interupt-Controller base addresses */
 #  define CLKDIVN	0x14800014	/* clock divisor register */
-#else
+# else
 #  define pWTCON	0x53000000
 #  define INTMSK	0x4A000008	/* Interupt-Controller base addresses */
 #  define INTSUBMSK	0x4A00001C
@@ -230,9 +209,8 @@ stack_setup:
 
 	adr	r0, _start
 	ldr	r2, _TEXT_BASE
-	ldr	r3, _bss_start
-	sub	r2, r3, r2		/* r2 <- size of armboot	    */
-	add	r2, r0, r2		/* r2 <- source end address	    */
+	ldr	r3, _bss_start_ofs
+	add	r2, r0, r3		/* r2 <- source end address	    */
 	cmp	r0, r6
 	beq	clear_bss
 
@@ -243,35 +221,53 @@ copy_loop:
 	blo	copy_loop
 
 #ifndef CONFIG_PRELOADER
-	/* fix got entries */
-	ldr	r1, _TEXT_BASE		/* Text base */
-	mov	r0, r7			/* reloc addr */
-	ldr	r2, _got_start		/* addr in Flash */
-	ldr	r3, _got_end		/* addr in Flash */
-	sub	r3, r3, r1
-	add	r3, r3, r0
-	sub	r2, r2, r1
-	add	r2, r2, r0
-
+	/*
+	 * fix .rel.dyn relocations
+	 */
+	ldr	r0, _TEXT_BASE		/*  r0 <- Text base */
+	sub	r9, r7, r0		/*  r9 <- relocation offset */
+	ldr	r10, _dynsym_start_ofs	/* r10 <- sym table offset */
+	add	r10, r10, r0		/* r10 <- sym table in FLASH */
+	ldr	r2, _rel_dyn_start_ofs	/*  r2 <- rel dyn start offset */
+	add	r2, r2, r0		/*  r2 <- rel dyn start in FLASH */
+	ldr	r3, _rel_dyn_end_ofs	/*  r3 <- rel dyn end offset */
+	add	r3, r3, r0		/*  r3 <- rel dyn end in FLASH */
 fixloop:
-	ldr	r4, [r2]
-	sub	r4, r4, r1
-	add	r4, r4, r0
-	str	r4, [r2]
-	add	r2, r2, #4
+	ldr	r0, [r2]		/*  r0 <- location to fix up in FLASH */
+	add	r0, r0, r9		/*  r0 <- location to fix up in RAM */
+	ldr	r1, [r2, #4]
+	and	r8, r1, #0xff
+	cmp	r8, #23			/* relative fixup? */
+	beq	fixrel
+	cmp	r8, #2			/* absolute fixup? */
+	beq	fixabs
+	/* ignore unknown type of fixup */
+	b	fixnext
+fixabs:
+	/* absolute fix: set location to (offste) symbol value */
+	mov	r1, r1, LSR #4		/*  r1 <- symbol index in .dynsym */
+	add	r1, r10, r1		/*  r1 <- address of symbol in table */
+	ldr	r1, [r1, #4]		/*  r1 <- symbol value */
+	add	r1, r9			/*  r1 <- relocated sym address */
+	b	fixnext
+fixrel:
+	/* relative fix: increase location by offset */
+	ldr	r1, [r0]
+	add	r1, r1, r9
+fixnext:
+	str	r1, [r0]
+	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
 	cmp	r2, r3
 	blo	fixloop
 #endif
 
 clear_bss:
 #ifndef CONFIG_PRELOADER
-	ldr	r0, _bss_start
-	ldr	r1, _bss_end
+	ldr	r0, _bss_start_ofs
+	ldr	r1, _bss_end_ofs
 	ldr	r3, _TEXT_BASE		/* Text base */
 	mov	r4, r7			/* reloc addr */
-	sub	r0, r0, r3
 	add	r0, r0, r4
-	sub	r1, r1, r3
 	add	r1, r1, r4
 	mov	r2, #0x00000000		/* clear			    */
 
@@ -289,24 +285,33 @@ clbss_l:str	r2, [r0]		/* clear loop...		    */
  * initialization, now running from RAM.
  */
 #ifdef CONFIG_NAND_SPL
-	ldr     pc, _nand_boot
+	ldr	r0, _nand_boot_ofs
+	mov	pc, r0
 
-_nand_boot: .word nand_boot
+_nand_boot_ofs:
+	.word nand_boot
 #else
-	ldr	r0, _TEXT_BASE
-	ldr	r2, _board_init_r
-	sub	r2, r2, r0
-	add	r2, r2, r7	/* position from board_init_r in RAM */
+	ldr	r0, _board_init_r_ofs
+	adr	r1, _start
+	add	lr, r0, r1
+	add	lr, lr, r9	/* position from board_init_r in RAM */
 	/* setup parameters for board_init_r */
 	mov	r0, r5		/* gd_t */
 	mov	r1, r7		/* dest_addr */
 	/* jump to it ... */
-	mov	lr, r2
 	mov	pc, lr
 
-_board_init_r: .word board_init_r
+_board_init_r_ofs:
+	.word board_init_r - _start
 #endif
 
+_rel_dyn_start_ofs:
+	.word __rel_dyn_start - _start
+_rel_dyn_end_ofs:
+	.word __rel_dyn_end - _start
+_dynsym_start_ofs:
+	.word __dynsym_start - _start
+
 /*
  *************************************************************************
  *
@@ -317,8 +322,6 @@ _board_init_r: .word board_init_r
  *
  *************************************************************************
  */
-
-
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
 cpu_init_crit:
 	/*
diff --git a/arch/arm/cpu/arm920t/u-boot.lds b/arch/arm/cpu/arm920t/u-boot.lds
index 6985434..d647952 100644
--- a/arch/arm/cpu/arm920t/u-boot.lds
+++ b/arch/arm/cpu/arm920t/u-boot.lds
@@ -49,28 +49,37 @@ SECTIONS
 	. = ALIGN(4);
 	.data : {
 		*(.data)
-	__datarel_start = .;
-		*(.data.rel)
-	__datarelrolocal_start = .;
-		*(.data.rel.ro.local)
-	__datarellocal_start = .;
-		*(.data.rel.local)
-	__datarelro_start = .;
-		*(.data.rel.ro)
 	}
 
-	__got_start = .;
 	. = ALIGN(4);
-	.got : { *(.got) }
 
-	__got_end = .;
 	. = .;
 	__u_boot_cmd_start = .;
 	.u_boot_cmd : { *(.u_boot_cmd) }
 	__u_boot_cmd_end = .;
 
 	. = ALIGN(4);
-	__bss_start = .;
-	.bss (NOLOAD) : { *(.bss) . = ALIGN(4); }
-	_end = .;
+	.bss (NOLOAD) : {
+		__bss_start = .;
+		*(.bss)
+		 . = ALIGN(4);
+		_bss_end = .;
+	}
+
+	.rel.dyn __bss_start (OVERLAY) : {
+		__rel_dyn_start = .;
+		*(.rel*)
+		__rel_dyn_end = .;
+	}
+
+	.dynsym : {
+		__dynsym_start = .;
+		*(.dynsym)
+	}
+
+	/DISCARD/ : { *(.dynstr*) }
+	/DISCARD/ : { *(.dynamic*) }
+	/DISCARD/ : { *(.plt*) }
+	/DISCARD/ : { *(.interp*) }
+	/DISCARD/ : { *(.gnu*) }
 }
-- 
1.7.3.2

[U-Boot] [PATCH v2 1/2] arm920t/at91/reset.c: fix weak reset_board()

[Joakim Tjernlund]

>
> The arm920t compiler/linker dif not handle weak functions correctely.
> Therefore the linker tried to link outside the ELF (isn't that lazy
> linking?). This leads to segfault of linker in the end.
>
> This patch adds a empty stub for weak function reset_board() to catch
> that case.

I believe this is the wrong approach.
Instead you should fix the relocation/fixup routine not to relocate
NULL ptrs. NULL ptrs are absolute values and should be left as is.

See http://git.denx.de/?p=u-boot.git;a=commit;h=d1e0b10accdbac2e0a8b2cbf7c589645442f87c5
for a fix to ppc that went in recently

 Jocke

[U-Boot] [PATCH v2 1/2] arm920t/at91/reset.c: fix weak reset_board()

[Graeme Russ]

On Thu, Nov 4, 2010 at 12:00 PM, Joakim Tjernlund
<joakim.tjernlund@transmode.se> wrote:
>
>>
>> The arm920t compiler/linker dif not handle weak functions correctely.
>> Therefore the linker tried to link outside the ELF (isn't that lazy
>> linking?). This leads to segfault of linker in the end.
>>
>> This patch adds a empty stub for weak function reset_board() to catch
>> that case.
>
> I believe this is the wrong approach.
> Instead you should fix the relocation/fixup routine not to relocate
> NULL ptrs. NULL ptrs are absolute values and should be left as is.
>
> See http://git.denx.de/?p=u-boot.git;a=commit;h=d1e0b10accdbac2e0a8b2cbf7c589645442f87c5
> for a fix to ppc that went in recently
>

I seem to recall a discussion some time ago regarding the use of the
"if (function())" construct versus calls to weakly defined empty functions
but cannot remember the outcome.

I personally think weak functions are 'cleaner' but may result in slightly
larger and slower code due to the call overhead (is the optimiser clever
enough to strip these out if the stub function is empty?)

Would converting all instances of "if (function())" to weak functions be
such a bad thing?

Regards,

Graeme

[U-Boot] [PATCH v2 1/2] arm920t/at91/reset.c: fix weak reset_board()

[Joakim Tjernlund]

Graeme Russ <graeme.russ@gmail.com> wrote on 2010/11/04 02:13:44:
>
> On Thu, Nov 4, 2010 at 12:00 PM, Joakim Tjernlund
> <joakim.tjernlund@transmode.se> wrote:
> >
> >>
> >> The arm920t compiler/linker dif not handle weak functions correctely.
> >> Therefore the linker tried to link outside the ELF (isn't that lazy
> >> linking?). This leads to segfault of linker in the end.
> >>
> >> This patch adds a empty stub for weak function reset_board() to catch
> >> that case.
> >
> > I believe this is the wrong approach.
> > Instead you should fix the relocation/fixup routine not to relocate
> > NULL ptrs. NULL ptrs are absolute values and should be left as is.
> >
> > See http://git.denx.de/?p=u-boot.git;a=commit;h=d1e0b10accdbac2e0a8b2cbf7c589645442f87c5
> > for a fix to ppc that went in recently
> >
>
> I seem to recall a discussion some time ago regarding the use of the
> "if (function())" construct versus calls to weakly defined empty functions
> but cannot remember the outcome.

Me neither.

>
> I personally think weak functions are 'cleaner' but may result in slightly
> larger and slower code due to the call overhead (is the optimiser clever
> enough to strip these out if the stub function is empty?)

No idea.

>
> Would converting all instances of "if (function())" to weak functions be
> such a bad thing?

nope, but I still think the reloc routine(s) needs to be fixed.

[U-Boot] [PATCH v2 1/2] arm920t/at91/reset.c: fix weak reset_board()

[Sebastien Carlier]

These weakly defined empty functions prevent the strong definition
from being linked in.

For example, libarm.a contains a weak symbol 'red_LED_on', which is
expected to be defined (strongly) in the board library. Because
archive libraries are being used, this fails (testing with binutils
2.20.1), and only the empty __red_LED_on stub is linked in; the
red_LED_on definition in the board library is throw away.

This behavior is documented and it is the intended one; from
http://www.sco.com/developers/gabi/latest/ch4.symtab.html:

> When the link editor searches archive libraries [see ``Archive
> File'' in Chapter 7], it extracts archive members that contain
> definitions of undefined global symbols.
> The member's definition may be either a global or a weak symbol.
> The link editor does not extract archive members to resolve
> undefined weak symbols. Unresolved weak symbols have a zero value.

Empty weak definitions would have to be supplied to the linker only
_after_ the strong definitions have been provided.

Leaving undefined weak symbols and testing for NULL-ity at call sites
seems to be a more robust approach.

Note that with some ld versions (at least with 2.20.1), ld creates PLT
entries for undefined weak symbols and crashes when the PLT-related
sections (.plt, .got.plt, and .rel.plt) are discarded...

--
Sebastien Carlier

[U-Boot] [PATCH v3] arm920t: implement elf relocation

[Albert ARIBAUD]

Hi Andreas,

Le 04/11/2010 00:29, Andreas Bie?mann a ?crit :
> This is mostly a copy of arm926ejs relocation and known to _not_ work
> correctly a.t.m!

Can you give a specific case of board and toolchain that does not work?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v3] arm920t: implement elf relocation

[Andreas Bießmann]

Hi Albert,

Am 04.11.2010 07:14, schrieb Albert ARIBAUD:
> Hi Andreas,
> 
> Le 04/11/2010 00:29, Andreas Bie?mann a ?crit :
>> This is mostly a copy of arm926ejs relocation and known to _not_ work
>> correctly a.t.m!
> 
> Can you give a specific case of board and toolchain that does not work?
at91rm9200ek_ram_config (patch sent these days, is at91rm9200ek_config
with TEXT_BASE in RAM and SKIP_LOWLEVEL_INIT set). Toolchain was a
selfbuilt gcc-4.5.1 + binutils 2.20.1.20100303, can check CS 2010-q1
this evening.

regards

Andreas Bie?mann

[U-Boot] [PATCH v2 1/2] arm920t/at91/reset.c: fix weak reset_board()

[Andreas Bießmann]

Dear all,

Am 04.11.2010 02:23, schrieb Joakim Tjernlund:
> Graeme Russ <graeme.russ@gmail.com> wrote on 2010/11/04 02:13:44:
>>
>> On Thu, Nov 4, 2010 at 12:00 PM, Joakim Tjernlund
>> <joakim.tjernlund@transmode.se> wrote:
>>>
>>>>
>>>> The arm920t compiler/linker dif not handle weak functions correctely.
>>>> Therefore the linker tried to link outside the ELF (isn't that lazy
>>>> linking?). This leads to segfault of linker in the end.
>>>>
>>>> This patch adds a empty stub for weak function reset_board() to catch
>>>> that case.
>>>
>>> I believe this is the wrong approach.
>>> Instead you should fix the relocation/fixup routine not to relocate
>>> NULL ptrs. NULL ptrs are absolute values and should be left as is.

This is ok and should be fixed in reloc routine. But it seems these
undefined weak functions are _not_ null for me (have to check this ...).

>> I personally think weak functions are 'cleaner' but may result in slightly
>> larger and slower code due to the call overhead (is the optimiser clever
>> enough to strip these out if the stub function is empty?)
> 
> No idea.

It seems my linker do not strip these functions. As Sebastian pointed
out the linker adds .plt sections for these undefined weak functions. Do
we use the linker in right way?

>> Would converting all instances of "if (function())" to weak functions be
>> such a bad thing?
> 
> nope, but I still think the reloc routine(s) needs to be fixed.

If reloc routine is defective, it will be fixed ... currently the link
of weak functions is the bigger problem. Without the empty stub for
reset_board() my linker segfaults. Can please one test this issue with
e.g. ELDK or other toolchains? CS 2010-q1 is known to be defective too!

To test the described issue use '[PATCH v3] arm920t: implement elf
relocation' and build for at91rm9200ek.

regards

Andreas Bie?mann

[U-Boot] [PATCH v2 1/2] arm920t/at91/reset.c: fix weak reset_board()

[Andreas Bießmann]

Dear Sebastien Carlier,

Am 04.11.2010 04:27, schrieb Sebastien Carlier:
> These weakly defined empty functions prevent the strong definition
> from being linked in.
> 
> For example, libarm.a contains a weak symbol 'red_LED_on', which is
> expected to be defined (strongly) in the board library. Because
> archive libraries are being used, this fails (testing with binutils
> 2.20.1), and only the empty __red_LED_on stub is linked in; the
> red_LED_on definition in the board library is throw away.

I have detected this issue yesterday evening too.

> This behavior is documented and it is the intended one; from
> http://www.sco.com/developers/gabi/latest/ch4.symtab.html:
> 
>> When the link editor searches archive libraries [see ``Archive
>> File'' in Chapter 7], it extracts archive members that contain
>> definitions of undefined global symbols.
>> The member's definition may be either a global or a weak symbol.
>> The link editor does not extract archive members to resolve
>> undefined weak symbols. Unresolved weak symbols have a zero value.
> 
> Empty weak definitions would have to be supplied to the linker only
> _after_ the strong definitions have been provided.

So it would be a work around to change Makefile from:

---8<---
__LIBS := $(subst $(obj),,$(LIBS)) $(subst $(obj),,$(LIBBOARD))
--->8---

to:

--->8---
__LIBS := $(subst $(obj),,$(LIBBOARD)) $(subst $(obj),,$(LIBS))
---8<---

This could work cause in most cases the strong definitions are in
$(LIBBOARD) and overload the weak functions in $(LIBS).

But this is just a work around and will not fit any use-case of weak
functions. The root cause seems to be a linker problem. But I dunno
whether it is a mis-usage or a bug. Any gcc-guys here to comment?

> Leaving undefined weak symbols and testing for NULL-ity at call sites
> seems to be a more robust approach.
> 
> Note that with some ld versions (at least with 2.20.1), ld creates PLT
> entries for undefined weak symbols and crashes when the PLT-related
> sections (.plt, .got.plt, and .rel.plt) are discarded...

This seems to be the main issue here ... but how to get it solved?

regards

Andreas Bie?mann

[U-Boot] [PATCH v2 1/2] arm920t/at91/reset.c: fix weak reset_board()

[Sebastien Carlier]

On Thu, Nov 4, 2010 at 11:54 AM, Andreas Bie?mann
<andreas.devel@googlemail.com> wrote:
>
> --->8---
> __LIBS := $(subst $(obj),,$(LIBBOARD)) $(subst $(obj),,$(LIBS))
> ---8<---

Still, why would the linker pull definitions from libboard.a?  Library
archive are only searched by the linker to resolved undefined
definitions.  Weak symbols do not count as undefined.

I think the following should work:

- In the u-boot code, forward declare (no 'weak' attribute) functions
  which are to have a default implementation.

- Keep the linking order as it is now (main u-boot libs, then board
  lib), and append a library containing all the weak definitions.

> But this is just a work around and will not fit any use-case of weak
> functions. The root cause seems to be a linker problem. But I dunno
> whether it is a mis-usage or a bug. Any gcc-guys here to comment?

I think the answer is that weak symbols are not	meant to be used with
library archives.  They work well with dynamic libraries.

>> Note that with some ld versions (at least with 2.20.1), ld creates PLT
>> entries for undefined weak symbols and crashes when the PLT-related
>> sections (.plt, .got.plt, and .rel.plt) are discarded...
>
> This seems to be the main issue here ... but how to get it solved?

I think there is no way around including the PLT.  If I understand
well, weak symbols are meant to be resolved during dynamic linking, so
the extra indirection going through the PLT is needed to alter the
address of the weak symbols when dynamic objects are loaded.

-- 
Sebastien

[U-Boot] [PATCH v3] arm920t: implement elf relocation

[Albert ARIBAUD]

Le 04/11/2010 11:15, Andreas Bie?mann a ?crit :

>> Can you give a specific case of board and toolchain that does not work?
> at91rm9200ek_ram_config (patch sent these days, is at91rm9200ek_config
> with TEXT_BASE in RAM and SKIP_LOWLEVEL_INIT set). Toolchain was a
> selfbuilt gcc-4.5.1 + binutils 2.20.1.20100303, can check CS 2010-q1
> this evening.

Thanks for the info. And yes please, check with a toolchain readily 
available to all.

> regards
>
> Andreas Bie?mann

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 2/2] arm920t: implement elf relocation

[Andreas Bießmann]

Dear Sebastien,

sorry for being late, I was quite busy last days.

Am 18.11.2010 um 11:44 schrieb Sebastien Carlier:

> Dear Andreas,
> 
> Could you please inform me of the status of your patch for ELF relocs
> for the arm920t?

I did some more changes to the posted patch. Mostely to address the 'do not fixup NULL pointer' mentioned by Joakim Tjernlund and to get a cleaner architecture (e.g. do not copy register content that often; before the decision if we need the fixup the original version does some unnecessary copies and on some other places also).

> I have been keeping my own version based on yours (see the attached
> patch based on u-boot/master) which has small fixes and cleanups and
> seems to run well here on an AT91RM9200.

Will have a look for your patch tomorrow (I'm still busy today ;(
Will send another version of my patch also tomorrow, maybe in the evening.

> U-Boot loads and relocates,
> but crashes later due to some problem with loading the initial
> environment because of a heap problem, which I am still investigating.
> 
> I am not sure what to do with my patch.  I had to copy your patch from
> an archive of the mailing list but this likely messed up whitespace.
> Can you please send me the latest version of your patch so that I can
> then send you a patch that applies cleanly on top of yours?  Also, are
> you working against u-boot, or u-boot-arm?

Normally I work on top of u-boot. There where some changes these days so I will have to rebase and see whats happened.

regards

Andreas Bie?mann