[Buildroot] [PATCH 1/2] package/Makefile.in: append endianness argument to TARGET_LD for MIPS

[Buildroot] [PATCH 1/2] package/Makefile.in: append endianness argument to TARGET_LD for MIPS

[Vicente Olivert Riera]

We don't use a wrapper for ld in Buildroot like we do for gcc, so when
using ld for linking (instead of gcc) the process can fail if the
appropriate arguments are not passed to it. For instance, this happens
when building perf (Linux -> Linux Kernel Tools -> perf) for MIPS little
endian, and this is how the error message looks like:

 LD    /br/output/build/perf-custom/fd/libapi-in.o
/br/output/host/usr/bin/mips-linux-gnu-ld:
/br/output/build/perf-custom/fd/array.o: compiled for a little endian
system and target is big endian

In this case the linker is called without passing any argument to
indicate the endianness, so the default one is used, which is big endian
and therefore the process fails because the object files were built for
little endian instead.

This patch doesn't fix the problem in perf. It just appends the right
endianness argument (-EL or -EB) to the TARGET_LD variable when building
for MIPS architecture.

Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
---
 package/Makefile.in | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/package/Makefile.in b/package/Makefile.in
index dd595e2..8422b96 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -190,6 +190,19 @@ TARGET_READELF  = $(TARGET_CROSS)readelf
 TARGET_OBJCOPY  = $(TARGET_CROSS)objcopy
 TARGET_OBJDUMP  = $(TARGET_CROSS)objdump
 
+# Append the endianness argument to ld for MIPS architecture in order to
+# avoid problems like this one when using ld instead of gcc for linking:
+# mips-linux-gnu-ld: failed to merge target specific data of file foo.o
+# mips-linux-gnu-ld: foo.o: compiled for a little endian system and
+# target is big endian
+ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
+ifeq ($(BR2_ENDIAN),"BIG")
+TARGET_LD += -EB
+else
+TARGET_LD += -EL
+endif
+endif
+
 ifeq ($(BR2_STRIP_strip),y)
 STRIP_STRIP_DEBUG := --strip-debug
 STRIP_STRIP_UNNEEDED := --strip-unneeded
-- 
2.4.10

[Buildroot] [PATCH 2/2] linux/linux.mk: add LD="$(TARGET_LD)" to LINUX_MAKE_FLAGS

[Vicente Olivert Riera]

Some Linux tools like perf use ld for linking instead of gcc, and that
may cause problems on certain architectures (MIPS, for instance) since
we don't use a wrapper for ld as we do for gcc and the call to the
linker may lack some necessary arguments (which would be included if we
were using a wrapper). The problem is explained in detail below.

perf defines the value of LD on its Makefile by doing this:

$(call allow-override,LD,$(CROSS_COMPILE)ld)

So, if we don't pass an LD variable to override its value it will set
the value using the content of the CROSS_COMPILE variable and the result
will be something like <arch>-linux-gnu-ld.

This works fine for all architectures (because the value will be the
same as the TARGET_LD variable) except for MIPS, because the value of
TARGET_LD when building for MIPS also contains the argument to set the
endianness.

Due to that, perf will fail to build for MIPS little endian because the
linker (ld) will not receive any argument to set the endianness and the
default one (-EB for big endian) will be used instead. The error looks
like this:

 LD    /br/output/build/perf-custom/fd/libapi-in.o
/br/output/host/usr/bin/mips-linux-gnu-ld:
/br/output/build/perf-custom/fd/array.o: compiled for a little endian
system and target is big endian

So in order to make sure that perf will call the linker as we want
(using the value of the TARGET_LD variable), we define an LD variable
which value is the same as TARGET_LD, and we add that variable to the
LINUX_MAKE_FLAGS one. Then, since the linux-tool-perf.mk file uses that
variable, the value of LD in the perf Makefile will be overriden with
ours and the problem will be fixed.

Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
---
 linux/linux.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux/linux.mk b/linux/linux.mk
index 7e20255..7f48155 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -79,6 +79,7 @@ LINUX_MAKE_FLAGS = \
 	HOSTCC="$(HOSTCC)" \
 	HOSTCFLAGS="$(HOSTCFLAGS)" \
 	ARCH=$(KERNEL_ARCH) \
+	LD="$(TARGET_LD)" \
 	INSTALL_MOD_PATH=$(TARGET_DIR) \
 	CROSS_COMPILE="$(TARGET_CROSS)" \
 	DEPMOD=$(HOST_DIR)/sbin/depmod
