Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified
@ 2013-06-05 21:59 Thomas Petazzoni
  2013-06-05 21:59 ` [Buildroot] [PATCH 2/2] arch: define appropriate ld emulation values for the MIPS architecture Thomas Petazzoni
  2013-06-06  8:31 ` [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified Markos Chandras
  0 siblings, 2 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2013-06-05 21:59 UTC (permalink / raw)
  To: buildroot

On some architectures, such as MIPS, the linker supports several 'ld
emulation', and depending on which flavor of the architecture you're
building for, one should be passing the proper -m <something> option
to ld, otherwise ld defaults to the default emulation, which may not
necessarily be compatible with the object files that are being
linked. See for example the following build failure:

  http://autobuild.buildroot.org/results/0e1/0e18e02e01148afb36acd2293b3fdc56c6bb8413/build-end.log

So, we extend the toolchain wrapper to also wrap the ld linker, and
allow a BR_LD_EMULATION variable to be passed to it.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/Config.in                                     |  3 +++
 toolchain/toolchain-external/ext-tool.mk           |  6 ++++-
 .../toolchain-external/ext-toolchain-wrapper.c     | 26 +++++++++++++++++-----
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/arch/Config.in b/arch/Config.in
index 5ca05cd..aab9922 100644
--- a/arch/Config.in
+++ b/arch/Config.in
@@ -192,6 +192,9 @@ config BR2_GCC_TARGET_CPU
 config BR2_GCC_TARGET_CPU_REVISION
 	string
 
+config BR2_LD_TARGET_EMULATION
+	string
+
 # Set up target binary format
 choice
 	prompt "Target Binary Format"
diff --git a/toolchain/toolchain-external/ext-tool.mk b/toolchain/toolchain-external/ext-tool.mk
index 93b3b15..c132e6a 100644
--- a/toolchain/toolchain-external/ext-tool.mk
+++ b/toolchain/toolchain-external/ext-tool.mk
@@ -141,6 +141,7 @@ CC_TARGET_CPU_:=$(call qstrip,$(BR2_GCC_TARGET_CPU)-$(BR2_GCC_TARGET_CPU_REVISIO
 endif
 CC_TARGET_ARCH_:=$(call qstrip,$(BR2_GCC_TARGET_ARCH))
 CC_TARGET_ABI_:=$(call qstrip,$(BR2_GCC_TARGET_ABI))
+LD_TARGET_EMULATION_:=$(call qstrip,$(BR2_LD_TARGET_EMULATION))
 
 # march/mtune/floating point mode needs to be passed to the external toolchain
 # to select the right multilib variant
@@ -164,6 +165,9 @@ ifneq ($(CC_TARGET_ABI_),)
 TOOLCHAIN_EXTERNAL_CFLAGS += -mabi=$(CC_TARGET_ABI_)
 TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_ABI='"$(CC_TARGET_ABI_)"'
 endif
+ifneq ($(LD_TARGET_EMULATION_),)
+TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_LD_EMULATION='"$(LD_TARGET_EMULATION_)"'
+endif
 ifeq ($(BR2_BINFMT_FLAT),y)
 TOOLCHAIN_EXTERNAL_CFLAGS += -Wl,-elf2flt
 TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_BINFMT_FLAT
@@ -468,7 +472,7 @@ $(HOST_DIR)/usr/bin/ext-toolchain-wrapper: $(STAMP_DIR)/ext-toolchain-installed
 	for i in $(TOOLCHAIN_EXTERNAL_CROSS)*; do \
 		base=$${i##*/}; \
 		case "$$base" in \
-		*cc|*cc-*|*++|*++-*|*cpp) \
+		*cc|*cc-*|*++|*++-*|*cpp|*ld) \
 			ln -sf $(@F) $$base; \
 			;; \
 		*) \
diff --git a/toolchain/toolchain-external/ext-toolchain-wrapper.c b/toolchain/toolchain-external/ext-toolchain-wrapper.c
index 9d79d68..e2b945e 100644
--- a/toolchain/toolchain-external/ext-toolchain-wrapper.c
+++ b/toolchain/toolchain-external/ext-toolchain-wrapper.c
@@ -23,7 +23,7 @@
 static char path[PATH_MAX];
 static char sysroot[PATH_MAX];
 
-static char *predef_args[] = {
+static char *gcc_predef_args[] = {
 	path,
 	"--sysroot", sysroot,
 #ifdef BR_ARCH
@@ -55,13 +55,21 @@ static char *predef_args[] = {
 #endif
 };
 
+static char *ld_predef_args[] = {
+	path,
+#ifdef BR_LD_EMULATION
+	"-m", BR_LD_EMULATION,
+#endif
+};
+
 int main(int argc, char **argv)
 {
 	char **args, **cur;
 	char *relbasedir, *absbasedir;
 	char *progpath = argv[0];
 	char *basename;
-	int ret, i, count = 0;
+	char **predef_args;
+	int ret, i, predef_args_sz, count = 0;
 
 	/* Calculate the relative paths */
 	basename = strrchr(progpath, '/');
@@ -113,15 +121,23 @@ int main(int argc, char **argv)
 		return 3;
 	}
 
-	cur = args = malloc(sizeof(predef_args) + (sizeof(char *) * argc));
+	if (!strncmp("-ld", basename + strlen(basename) - 3, 3)) {
+		predef_args_sz = sizeof(ld_predef_args);
+		predef_args = ld_predef_args;
+	} else {
+		predef_args_sz = sizeof(gcc_predef_args);
+		predef_args = gcc_predef_args;
+	}
+
+	cur = args = malloc(predef_args_sz + (sizeof(char *) * argc));
 	if (args == NULL) {
 		perror(__FILE__ ": malloc");
 		return 2;
 	}
 
 	/* start with predefined args */
-	memcpy(cur, predef_args, sizeof(predef_args));
-	cur += sizeof(predef_args) / sizeof(predef_args[0]);
+	memcpy(cur, predef_args, predef_args_sz);
+	cur += predef_args_sz / sizeof(char *);
 
 	/* append forward args */
 	memcpy(cur, &argv[1], sizeof(char *) * (argc - 1));
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 2/2] arch: define appropriate ld emulation values for the MIPS architecture
  2013-06-05 21:59 [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified Thomas Petazzoni
@ 2013-06-05 21:59 ` Thomas Petazzoni
  2013-06-06  8:37   ` Markos Chandras
  2013-06-06  8:31 ` [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified Markos Chandras
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Petazzoni @ 2013-06-05 21:59 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/Config.in.mips | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/Config.in.mips b/arch/Config.in.mips
index 1454fb4..7f134f8 100644
--- a/arch/Config.in.mips
+++ b/arch/Config.in.mips
@@ -76,3 +76,11 @@ config BR2_GCC_TARGET_ABI
 	default "32"		if BR2_MIPS_OABI32
 	default "n32"		if BR2_MIPS_NABI32
 	default "64"		if BR2_MIPS_NABI64
+
+config BR2_LD_TARGET_EMULATION
+	default "elf64ltsmip"	   if BR2_mips64el
+	default "elf64btsmip"	   if BR2_mips64
+	default "elf32ltsmip"	   if BR2_mipsel && !BR2_MIPS_NABI32
+	default "elf32btsmip"	   if BR2_mips && !BR2_MIPS_NABI32
+	default "elf32ltsmipn32"   if BR2_mipsel && BR2_MIPS_NABI32
+	default "elf32btsmipn32"   if BR2_mips && BR2_MIPS_NABI32
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified
  2013-06-05 21:59 [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified Thomas Petazzoni
  2013-06-05 21:59 ` [Buildroot] [PATCH 2/2] arch: define appropriate ld emulation values for the MIPS architecture Thomas Petazzoni
@ 2013-06-06  8:31 ` Markos Chandras
  2013-06-06  8:51   ` Thomas Petazzoni
  1 sibling, 1 reply; 14+ messages in thread
From: Markos Chandras @ 2013-06-06  8:31 UTC (permalink / raw)
  To: buildroot

On 5 June 2013 22:59, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> On some architectures, such as MIPS, the linker supports several 'ld
> emulation', and depending on which flavor of the architecture you're
> building for, one should be passing the proper -m <something> option
> to ld, otherwise ld defaults to the default emulation, which may not
> necessarily be compatible with the object files that are being
> linked. See for example the following build failure:
>
>   http://autobuild.buildroot.org/results/0e1/0e18e02e01148afb36acd2293b3fdc56c6bb8413/build-end.log
>
> So, we extend the toolchain wrapper to also wrap the ld linker, and
> allow a BR_LD_EMULATION variable to be passed to it.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/Config.in                                     |  3 +++
>  toolchain/toolchain-external/ext-tool.mk           |  6 ++++-
>  .../toolchain-external/ext-toolchain-wrapper.c     | 26 +++++++++++++++++-----
>  3 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/arch/Config.in b/arch/Config.in
> index 5ca05cd..aab9922 100644
> --- a/arch/Config.in
> +++ b/arch/Config.in
> @@ -192,6 +192,9 @@ config BR2_GCC_TARGET_CPU
>  config BR2_GCC_TARGET_CPU_REVISION
>         string
>
> +config BR2_LD_TARGET_EMULATION
> +       string
> +
>  # Set up target binary format
>  choice
>         prompt "Target Binary Format"
> diff --git a/toolchain/toolchain-external/ext-tool.mk b/toolchain/toolchain-external/ext-tool.mk
> index 93b3b15..c132e6a 100644
> --- a/toolchain/toolchain-external/ext-tool.mk
> +++ b/toolchain/toolchain-external/ext-tool.mk
> @@ -141,6 +141,7 @@ CC_TARGET_CPU_:=$(call qstrip,$(BR2_GCC_TARGET_CPU)-$(BR2_GCC_TARGET_CPU_REVISIO
>  endif
>  CC_TARGET_ARCH_:=$(call qstrip,$(BR2_GCC_TARGET_ARCH))
>  CC_TARGET_ABI_:=$(call qstrip,$(BR2_GCC_TARGET_ABI))
> +LD_TARGET_EMULATION_:=$(call qstrip,$(BR2_LD_TARGET_EMULATION))
>
>  # march/mtune/floating point mode needs to be passed to the external toolchain
>  # to select the right multilib variant
> @@ -164,6 +165,9 @@ ifneq ($(CC_TARGET_ABI_),)
>  TOOLCHAIN_EXTERNAL_CFLAGS += -mabi=$(CC_TARGET_ABI_)
>  TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_ABI='"$(CC_TARGET_ABI_)"'
>  endif
> +ifneq ($(LD_TARGET_EMULATION_),)
> +TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_LD_EMULATION='"$(LD_TARGET_EMULATION_)"'
> +endif
>  ifeq ($(BR2_BINFMT_FLAT),y)
>  TOOLCHAIN_EXTERNAL_CFLAGS += -Wl,-elf2flt
>  TOOLCHAIN_EXTERNAL_WRAPPER_ARGS += -DBR_BINFMT_FLAT
> @@ -468,7 +472,7 @@ $(HOST_DIR)/usr/bin/ext-toolchain-wrapper: $(STAMP_DIR)/ext-toolchain-installed
>         for i in $(TOOLCHAIN_EXTERNAL_CROSS)*; do \
>                 base=$${i##*/}; \
>                 case "$$base" in \
> -               *cc|*cc-*|*++|*++-*|*cpp) \
> +               *cc|*cc-*|*++|*++-*|*cpp|*ld) \
>                         ln -sf $(@F) $$base; \
>                         ;; \
>                 *) \
> diff --git a/toolchain/toolchain-external/ext-toolchain-wrapper.c b/toolchain/toolchain-external/ext-toolchain-wrapper.c
> index 9d79d68..e2b945e 100644
> --- a/toolchain/toolchain-external/ext-toolchain-wrapper.c
> +++ b/toolchain/toolchain-external/ext-toolchain-wrapper.c
> @@ -23,7 +23,7 @@
>  static char path[PATH_MAX];
>  static char sysroot[PATH_MAX];
>
> -static char *predef_args[] = {
> +static char *gcc_predef_args[] = {
>         path,
>         "--sysroot", sysroot,
>  #ifdef BR_ARCH
> @@ -55,13 +55,21 @@ static char *predef_args[] = {
>  #endif
>  };
>
> +static char *ld_predef_args[] = {
> +       path,
> +#ifdef BR_LD_EMULATION
> +       "-m", BR_LD_EMULATION,
> +#endif
> +};
> +
>  int main(int argc, char **argv)
>  {
>         char **args, **cur;
>         char *relbasedir, *absbasedir;
>         char *progpath = argv[0];
>         char *basename;
> -       int ret, i, count = 0;
> +       char **predef_args;
> +       int ret, i, predef_args_sz, count = 0;
>
>         /* Calculate the relative paths */
>         basename = strrchr(progpath, '/');
> @@ -113,15 +121,23 @@ int main(int argc, char **argv)
>                 return 3;
>         }
>
> -       cur = args = malloc(sizeof(predef_args) + (sizeof(char *) * argc));
> +       if (!strncmp("-ld", basename + strlen(basename) - 3, 3)) {
> +               predef_args_sz = sizeof(ld_predef_args);
> +               predef_args = ld_predef_args;
> +       } else {
> +               predef_args_sz = sizeof(gcc_predef_args);
> +               predef_args = gcc_predef_args;
> +       }
> +
> +       cur = args = malloc(predef_args_sz + (sizeof(char *) * argc));
>         if (args == NULL) {
>                 perror(__FILE__ ": malloc");
>                 return 2;
>         }
>
>         /* start with predefined args */
> -       memcpy(cur, predef_args, sizeof(predef_args));
> -       cur += sizeof(predef_args) / sizeof(predef_args[0]);
> +       memcpy(cur, predef_args, predef_args_sz);
> +       cur += predef_args_sz / sizeof(char *);
>
>         /* append forward args */
>         memcpy(cur, &argv[1], sizeof(char *) * (argc - 1));
> --
> 1.8.1.2
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Hi Thomas,

My understanding is that this is also a problem with buildroot
toolchains as well. So whilst your patch fixes the problem with
external
toolchains, MIPS64/n64 buildroot toolchains will still have the same problem.

--
Regards,
Markos Chandras - Gentoo Linux Developer
http://dev.gentoo.org/~hwoarang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 2/2] arch: define appropriate ld emulation values for the MIPS architecture
  2013-06-05 21:59 ` [Buildroot] [PATCH 2/2] arch: define appropriate ld emulation values for the MIPS architecture Thomas Petazzoni
@ 2013-06-06  8:37   ` Markos Chandras
  2013-06-06  8:57     ` Thomas Petazzoni
  0 siblings, 1 reply; 14+ messages in thread
From: Markos Chandras @ 2013-06-06  8:37 UTC (permalink / raw)
  To: buildroot

On 5 June 2013 22:59, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/Config.in.mips | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/Config.in.mips b/arch/Config.in.mips
> index 1454fb4..7f134f8 100644
> --- a/arch/Config.in.mips
> +++ b/arch/Config.in.mips
> @@ -76,3 +76,11 @@ config BR2_GCC_TARGET_ABI
>         default "32"            if BR2_MIPS_OABI32
>         default "n32"           if BR2_MIPS_NABI32
>         default "64"            if BR2_MIPS_NABI64
> +
> +config BR2_LD_TARGET_EMULATION
> +       default "elf64ltsmip"      if BR2_mips64el
> +       default "elf64btsmip"      if BR2_mips64
> +       default "elf32ltsmip"      if BR2_mipsel && !BR2_MIPS_NABI32
> +       default "elf32btsmip"      if BR2_mips && !BR2_MIPS_NABI32
> +       default "elf32ltsmipn32"   if BR2_mipsel && BR2_MIPS_NABI32
> +       default "elf32btsmipn32"   if BR2_mips && BR2_MIPS_NABI32
> --
> 1.8.1.2
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Hi Thomas,

binutils set el32{l,b}smipn32 as default ABI for MIPS64 as well. The
elf64{l,b}tsmip one need to be used only if you want to use the n64
ABI. My opinion is that this patch needs to be changed to something
like this:

 +config BR2_LD_TARGET_EMULATION
 +       default "elf64ltsmip"      if BR2_mips64el && BR2_MIPS_NABI64
 +       default "elf64btsmip"      if BR2_mips64 && BR2_MIPS_NABI64
 +       default "elf32ltsmip"      if BR2_mipsel && !BR2_MIPS_NABI32
 +       default "elf32btsmip"      if BR2_mips && !BR2_MIPS_NABI32
 +       default "elf32ltsmipn32"   if BR2_mipsel && BR2_MIPS_NABI32
 +       default "elf32btsmipn32"   if BR2_mips && BR2_MIPS_NABI32


--
Regards,
Markos Chandras - Gentoo Linux Developer
http://dev.gentoo.org/~hwoarang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified
  2013-06-06  8:31 ` [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified Markos Chandras
@ 2013-06-06  8:51   ` Thomas Petazzoni
  2013-06-06  8:56     ` Markos Chandras
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni @ 2013-06-06  8:51 UTC (permalink / raw)
  To: buildroot

Dear Markos Chandras,

On Thu, 6 Jun 2013 09:31:25 +0100, Markos Chandras wrote:

> My understanding is that this is also a problem with buildroot
> toolchains as well. So whilst your patch fixes the problem with
> external
> toolchains, MIPS64/n64 buildroot toolchains will still have the same
> problem.

Yes, I do remember your patch
http://patchwork.ozlabs.org/patch/244694/. Normally, for internal
toolchains, the idea is that the tools (gcc, ld) are configured, at
build time, to produce the 'right' code by default, without requiring
any tuning. The man page of 'ld' says:

       -m emulation
           Emulate the emulation linker.  You can list the available
           emulations with the --verbose or -V options.

           If the -m option is not used, the emulation is taken from the
           "LDEMULATION" environment variable, if that is defined.

           Otherwise, the default emulation depends upon how the linker
           was configured.

The last paragraph being the important one here. See what OpenEmbedded
is doing:
https://github.com/openembedded/oe-core/blob/master/meta/recipes-devtools/binutils/binutils-2.23.2/mips64-default-ld-emulation.patch.

The problem here is that the MIPS tuples do not contain the ABI, so the
ld/configure.tgt and bfd/config.bfd scripts of binutils have no way of
knowing which emulation you would like to have by default. Maybe we
need to improve binutils to make this configurable?

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified
  2013-06-06  8:51   ` Thomas Petazzoni
@ 2013-06-06  8:56     ` Markos Chandras
  2013-06-06  9:13       ` Markos Chandras
  0 siblings, 1 reply; 14+ messages in thread
From: Markos Chandras @ 2013-06-06  8:56 UTC (permalink / raw)
  To: buildroot

On 6 June 2013 09:51, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Markos Chandras,
>
> On Thu, 6 Jun 2013 09:31:25 +0100, Markos Chandras wrote:
>
>> My understanding is that this is also a problem with buildroot
>> toolchains as well. So whilst your patch fixes the problem with
>> external
>> toolchains, MIPS64/n64 buildroot toolchains will still have the same
>> problem.
>
> Yes, I do remember your patch
> http://patchwork.ozlabs.org/patch/244694/. Normally, for internal
> toolchains, the idea is that the tools (gcc, ld) are configured, at
> build time, to produce the 'right' code by default, without requiring
> any tuning. The man page of 'ld' says:
>
>        -m emulation
>            Emulate the emulation linker.  You can list the available
>            emulations with the --verbose or -V options.
>
>            If the -m option is not used, the emulation is taken from the
>            "LDEMULATION" environment variable, if that is defined.
>
>            Otherwise, the default emulation depends upon how the linker
>            was configured.
>
> The last paragraph being the important one here. See what OpenEmbedded
> is doing:
> https://github.com/openembedded/oe-core/blob/master/meta/recipes-devtools/binutils/binutils-2.23.2/mips64-default-ld-emulation.patch.
>
> The problem here is that the MIPS tuples do not contain the ABI, so the
> ld/configure.tgt and bfd/config.bfd scripts of binutils have no way of
> knowing which emulation you would like to have by default. Maybe we
> need to improve binutils to make this configurable?
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
>

Hi Thomas,

Yes I've seen this patch before but given this is not in the upstream
binutils code I had to "workaround" it by using the exported
LDEMULATION variable. It might worth taking this to the binutils
mailing list.

--
Regards,
Markos Chandras - Gentoo Linux Developer
http://dev.gentoo.org/~hwoarang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 2/2] arch: define appropriate ld emulation values for the MIPS architecture
  2013-06-06  8:37   ` Markos Chandras
@ 2013-06-06  8:57     ` Thomas Petazzoni
  2013-06-06  9:04       ` Markos Chandras
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni @ 2013-06-06  8:57 UTC (permalink / raw)
  To: buildroot

Dear Markos Chandras,

On Thu, 6 Jun 2013 09:37:41 +0100, Markos Chandras wrote:

> binutils set el32{l,b}smipn32 as default ABI for MIPS64 as well. The
> elf64{l,b}tsmip one need to be used only if you want to use the n64
> ABI. My opinion is that this patch needs to be changed to something
> like this:
> 
>  +config BR2_LD_TARGET_EMULATION
>  +       default "elf64ltsmip"      if BR2_mips64el && BR2_MIPS_NABI64
>  +       default "elf64btsmip"      if BR2_mips64 && BR2_MIPS_NABI64
>  +       default "elf32ltsmip"      if BR2_mipsel && !BR2_MIPS_NABI32
>  +       default "elf32btsmip"      if BR2_mips && !BR2_MIPS_NABI32
>  +       default "elf32ltsmipn32"   if BR2_mipsel && BR2_MIPS_NABI32
>  +       default "elf32btsmipn32"   if BR2_mips && BR2_MIPS_NABI32

Ok, but I'm not too happy with the fact that the BR2_mips64[el]
&& !BR2_MIPS_NABI64 is not being handled here. And according to
arch/Config.in.mips, in fact the n32 ABI does not make sense on 32 bits
BR2_mips and BR2_mipsel.

So, shouldn't this thing be:

config BR2_LD_TARGET_EMULATION
	default "elf64ltsmip"		if  BR2_mips64el && BR2_MIPS_NABI64
	default "elf64btsmip"		if  BR2_mips64   && BR2_MIPS_NABI64
	default "elf32ltsmipn32"	if  BR2_mips64el && BR2_MIPS_NABI32
	default "elf32btsmipn32"	if  BR2_mips64   && BR2_MIPS_NABI32
	default "elf32ltsmip"		if (BR2_mips64el && BR2_MIPS_OABI32) || BR2_mipsel
	default "elf32btsmip"		if (BR2_mips64   && BR2_MIPS_OABI32) || BR2_mips

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 2/2] arch: define appropriate ld emulation values for the MIPS architecture
  2013-06-06  8:57     ` Thomas Petazzoni
@ 2013-06-06  9:04       ` Markos Chandras
  0 siblings, 0 replies; 14+ messages in thread
From: Markos Chandras @ 2013-06-06  9:04 UTC (permalink / raw)
  To: buildroot

On 6 June 2013 09:57, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Markos Chandras,
>
> On Thu, 6 Jun 2013 09:37:41 +0100, Markos Chandras wrote:
>
>> binutils set el32{l,b}smipn32 as default ABI for MIPS64 as well. The
>> elf64{l,b}tsmip one need to be used only if you want to use the n64
>> ABI. My opinion is that this patch needs to be changed to something
>> like this:
>>
>>  +config BR2_LD_TARGET_EMULATION
>>  +       default "elf64ltsmip"      if BR2_mips64el && BR2_MIPS_NABI64
>>  +       default "elf64btsmip"      if BR2_mips64 && BR2_MIPS_NABI64
>>  +       default "elf32ltsmip"      if BR2_mipsel && !BR2_MIPS_NABI32
>>  +       default "elf32btsmip"      if BR2_mips && !BR2_MIPS_NABI32
>>  +       default "elf32ltsmipn32"   if BR2_mipsel && BR2_MIPS_NABI32
>>  +       default "elf32btsmipn32"   if BR2_mips && BR2_MIPS_NABI32
>
> Ok, but I'm not too happy with the fact that the BR2_mips64[el]
> && !BR2_MIPS_NABI64 is not being handled here. And according to
> arch/Config.in.mips, in fact the n32 ABI does not make sense on 32 bits
> BR2_mips and BR2_mipsel.
>
> So, shouldn't this thing be:
>
> config BR2_LD_TARGET_EMULATION
>         default "elf64ltsmip"           if  BR2_mips64el && BR2_MIPS_NABI64
>         default "elf64btsmip"           if  BR2_mips64   && BR2_MIPS_NABI64
>         default "elf32ltsmipn32"        if  BR2_mips64el && BR2_MIPS_NABI32
>         default "elf32btsmipn32"        if  BR2_mips64   && BR2_MIPS_NABI32
>         default "elf32ltsmip"           if (BR2_mips64el && BR2_MIPS_OABI32) || BR2_mipsel
>         default "elf32btsmip"           if (BR2_mips64   && BR2_MIPS_OABI32) || BR2_mips
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
>

Hi Thomas,

Yes I believe this makes sense

Reviewed-by: Markos Chandras <markos.chandras@imgtec.com>

--
Regards,
Markos Chandras - Gentoo Linux Developer
http://dev.gentoo.org/~hwoarang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified
  2013-06-06  8:56     ` Markos Chandras
@ 2013-06-06  9:13       ` Markos Chandras
  2013-06-06 10:04         ` Markos Chandras
  0 siblings, 1 reply; 14+ messages in thread
From: Markos Chandras @ 2013-06-06  9:13 UTC (permalink / raw)
  To: buildroot

On 6 June 2013 09:56, Markos Chandras <hwoarang@gentoo.org> wrote:
> On 6 June 2013 09:51, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> Dear Markos Chandras,
>>
>> On Thu, 6 Jun 2013 09:31:25 +0100, Markos Chandras wrote:
>>
>>> My understanding is that this is also a problem with buildroot
>>> toolchains as well. So whilst your patch fixes the problem with
>>> external
>>> toolchains, MIPS64/n64 buildroot toolchains will still have the same
>>> problem.
>>
>> Yes, I do remember your patch
>> http://patchwork.ozlabs.org/patch/244694/. Normally, for internal
>> toolchains, the idea is that the tools (gcc, ld) are configured, at
>> build time, to produce the 'right' code by default, without requiring
>> any tuning. The man page of 'ld' says:
>>
>>        -m emulation
>>            Emulate the emulation linker.  You can list the available
>>            emulations with the --verbose or -V options.
>>
>>            If the -m option is not used, the emulation is taken from the
>>            "LDEMULATION" environment variable, if that is defined.
>>
>>            Otherwise, the default emulation depends upon how the linker
>>            was configured.
>>
>> The last paragraph being the important one here. See what OpenEmbedded
>> is doing:
>> https://github.com/openembedded/oe-core/blob/master/meta/recipes-devtools/binutils/binutils-2.23.2/mips64-default-ld-emulation.patch.
>>
>> The problem here is that the MIPS tuples do not contain the ABI, so the
>> ld/configure.tgt and bfd/config.bfd scripts of binutils have no way of
>> knowing which emulation you would like to have by default. Maybe we
>> need to improve binutils to make this configurable?
>>
>> Thomas
>> --
>> Thomas Petazzoni, Free Electrons
>> Kernel, drivers, real-time and embedded Linux
>> development, consulting, training and support.
>> http://free-electrons.com
>>
>
> Hi Thomas,
>
> Yes I've seen this patch before but given this is not in the upstream
> binutils code I had to "workaround" it by using the exported
> LDEMULATION variable. It might worth taking this to the binutils
> mailing list.
>
> --
> Regards,
> Markos Chandras - Gentoo Linux Developer
> http://dev.gentoo.org/~hwoarang

Hi Thomas,

Just tested this patch. Looks good to me.

Tested-by: Markos Chandras <markos.chandras@imgtec.com>

--
Regards,
Markos Chandras - Gentoo Linux Developer
http://dev.gentoo.org/~hwoarang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified
  2013-06-06  9:13       ` Markos Chandras
@ 2013-06-06 10:04         ` Markos Chandras
  0 siblings, 0 replies; 14+ messages in thread
From: Markos Chandras @ 2013-06-06 10:04 UTC (permalink / raw)
  To: buildroot

On 6 June 2013 10:13, Markos Chandras <hwoarang@gentoo.org> wrote:
> On 6 June 2013 09:56, Markos Chandras <hwoarang@gentoo.org> wrote:
>> On 6 June 2013 09:51, Thomas Petazzoni
>> <thomas.petazzoni@free-electrons.com> wrote:
>>> Dear Markos Chandras,
>>>
>>> On Thu, 6 Jun 2013 09:31:25 +0100, Markos Chandras wrote:
>>>
>>>> My understanding is that this is also a problem with buildroot
>>>> toolchains as well. So whilst your patch fixes the problem with
>>>> external
>>>> toolchains, MIPS64/n64 buildroot toolchains will still have the same
>>>> problem.
>>>
>>> Yes, I do remember your patch
>>> http://patchwork.ozlabs.org/patch/244694/. Normally, for internal
>>> toolchains, the idea is that the tools (gcc, ld) are configured, at
>>> build time, to produce the 'right' code by default, without requiring
>>> any tuning. The man page of 'ld' says:
>>>
>>>        -m emulation
>>>            Emulate the emulation linker.  You can list the available
>>>            emulations with the --verbose or -V options.
>>>
>>>            If the -m option is not used, the emulation is taken from the
>>>            "LDEMULATION" environment variable, if that is defined.
>>>
>>>            Otherwise, the default emulation depends upon how the linker
>>>            was configured.
>>>
>>> The last paragraph being the important one here. See what OpenEmbedded
>>> is doing:
>>> https://github.com/openembedded/oe-core/blob/master/meta/recipes-devtools/binutils/binutils-2.23.2/mips64-default-ld-emulation.patch.
>>>
>>> The problem here is that the MIPS tuples do not contain the ABI, so the
>>> ld/configure.tgt and bfd/config.bfd scripts of binutils have no way of
>>> knowing which emulation you would like to have by default. Maybe we
>>> need to improve binutils to make this configurable?
>>>
>>> Thomas
>>> --
>>> Thomas Petazzoni, Free Electrons
>>> Kernel, drivers, real-time and embedded Linux
>>> development, consulting, training and support.
>>> http://free-electrons.com
>>>
>>
>> Hi Thomas,
>>
>> Yes I've seen this patch before but given this is not in the upstream
>> binutils code I had to "workaround" it by using the exported
>> LDEMULATION variable. It might worth taking this to the binutils
>> mailing list.
>>
>> --
>> Regards,
>> Markos Chandras - Gentoo Linux Developer
>> http://dev.gentoo.org/~hwoarang
>
> Hi Thomas,
>
> Just tested this patch. Looks good to me.
>
> Tested-by: Markos Chandras <markos.chandras@imgtec.com>
>
> --
> Regards,
> Markos Chandras - Gentoo Linux Developer
> http://dev.gentoo.org/~hwoarang

Hi Thomas,

I am also taking a look at the log you provided. It seems even though
libtool uses gcc to do the linking it actually invokes the linker
without the proper options. A mips64/n64 gcc will pass the correct -m
option to the linker but here it seems to ignore that. I could be a
problem with libtool itself.

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

--
Regards,
Markos Chandras - Gentoo Linux Developer
http://dev.gentoo.org/~hwoarang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified
@ 2016-02-20  9:33 Jan Heylen
  2016-03-22 16:19 ` Jan Heylen
  2016-03-30 21:55 ` Arnout Vandecappelle
  0 siblings, 2 replies; 14+ messages in thread
From: Jan Heylen @ 2016-02-20  9:33 UTC (permalink / raw)
  To: buildroot

On some architectures, such as MIPS, the linker supports several 'ld
emulation', and depending on which flavor of the architecture you're
building for, one should be passing the proper -m <something> option
to ld, otherwise ld defaults to the default emulation, which may not
necessarily be compatible with the object files that are being
linked.

  So, we extend the toolchain wrapper to also wrap the ld linker, and
  allow a BR_LD_EMULATION variable to be passed to it.

Based upon patch from Thomas Petazzoni:
http://thread.gmane.org/gmane.comp.lib.uclibc.buildroot/60942

Signed-off-by: Jan Heylen <heyleke@gmail.com>
---
 arch/Config.in                                     |  3 +
 toolchain/toolchain-external/toolchain-external.mk |  6 +-
 toolchain/toolchain-wrapper.c                      | 75 ++++++++++++++--------
 3 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/arch/Config.in b/arch/Config.in
index 401bd28..50816c6 100644
--- a/arch/Config.in
+++ b/arch/Config.in
@@ -266,6 +266,9 @@ config BR2_GCC_TARGET_CPU
 config BR2_GCC_TARGET_CPU_REVISION
 	string
 
+config BR2_LD_TARGET_EMULATION
+	string
+
 # The value of this option will be passed as --with-fpu=<value> when
 # building gcc (internal backend) or -mfpu=<value> in the toolchain
 # wrapper (external toolchain)
diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
index 6c3022a..9396377 100644
--- a/toolchain/toolchain-external/toolchain-external.mk
+++ b/toolchain/toolchain-external/toolchain-external.mk
@@ -182,6 +182,7 @@ CC_TARGET_ABI_ := $(call qstrip,$(BR2_GCC_TARGET_ABI))
 CC_TARGET_FPU_ := $(call qstrip,$(BR2_GCC_TARGET_FPU))
 CC_TARGET_FLOAT_ABI_ := $(call qstrip,$(BR2_GCC_TARGET_FLOAT_ABI))
 CC_TARGET_MODE_ := $(call qstrip,$(BR2_GCC_TARGET_MODE))
+LD_TARGET_EMULATION_:=$(call qstrip,$(BR2_LD_TARGET_EMULATION))
 
 # march/mtune/floating point mode needs to be passed to the external toolchain
 # to select the right multilib variant
@@ -213,6 +214,9 @@ ifneq ($(CC_TARGET_MODE_),)
 TOOLCHAIN_EXTERNAL_CFLAGS += -m$(CC_TARGET_MODE_)
 TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_MODE='"$(CC_TARGET_MODE_)"'
 endif
+ifneq ($(LD_TARGET_EMULATION_),)
+TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_LD_EMULATION='"$(LD_TARGET_EMULATION_)"'
+endif
 ifeq ($(BR2_BINFMT_FLAT),y)
 TOOLCHAIN_EXTERNAL_CFLAGS += -Wl,-elf2flt
 TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_BINFMT_FLAT
@@ -711,7 +715,7 @@ define TOOLCHAIN_EXTERNAL_INSTALL_WRAPPER
 		*-ar|*-ranlib|*-nm) \
 			ln -sf $$(echo $$i | sed 's%^$(HOST_DIR)%../..%') .; \
 			;; \
-		*cc|*cc-*|*++|*++-*|*cpp) \
+		*cc|*cc-*|*++|*++-*|*cpp|*ld) \
 			ln -sf toolchain-wrapper $$base; \
 			;; \
 		*gdb|*gdbtui) \
diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
index 887058f..25ce640 100644
--- a/toolchain/toolchain-wrapper.c
+++ b/toolchain/toolchain-wrapper.c
@@ -42,7 +42,7 @@ static char sysroot[PATH_MAX];
  */
 #define EXCLUSIVE_ARGS	3
 
-static char *predef_args[] = {
+static char *gcc_predef_args[] = {
 #ifdef BR_CCACHE
 	ccache_path,
 #endif
@@ -80,6 +80,13 @@ static char *predef_args[] = {
 #endif
 };
 
+static char *ld_predef_args[] = {
+	path,
+#ifdef BR_LD_EMULATION
+	"-m", BR_LD_EMULATION,
+#endif
+};
+
 static void check_unsafe_path(const char *path, int paranoid)
 {
 	char **c;
@@ -108,7 +115,8 @@ int main(int argc, char **argv)
 	char *env_debug;
 	char *paranoid_wrapper;
 	int paranoid;
-	int ret, i, count = 0, debug;
+	char **predef_args;
+	int ret, i, predef_args_sz, count = 0, debug;
 
 	/* Calculate the relative paths */
 	basename = strrchr(progpath, '/');
@@ -169,7 +177,15 @@ int main(int argc, char **argv)
 		return 3;
 	}
 
-	cur = args = malloc(sizeof(predef_args) +
+	if (!strncmp("-ld", basename + strlen(basename) - 3, 3)) {
+		predef_args_sz = sizeof(ld_predef_args);
+		predef_args = ld_predef_args;
+	} else {
+		predef_args_sz = sizeof(gcc_predef_args);
+		predef_args = gcc_predef_args;
+	}
+
+	cur = args = malloc(predef_args_sz +
 			    (sizeof(char *) * (argc + EXCLUSIVE_ARGS)));
 	if (args == NULL) {
 		perror(__FILE__ ": malloc");
@@ -177,42 +193,45 @@ int main(int argc, char **argv)
 	}
 
 	/* start with predefined args */
-	memcpy(cur, predef_args, sizeof(predef_args));
-	cur += sizeof(predef_args) / sizeof(predef_args[0]);
+	memcpy(cur, predef_args, predef_args_sz);
+	cur += predef_args_sz / sizeof(char *);
 
+	/* following extras should only happen for non-ld wrappers */
+	if (predef_args != ld_predef_args) {
 #ifdef BR_FLOAT_ABI
-	/* add float abi if not overridden in args */
-	for (i = 1; i < argc; i++) {
-		if (!strncmp(argv[i], "-mfloat-abi=", strlen("-mfloat-abi=")) ||
-		    !strcmp(argv[i], "-msoft-float") ||
-		    !strcmp(argv[i], "-mhard-float"))
-			break;
-	}
+		/* add float abi if not overridden in args */
+		for (i = 1; i < argc; i++) {
+			if (!strncmp(argv[i], "-mfloat-abi=", strlen("-mfloat-abi=")) ||
+					!strcmp(argv[i], "-msoft-float") ||
+					!strcmp(argv[i], "-mhard-float"))
+				break;
+		}
 
-	if (i == argc)
-		*cur++ = "-mfloat-abi=" BR_FLOAT_ABI;
+		if (i == argc)
+			*cur++ = "-mfloat-abi=" BR_FLOAT_ABI;
 #endif
 
 #if defined(BR_ARCH) || \
-    defined(BR_CPU)
-	/* Add our -march/cpu flags, but only if none of
-	 * -march/mtune/mcpu are already specified on the commandline
-	 */
-	for (i = 1; i < argc; i++) {
-		if (!strncmp(argv[i], "-march=", strlen("-march=")) ||
-		    !strncmp(argv[i], "-mtune=", strlen("-mtune=")) ||
-		    !strncmp(argv[i], "-mcpu=",  strlen("-mcpu=" )))
-			break;
-	}
-	if (i == argc) {
+		defined(BR_CPU)
+		/* Add our -march/cpu flags, but only if none of
+		 * -march/mtune/mcpu are already specified on the commandline
+		 */
+		for (i = 1; i < argc; i++) {
+			if (!strncmp(argv[i], "-march=", strlen("-march=")) ||
+					!strncmp(argv[i], "-mtune=", strlen("-mtune=")) ||
+					!strncmp(argv[i], "-mcpu=",  strlen("-mcpu=" )))
+				break;
+		}
+		if (i == argc) {
 #ifdef BR_ARCH
-		*cur++ = "-march=" BR_ARCH;
+			*cur++ = "-march=" BR_ARCH;
 #endif
 #ifdef BR_CPU
-		*cur++ = "-mcpu=" BR_CPU;
+			*cur++ = "-mcpu=" BR_CPU;
 #endif
-	}
+		}
 #endif /* ARCH || CPU */
+	}
 
 	paranoid_wrapper = getenv("BR_COMPILER_PARANOID_UNSAFE_PATH");
 	if (paranoid_wrapper && strlen(paranoid_wrapper) > 0)
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified
  2016-02-20  9:33 Jan Heylen
@ 2016-03-22 16:19 ` Jan Heylen
  2016-03-30 21:55 ` Arnout Vandecappelle
  1 sibling, 0 replies; 14+ messages in thread
From: Jan Heylen @ 2016-03-22 16:19 UTC (permalink / raw)
  To: buildroot

Hi,

it has been a month, I know the amount of patches in the queue is
high, but any luck at looking at this one, based upon a patch from
Thomas Petazzoni that was in the queue before, so should be ok ;-) ?

br,

Jan

On Sat, Feb 20, 2016 at 10:33 AM, Jan Heylen <heyleke@gmail.com> wrote:
> On some architectures, such as MIPS, the linker supports several 'ld
> emulation', and depending on which flavor of the architecture you're
> building for, one should be passing the proper -m <something> option
> to ld, otherwise ld defaults to the default emulation, which may not
> necessarily be compatible with the object files that are being
> linked.
>
>   So, we extend the toolchain wrapper to also wrap the ld linker, and
>   allow a BR_LD_EMULATION variable to be passed to it.
>
> Based upon patch from Thomas Petazzoni:
> http://thread.gmane.org/gmane.comp.lib.uclibc.buildroot/60942
>
> Signed-off-by: Jan Heylen <heyleke@gmail.com>
> ---
>  arch/Config.in                                     |  3 +
>  toolchain/toolchain-external/toolchain-external.mk |  6 +-
>  toolchain/toolchain-wrapper.c                      | 75 ++++++++++++++--------
>  3 files changed, 55 insertions(+), 29 deletions(-)
>
> diff --git a/arch/Config.in b/arch/Config.in
> index 401bd28..50816c6 100644
> --- a/arch/Config.in
> +++ b/arch/Config.in
> @@ -266,6 +266,9 @@ config BR2_GCC_TARGET_CPU
>  config BR2_GCC_TARGET_CPU_REVISION
>         string
>
> +config BR2_LD_TARGET_EMULATION
> +       string
> +
>  # The value of this option will be passed as --with-fpu=<value> when
>  # building gcc (internal backend) or -mfpu=<value> in the toolchain
>  # wrapper (external toolchain)
> diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
> index 6c3022a..9396377 100644
> --- a/toolchain/toolchain-external/toolchain-external.mk
> +++ b/toolchain/toolchain-external/toolchain-external.mk
> @@ -182,6 +182,7 @@ CC_TARGET_ABI_ := $(call qstrip,$(BR2_GCC_TARGET_ABI))
>  CC_TARGET_FPU_ := $(call qstrip,$(BR2_GCC_TARGET_FPU))
>  CC_TARGET_FLOAT_ABI_ := $(call qstrip,$(BR2_GCC_TARGET_FLOAT_ABI))
>  CC_TARGET_MODE_ := $(call qstrip,$(BR2_GCC_TARGET_MODE))
> +LD_TARGET_EMULATION_:=$(call qstrip,$(BR2_LD_TARGET_EMULATION))
>
>  # march/mtune/floating point mode needs to be passed to the external toolchain
>  # to select the right multilib variant
> @@ -213,6 +214,9 @@ ifneq ($(CC_TARGET_MODE_),)
>  TOOLCHAIN_EXTERNAL_CFLAGS += -m$(CC_TARGET_MODE_)
>  TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_MODE='"$(CC_TARGET_MODE_)"'
>  endif
> +ifneq ($(LD_TARGET_EMULATION_),)
> +TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_LD_EMULATION='"$(LD_TARGET_EMULATION_)"'
> +endif
>  ifeq ($(BR2_BINFMT_FLAT),y)
>  TOOLCHAIN_EXTERNAL_CFLAGS += -Wl,-elf2flt
>  TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_BINFMT_FLAT
> @@ -711,7 +715,7 @@ define TOOLCHAIN_EXTERNAL_INSTALL_WRAPPER
>                 *-ar|*-ranlib|*-nm) \
>                         ln -sf $$(echo $$i | sed 's%^$(HOST_DIR)%../..%') .; \
>                         ;; \
> -               *cc|*cc-*|*++|*++-*|*cpp) \
> +               *cc|*cc-*|*++|*++-*|*cpp|*ld) \
>                         ln -sf toolchain-wrapper $$base; \
>                         ;; \
>                 *gdb|*gdbtui) \
> diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
> index 887058f..25ce640 100644
> --- a/toolchain/toolchain-wrapper.c
> +++ b/toolchain/toolchain-wrapper.c
> @@ -42,7 +42,7 @@ static char sysroot[PATH_MAX];
>   */
>  #define EXCLUSIVE_ARGS 3
>
> -static char *predef_args[] = {
> +static char *gcc_predef_args[] = {
>  #ifdef BR_CCACHE
>         ccache_path,
>  #endif
> @@ -80,6 +80,13 @@ static char *predef_args[] = {
>  #endif
>  };
>
> +static char *ld_predef_args[] = {
> +       path,
> +#ifdef BR_LD_EMULATION
> +       "-m", BR_LD_EMULATION,
> +#endif
> +};
> +
>  static void check_unsafe_path(const char *path, int paranoid)
>  {
>         char **c;
> @@ -108,7 +115,8 @@ int main(int argc, char **argv)
>         char *env_debug;
>         char *paranoid_wrapper;
>         int paranoid;
> -       int ret, i, count = 0, debug;
> +       char **predef_args;
> +       int ret, i, predef_args_sz, count = 0, debug;
>
>         /* Calculate the relative paths */
>         basename = strrchr(progpath, '/');
> @@ -169,7 +177,15 @@ int main(int argc, char **argv)
>                 return 3;
>         }
>
> -       cur = args = malloc(sizeof(predef_args) +
> +       if (!strncmp("-ld", basename + strlen(basename) - 3, 3)) {
> +               predef_args_sz = sizeof(ld_predef_args);
> +               predef_args = ld_predef_args;
> +       } else {
> +               predef_args_sz = sizeof(gcc_predef_args);
> +               predef_args = gcc_predef_args;
> +       }
> +
> +       cur = args = malloc(predef_args_sz +
>                             (sizeof(char *) * (argc + EXCLUSIVE_ARGS)));
>         if (args == NULL) {
>                 perror(__FILE__ ": malloc");
> @@ -177,42 +193,45 @@ int main(int argc, char **argv)
>         }
>
>         /* start with predefined args */
> -       memcpy(cur, predef_args, sizeof(predef_args));
> -       cur += sizeof(predef_args) / sizeof(predef_args[0]);
> +       memcpy(cur, predef_args, predef_args_sz);
> +       cur += predef_args_sz / sizeof(char *);
>
> +       /* following extras should only happen for non-ld wrappers */
> +       if (predef_args != ld_predef_args) {
>  #ifdef BR_FLOAT_ABI
> -       /* add float abi if not overridden in args */
> -       for (i = 1; i < argc; i++) {
> -               if (!strncmp(argv[i], "-mfloat-abi=", strlen("-mfloat-abi=")) ||
> -                   !strcmp(argv[i], "-msoft-float") ||
> -                   !strcmp(argv[i], "-mhard-float"))
> -                       break;
> -       }
> +               /* add float abi if not overridden in args */
> +               for (i = 1; i < argc; i++) {
> +                       if (!strncmp(argv[i], "-mfloat-abi=", strlen("-mfloat-abi=")) ||
> +                                       !strcmp(argv[i], "-msoft-float") ||
> +                                       !strcmp(argv[i], "-mhard-float"))
> +                               break;
> +               }
>
> -       if (i == argc)
> -               *cur++ = "-mfloat-abi=" BR_FLOAT_ABI;
> +               if (i == argc)
> +                       *cur++ = "-mfloat-abi=" BR_FLOAT_ABI;
>  #endif
>
>  #if defined(BR_ARCH) || \
> -    defined(BR_CPU)
> -       /* Add our -march/cpu flags, but only if none of
> -        * -march/mtune/mcpu are already specified on the commandline
> -        */
> -       for (i = 1; i < argc; i++) {
> -               if (!strncmp(argv[i], "-march=", strlen("-march=")) ||
> -                   !strncmp(argv[i], "-mtune=", strlen("-mtune=")) ||
> -                   !strncmp(argv[i], "-mcpu=",  strlen("-mcpu=" )))
> -                       break;
> -       }
> -       if (i == argc) {
> +               defined(BR_CPU)
> +               /* Add our -march/cpu flags, but only if none of
> +                * -march/mtune/mcpu are already specified on the commandline
> +                */
> +               for (i = 1; i < argc; i++) {
> +                       if (!strncmp(argv[i], "-march=", strlen("-march=")) ||
> +                                       !strncmp(argv[i], "-mtune=", strlen("-mtune=")) ||
> +                                       !strncmp(argv[i], "-mcpu=",  strlen("-mcpu=" )))
> +                               break;
> +               }
> +               if (i == argc) {
>  #ifdef BR_ARCH
> -               *cur++ = "-march=" BR_ARCH;
> +                       *cur++ = "-march=" BR_ARCH;
>  #endif
>  #ifdef BR_CPU
> -               *cur++ = "-mcpu=" BR_CPU;
> +                       *cur++ = "-mcpu=" BR_CPU;
>  #endif
> -       }
> +               }
>  #endif /* ARCH || CPU */
> +       }
>
>         paranoid_wrapper = getenv("BR_COMPILER_PARANOID_UNSAFE_PATH");
>         if (paranoid_wrapper && strlen(paranoid_wrapper) > 0)
> --
> 2.5.0
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified
  2016-02-20  9:33 Jan Heylen
  2016-03-22 16:19 ` Jan Heylen
@ 2016-03-30 21:55 ` Arnout Vandecappelle
  2016-03-31 12:26   ` Jan Heylen
  1 sibling, 1 reply; 14+ messages in thread
From: Arnout Vandecappelle @ 2016-03-30 21:55 UTC (permalink / raw)
  To: buildroot

On 02/20/16 10:33, Jan Heylen wrote:
> On some architectures, such as MIPS, the linker supports several 'ld
> emulation', and depending on which flavor of the architecture you're
> building for, one should be passing the proper -m <something> option
> to ld, otherwise ld defaults to the default emulation, which may not
> necessarily be compatible with the object files that are being
> linked.
>
>    So, we extend the toolchain wrapper to also wrap the ld linker, and
>    allow a BR_LD_EMULATION variable to be passed to it.

  Why this sudden indentation?

  I would split this off into a separate patch, i.e. patch 1 only adds the ld 
wrapper, patch 2 introduces LD_EMULATION.

>
> Based upon patch from Thomas Petazzoni:
> http://thread.gmane.org/gmane.comp.lib.uclibc.buildroot/60942
>
> Signed-off-by: Jan Heylen <heyleke@gmail.com>
> ---
>   arch/Config.in                                     |  3 +
>   toolchain/toolchain-external/toolchain-external.mk |  6 +-

  You only do this for the external toolchain, it should probably be done for 
the internal toolchain as well. However, that is more tricky...

1. Where should it be done? It's currently done as a post-install hook of 
host-gcc-initial and again for host-gcc-final. For the ld wrapper, it would make 
sense to do it as part of host-binutils. However, the wrapper itself is 
currently compiled in host-gcc-initial, which comes after host-binutils...

2. For external toolchains, gcc itself will call the unwrapped ld, because it 
refers directly to the ld path at installation time. However, for internal 
toolchains, it refers to the host dir so to the wrapped ld.

3. Do the workarounds that we (I) added for 
internal-toolchain-used-as-external-toolchain still work with the ld wrapper?

  Actually, it turns out that 2. is not valid: gcc will call 
host/usr/<tuple>/bin/ld, which is not the wrapped one. One less thing to worry 
about :-)

  Note that you can add the wrapper for the external and the internal toolchain 
in separate patches as well.


>   toolchain/toolchain-wrapper.c                      | 75 ++++++++++++++--------
>   3 files changed, 55 insertions(+), 29 deletions(-)
>
> diff --git a/arch/Config.in b/arch/Config.in
> index 401bd28..50816c6 100644
> --- a/arch/Config.in
> +++ b/arch/Config.in
> @@ -266,6 +266,9 @@ config BR2_GCC_TARGET_CPU
>   config BR2_GCC_TARGET_CPU_REVISION
>   	string
>
> +config BR2_LD_TARGET_EMULATION
> +	string
> +
>   # The value of this option will be passed as --with-fpu=<value> when
>   # building gcc (internal backend) or -mfpu=<value> in the toolchain
>   # wrapper (external toolchain)
> diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
> index 6c3022a..9396377 100644
> --- a/toolchain/toolchain-external/toolchain-external.mk
> +++ b/toolchain/toolchain-external/toolchain-external.mk
> @@ -182,6 +182,7 @@ CC_TARGET_ABI_ := $(call qstrip,$(BR2_GCC_TARGET_ABI))
>   CC_TARGET_FPU_ := $(call qstrip,$(BR2_GCC_TARGET_FPU))
>   CC_TARGET_FLOAT_ABI_ := $(call qstrip,$(BR2_GCC_TARGET_FLOAT_ABI))
>   CC_TARGET_MODE_ := $(call qstrip,$(BR2_GCC_TARGET_MODE))
> +LD_TARGET_EMULATION_:=$(call qstrip,$(BR2_LD_TARGET_EMULATION))

  There should be a space before and after :=

  Also it should be = instead of :=, but since the other assignments don't do 
that, let's keep it.

>
>   # march/mtune/floating point mode needs to be passed to the external toolchain
>   # to select the right multilib variant
> @@ -213,6 +214,9 @@ ifneq ($(CC_TARGET_MODE_),)
>   TOOLCHAIN_EXTERNAL_CFLAGS += -m$(CC_TARGET_MODE_)
>   TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_MODE='"$(CC_TARGET_MODE_)"'
>   endif
> +ifneq ($(LD_TARGET_EMULATION_),)
> +TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_LD_EMULATION='"$(LD_TARGET_EMULATION_)"'
> +endif
>   ifeq ($(BR2_BINFMT_FLAT),y)
>   TOOLCHAIN_EXTERNAL_CFLAGS += -Wl,-elf2flt
>   TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_BINFMT_FLAT
> @@ -711,7 +715,7 @@ define TOOLCHAIN_EXTERNAL_INSTALL_WRAPPER
>   		*-ar|*-ranlib|*-nm) \
>   			ln -sf $$(echo $$i | sed 's%^$(HOST_DIR)%../..%') .; \
>   			;; \
> -		*cc|*cc-*|*++|*++-*|*cpp) \
> +		*cc|*cc-*|*++|*++-*|*cpp|*ld) \
>   			ln -sf toolchain-wrapper $$base; \
>   			;; \
>   		*gdb|*gdbtui) \
> diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
> index 887058f..25ce640 100644
> --- a/toolchain/toolchain-wrapper.c
> +++ b/toolchain/toolchain-wrapper.c
> @@ -42,7 +42,7 @@ static char sysroot[PATH_MAX];
>    */
>   #define EXCLUSIVE_ARGS	3
>
> -static char *predef_args[] = {
> +static char *gcc_predef_args[] = {
>   #ifdef BR_CCACHE
>   	ccache_path,
>   #endif
> @@ -80,6 +80,13 @@ static char *predef_args[] = {
>   #endif
>   };
>
> +static char *ld_predef_args[] = {
> +	path,
> +#ifdef BR_LD_EMULATION
> +	"-m", BR_LD_EMULATION,
> +#endif

  In addition to these, you should take over some more options from gcc:
BR_BINFMT_FLAT -> -elf2flt
BR_MIPS_TARGET_LITTLE_ENDIAN -> -EB
BR_MIPS/ARC_TARGET_BIG_ENDIAN -> -EL

  Probably also --sysroot, but I'm not sure what will happen if an external 
toolchain's binutils wasn't configured with --with-sysroot.

  Also we want to add BR2_TARGET_LDFLAGS as BR_ADDITIONAL_LDFLAGS, like we do 
for CFLAGS.

> +};
> +
>   static void check_unsafe_path(const char *path, int paranoid)
>   {
>   	char **c;
> @@ -108,7 +115,8 @@ int main(int argc, char **argv)
>   	char *env_debug;
>   	char *paranoid_wrapper;
>   	int paranoid;
> -	int ret, i, count = 0, debug;
> +	char **predef_args;
> +	int ret, i, predef_args_sz, count = 0, debug;

  Small nit: I consider it bad style to declare multiple variables in a single 
line (especially when one of them is initialised!), so let's not make that worse 
:-). I.e., put predef_args_sz on a separate line.

>
>   	/* Calculate the relative paths */
>   	basename = strrchr(progpath, '/');
> @@ -169,7 +177,15 @@ int main(int argc, char **argv)
>   		return 3;
>   	}
>
> -	cur = args = malloc(sizeof(predef_args) +
> +	if (!strncmp("-ld", basename + strlen(basename) - 3, 3)) {

  Actually, strcmp would be OK here, because it's at the end of the string. But 
explaining that takes more time than just adding the n :-)

  We will eventually also have gold, so don't match the -. That anyway also 
corresponds to the pattern you use to create the symlink to the wrapper.

> +		predef_args_sz = sizeof(ld_predef_args);
> +		predef_args = ld_predef_args;
> +	} else {
> +		predef_args_sz = sizeof(gcc_predef_args);
> +		predef_args = gcc_predef_args;
> +	}
> +
> +	cur = args = malloc(predef_args_sz +
>   			    (sizeof(char *) * (argc + EXCLUSIVE_ARGS)));
>   	if (args == NULL) {
>   		perror(__FILE__ ": malloc");
> @@ -177,42 +193,45 @@ int main(int argc, char **argv)
>   	}
>
>   	/* start with predefined args */
> -	memcpy(cur, predef_args, sizeof(predef_args));
> -	cur += sizeof(predef_args) / sizeof(predef_args[0]);
> +	memcpy(cur, predef_args, predef_args_sz);
> +	cur += predef_args_sz / sizeof(char *);
>
> +	/* following extras should only happen for non-ld wrappers */
> +	if (predef_args != ld_predef_args) {

  Since you have this condition, perhaps it's better to store the result of the 
strncmp in a bool and use a ?: in the one place where predef_args is used (in 
the memcpy). I think that would be more readable.

>   #ifdef BR_FLOAT_ABI
> -	/* add float abi if not overridden in args */
> -	for (i = 1; i < argc; i++) {
> -		if (!strncmp(argv[i], "-mfloat-abi=", strlen("-mfloat-abi=")) ||
> -		    !strcmp(argv[i], "-msoft-float") ||
> -		    !strcmp(argv[i], "-mhard-float"))
> -			break;
> -	}
> +		/* add float abi if not overridden in args */
> +		for (i = 1; i < argc; i++) {
> +			if (!strncmp(argv[i], "-mfloat-abi=", strlen("-mfloat-abi=")) ||
> +					!strcmp(argv[i], "-msoft-float") ||
> +					!strcmp(argv[i], "-mhard-float"))
> +				break;
> +		}
>
> -	if (i == argc)
> -		*cur++ = "-mfloat-abi=" BR_FLOAT_ABI;
> +		if (i == argc)
> +			*cur++ = "-mfloat-abi=" BR_FLOAT_ABI;
>   #endif
>
>   #if defined(BR_ARCH) || \
> -    defined(BR_CPU)
> -	/* Add our -march/cpu flags, but only if none of
> -	 * -march/mtune/mcpu are already specified on the commandline
> -	 */
> -	for (i = 1; i < argc; i++) {
> -		if (!strncmp(argv[i], "-march=", strlen("-march=")) ||
> -		    !strncmp(argv[i], "-mtune=", strlen("-mtune=")) ||
> -		    !strncmp(argv[i], "-mcpu=",  strlen("-mcpu=" )))
> -			break;
> -	}
> -	if (i == argc) {
> +		defined(BR_CPU)
> +		/* Add our -march/cpu flags, but only if none of
> +		 * -march/mtune/mcpu are already specified on the commandline
> +		 */
> +		for (i = 1; i < argc; i++) {
> +			if (!strncmp(argv[i], "-march=", strlen("-march=")) ||
> +					!strncmp(argv[i], "-mtune=", strlen("-mtune=")) ||
> +					!strncmp(argv[i], "-mcpu=",  strlen("-mcpu=" )))
> +				break;
> +		}
> +		if (i == argc) {
>   #ifdef BR_ARCH
> -		*cur++ = "-march=" BR_ARCH;
> +			*cur++ = "-march=" BR_ARCH;
>   #endif
>   #ifdef BR_CPU
> -		*cur++ = "-mcpu=" BR_CPU;
> +			*cur++ = "-mcpu=" BR_CPU;
>   #endif
> -	}
> +		}
>   #endif /* ARCH || CPU */
> +	}
>
>   	paranoid_wrapper = getenv("BR_COMPILER_PARANOID_UNSAFE_PATH");
>   	if (paranoid_wrapper && strlen(paranoid_wrapper) > 0)
>

  BR_NO_CCACHE handling should also be skipped for ld, since ld_predef_args[0] 
is NOT ccache. Otherwise you'll get spectacular results when BR_NO_CCACHE is 
exported...

  Probably the other things under CCACHE ifdefs should not be done for ld 
either, though they don't really hurt.

  Regards,
  Arnout

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified
  2016-03-30 21:55 ` Arnout Vandecappelle
@ 2016-03-31 12:26   ` Jan Heylen
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Heylen @ 2016-03-31 12:26 UTC (permalink / raw)
  To: buildroot

On Wed, Mar 30, 2016 at 11:55 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 02/20/16 10:33, Jan Heylen wrote:
>>
>> On some architectures, such as MIPS, the linker supports several 'ld
>> emulation', and depending on which flavor of the architecture you're
>> building for, one should be passing the proper -m <something> option
>> to ld, otherwise ld defaults to the default emulation, which may not
>> necessarily be compatible with the object files that are being
>> linked.
>>
>>    So, we extend the toolchain wrapper to also wrap the ld linker, and
>>    allow a BR_LD_EMULATION variable to be passed to it.
>
>
>  Why this sudden indentation?

probably a typo, don't remember any reason

>
>  I would split this off into a separate patch, i.e. patch 1 only adds the ld
> wrapper, patch 2 introduces LD_EMULATION.
>
>>
>> Based upon patch from Thomas Petazzoni:
>> http://thread.gmane.org/gmane.comp.lib.uclibc.buildroot/60942
>>
>> Signed-off-by: Jan Heylen <heyleke@gmail.com>
>> ---
>>   arch/Config.in                                     |  3 +
>>   toolchain/toolchain-external/toolchain-external.mk |  6 +-
>
>
>  You only do this for the external toolchain, it should probably be done for
> the internal toolchain as well. However, that is more tricky...
>
> 1. Where should it be done? It's currently done as a post-install hook of
> host-gcc-initial and again for host-gcc-final. For the ld wrapper, it would
> make sense to do it as part of host-binutils. However, the wrapper itself is
> currently compiled in host-gcc-initial, which comes after host-binutils...

Without looking into the details: would it be ok to initially not do
it, (as host-binutils is not affected itself) and only do it after
host-gcc-initial. Building host-binutils only, and linking a target
package with LD_EMULATION without host-gcc build yet would be a
strange use case?

And, making it available for internal toolchains as well, would mean
moving LD_EMULATION to toolchain-wrapper.mk?

>
> 2. For external toolchains, gcc itself will call the unwrapped ld, because
> it refers directly to the ld path at installation time. However, for
> internal toolchains, it refers to the host dir so to the wrapped ld.
>
> 3. Do the workarounds that we (I) added for
> internal-toolchain-used-as-external-toolchain still work with the ld
> wrapper?

I looked up this patch you did, and verified (by reading the code),
this should be handled generically with BR_CROSS_PATH_SUFFIX which
gets defined when using a buildroot toolchain externally.

>
>  Actually, it turns out that 2. is not valid: gcc will call
> host/usr/<tuple>/bin/ld, which is not the wrapped one. One less thing to
> worry about :-)
>
>  Note that you can add the wrapper for the external and the internal
> toolchain in separate patches as well.
>
>
>>   toolchain/toolchain-wrapper.c                      | 75
>> ++++++++++++++--------
>>   3 files changed, 55 insertions(+), 29 deletions(-)
>>
>> diff --git a/arch/Config.in b/arch/Config.in
>> index 401bd28..50816c6 100644
>> --- a/arch/Config.in
>> +++ b/arch/Config.in
>> @@ -266,6 +266,9 @@ config BR2_GCC_TARGET_CPU
>>   config BR2_GCC_TARGET_CPU_REVISION
>>         string
>>
>> +config BR2_LD_TARGET_EMULATION
>> +       string
>> +
>>   # The value of this option will be passed as --with-fpu=<value> when
>>   # building gcc (internal backend) or -mfpu=<value> in the toolchain
>>   # wrapper (external toolchain)
>> diff --git a/toolchain/toolchain-external/toolchain-external.mk
>> b/toolchain/toolchain-external/toolchain-external.mk
>> index 6c3022a..9396377 100644
>> --- a/toolchain/toolchain-external/toolchain-external.mk
>> +++ b/toolchain/toolchain-external/toolchain-external.mk
>> @@ -182,6 +182,7 @@ CC_TARGET_ABI_ := $(call qstrip,$(BR2_GCC_TARGET_ABI))
>>   CC_TARGET_FPU_ := $(call qstrip,$(BR2_GCC_TARGET_FPU))
>>   CC_TARGET_FLOAT_ABI_ := $(call qstrip,$(BR2_GCC_TARGET_FLOAT_ABI))
>>   CC_TARGET_MODE_ := $(call qstrip,$(BR2_GCC_TARGET_MODE))
>> +LD_TARGET_EMULATION_:=$(call qstrip,$(BR2_LD_TARGET_EMULATION))
>
>
>  There should be a space before and after :=
>
>  Also it should be = instead of :=, but since the other assignments don't do
> that, let's keep it.
>
>
>>
>>   # march/mtune/floating point mode needs to be passed to the external
>> toolchain
>>   # to select the right multilib variant
>> @@ -213,6 +214,9 @@ ifneq ($(CC_TARGET_MODE_),)
>>   TOOLCHAIN_EXTERNAL_CFLAGS += -m$(CC_TARGET_MODE_)
>>   TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS +=
>> -DBR_MODE='"$(CC_TARGET_MODE_)"'
>>   endif
>> +ifneq ($(LD_TARGET_EMULATION_),)
>> +TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS +=
>> -DBR_LD_EMULATION='"$(LD_TARGET_EMULATION_)"'
>> +endif
>>   ifeq ($(BR2_BINFMT_FLAT),y)
>>   TOOLCHAIN_EXTERNAL_CFLAGS += -Wl,-elf2flt
>>   TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_BINFMT_FLAT
>> @@ -711,7 +715,7 @@ define TOOLCHAIN_EXTERNAL_INSTALL_WRAPPER
>>                 *-ar|*-ranlib|*-nm) \
>>                         ln -sf $$(echo $$i | sed 's%^$(HOST_DIR)%../..%')
>> .; \
>>                         ;; \
>> -               *cc|*cc-*|*++|*++-*|*cpp) \
>> +               *cc|*cc-*|*++|*++-*|*cpp|*ld) \
>>                         ln -sf toolchain-wrapper $$base; \
>>                         ;; \
>>                 *gdb|*gdbtui) \
>> diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
>> index 887058f..25ce640 100644
>> --- a/toolchain/toolchain-wrapper.c
>> +++ b/toolchain/toolchain-wrapper.c
>> @@ -42,7 +42,7 @@ static char sysroot[PATH_MAX];
>>    */
>>   #define EXCLUSIVE_ARGS        3
>>
>> -static char *predef_args[] = {
>> +static char *gcc_predef_args[] = {
>>   #ifdef BR_CCACHE
>>         ccache_path,
>>   #endif
>> @@ -80,6 +80,13 @@ static char *predef_args[] = {
>>   #endif
>>   };
>>
>> +static char *ld_predef_args[] = {
>> +       path,
>> +#ifdef BR_LD_EMULATION
>> +       "-m", BR_LD_EMULATION,
>> +#endif
>
>
>  In addition to these, you should take over some more options from gcc:
> BR_BINFMT_FLAT -> -elf2flt
> BR_MIPS_TARGET_LITTLE_ENDIAN -> -EB
> BR_MIPS/ARC_TARGET_BIG_ENDIAN -> -EL
>
>  Probably also --sysroot, but I'm not sure what will happen if an external
> toolchain's binutils wasn't configured with --with-sysroot.
I see, would be logical to add it, bit strange Thomas Petazzoni didn't
add it in the original? But the issue
you describe is also a valid issue:
man ld
'--sysroot=directory
Use directory as the location of the sysroot, overriding the
configure-time default. This option is only supported by linkers that
were configured using --with-sysroot.'

Don't know how I should proceed with this case...

>
>  Also we want to add BR2_TARGET_LDFLAGS as BR_ADDITIONAL_LDFLAGS, like we do
> for CFLAGS.
Would BR_ADDITIONAL_LDFLAGS then remove the -Wl, and pass those to the linker?

And more general, do we still need LD_FLAGS being set, or should we
pass them too directly via the wrapper? (like was done for the
OPTIMIZATION flags in 50f44d877e5246198e692ecab3579ec36779da74)
Although that would probably change the behavior in various makefiles
of packages, depending how they use LD_FLAGS?

Sorry, more questions than answers...

>
>> +};
>> +
>>   static void check_unsafe_path(const char *path, int paranoid)
>>   {
>>         char **c;
>> @@ -108,7 +115,8 @@ int main(int argc, char **argv)
>>         char *env_debug;
>>         char *paranoid_wrapper;
>>         int paranoid;
>> -       int ret, i, count = 0, debug;
>> +       char **predef_args;
>> +       int ret, i, predef_args_sz, count = 0, debug;
>
>
>  Small nit: I consider it bad style to declare multiple variables in a
> single line (especially when one of them is initialised!), so let's not make
> that worse :-). I.e., put predef_args_sz on a separate line.
>
>>
>>         /* Calculate the relative paths */
>>         basename = strrchr(progpath, '/');
>> @@ -169,7 +177,15 @@ int main(int argc, char **argv)
>>                 return 3;
>>         }
>>
>> -       cur = args = malloc(sizeof(predef_args) +
>> +       if (!strncmp("-ld", basename + strlen(basename) - 3, 3)) {
>
>
>  Actually, strcmp would be OK here, because it's at the end of the string.
> But explaining that takes more time than just adding the n :-)
>
>  We will eventually also have gold, so don't match the -. That anyway also
> corresponds to the pattern you use to create the symlink to the wrapper.
>
>> +               predef_args_sz = sizeof(ld_predef_args);
>> +               predef_args = ld_predef_args;
>> +       } else {
>> +               predef_args_sz = sizeof(gcc_predef_args);
>> +               predef_args = gcc_predef_args;
>> +       }
>> +
>> +       cur = args = malloc(predef_args_sz +
>>                             (sizeof(char *) * (argc + EXCLUSIVE_ARGS)));
>>         if (args == NULL) {
>>                 perror(__FILE__ ": malloc");
>> @@ -177,42 +193,45 @@ int main(int argc, char **argv)
>>         }
>>
>>         /* start with predefined args */
>> -       memcpy(cur, predef_args, sizeof(predef_args));
>> -       cur += sizeof(predef_args) / sizeof(predef_args[0]);
>> +       memcpy(cur, predef_args, predef_args_sz);
>> +       cur += predef_args_sz / sizeof(char *);
>>
>> +       /* following extras should only happen for non-ld wrappers */
>> +       if (predef_args != ld_predef_args) {
>
>
>  Since you have this condition, perhaps it's better to store the result of
> the strncmp in a bool and use a ?: in the one place where predef_args is
> used (in the memcpy). I think that would be more readable.
>
>
>>   #ifdef BR_FLOAT_ABI
>> -       /* add float abi if not overridden in args */
>> -       for (i = 1; i < argc; i++) {
>> -               if (!strncmp(argv[i], "-mfloat-abi=",
>> strlen("-mfloat-abi=")) ||
>> -                   !strcmp(argv[i], "-msoft-float") ||
>> -                   !strcmp(argv[i], "-mhard-float"))
>> -                       break;
>> -       }
>> +               /* add float abi if not overridden in args */
>> +               for (i = 1; i < argc; i++) {
>> +                       if (!strncmp(argv[i], "-mfloat-abi=",
>> strlen("-mfloat-abi=")) ||
>> +                                       !strcmp(argv[i], "-msoft-float")
>> ||
>> +                                       !strcmp(argv[i], "-mhard-float"))
>> +                               break;
>> +               }
>>
>> -       if (i == argc)
>> -               *cur++ = "-mfloat-abi=" BR_FLOAT_ABI;
>> +               if (i == argc)
>> +                       *cur++ = "-mfloat-abi=" BR_FLOAT_ABI;
>>   #endif
>>
>>   #if defined(BR_ARCH) || \
>> -    defined(BR_CPU)
>> -       /* Add our -march/cpu flags, but only if none of
>> -        * -march/mtune/mcpu are already specified on the commandline
>> -        */
>> -       for (i = 1; i < argc; i++) {
>> -               if (!strncmp(argv[i], "-march=", strlen("-march=")) ||
>> -                   !strncmp(argv[i], "-mtune=", strlen("-mtune=")) ||
>> -                   !strncmp(argv[i], "-mcpu=",  strlen("-mcpu=" )))
>> -                       break;
>> -       }
>> -       if (i == argc) {
>> +               defined(BR_CPU)
>> +               /* Add our -march/cpu flags, but only if none of
>> +                * -march/mtune/mcpu are already specified on the
>> commandline
>> +                */
>> +               for (i = 1; i < argc; i++) {
>> +                       if (!strncmp(argv[i], "-march=",
>> strlen("-march=")) ||
>> +                                       !strncmp(argv[i], "-mtune=",
>> strlen("-mtune=")) ||
>> +                                       !strncmp(argv[i], "-mcpu=",
>> strlen("-mcpu=" )))
>> +                               break;
>> +               }
>> +               if (i == argc) {
>>   #ifdef BR_ARCH
>> -               *cur++ = "-march=" BR_ARCH;
>> +                       *cur++ = "-march=" BR_ARCH;
>>   #endif
>>   #ifdef BR_CPU
>> -               *cur++ = "-mcpu=" BR_CPU;
>> +                       *cur++ = "-mcpu=" BR_CPU;
>>   #endif
>> -       }
>> +               }
>>   #endif /* ARCH || CPU */
>> +       }
>>
>>         paranoid_wrapper = getenv("BR_COMPILER_PARANOID_UNSAFE_PATH");
>>         if (paranoid_wrapper && strlen(paranoid_wrapper) > 0)
>>
>
>  BR_NO_CCACHE handling should also be skipped for ld, since
> ld_predef_args[0] is NOT ccache. Otherwise you'll get spectacular results
> when BR_NO_CCACHE is exported...
>
>  Probably the other things under CCACHE ifdefs should not be done for ld
> either, though they don't really hurt.
Ok, I'll rework the patch according to your comments, and think a bit
more on the open points.

Jan

>
>  Regards,
>  Arnout
>
> --
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-03-31 12:26 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-05 21:59 [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified Thomas Petazzoni
2013-06-05 21:59 ` [Buildroot] [PATCH 2/2] arch: define appropriate ld emulation values for the MIPS architecture Thomas Petazzoni
2013-06-06  8:37   ` Markos Chandras
2013-06-06  8:57     ` Thomas Petazzoni
2013-06-06  9:04       ` Markos Chandras
2013-06-06  8:31 ` [Buildroot] [PATCH 1/2] toolchain: wrap 'ld' so that a ld emulation can be specified Markos Chandras
2013-06-06  8:51   ` Thomas Petazzoni
2013-06-06  8:56     ` Markos Chandras
2013-06-06  9:13       ` Markos Chandras
2013-06-06 10:04         ` Markos Chandras
  -- strict thread matches above, loose matches on Subject: below --
2016-02-20  9:33 Jan Heylen
2016-03-22 16:19 ` Jan Heylen
2016-03-30 21:55 ` Arnout Vandecappelle
2016-03-31 12:26   ` Jan Heylen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox