[PATCH] Fix v850 bfd arch info printable names (was: Re: Commit: V850: Add support for fixed ABI implementation)

[Pedro Alves <palves at redhat dot com>]

Hi Nick,

On 11/09/2012 05:30 PM, Nick Clifton wrote:

> 	* cpu-v850.c: Update printed description.
> 	* cpu-v850_rh850.c: New file.

I wrote a GDB test that tries all "(gdb) set architecture foo"
options, and surprisingly found that it failed with an
--enable-targets=all gdb build, when it tried to set some of the v850
archs:

 (gdb) set architecture v850<TAB>
 v850 (using old gcc ABI)
 v850-rh850
 v850e
 v850e (using old gcc ABI)
 v850e1
 v850e1 (using old gcc ABI)
 v850e2
 v850e2 (using old gcc ABI)
 v850e2v3
 v850e2v3 (using old gcc ABI)
 v850e2v4
 v850e2v4 (using old gcc ABI)
 v850e3v5
 v850e3v5 (using old gcc ABI)
 (gdb) set architecture v850 (using old gcc ABI)
 Ambiguous item "v850 (using old gcc ABI)".

The problem above is that "set architecture" is a GDB "enum command",
and GDB only considers an enum value to be the string up until the
first space.  So writing "v850 (using old gcc ABI)" is the same as
writing "v850", and then that's obviously ambiguous.

v850 is actually the only arch that has spaces in its printable name.
One can conveniently see that with (e.g.):

 $ gdb
  (...)
 (gdb) set max-completions unlimited
 (gdb) set architecture <TAB>
 Display all 295 possibilities? (y or n)
  (... many snipped ...)
 m68k:5407                      sparc:sparclite
 m68k:547x                      sparc:sparclite_le
 m68k:548x                      sparc:v8plus
 m68k:68000                     sparc:v8plusa
 m68k:68008                     sparc:v8plusb
 m68k:68010                     sparc:v9
 m68k:68020                     sparc:v9a
 m68k:68030                     sparc:v9b
 m68k:68040                     spu:256K
 m68k:68060                     tic6x
 m68k:cfv4e                     tilegx
 m68k:cpu32                     tilegx32
 m68k:fido                      tomcat
 m68k:isa-a                     v850 (using old gcc ABI)
 m68k:isa-a:emac                v850-rh850
 m68k:isa-a:mac                 v850e
 m68k:isa-a:nodiv               v850e (using old gcc ABI)
 m68k:isa-aplus                 v850e1
 m68k:isa-aplus:emac            v850e1 (using old gcc ABI)
 m68k:isa-aplus:mac             v850e2
 m68k:isa-b                     v850e2 (using old gcc ABI)
 m68k:isa-b:emac                v850e2v3
 m68k:isa-b:float               v850e2v3 (using old gcc ABI)
 m68k:isa-b:float:emac          v850e2v4
 m68k:isa-b:float:mac           v850e2v4 (using old gcc ABI)
 m68k:isa-b:mac                 v850e3v5
 m68k:isa-b:nousp               v850e3v5 (using old gcc ABI)
 m68k:isa-b:nousp:emac          vax
 m68k:isa-b:nousp:mac           xscale
 m68k:isa-c                     xstormy16
  (... many snipped ...)
 (gdb)

Rather than hack GDB into accepting this somehow, maybe we can make
v850 arch printable names more like the others, and put the abi
variant in the "machine" part, after a ':'.  There's precedent for
that in several other archs, e.g.:

 (gdb) set architecture i386<TAB>
 i386
 i386:intel
 i386:nacl
 i386:x64-32
 i386:x64-32:intel
 i386:x64-32:nacl
 i386:x86-64
 i386:x86-64:intel
 i386:x86-64:nacl

 (gdb) set architecture aarch64<TAB>
 aarch64
 aarch64:ilp32

After the patch we now get:

 (gdb) set architecture v850<TAB>
 v850:old-gcc-abi
 v850:rh850
 v850e
 v850e1
 v850e1:old-gcc-abi
 v850e2
 v850e2:old-gcc-abi
 v850e2v3
 v850e2v3:old-gcc-abi
 v850e2v4
 v850e2v4:old-gcc-abi
 v850e3v5
 v850e3v5:old-gcc-abi
 v850e:old-gcc-abi

And now "set architecture v850:old-gcc-abi" works as expected.