-- 
2.4.10

[Buildroot] [PATCH 1/2] package/Makefile.in: append endianness argument to TARGET_LD for MIPS

[Thomas Petazzoni]

Dear Vicente Olivert Riera,

On Wed, 17 Feb 2016 17:07:53 +0000, Vicente Olivert Riera wrote:
> We don't use a wrapper for ld in Buildroot like we do for gcc, so when
> using ld for linking (instead of gcc) the process can fail if the
> appropriate arguments are not passed to it. For instance, this happens
> when building perf (Linux -> Linux Kernel Tools -> perf) for MIPS little
> endian, and this is how the error message looks like:
> 
>  LD    /br/output/build/perf-custom/fd/libapi-in.o
> /br/output/host/usr/bin/mips-linux-gnu-ld:
> /br/output/build/perf-custom/fd/array.o: compiled for a little endian
> system and target is big endian
> 
> In this case the linker is called without passing any argument to
> indicate the endianness, so the default one is used, which is big endian
> and therefore the process fails because the object files were built for
> little endian instead.
> 
> This patch doesn't fix the problem in perf. It just appends the right
> endianness argument (-EL or -EB) to the TARGET_LD variable when building
> for MIPS architecture.
> 
> Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
> ---
>  package/Makefile.in | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index dd595e2..8422b96 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -190,6 +190,19 @@ TARGET_READELF  = $(TARGET_CROSS)readelf
>  TARGET_OBJCOPY  = $(TARGET_CROSS)objcopy
>  TARGET_OBJDUMP  = $(TARGET_CROSS)objdump
>  
> +# Append the endianness argument to ld for MIPS architecture in order to
> +# avoid problems like this one when using ld instead of gcc for linking:
> +# mips-linux-gnu-ld: failed to merge target specific data of file foo.o
> +# mips-linux-gnu-ld: foo.o: compiled for a little endian system and
> +# target is big endian
> +ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
> +ifeq ($(BR2_ENDIAN),"BIG")
> +TARGET_LD += -EB
> +else
> +TARGET_LD += -EL
> +endif
> +endif

This is really an horrible fix IMO. First because TARGET_LD is only
meant to contain the path to the linker, and not additional flags.
Additional flags should go in TARGET_LDFLAGS.

And secondly because programs should not use ld directly for linking,
but gcc. Remember Vicente, you already fixed this issue in other
packages and the fix was to use gcc instead of ld.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/2] package/Makefile.in: append endianness argument to TARGET_LD for MIPS

[Arnout Vandecappelle]

On 17-02-16 22:13, Thomas Petazzoni wrote:
> Dear Vicente Olivert Riera,
> 
> On Wed, 17 Feb 2016 17:07:53 +0000, Vicente Olivert Riera wrote:
>> We don't use a wrapper for ld in Buildroot like we do for gcc, so when
>> using ld for linking (instead of gcc) the process can fail if the
>> appropriate arguments are not passed to it. For instance, this happens
>> when building perf (Linux -> Linux Kernel Tools -> perf) for MIPS little
>> endian, and this is how the error message looks like:
[snip]
>> +# Append the endianness argument to ld for MIPS architecture in order to
>> +# avoid problems like this one when using ld instead of gcc for linking:
>> +# mips-linux-gnu-ld: failed to merge target specific data of file foo.o
>> +# mips-linux-gnu-ld: foo.o: compiled for a little endian system and
>> +# target is big endian
>> +ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
>> +ifeq ($(BR2_ENDIAN),"BIG")
>> +TARGET_LD += -EB
>> +else
>> +TARGET_LD += -EL
>> +endif
>> +endif
> 
> This is really an horrible fix IMO. First because TARGET_LD is only
> meant to contain the path to the linker, and not additional flags.
> Additional flags should go in TARGET_LDFLAGS.
> 
> And secondly because programs should not use ld directly for linking,
> but gcc. Remember Vicente, you already fixed this issue in other
> packages and the fix was to use gcc instead of ld.

 So, are we really going to "fix" all packages that use ld directly? I don't see
why a package shouldn't be allowed to call ld when it is really doing linking
without any other toolchainy stuff.

 LDFLAGS has a bit of the same problem: there will probably be packages that
don't (completely) honour it.

 For CFLAGS, that was the reason why we introduced the wrapper. So I think we
should just add a wrapper for ld as well.

 Ideally the ld wrapper and the gcc wrapper should be refactored into the same
