[U-Boot] ARM: problems with elf relocation on arm920t / maybe toolchain related
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
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*) } }
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ßmannandreas.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,
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ßmannandreas.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
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ßmannandreas.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,
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),
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=d1e0b10accdbac2e0a8b2cbf7c589645... for a fix to ppc that went in recently
Jocke
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=d1e0b10accdbac2e0a8b2cbf7c589645... 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
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=d1e0b10accdbac2e0a8b2cbf7c589645... 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.
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
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
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.
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
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*) } }
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*) } }
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,
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
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,
participants (5)
-
Albert ARIBAUD -
Andreas Bießmann -
Graeme Russ -
Joakim Tjernlund -
Sebastien Carlier