(I'd find it a bit more consistent to either drop ":rh850" from the
"v850" entry, or add it to all others !old-gcc-abi cases as well, but
I didn't do that.)

I ran the binutils/gas/ld testsuites, and found no regressions.  I
don't have a cross compiler handy, but I ran the gdb tests anyway,
which covers at least some snoke testing.  Surprisingly, while the ld
testsuite didn't catch the need to adjust
ld/scripttempl/v850_rh850.sc, the gdb.asm/asm-source.exp gdb test did.

Actually, I think that the OUTPUT_ARCH in ld/scripttempl/v850.sc may
have got broken with the 2012 change?  I hacked v850_rh850.sc to
output "v850" and ld failed to grok it.  Maybe it only works if the
old GCC ABI is the configured default -- at least, I don't see how
bfd_default_scan would handle bare "v850" otherwise.

And it turns out the patch actually "fixes" an existing GDB test,
which isn't likewise expecting spaces in arch names:

  (gdb) FAIL: gdb.xml/tdesc-arch.exp: read valid architectures

bfd/ChangeLog:
2016-03-07  Pedro Alves  <palves@redhat.com>

	* cpu-v850.c (N): Append ":old-gcc-abi" instead of " (using old
	gcc ABI)" to printable name.
	* cpu-v850_rh850.c (bfd_v850_rh850_arch): Use "v850:rh850" instead
	of "v850-rh850" as printable name.

ld/ChangeLog:
2016-03-07  Pedro Alves  <palves@redhat.com>

	* scripttempl/v850.sc: Use "v850:old-gcc-abi" as OUTPUT_ARCH.
	* scripttempl/v850_rh850.sc: Use "v850:rh850" as OUTPUT_ARCH.
---
 bfd/cpu-v850.c               | 2 +-
 bfd/cpu-v850_rh850.c         | 2 +-
 ld/scripttempl/v850.sc       | 2 +-
 ld/scripttempl/v850_rh850.sc | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/bfd/cpu-v850.c b/bfd/cpu-v850.c
index 472c578..d82d4c0 100644
--- a/bfd/cpu-v850.c
+++ b/bfd/cpu-v850.c
@@ -24,7 +24,7 @@
 #include "safe-ctype.h"
 
 #define N(number, print, default, next)  \
-{  32, 32, 8, bfd_arch_v850, number, "v850", print " (using old gcc ABI)", 2, default, \
+{  32, 32, 8, bfd_arch_v850, number, "v850", print ":old-gcc-abi", 2, default, \
    bfd_default_compatible, bfd_default_scan, bfd_arch_default_fill, next }
 
 #define NEXT NULL
diff --git a/bfd/cpu-v850_rh850.c b/bfd/cpu-v850_rh850.c
index 3b58887..e86749b 100644
--- a/bfd/cpu-v850_rh850.c
+++ b/bfd/cpu-v850_rh850.c
@@ -38,4 +38,4 @@ static const bfd_arch_info_type arch_info_struct[] =
 };
 
 const bfd_arch_info_type bfd_v850_rh850_arch =
-  R (bfd_mach_v850,     "v850-rh850",   TRUE,  & arch_info_struct[0]);
+  R (bfd_mach_v850,     "v850:rh850",   TRUE,  & arch_info_struct[0]);
diff --git a/ld/scripttempl/v850.sc b/ld/scripttempl/v850.sc
index e338bf1..cf7cd20 100644
--- a/ld/scripttempl/v850.sc
+++ b/ld/scripttempl/v850.sc
@@ -13,7 +13,7 @@ cat << EOF
 
 OUTPUT_FORMAT("elf32-v850", "elf32-v850",
 	      "elf32-v850")
-OUTPUT_ARCH(v850)
+OUTPUT_ARCH(v850:old-gcc-abi)
 ${RELOCATING+ENTRY(_start)}
 SEARCH_DIR(.);
 EXTERN(__ctbp __ep __gp);
diff --git a/ld/scripttempl/v850_rh850.sc b/ld/scripttempl/v850_rh850.sc
index 06e5243..a44b9b5 100644
--- a/ld/scripttempl/v850_rh850.sc
+++ b/ld/scripttempl/v850_rh850.sc
@@ -13,7 +13,7 @@ cat << EOF
 
 OUTPUT_FORMAT("elf32-v850-rh850", "elf32-v850-rh850",
 	      "elf32-v850-rh850")
-OUTPUT_ARCH(v850-rh850)
+OUTPUT_ARCH(v850:rh850)
 ${RELOCATING+ENTRY(_start)}
 SEARCH_DIR(.);
 EXTERN(__ctbp __ep __gp);
-- 
2.5.0