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

2016-12-15 Thread Joel Stanley 
On Tue, Dec 13, 2016 at 10:51 PM, Tom Rini  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  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, 2260, --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 22400 for the version.  Perhaps it would
> give 22600 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 22600 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-12-13 Thread Måns Rullgård 
Joel Stanley  writes:

> Hello Tom,
>
> On Sat, Dec 10, 2016 at 12:53 AM, Tom Rini  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, 2260, --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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-12-13 Thread 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  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, 2260, --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 22400 for the version.  Perhaps it would
give 22600 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


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-12-13 Thread Joel Stanley 
Hello Tom,

On Sat, Dec 10, 2016 at 12:53 AM, Tom Rini  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, 2260, --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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-12-09 Thread 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=ff45000fcb56b5b0f1a14a865d3541746d838a0a
> 
> Signed-off-by: Joel Stanley 
> Reviewed-by: Simon Glass 
> ---
>  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, 2260, --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


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot