[PATCH 1/3] makefile: Fix symbol typo in binary_size_check

[PATCH 1/3] makefile: Fix symbol typo in binary_size_check

[Sam Edwards]

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 \
-- 
2.39.2

[PATCH 2/3] makefile: Consider "no relocs" to be "no unexpected relocs"

[Sam Edwards]

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; \
-- 
2.39.2

[PATCH 3/3] clang: Link with --no-pie instead of --apply-dynamic-relocs

[Sam Edwards]

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
 
-- 
2.39.2

Re: [PATCH 1/3] makefile: Fix symbol typo in binary_size_check

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 349 bytes --]

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>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 3/3] clang: Link with --no-pie instead of --apply-dynamic-relocs

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 3/3] clang: Link with --no-pie instead of --apply-dynamic-relocs

[Sam Edwards]

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