source file, but I think we could start out with separate sources and factor later.

 Any takers?


 Unless Vicente volunteers to work on an ld wrapper, I guess on the short term
we should take patches like these. That said, it should really be passed through
LDFLAGS and not LD itself.

 Regards,
 Arnout

> 
> Thomas
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/2] package/Makefile.in: append endianness argument to TARGET_LD for MIPS

[Thomas Petazzoni]

Arnout,

On Wed, 17 Feb 2016 23:41:14 +0100, Arnout Vandecappelle wrote:

>  So, are we really going to "fix" all packages that use ld directly? I don't see
> why a package shouldn't be allowed to call ld when it is really doing linking
> without any other toolchainy stuff.

Which isn't the case here. It's linking perf, i.e a normal userspace
application.

But it might indeed be difficult to convince the kernel developers to
change this, except if you have a solid argumentation.

>  LDFLAGS has a bit of the same problem: there will probably be packages that
> don't (completely) honour it.

Yes, and we should fix this IMO.

>  For CFLAGS, that was the reason why we introduced the wrapper. So I think we
> should just add a wrapper for ld as well.

Why not.

>  Unless Vicente volunteers to work on an ld wrapper, I guess on the short term
> we should take patches like these. That said, it should really be passed through
> LDFLAGS and not LD itself.

And the hack be limited to those specific packages that are broken,
IMO. At least until we realize how many packages are affected by this.
It would be annoying to put those additional flags globally just to fix
one single package.

So I would like a patch that touches only the perf build logic for now,
until we see other packages in the same situation.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/2] package/Makefile.in: append endianness argument to TARGET_LD for MIPS

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

 >> Unless Vicente volunteers to work on an ld wrapper, I guess on the short term
 >> we should take patches like these. That said, it should really be passed through
 >> LDFLAGS and not LD itself.

 > And the hack be limited to those specific packages that are broken,
 > IMO. At least until we realize how many packages are affected by this.
 > It would be annoying to put those additional flags globally just to fix
 > one single package.

As I understood it from Vicente on IRC:

- The endian flags are always required when linking
- The options are understood both when linking with gcc and ld

Which is why I suggested we do it globally.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/2] package/Makefile.in: append endianness argument to TARGET_LD for MIPS

[Vicente Olivert Riera]

Hello Peter, Thomas and Arnout,

so, do you prefer a patch to just fix perf?

The perf Makefile doesn't read an environmental LDFLAGS variable, so
adding -EL/-EB to LDFLAGS will not work.

Also, doing LD="$(TARGET_CC)" in order to use gcc for linking doesn't
work either and it fails with errors about '-lgcc_s' not found.

Are you ok with doing LD="$(TARGET_LD) -EL/-EB" only when building perf
for MIPS architecture?

Regards,

Vincent.

On 18/02/16 09:25, Peter Korsgaard wrote:
>>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> 
> Hi,
> 
>  >> Unless Vicente volunteers to work on an ld wrapper, I guess on the short term
>  >> we should take patches like these. That said, it should really be passed through
>  >> LDFLAGS and not LD itself.
> 
>  > And the hack be limited to those specific packages that are broken,
>  > IMO. At least until we realize how many packages are affected by this.
>  > It would be annoying to put those additional flags globally just to fix
>  > one single package.
> 
> As I understood it from Vicente on IRC:
> 
> - The endian flags are always required when linking
> - The options are understood both when linking with gcc and ld
> 
> Which is why I suggested we do it globally.
> 

[Buildroot] [PATCH 1/2] package/Makefile.in: append endianness argument to TARGET_LD for MIPS

[Peter Korsgaard]

>>>>> "Vicente" == Vicente Olivert Riera <Vincent.Riera@imgtec.com> writes:

 > Hello Peter, Thomas and Arnout,
 > so, do you prefer a patch to just fix perf?

 > The perf Makefile doesn't read an environmental LDFLAGS variable, so
 > adding -EL/-EB to LDFLAGS will not work.

 > Also, doing LD="$(TARGET_CC)" in order to use gcc for linking doesn't
 > work either and it fails with errors about '-lgcc_s' not found.

 > Are you ok with doing LD="$(TARGET_LD) -EL/-EB" only when building perf
 > for MIPS architecture?

Yes, for 2016.02 I think this is fine.

-- 
Venlig hilsen,
Peter Korsgaard 

[Buildroot] [PATCH 1/2] package/Makefile.in: append endianness argument to TARGET_LD for MIPS

