[PATCH 1/3] makefile: Fix symbol typo in binary_size_check
The start-of-image marker symbol is `__image_copy_start`; by searching for `_image_copy_start` instead, this check can accidentally match `_image_copy_start_ofs`.
Signed-off-by: Sam Edwards CFSworks@gmail.com --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index fb02bba08f..22bdc30109 100644 --- a/Makefile +++ b/Makefile @@ -1262,7 +1262,7 @@ OBJCOPYFLAGS_u-boot-nodtb.bin := -O binary \ binary_size_check: u-boot-nodtb.bin FORCE @file_size=$(shell wc -c u-boot-nodtb.bin | awk '{print $$1}') ; \ map_size=$(shell cat u-boot.map | \ - awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \ + awk '/__image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \ | sed 's/0X//g' \ | bc); \ if [ "" != "$$map_size" ]; then \
Signed-off-by: Sam Edwards CFSworks@gmail.com --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 22bdc30109..20785860f7 100644 --- a/Makefile +++ b/Makefile @@ -2109,7 +2109,8 @@ System.map: u-boot checkarmreloc: u-boot @RELOC="`$(CROSS_COMPILE)readelf -r -W $< | cut -d ' ' -f 4 | \ grep R_A | sort -u`"; \ - if test "$$RELOC" != "R_ARM_RELATIVE" -a \ + if test -n "$$RELOC" -a \ + "$$RELOC" != "R_ARM_RELATIVE" -a \ "$$RELOC" != "R_AARCH64_RELATIVE"; then \ echo "$< contains unexpected relocations: $$RELOC"; \ false; \
The goal of using --apply-dynamic-relocs seems to be to have all relocations applied "statically" to the output binary, so that the dynamic segment can be dropped safely. However, LLD still creates sections to support the dynamic section, and platform linker scripts may not know to discard these.
Since the build process does not appear to make use of the dynamic segment at all, it would be more sensible to suppress it entirely.
Signed-off-by: Sam Edwards CFSworks@gmail.com --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 20785860f7..d71127e412 100644 --- a/Makefile +++ b/Makefile @@ -1024,7 +1024,7 @@ LDFLAGS_u-boot += $(LDFLAGS_FINAL) LDFLAGS_u-boot += $(call ld-option, --no-dynamic-linker)
# ld.lld support -LDFLAGS_u-boot += -z notext $(call ld-option,--apply-dynamic-relocs) +LDFLAGS_u-boot += -z notext $(call ld-option,--no-pie)
LDFLAGS_u-boot += --build-id=none
On Fri, May 12, 2023 at 05:01:35PM -0600, Sam Edwards wrote:
The goal of using --apply-dynamic-relocs seems to be to have all relocations applied "statically" to the output binary, so that the dynamic segment can be dropped safely. However, LLD still creates sections to support the dynamic section, and platform linker scripts may not know to discard these.
Since the build process does not appear to make use of the dynamic segment at all, it would be more sensible to suppress it entirely.
Signed-off-by: Sam Edwards CFSworks@gmail.com
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 20785860f7..d71127e412 100644 --- a/Makefile +++ b/Makefile @@ -1024,7 +1024,7 @@ LDFLAGS_u-boot += $(LDFLAGS_FINAL) LDFLAGS_u-boot += $(call ld-option, --no-dynamic-linker)
# ld.lld support -LDFLAGS_u-boot += -z notext $(call ld-option,--apply-dynamic-relocs) +LDFLAGS_u-boot += -z notext $(call ld-option,--no-pie)
LDFLAGS_u-boot += --build-id=none
How extensively have you tested this change? We don't install ld.lld in our container environment right now (but should..) and from some local testing I forget if we need to pass further make logic in to have ld.lld be used instead.
On 5/14/23 09:28, Tom Rini wrote:
Hi Tom!
How extensively have you tested this change?
I tested it in building for the arm/sunxi target. U-Boot does not build at all (on Clang+LLD) in its current state:
ld.lld: error: section type mismatch for .gnu.version_r
<internal>:(.gnu.version_r): SHT_GNU_verneed output section .gnu: SHT_GNU_versym
However, I don't think I've reasoned correctly that --no-pie is the correct flag to pass, since I do see in arch/arm/config.mk:
# needed for relocation LDFLAGS_u-boot += -pie
...and by passing --no-pie, I am just turning that right back off, getting a non-relocatable binary. So I should (and do) withdraw this patch, and will work to understand the situation better before retrying.
It does seem that the present rule in the ARM target is to make a binary that assumes a .text base but includes some .rel.dyn relocations to make it optionally relocatable, so whatever flag we pass to LLD should preserve that behavior without bringing in unwanted sections (versym sections, .dynsym, .dynstr, ...)
We don't install ld.lld in our container environment right now (but should..) and from some local testing I forget if we need to pass further make logic in to have ld.lld be used instead.
To support both LTO and non-LTO cases, I pass: LTO_FINAL_LDFLAGS=-fuse-ld=lld LD=ld.lld
Cheers, Sam
On Fri, May 12, 2023 at 05:01:33PM -0600, Sam Edwards wrote:
The start-of-image marker symbol is `__image_copy_start`; by searching for `_image_copy_start` instead, this check can accidentally match `_image_copy_start_ofs`.
Signed-off-by: Sam Edwards CFSworks@gmail.com
Reviewed-by: Tom Rini trini@konsulko.com
participants (2)
-
Sam Edwards -
Tom Rini