[U-Boot] [PATCH] Makefile: Fix linking with modern binutils

[U-Boot] [PATCH] Makefile: Fix linking with modern binutils

[Joel Stanley]

Since Binutils 1a9ccd70f9a7[1] u-boot will not link targets that set
CONFIG_SYS_TEXT_BASE=0 with the following error:

  LD      u-boot
arm-linux-gnueabi-ld.bfd: u-boot: Not enough room for program headers, try
 linking with -N
arm-linux-gnueabi-ld.bfd: final link failed: Bad value

The issue can reproduced the bad binutils and the rock2_defconfig target.

This issue was also encountered by the powerpc kernel[2], with the fix
being to pass --no-dynamic-linker for linkers newer than 2.26 when this
flag was introduced. While version 2.26 and 2.27 will not show this
error, we are technically creating invalid ELF files without this
option.

Ubuntu Zesty's Binutils 2.27.51.20161202 hits this error.

[1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=1a9ccd70f9a7
[2] https://git.kernel.org/cgit/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=ff45000fcb56b5b0f1a14a865d3541746d838a0a

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 Makefile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index dfed58bec661..0bfd57d81646 100644
--- a/Makefile
+++ b/Makefile
@@ -792,7 +792,11 @@ ifneq ($(CONFIG_BUILD_TARGET),)
 ALL-y += $(CONFIG_BUILD_TARGET:"%"=%)
 endif
 
-LDFLAGS_u-boot += $(LDFLAGS_FINAL)
+# Avoid Not enough room for program headers on binutils 2.28 onwards.
+# Flag was introduced in 2.26
+LDFLAGS_u-boot += $(LDFLAGS_FINAL) \
+	$(call ld-ifversion, -ge, 22600000, --no-dynamic-linker)
+
 ifneq ($(CONFIG_SYS_TEXT_BASE),)
 LDFLAGS_u-boot += -Ttext $(CONFIG_SYS_TEXT_BASE)
 endif
-- 
2.11.0

[U-Boot] [PATCH] Makefile: Fix linking with modern binutils

[Alan Modra]

On Thu, Dec 08, 2016 at 02:22:51PM +1100, Joel Stanley wrote:
> error, we are technically creating invalid ELF files without this
> option.

I don't think you were creating invalid ELF files, just unusual ones.

Anyway, passing --no-dynamic-linker is a good idea.  That tells ld
that the PIE or shared lib that you're creating does not need loaded
program headers.

-- 
Alan Modra
Australia Development Lab, IBM

[U-Boot] [PATCH] Makefile: Fix linking with modern binutils

[Simon Glass]

On 7 December 2016 at 22:22, Joel Stanley <joel@jms.id.au> wrote:
> Since Binutils 1a9ccd70f9a7[1] u-boot will not link targets that set
> CONFIG_SYS_TEXT_BASE=0 with the following error:
>
>   LD      u-boot
> arm-linux-gnueabi-ld.bfd: u-boot: Not enough room for program headers, try
>  linking with -N
> arm-linux-gnueabi-ld.bfd: final link failed: Bad value
>
> The issue can reproduced the bad binutils and the rock2_defconfig target.
>
> This issue was also encountered by the powerpc kernel[2], with the fix
> being to pass --no-dynamic-linker for linkers newer than 2.26 when this
> flag was introduced. While version 2.26 and 2.27 will not show this
> error, we are technically creating invalid ELF files without this
> option.
>
> Ubuntu Zesty's Binutils 2.27.51.20161202 hits this error.
>
> [1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=1a9ccd70f9a7
> [2] https://git.kernel.org/cgit/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=ff45000fcb56b5b0f1a14a865d3541746d838a0a
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  Makefile | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] Makefile: Fix linking with modern binutils

[Tom Rini]

On Thu, Dec 08, 2016 at 02:22:51PM +1100, Joel Stanley wrote:

> Since Binutils 1a9ccd70f9a7[1] u-boot will not link targets that set
> CONFIG_SYS_TEXT_BASE=0 with the following error:
> 
>   LD      u-boot
> arm-linux-gnueabi-ld.bfd: u-boot: Not enough room for program headers, try
>  linking with -N
> arm-linux-gnueabi-ld.bfd: final link failed: Bad value
> 
> The issue can reproduced the bad binutils and the rock2_defconfig target.
> 
> This issue was also encountered by the powerpc kernel[2], with the fix
> being to pass --no-dynamic-linker for linkers newer than 2.26 when this
> flag was introduced. While version 2.26 and 2.27 will not show this
> error, we are technically creating invalid ELF files without this
> option.
> 
> Ubuntu Zesty's Binutils 2.27.51.20161202 hits this error.
> 
> [1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=1a9ccd70f9a7
> [2] https://git.kernel.org/cgit/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=ff45000fcb56b5b0f1a14a865d3541746d838a0a
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>  Makefile | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index dfed58bec661..0bfd57d81646 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -792,7 +792,11 @@ ifneq ($(CONFIG_BUILD_TARGET),)
>  ALL-y += $(CONFIG_BUILD_TARGET:"%"=%)
>  endif
>  
> -LDFLAGS_u-boot += $(LDFLAGS_FINAL)
> +# Avoid Not enough room for program headers on binutils 2.28 onwards.
> +# Flag was introduced in 2.26
> +LDFLAGS_u-boot += $(LDFLAGS_FINAL) \
> +	$(call ld-ifversion, -ge, 22600000, --no-dynamic-linker)

This breaks on things like:
$ arm-none-eabi-ld --version
GNU ld (GNU Tools for ARM Embedded Processors) 2.24.0.20150921

so please use ld-option instead to add this when supported, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161209/6470de6b/attachment.sig>

[U-Boot] Makefile: Fix linking with modern binutils

[Joel Stanley]

Hello Tom,

On Sat, Dec 10, 2016 at 12:53 AM, Tom Rini <trini@konsulko.com> wrote:
>> -LDFLAGS_u-boot += $(LDFLAGS_FINAL)
>> +# Avoid Not enough room for program headers on binutils 2.28 onwards.
>> +# Flag was introduced in 2.26
>> +LDFLAGS_u-boot += $(LDFLAGS_FINAL) \
>> +     $(call ld-ifversion, -ge, 22600000, --no-dynamic-linker)
>
> This breaks on things like:
> $ arm-none-eabi-ld --version
> GNU ld (GNU Tools for ARM Embedded Processors) 2.24.0.20150921

The flag will only be added when the version is >= 2.26. Which part of
that version string will break the test?

> so please use ld-option instead to add this when supported, thanks!

If you'd just prefer ld-option over ld-ifversion then I can send a
different patch.

Cheers,

Joel

[U-Boot] Makefile: Fix linking with modern binutils

[Tom Rini]

On Tue, Dec 13, 2016 at 03:31:24PM +1030, Joel Stanley wrote:
> Hello Tom,
> 
> On Sat, Dec 10, 2016 at 12:53 AM, Tom Rini <trini@konsulko.com> wrote:
> >> -LDFLAGS_u-boot += $(LDFLAGS_FINAL)
> >> +# Avoid Not enough room for program headers on binutils 2.28 onwards.
> >> +# Flag was introduced in 2.26
> >> +LDFLAGS_u-boot += $(LDFLAGS_FINAL) \
> >> +     $(call ld-ifversion, -ge, 22600000, --no-dynamic-linker)
> >
> > This breaks on things like:
> > $ arm-none-eabi-ld --version
> > GNU ld (GNU Tools for ARM Embedded Processors) 2.24.0.20150921
> 
> The flag will only be added when the version is >= 2.26. Which part of
> that version string will break the test?

The whole of the string breaks the test.  It comes out to 2037492100, is
greater than is true, and tries to pass the flag which isn't know about
in that version and fails.  Note that we're slightly behind the Linux
kernel version of the ld-version script, but dropping that one in (which
does have a bugfix) gives 224000000 for the version.  Perhaps it would
give 226000000 or similar instead for what you've got and we need to
update ld-version.sh in order to use this patch as-is?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161213/e7a06ede/attachment.sig>

[U-Boot] Makefile: Fix linking with modern binutils

[Måns Rullgård]

Joel Stanley <joel@jms.id.au> writes:

> Hello Tom,
>
> On Sat, Dec 10, 2016 at 12:53 AM, Tom Rini <trini@konsulko.com> wrote:
>>> -LDFLAGS_u-boot += $(LDFLAGS_FINAL)
>>> +# Avoid Not enough room for program headers on binutils 2.28 onwards.
>>> +# Flag was introduced in 2.26
>>> +LDFLAGS_u-boot += $(LDFLAGS_FINAL) \
>>> +     $(call ld-ifversion, -ge, 22600000, --no-dynamic-linker)
>>
>> This breaks on things like:
>> $ arm-none-eabi-ld --version
>> GNU ld (GNU Tools for ARM Embedded Processors) 2.24.0.20150921
>
> The flag will only be added when the version is >= 2.26. Which part of
> that version string will break the test?
>
>> so please use ld-option instead to add this when supported, thanks!
>
> If you'd just prefer ld-option over ld-ifversion then I can send a
> different patch.

Always test for the actual thing you need whenever possible.  Version
checks are fragile and should be a last resort.  Consider what would
happen if someone backported that functionality to an older binutils
version.  Going by version number alone, the flag would not be used and
the build would fail.

-- 
M?ns Rullg?rd

[U-Boot] Makefile: Fix linking with modern binutils

[Joel Stanley]

On Tue, Dec 13, 2016 at 10:51 PM, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Dec 13, 2016 at 03:31:24PM +1030, Joel Stanley wrote:
>> Hello Tom,
>>
>> On Sat, Dec 10, 2016 at 12:53 AM, Tom Rini <trini@konsulko.com> wrote:
>> >> -LDFLAGS_u-boot += $(LDFLAGS_FINAL)
>> >> +# Avoid Not enough room for program headers on binutils 2.28 onwards.
>> >> +# Flag was introduced in 2.26
>> >> +LDFLAGS_u-boot += $(LDFLAGS_FINAL) \
>> >> +     $(call ld-ifversion, -ge, 22600000, --no-dynamic-linker)
>> >
>> > This breaks on things like:
>> > $ arm-none-eabi-ld --version
>> > GNU ld (GNU Tools for ARM Embedded Processors) 2.24.0.20150921
>>
>> The flag will only be added when the version is >= 2.26. Which part of
>> that version string will break the test?
>
> The whole of the string breaks the test.  It comes out to 2037492100, is
> greater than is true, and tries to pass the flag which isn't know about
> in that version and fails.  Note that we're slightly behind the Linux
> kernel version of the ld-version script, but dropping that one in (which
> does have a bugfix) gives 224000000 for the version.  Perhaps it would
> give 226000000 or similar instead for what you've got and we need to
> update ld-version.sh in order to use this patch as-is?

Thanks for pointing that out. I don't have an old enough binutils
around to test that case.

You're correct; it does give 226000000 with the version of
ld-version.sh in the kernel tree. I recommend updating the u-boot copy
of the scripts to avoid this mistake in the future.

I will instead send a patch that tests for ld-option as M?ns suggested.

Cheers,

Joel