[Jan Heylen]

On Wed, Feb 17, 2016 at 11:41 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 17-02-16 22:13, Thomas Petazzoni wrote:
>> Dear Vicente Olivert Riera,
>>
>> On Wed, 17 Feb 2016 17:07:53 +0000, Vicente Olivert Riera wrote:
>>> We don't use a wrapper for ld in Buildroot like we do for gcc, so when
>>> using ld for linking (instead of gcc) the process can fail if the
>>> appropriate arguments are not passed to it. For instance, this happens
>>> when building perf (Linux -> Linux Kernel Tools -> perf) for MIPS little
>>> endian, and this is how the error message looks like:
> [snip]
>>> +# Append the endianness argument to ld for MIPS architecture in order to
>>> +# avoid problems like this one when using ld instead of gcc for linking:
>>> +# mips-linux-gnu-ld: failed to merge target specific data of file foo.o
>>> +# mips-linux-gnu-ld: foo.o: compiled for a little endian system and
>>> +# target is big endian
>>> +ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
>>> +ifeq ($(BR2_ENDIAN),"BIG")
>>> +TARGET_LD += -EB
>>> +else
>>> +TARGET_LD += -EL
>>> +endif
>>> +endif
>>
>> This is really an horrible fix IMO. First because TARGET_LD is only
>> meant to contain the path to the linker, and not additional flags.
>> Additional flags should go in TARGET_LDFLAGS.
>>
>> And secondly because programs should not use ld directly for linking,
>> but gcc. Remember Vicente, you already fixed this issue in other
>> packages and the fix was to use gcc instead of ld.
>
>  So, are we really going to "fix" all packages that use ld directly? I don't see
> why a package shouldn't be allowed to call ld when it is really doing linking
> without any other toolchainy stuff.
>
>  LDFLAGS has a bit of the same problem: there will probably be packages that
> don't (completely) honour it.
>
>  For CFLAGS, that was the reason why we introduced the wrapper. So I think we
> should just add a wrapper for ld as well.
>
>  Ideally the ld wrapper and the gcc wrapper should be refactored into the same
> source file, but I think we could start out with separate sources and factor later.
>
>  Any takers?
>
>
>  Unless Vicente volunteers to work on an ld wrapper, I guess on the short term
> we should take patches like these. That said, it should really be passed through
> LDFLAGS and not LD itself.

Hi,

I'm following this discussion in the scope of this 'ld wrapper' in
toolchain-wrapper, as there are still other packages out there that
have this issue (e.g. Dmalloc using ld directly). Without proper
knowledge on what is the 'correct' way I saw also following (old)
comment I wanted to point to:

From: http://lists.busybox.net/pipermail/buildroot/2013-june/073430.html
On 6 June 2013, Markos Chandras <hwoarang@gentoo.org> wrote:
[snip]
> I am  inclined to say that binutils do not need to be changed, because
> you should never call the linker directly to do the linking (I recall
> directfb does that and fails with the same problem). You should be
> using gcc instead and it will pass all the appropriate linker flags to
> the linker. Such build systems need fixing instead of trying to
> workaround the problem in the toolchain.
>
> Having said that, the entire patchset [heyleke: topic was ld-toolchain-wrapper] could be seen as a temporary
> workaround for this problem. In the libiscsi case, I need to
> understand why libtool invokes the linker without the correct flags.
> If you try to compile a simple test program using the buildroot
> mip64-linux-gcc toolchain (pass -v as well) you will  notice that gcc
> passes -melf64btsmip correctly.

That  being said and pointed to, there is this old patchset from
Thomas Petazzoni on toolchain-wrapper for 'ld'. The  main principles
are not changes since the original patch, but toolchain-wrapper was
moved and other patches were added later, so I had to rework it a bit,
and tested it, it fixes the issue for mips and dmalloc package, the
original patch and discussion:
Http://thread.gmane.org/gmane.comp.lib.uclibc.buildroot/60942

Although it was reviewed back then, it looks like it never made into
the buildroot repository. As said, I've rebased and reworked it a bit,
I'll put it on the mailing list. But as said, I'm not saying it is the
'correct' or 'incorrect' way of fixing this mips-ld stuff, but it does
fix it for the way dmalloc's buildsystem is constructed (using ld
directly).

Best regards,

Jan

>
>  Regards,
>  Arnout
>
>>
>> Thomas
>>
>
>
> --
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot