toolchain/binutils: fix broken build of binutils 2.34 on mips64

[guidosarducci]

Review/Distribution List: @hauke, @diizzyy (based on similar fixes and commits in the repo)
Compile tested: mips64-le-malta, master branch
Run tested: mips64-le-malta, master branch

Description:
Commit 53470bd ("toolchain/binutils: Add binutils 2.34") logs refreshed patches, but also added a typo causing failed builds on mipsel64 platforms, including the malta subtarget. Update the patch to fix this.

This also fixes FS#3276.

Thanks for your review and any feedback!

[neheb]

openembedded has these:

https://git.openembedded.org/openembedded-core/tree/meta/recipes-devtools/binutils/binutils/0012-Detect-64-bit-MIPS-targets.patch?id=7a7667127625244aae2b1ff179e431fc9ad25737
https://git.openembedded.org/openembedded-core/tree/meta/recipes-devtools/binutils/binutils/0009-Change-default-emulation-for-mips64-linux.patch?id=7a7667127625244aae2b1ff179e431fc9ad25737

Seems this is already done there.

edit: actually...why don't we use their patch instead? It even has a description.

[diizzyy]

Pretty sure that I used openembedded as template, LGTM. Thanks

[neheb]

The openembedded patch is actually slightly different (le vs. be differences). Trying it out now.

[neheb]

Still panics. Issue is different looks like/

[guidosarducci]

Pretty sure that I used openembedded as template, LGTM. Thanks

You did, but it took me while to catch a small but lethal typo which crept into our version: targ_emul=lf64ltsmip vs. the correct targ_emul=elf64ltsmip.

Since this is clearly just a typo and does fix building, can we cherry-pick/merge this? Or did you want to make further updates to the patch set? I do think keeping the original email headers from the OE patches would be good for traceability/maintenance.

The openembedded patch is actually slightly different (le vs. be differences). Trying it out now.

I didn't see any other differences. Could you highlight them?

[neheb]

Pretty sure that I used openembedded as template, LGTM. Thanks

You did, but it took me while to catch a small but lethal typo which crept into our version: targ_emul=lf64ltsmip vs. the correct targ_emul=elf64ltsmip.

Since this is clearly just a typo and does fix building, can we cherry-pick/merge this? Or did you want to make further updates to the patch set? I do think keeping the original email headers from the OE patches would be good for traceability/maintenance.

The openembedded patch is actually slightly different (le vs. be differences). Trying it out now.

I didn't see any other differences. Could you highlight them?

--- a/toolchain/binutils/patches/2.34/500-Change-default-emulation-for-mips64-linux.patch
+++ b/toolchain/binutils/patches/2.34/500-Change-default-emulation-for-mips64-linux.patch
@@ -1,25 +1,44 @@
+From 13a67e9040c01abd284fe506471e0eab668ee3dc Mon Sep 17 00:00:00 2001
+From: Khem Raj <raj.khem@gmail.com>
+Date: Mon, 2 Mar 2015 01:44:14 +0000
+Subject: [PATCH 09/17] Change default emulation for mips64*-*-linux
+
+we change the default emulations to be N64 instead of N32
+
+Upstream-Status: Inappropriate [ OE configuration Specific]
+
+Signed-off-by: Khem Raj <raj.khem@gmail.com>
+---
+ bfd/config.bfd   | 8 ++++----
+ ld/configure.tgt | 8 ++++----
+ 2 files changed, 8 insertions(+), 8 deletions(-)
+
+diff --git a/bfd/config.bfd b/bfd/config.bfd
+index 14523caf0c5..e5233cd1f7e 100644
 --- a/bfd/config.bfd
 +++ b/bfd/config.bfd
-@@ -911,12 +911,12 @@ case "${targ}" in
+@@ -894,12 +894,12 @@ case "${targ}" in
      targ_selvecs="mips_elf32_le_vec mips_elf64_be_vec mips_elf64_le_vec mips_ecoff_be_vec mips_ecoff_le_vec"
      ;;
    mips64*el-*-linux*)
 -    targ_defvec=mips_elf32_ntrad_le_vec
 -    targ_selvecs="mips_elf32_ntrad_be_vec mips_elf32_trad_le_vec mips_elf32_trad_be_vec mips_elf64_trad_le_vec mips_elf64_trad_be_vec"
 +    targ_defvec=mips_elf64_trad_le_vec
-+    targ_selvecs="mips_elf32_ntrad_le_vec mips_elf32_ntrad_be_vec mips_elf32_trad_le_vec mips_elf32_trad_be_vec mips_elf64_trad_be_vec"
++    targ_selvecs="mips_elf32_ntrad_be_vec mips_elf32_ntrad_le_vec mips_elf32_trad_le_vec mips_elf32_trad_be_vec mips_elf64_trad_be_vec"
      ;;
    mips64*-*-linux*)
 -    targ_defvec=mips_elf32_ntrad_be_vec
 -    targ_selvecs="mips_elf32_ntrad_le_vec mips_elf32_trad_be_vec mips_elf32_trad_le_vec mips_elf64_trad_be_vec mips_elf64_trad_le_vec"
 +    targ_defvec=mips_elf64_trad_be_vec
-+    targ_selvecs="mips_elf32_ntrad_be_vec mips_elf32_ntrad_le_vec mips_elf32_trad_be_vec mips_elf32_trad_le_vec mips_elf64_trad_le_vec"
++    targ_selvecs="mips_elf32_ntrad_be_vec mips_elf32_ntrad_be_vec mips_elf32_trad_be_vec mips_elf32_trad_le_vec mips_elf64_trad_le_vec"
      ;;
    mips*el-*-linux*)
      targ_defvec=mips_elf32_trad_le_vec
+diff --git a/ld/configure.tgt b/ld/configure.tgt
+index 87c7d9a4cad..9b4bf2ca964 100644
 --- a/ld/configure.tgt
 +++ b/ld/configure.tgt
-@@ -541,12 +541,12 @@ mips*-*-vxworks*)        targ_emul=elf32ebmipvx
+@@ -531,12 +531,12 @@ mips*-*-vxworks*)        targ_emul=elf32ebmipvxworks
                        ;;
  mips*-*-windiss)      targ_emul=elf32mipswindiss
                        ;;
@@ -36,3 +55,6 @@
                        targ_extra_libpath=$targ_extra_emuls
                        ;;
  mips*el-*-linux-*)    targ_emul=elf32ltsmip
+-- 
+2.28.0

edit: note the ++ and -+

[guidosarducci]

OK, I forgot I reviewed this, and actually think there's a bug in the OE version of the patch. The general pattern for switching to the mips64 default is to exchange the targ_defvec="<vec>" entry with an entry from the list targ_selvecs="<vec> ... <vec>". You can see this both pre-patch and post-patch, and the entries in the combined set of targ_defvec and targ_selvecs are/should be unique.

Our patch does this correctly and the first section of the OE patch does this, but the second OE patch section instead sets:
targ_selvecs="mips_elf32_ntrad_be_vec mips_elf32_ntrad_be_vec mips_elf32_trad_be_vec mips_elf32_trad_le_vec mips_elf64_trad_le_vec"

It duplicates mips_elf32_ntrad_be_vec and omits mips_elf32_ntrad_le_vec, which I think is a mistake.

[hauke]

Thank you for the patch it was applied to master.
This is fixing the build problem, if we should change more we should do a separate commit for that.