use MIPS NewABI register names when disassembling NewABI code

use MIPS NewABI register names when disassembling NewABI code

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

mips-opc.c:_print_insn_mips decides which register naming convention
to use obtaining a pointer to the bfd from the first symbol in the
symbols field from the disassemble_info argument, and then looking for
ABI information in the ELF headers.  Unfortunately, gdb doesn't set up
symbols, so we end up always using the o32 register naming
convention.  This patch arranges for the information the disassembler
uses to be passed to it, in an admittedly ugly, but effective way.  Ok
to install?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mips-gdb-abi-disassemble.patch --]
[-- Type: text/x-patch, Size: 1338 bytes --]

Index: gdb/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* mips-tdep.c (mips_gdbarch_init): Set tm_print_insn_info.symbols
	to at least an empty symbol.

Index: gdb/mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.185
diff -u -p -r1.185 mips-tdep.c
--- gdb/mips-tdep.c 7 Apr 2003 18:38:04 -0000 1.185
+++ gdb/mips-tdep.c 8 Apr 2003 02:26:04 -0000
@@ -5656,6 +5656,24 @@ mips_gdbarch_init (struct gdbarch_info i
 
   if (info.abfd)
     {
+      static asymbol *symbols = NULL;
+
+      /* The disassembler uses *info.symbols to get to the bfd elf
+	 flags, which it uses to tell the executable ABI, so we have
+	 to give it at least one symbol.  */
+      if (tm_print_insn_info.symbols == NULL
+	  || tm_print_insn_info.symbols == &symbols)
+	{
+	  /* It would be nice if we could bfd_release the previous
+	     symbol, but we'd need a pointer to its bfd then.  */
+	  symbols = bfd_make_empty_symbol (info.abfd);
+	  if (symbols != NULL)
+	    {
+	      tm_print_insn_info.symbols = &symbols;
+	      tm_print_insn_info.num_symbols = 0;
+	    }
+	}
+
       /* First of all, extract the elf_flags, if available.  */
       if (bfd_get_flavour (info.abfd) == bfd_target_elf_flavour)
 	elf_flags = elf_elfheader (info.abfd)->e_flags;

[-- Attachment #3: Type: text/plain, Size: 289 bytes --]


-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: use MIPS NewABI register names when disassembling NewABI code

[Andrew Cagney]

> mips-opc.c:_print_insn_mips decides which register naming convention
> to use obtaining a pointer to the bfd from the first symbol in the
> symbols field from the disassemble_info argument, and then looking for
> ABI information in the ELF headers.  Unfortunately, gdb doesn't set up
> symbols, so we end up always using the o32 register naming
> convention.  This patch arranges for the information the disassembler
> uses to be passed to it, in an admittedly ugly, but effective way.

Ugly? This bit:
 > +      static asymbol *symbols = NULL;
is wrong.  There is more than one instance of an architecture.  The 
symbols lifetime is different to that of the architecture (assuming that 
the symbol's lifetime is tied to the corresponding bfd).

How does objdump manage to correctly disassemble something like an 
srecord?  GDB should be using that same mechanism.

Andrew


> Index: gdb/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	* mips-tdep.c (mips_gdbarch_init): Set tm_print_insn_info.symbols
> 	to at least an empty symbol.
> 
> Index: gdb/mips-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mips-tdep.c,v
> retrieving revision 1.185
> diff -u -p -r1.185 mips-tdep.c
> --- gdb/mips-tdep.c 7 Apr 2003 18:38:04 -0000 1.185
> +++ gdb/mips-tdep.c 8 Apr 2003 02:26:04 -0000
> @@ -5656,6 +5656,24 @@ mips_gdbarch_init (struct gdbarch_info i
>  
>    if (info.abfd)
>      {
> +      static asymbol *symbols = NULL;
> +
> +      /* The disassembler uses *info.symbols to get to the bfd elf
> +	 flags, which it uses to tell the executable ABI, so we have
> +	 to give it at least one symbol.  */
> +      if (tm_print_insn_info.symbols == NULL
> +	  || tm_print_insn_info.symbols == &symbols)
> +	{
> +	  /* It would be nice if we could bfd_release the previous
> +	     symbol, but we'd need a pointer to its bfd then.  */
> +	  symbols = bfd_make_empty_symbol (info.abfd);
> +	  if (symbols != NULL)
> +	    {
> +	      tm_print_insn_info.symbols = &symbols;
> +	      tm_print_insn_info.num_symbols = 0;
> +	    }
> +	}
> +
>        /* First of all, extract the elf_flags, if available.  */
>        if (bfd_get_flavour (info.abfd) == bfd_target_elf_flavour)
>  	elf_flags = elf_elfheader (info.abfd)->e_flags;
> 
> 

Re: use MIPS NewABI register names when disassembling NewABI code

[Alexandre Oliva]

On Apr  8, 2003, Andrew Cagney <ac131313@redhat.com> wrote:

> Ugly? This bit:
>> +      static asymbol *symbols = NULL;
> is wrong.  There is more than one instance of an architecture.

Uh.  I see.  So much for trying to get rid of one more call to
bfd_alloc.  This is easy to fix, though.

> How does objdump manage to correctly disassemble something like an
> srecord?

Presumably, it doesn't.  If it's not an ELF bfd, it has no way to tell
which ABI to disassemble for.

> GDB should be using that same mechanism.

That's exactly what I'm trying to do :-)

Is there any chance of getting this in after fixing the memory
allocation issue, or must I try to introduce some means for the caller
to specify which ABI to disassemble for?

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: use MIPS NewABI register names when disassembling NewABI code

[Andrew Cagney]

> On Apr  8, 2003, Andrew Cagney <ac131313@redhat.com> wrote:
> 
> 
>> Ugly? This bit:
> 
>>> +      static asymbol *symbols = NULL;
> 
>> is wrong.  There is more than one instance of an architecture.
> 
> 
> Uh.  I see.  So much for trying to get rid of one more call to
> bfd_alloc.  This is easy to fix, though.

What about the problem of lifetimes?

>> How does objdump manage to correctly disassemble something like an
>> srecord?
> 
> 
> Presumably, it doesn't.  If it's not an ELF bfd, it has no way to tell
> which ABI to disassemble for.

Fixing that will cause the gdb side of this problem to fall out.  GDB 
can then wiggle knobs very similar to what the user running objdump 
would be playing with.  See ARM and i386.

Andrew

Re: use MIPS NewABI register names when disassembling NewABI code

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]

On Apr  8, 2003, Andrew Cagney <ac131313@redhat.com> wrote:

>> On Apr  8, 2003, Andrew Cagney <ac131313@redhat.com> wrote:
>> 
>>> Ugly? This bit:
>> 
>>>> +      static asymbol *symbols = NULL;
>> 
>>> is wrong.  There is more than one instance of an architecture.
>> Uh.  I see.  So much for trying to get rid of one more call to
>> bfd_alloc.  This is easy to fix, though.

> What about the problem of lifetimes?

What do you mean?  I plan to use bfd_alloc for it, like
bfd_make_empty_symbol does, which means it's allocated on the bfd's
obstack, which gets it deallocated at the right time.

>>> How does objdump manage to correctly disassemble something like an
>>> srecord?

>> Presumably, it doesn't.  If it's not an ELF bfd, it has no way to
>> tell which ABI to disassemble for.

> Fixing that will cause the gdb side of this problem to fall out.

I don't see what's there to be fixed, really, and I can't see how to
change it either.  n32 and n64, the only ABIs that use different
register naming conventions, don't support anything other than ELF, so
it's only fair for it to use information from the ELF headers to
decide which naming convention to use.  The only missing bit in gdb is
to pass the needed information (namely the pointer to the bfd) to the
disassembler.

I don't have strong feelings against an approach such as
disassembly-flavor or so, I just think it's overkill in this case.
objdump already has code to figure it out itself, it's just that gdb
is not giving it means to obtain the information.

Or, put another way, I see gdb may want to standardize the way it
passes information to the disassembler, but my take is that, before
imposing changes on another module that already offers means to
implement a feature, an attempt should be made to implement the needed
feature that doesn't require changes in other modules.

Here's a revised patch.  Ok to install?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mips-gdb-abi-disassemble.patch --]
[-- Type: text/x-patch, Size: 2751 bytes --]

Index: gdb/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* Makefile.in (libbfd_h): New.
	(mips-tdep.o): Depend on it.
	* mips-tdep.c: Include it.
	(mips_gdbarch_init): Set tm_print_insn_info.symbols to at least
	an empty symbol.

Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/uberbaum/gdb/Makefile.in,v
retrieving revision 1.360
diff -u -p -r1.360 Makefile.in
--- gdb/Makefile.in 6 Apr 2003 01:13:58 -0000 1.360
+++ gdb/Makefile.in 9 Apr 2003 03:44:40 -0000
@@ -574,6 +574,7 @@ elf_sh_h =	$(INCLUDE_DIR)/elf/sh.h
 elf_arm_h =	$(INCLUDE_DIR)/elf/arm.h $(elf_reloc_macros_h)
 elf_bfd_h =	$(BFD_SRC)/elf-bfd.h
 libaout_h =	$(BFD_SRC)/libaout.h
+libbfd_h =	$(BFD_SRC)/libbfd.h
 remote_sim_h =	$(INCLUDE_DIR)/gdb/remote-sim.h
 demangle_h =    $(INCLUDE_DIR)/demangle.h
 obstack_h =     $(INCLUDE_DIR)/obstack.h
@@ -1955,7 +1956,7 @@ mips-tdep.o: mips-tdep.c $(defs_h) $(gdb
 	$(language_h) $(gdbcore_h) $(symfile_h) $(objfiles_h) \
 	$(gdbtypes_h) $(target_h) $(arch_utils_h) $(regcache_h) \
 	$(osabi_h) $(mips_tdep_h) $(block_h) $(opcode_mips_h) \
-	$(elf_mips_h) $(elf_bfd_h) $(symcat_h)
+	$(elf_mips_h) $(elf_bfd_h) $(libbfd_h) $(symcat_h)
 mipsm3-nat.o: mipsm3-nat.c $(defs_h) $(inferior_h) $(regcache_h)
 mipsnbsd-nat.o: mipsnbsd-nat.c $(defs_h) $(inferior_h) $(regcache_h) \
 	$(mipsnbsd_tdep_h)
Index: gdb/mips-tdep.c
===================================================================
RCS file: /cvs/uberbaum/gdb/mips-tdep.c,v
retrieving revision 1.187
diff -u -p -r1.187 mips-tdep.c
--- gdb/mips-tdep.c 8 Apr 2003 19:21:15 -0000 1.187
+++ gdb/mips-tdep.c 9 Apr 2003 03:44:43 -0000
@@ -45,6 +45,7 @@
 
 #include "opcode/mips.h"
 #include "elf/mips.h"
+#include "libbfd.h"
 #include "elf-bfd.h"
 #include "symcat.h"
 
@@ -5656,6 +5657,26 @@ mips_gdbarch_init (struct gdbarch_info i
 
   if (info.abfd)
     {
+      /* The disassembler uses *info.symbols to get to the bfd elf
+	 flags, which it uses to tell the executable ABI, so we have
+	 to give it at least one symbol.  */
+      if (tm_print_insn_info.symbols == NULL)
+	{
+	  asymbol *symbol = NULL;
+
+	  /* It would be nice if we could bfd_release the previous
+	     symbol, but we'd need a pointer to its bfd then.  */
+	  symbol = bfd_make_empty_symbol (info.abfd);
+	  if (symbol != NULL)
+	    {
+	      tm_print_insn_info.symbols = bfd_alloc (info.abfd,
+						      sizeof (symbol));
+	      if (tm_print_insn_info.symbols != NULL)
+		*tm_print_insn_info.symbols = symbol;
+	      tm_print_insn_info.num_symbols = 0;
+	    }
+	}
+
       /* First of all, extract the elf_flags, if available.  */
       if (bfd_get_flavour (info.abfd) == bfd_target_elf_flavour)
 	elf_flags = elf_elfheader (info.abfd)->e_flags;

[-- Attachment #3: Type: text/plain, Size: 289 bytes --]


-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: use MIPS NewABI register names when disassembling NewABI code

[Daniel Jacobowitz]

On Wed, Apr 09, 2003 at 12:45:28AM -0300, Alexandre Oliva wrote:
> +	  /* It would be nice if we could bfd_release the previous
> +	     symbol, but we'd need a pointer to its bfd then.  */
> +	  symbol = bfd_make_empty_symbol (info.abfd);

I'd rather lose that comment, lest someone be tempted to save a bfd
pointer in the right place to let us call bfd_release here.  Remember,
bfd_release frees its argument and all more recent allocations.  Hardly
wise in this context...

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: use MIPS NewABI register names when disassembling NewABI code

[Andrew Cagney]

>>> Uh.  I see.  So much for trying to get rid of one more call to
>>> bfd_alloc.  This is easy to fix, though.
> 
> 
>> What about the problem of lifetimes?
> 
> 
> What do you mean?  I plan to use bfd_alloc for it, like
> bfd_make_empty_symbol does, which means it's allocated on the bfd's
> obstack, which gets it deallocated at the right time.

In my original post I made two points:

> Ugly? This bit:
>> +      static asymbol *symbols = NULL;
> is wrong.  There is more than one instance of an architecture.  The symbols lifetime is different to that of the architecture (assuming that the symbol's lifetime is tied to the corresponding bfd).

An architecture can't point at anything tied to the lifetime of an 
instance of a BFD.

>>>> How does objdump manage to correctly disassemble something like an
>>>> srecord?
> 
> 
>>> Presumably, it doesn't.  If it's not an ELF bfd, it has no way to
>>> tell which ABI to disassemble for.
> 
> 
>> Fixing that will cause the gdb side of this problem to fall out.
> 
> 
> I don't see what's there to be fixed, really, and I can't see how to
> change it either.  n32 and n64, the only ABIs that use different
> register naming conventions, don't support anything other than ELF, so
> it's only fair for it to use information from the ELF headers to
> decide which naming convention to use.  The only missing bit in gdb is
> to pass the needed information (namely the pointer to the bfd) to the
> disassembler.

It is wrong to assume that there is a BFD.  GDB needs to be able to work 
correctly vis:

$ gdb
(gdb) set architecture <mips-variant>
(gdb) set mips disassembler <disassembler-variant>
(gdb) target remote
(gdb) disassemble

and this is identical to the situtation that objdump encounters when 
presented with an srecord.

Please first fix objdump, and then we can see about fixing GDB.

Andrew

Re: use MIPS NewABI register names when disassembling NewABI code

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

On Apr  9, 2003, Andrew Cagney <ac131313@redhat.com> wrote:

> An architecture can't point at anything tied to the lifetime of an
> instance of a BFD.

tm_print_insn_info doesn't seem to be very tied to a gdbarch, anyway.
To wit, note the bug fix in this revised patch.  Explicitly setting
the ABI to a non-default one, then back, then the non-default one
again, would result the wrong tm_print_insn_info settings.  This works
now, and also disassembles in accordance with gdb's notion of the
current ABI.  Assuming tm_print_insn_info is actually the right thing
to use, of course.  Ok to install?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mips-gdb-abi-disassemble.patch --]
[-- Type: text/x-patch, Size: 4930 bytes --]

Index: opcodes/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* mips-dis.c (_print_insn_mips): Override reg_names from
	disassembler_options.

Index: opcodes/mips-dis.c
===================================================================
RCS file: /cvs/src/src/opcodes/mips-dis.c,v
retrieving revision 1.43
diff -u -p -r1.43 mips-dis.c
--- opcodes/mips-dis.c 8 Apr 2003 07:14:47 -0000 1.43
+++ opcodes/mips-dis.c 11 Apr 2003 05:39:20 -0000
@@ -1157,6 +1157,14 @@ _print_insn_mips (memaddr, info, endiann
     return print_insn_mips16 (memaddr, info);
 #endif
 
+  if (info->disassembler_options)
+    {
+      if (strcmp (info->disassembler_options, "NewABI") == 0)
+	reg_names = mips64_reg_names;
+      else
+	reg_names = mips32_reg_names;
+    }
+
   status = (*info->read_memory_func) (memaddr, buffer, INSNLEN, info);
   if (status == 0)
     {
Index: gdb/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* Makefile.in (libbfd_h): Added missing setting.
	* mips-tdep.c (mips_gdbarch_init): Set disassembler_options
	according to the selected ABI.

Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.361
diff -u -p -r1.361 Makefile.in
--- gdb/Makefile.in 10 Apr 2003 13:15:50 -0000 1.361
+++ gdb/Makefile.in 11 Apr 2003 05:39:25 -0000
@@ -574,6 +574,7 @@ elf_sh_h =	$(INCLUDE_DIR)/elf/sh.h
 elf_arm_h =	$(INCLUDE_DIR)/elf/arm.h $(elf_reloc_macros_h)
 elf_bfd_h =	$(BFD_SRC)/elf-bfd.h
 libaout_h =	$(BFD_SRC)/libaout.h
+libbfd_h =	$(BFD_SRC)/libbfd.h
 remote_sim_h =	$(INCLUDE_DIR)/gdb/remote-sim.h
 demangle_h =    $(INCLUDE_DIR)/demangle.h
 obstack_h =     $(INCLUDE_DIR)/obstack.h
Index: gdb/mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.187
diff -u -p -r1.187 mips-tdep.c
--- gdb/mips-tdep.c 8 Apr 2003 19:21:15 -0000 1.187
+++ gdb/mips-tdep.c 11 Apr 2003 05:39:29 -0000
@@ -5731,6 +5731,31 @@ mips_gdbarch_init (struct gdbarch_info i
   if (wanted_abi != MIPS_ABI_UNKNOWN)
     mips_abi = wanted_abi;
 
+  /* We have to set tm_print_insn_info before looking for a
+     pre-existing architecture, otherwise we may return before we get
+     a chance to set it up.  */
+  if (mips_abi == MIPS_ABI_N32 || mips_abi == MIPS_ABI_N64)
+    {
+      /* Set up the disassembler info, so that we get the right
+	 register names from libopcodes.  */
+      tm_print_insn_info.disassembler_options = "NewABI";
+      tm_print_insn_info.flavour = bfd_target_elf_flavour;
+      tm_print_insn_info.arch = bfd_arch_mips;
+      if (info.bfd_arch_info != NULL
+	  && info.bfd_arch_info->arch == bfd_arch_mips
+	  && info.bfd_arch_info->mach)
+	tm_print_insn_info.mach = info.bfd_arch_info->mach;
+      else
+	tm_print_insn_info.mach = bfd_mach_mips8000;
+    }
+  else
+    /* This string is not recognized explicitly by the disassembler,
+       but it tells the disassembler to not try to guess the ABI from
+       the bfd elf headers, such that, if the user overrides the ABI
+       of a program linked as NewABI, the disassembly will follow the
+       register naming conventions specified by the user.  */
+    tm_print_insn_info.disassembler_options = "OldABI";
+
   if (gdbarch_debug)
     {
       fprintf_unfiltered (gdb_stdlog,
@@ -5875,18 +5900,6 @@ mips_gdbarch_init (struct gdbarch_info i
       set_gdbarch_long_bit (gdbarch, 32);
       set_gdbarch_ptr_bit (gdbarch, 32);
       set_gdbarch_long_long_bit (gdbarch, 64);
-
-      /* Set up the disassembler info, so that we get the right
-	 register names from libopcodes.  */
-      tm_print_insn_info.flavour = bfd_target_elf_flavour;
-      tm_print_insn_info.arch = bfd_arch_mips;
-      if (info.bfd_arch_info != NULL
-	  && info.bfd_arch_info->arch == bfd_arch_mips
-	  && info.bfd_arch_info->mach)
-	tm_print_insn_info.mach = info.bfd_arch_info->mach;
-      else
-	tm_print_insn_info.mach = bfd_mach_mips8000;
-
       set_gdbarch_use_struct_convention (gdbarch, 
 					 mips_n32n64_use_struct_convention);
       set_gdbarch_reg_struct_has_addr (gdbarch, 
@@ -5906,18 +5919,6 @@ mips_gdbarch_init (struct gdbarch_info i
       set_gdbarch_long_bit (gdbarch, 64);
       set_gdbarch_ptr_bit (gdbarch, 64);
       set_gdbarch_long_long_bit (gdbarch, 64);
-
-      /* Set up the disassembler info, so that we get the right
-	 register names from libopcodes.  */
-      tm_print_insn_info.flavour = bfd_target_elf_flavour;
-      tm_print_insn_info.arch = bfd_arch_mips;
-      if (info.bfd_arch_info != NULL
-	  && info.bfd_arch_info->arch == bfd_arch_mips
-	  && info.bfd_arch_info->mach)
-	tm_print_insn_info.mach = info.bfd_arch_info->mach;
-      else
-	tm_print_insn_info.mach = bfd_mach_mips8000;
-
       set_gdbarch_use_struct_convention (gdbarch, 
 					 mips_n32n64_use_struct_convention);
       set_gdbarch_reg_struct_has_addr (gdbarch, 

[-- Attachment #3: Type: text/plain, Size: 289 bytes --]


-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: use MIPS NewABI register names when disassembling NewABI code

[cgd]

At Fri, 11 Apr 2003 05:43:18 +0000 (UTC), "Alexandre Oliva" wrote:
> 	* mips-dis.c (_print_insn_mips): Override reg_names from
> 	disassembler_options.

Uh...  Well...  "I'm not the maintainer, but i'd say no."  8-)

Note the line:

  parse_mips_dis_options (info->disassembler_options);

about 18 lines above your new lines.  It already supports what you
want, but with a different syntax.

> +      tm_print_insn_info.disassembler_options = "NewABI";

"gpr_names=ABI" instead, for values of ABI: 32, n32, 64.

note that, as you probably expect 32 -> oldabi and n32,64 -> newabi.
(default -> oldabi)

IMO it's best to use the correct names for ABIs (i.e., for the
above-named ones or others) so that somebody can add a new ABI easily
to "the obvious place" in GDB, and to the obvious place in the
disassembler, and the right thing should happen automatically.

(It's a real bummer that there's no existing way to return an error
for invalid disassembler options, or i would have coded in some error
reporting.  8-)

(Also note that there's also support for doing the ABI-specific FP
register names, but that's a different option because everybody refers
to them numerically anyway.)


The rest of the gdb disassembler_options setup looks compatible with
the code in the disassembler, but it was a very quick look.  8-)


chris

Re: use MIPS NewABI register names when disassembling NewABI code

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 973 bytes --]

On Apr 11, 2003, cgd@broadcom.com wrote:

> At Fri, 11 Apr 2003 05:43:18 +0000 (UTC), "Alexandre Oliva" wrote:
>> * mips-dis.c (_print_insn_mips): Override reg_names from
>> disassembler_options.

> Uh...  Well...  "I'm not the maintainer, but i'd say no."  8-)

Fair enough :-)

> Note the line:

>   parse_mips_dis_options (info->disassembler_options);

> about 18 lines above your new lines.  It already supports what you
> want, but with a different syntax.

Sorry that I didn't try diff -u18 :-)

It really sucks to be working out of a relatively old branch and
trying to contribute to the net at the same time :-(

> "gpr_names=ABI" instead, for values of ABI: 32, n32, 64.

Thanks for the pointer.  Here's a fixed patch that refrains from
``fixing'' opcodes.

> IMO it's best to use the correct names for ABIs

Except that I've never been able to figure out whether the correct
name for the N64 abi is N64 or just 64.  Not that I care, mind you ;-)

Ok to install?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mips-gdb-abi-disassemble.patch --]
[-- Type: text/x-patch, Size: 4190 bytes --]

Index: gdb/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* Makefile.in (libbfd_h): Added missing setting.
	* mips-tdep.c (mips_gdbarch_init): Set disassembler_options
	according to the selected ABI.

Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.361
diff -u -p -r1.361 Makefile.in
--- gdb/Makefile.in 10 Apr 2003 13:15:50 -0000 1.361
+++ gdb/Makefile.in 11 Apr 2003 07:43:10 -0000
@@ -574,6 +574,7 @@ elf_sh_h =	$(INCLUDE_DIR)/elf/sh.h
 elf_arm_h =	$(INCLUDE_DIR)/elf/arm.h $(elf_reloc_macros_h)
 elf_bfd_h =	$(BFD_SRC)/elf-bfd.h
 libaout_h =	$(BFD_SRC)/libaout.h
+libbfd_h =	$(BFD_SRC)/libbfd.h
 remote_sim_h =	$(INCLUDE_DIR)/gdb/remote-sim.h
 demangle_h =    $(INCLUDE_DIR)/demangle.h
 obstack_h =     $(INCLUDE_DIR)/obstack.h
Index: gdb/mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.187
diff -u -p -r1.187 mips-tdep.c
--- gdb/mips-tdep.c 8 Apr 2003 19:21:15 -0000 1.187
+++ gdb/mips-tdep.c 11 Apr 2003 07:43:14 -0000
@@ -5731,6 +5731,34 @@ mips_gdbarch_init (struct gdbarch_info i
   if (wanted_abi != MIPS_ABI_UNKNOWN)
     mips_abi = wanted_abi;
 
+  /* We have to set tm_print_insn_info before looking for a
+     pre-existing architecture, otherwise we may return before we get
+     a chance to set it up.  */
+  if (mips_abi == MIPS_ABI_N32 || mips_abi == MIPS_ABI_N64)
+    {
+      /* Set up the disassembler info, so that we get the right
+	 register names from libopcodes.  */
+      if (mips_abi == MIPS_ABI_N32)
+	tm_print_insn_info.disassembler_options = "gpr-names=n32";
+      else
+	tm_print_insn_info.disassembler_options = "gpr-names=64";
+      tm_print_insn_info.flavour = bfd_target_elf_flavour;
+      tm_print_insn_info.arch = bfd_arch_mips;
+      if (info.bfd_arch_info != NULL
+	  && info.bfd_arch_info->arch == bfd_arch_mips
+	  && info.bfd_arch_info->mach)
+	tm_print_insn_info.mach = info.bfd_arch_info->mach;
+      else
+	tm_print_insn_info.mach = bfd_mach_mips8000;
+    }
+  else
+    /* This string is not recognized explicitly by the disassembler,
+       but it tells the disassembler to not try to guess the ABI from
+       the bfd elf headers, such that, if the user overrides the ABI
+       of a program linked as NewABI, the disassembly will follow the
+       register naming conventions specified by the user.  */
+    tm_print_insn_info.disassembler_options = "gpr-names=32";
+
   if (gdbarch_debug)
     {
       fprintf_unfiltered (gdb_stdlog,
@@ -5875,18 +5903,6 @@ mips_gdbarch_init (struct gdbarch_info i
       set_gdbarch_long_bit (gdbarch, 32);
       set_gdbarch_ptr_bit (gdbarch, 32);
       set_gdbarch_long_long_bit (gdbarch, 64);
-
-      /* Set up the disassembler info, so that we get the right
-	 register names from libopcodes.  */
-      tm_print_insn_info.flavour = bfd_target_elf_flavour;
-      tm_print_insn_info.arch = bfd_arch_mips;
-      if (info.bfd_arch_info != NULL
-	  && info.bfd_arch_info->arch == bfd_arch_mips
-	  && info.bfd_arch_info->mach)
-	tm_print_insn_info.mach = info.bfd_arch_info->mach;
-      else
-	tm_print_insn_info.mach = bfd_mach_mips8000;
-
       set_gdbarch_use_struct_convention (gdbarch, 
 					 mips_n32n64_use_struct_convention);
       set_gdbarch_reg_struct_has_addr (gdbarch, 
@@ -5906,18 +5922,6 @@ mips_gdbarch_init (struct gdbarch_info i
       set_gdbarch_long_bit (gdbarch, 64);
       set_gdbarch_ptr_bit (gdbarch, 64);
       set_gdbarch_long_long_bit (gdbarch, 64);
-
-      /* Set up the disassembler info, so that we get the right
-	 register names from libopcodes.  */
-      tm_print_insn_info.flavour = bfd_target_elf_flavour;
-      tm_print_insn_info.arch = bfd_arch_mips;
-      if (info.bfd_arch_info != NULL
-	  && info.bfd_arch_info->arch == bfd_arch_mips
-	  && info.bfd_arch_info->mach)
-	tm_print_insn_info.mach = info.bfd_arch_info->mach;
-      else
-	tm_print_insn_info.mach = bfd_mach_mips8000;
-
       set_gdbarch_use_struct_convention (gdbarch, 
 					 mips_n32n64_use_struct_convention);
       set_gdbarch_reg_struct_has_addr (gdbarch, 

[-- Attachment #3: Type: text/plain, Size: 289 bytes --]


-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: use MIPS NewABI register names when disassembling NewABI code

[Andrew Cagney]

> On Apr 11, 2003, cgd@broadcom.com wrote:
> 
> 
>> At Fri, 11 Apr 2003 05:43:18 +0000 (UTC), "Alexandre Oliva" wrote:
> 
>>> * mips-dis.c (_print_insn_mips): Override reg_names from
>>> disassembler_options.
> 
> 
>> Uh...  Well...  "I'm not the maintainer, but i'd say no."  8-)

Anything with the word `new' in it should set of alarm bells.  "New" (in 
edit case) doublely so :-)

> Index: gdb/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	* Makefile.in (libbfd_h): Added missing setting.

Obvious, thanks (Is the use of libbfd.h permitted though?).

> 	* mips-tdep.c (mips_gdbarch_init): Set disassembler_options
> 	according to the selected ABI.

Yes!!

Can you please create a bug report 
(http://www.gnu.org/software/gdb/bugs/, pointing out that the MIPS needs 
a ``set mips disassembler' command.

thanks,
Andrew


> Index: gdb/Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/Makefile.in,v
> retrieving revision 1.361
> diff -u -p -r1.361 Makefile.in
> --- gdb/Makefile.in 10 Apr 2003 13:15:50 -0000 1.361
> +++ gdb/Makefile.in 11 Apr 2003 07:43:10 -0000
> @@ -574,6 +574,7 @@ elf_sh_h =	$(INCLUDE_DIR)/elf/sh.h
>  elf_arm_h =	$(INCLUDE_DIR)/elf/arm.h $(elf_reloc_macros_h)
>  elf_bfd_h =	$(BFD_SRC)/elf-bfd.h
>  libaout_h =	$(BFD_SRC)/libaout.h
> +libbfd_h =	$(BFD_SRC)/libbfd.h
>  remote_sim_h =	$(INCLUDE_DIR)/gdb/remote-sim.h
>  demangle_h =    $(INCLUDE_DIR)/demangle.h
>  obstack_h =     $(INCLUDE_DIR)/obstack.h
> Index: gdb/mips-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mips-tdep.c,v
> retrieving revision 1.187
> diff -u -p -r1.187 mips-tdep.c
> --- gdb/mips-tdep.c 8 Apr 2003 19:21:15 -0000 1.187
> +++ gdb/mips-tdep.c 11 Apr 2003 07:43:14 -0000
> @@ -5731,6 +5731,34 @@ mips_gdbarch_init (struct gdbarch_info i
>    if (wanted_abi != MIPS_ABI_UNKNOWN)
>      mips_abi = wanted_abi;
>  
> +  /* We have to set tm_print_insn_info before looking for a
> +     pre-existing architecture, otherwise we may return before we get
> +     a chance to set it up.  */
> +  if (mips_abi == MIPS_ABI_N32 || mips_abi == MIPS_ABI_N64)
> +    {
> +      /* Set up the disassembler info, so that we get the right
> +	 register names from libopcodes.  */
> +      if (mips_abi == MIPS_ABI_N32)
> +	tm_print_insn_info.disassembler_options = "gpr-names=n32";
> +      else
> +	tm_print_insn_info.disassembler_options = "gpr-names=64";
> +      tm_print_insn_info.flavour = bfd_target_elf_flavour;
> +      tm_print_insn_info.arch = bfd_arch_mips;
> +      if (info.bfd_arch_info != NULL
> +	  && info.bfd_arch_info->arch == bfd_arch_mips
> +	  && info.bfd_arch_info->mach)
> +	tm_print_insn_info.mach = info.bfd_arch_info->mach;
> +      else
> +	tm_print_insn_info.mach = bfd_mach_mips8000;
> +    }
> +  else
> +    /* This string is not recognized explicitly by the disassembler,
> +       but it tells the disassembler to not try to guess the ABI from
> +       the bfd elf headers, such that, if the user overrides the ABI
> +       of a program linked as NewABI, the disassembly will follow the
> +       register naming conventions specified by the user.  */
> +    tm_print_insn_info.disassembler_options = "gpr-names=32";
> +
>    if (gdbarch_debug)
>      {
>        fprintf_unfiltered (gdb_stdlog,
> @@ -5875,18 +5903,6 @@ mips_gdbarch_init (struct gdbarch_info i
>        set_gdbarch_long_bit (gdbarch, 32);
>        set_gdbarch_ptr_bit (gdbarch, 32);
>        set_gdbarch_long_long_bit (gdbarch, 64);
> -
> -      /* Set up the disassembler info, so that we get the right
> -	 register names from libopcodes.  */
> -      tm_print_insn_info.flavour = bfd_target_elf_flavour;
> -      tm_print_insn_info.arch = bfd_arch_mips;
> -      if (info.bfd_arch_info != NULL
> -	  && info.bfd_arch_info->arch == bfd_arch_mips
> -	  && info.bfd_arch_info->mach)
> -	tm_print_insn_info.mach = info.bfd_arch_info->mach;
> -      else
> -	tm_print_insn_info.mach = bfd_mach_mips8000;
> -
>        set_gdbarch_use_struct_convention (gdbarch, 
>  					 mips_n32n64_use_struct_convention);
>        set_gdbarch_reg_struct_has_addr (gdbarch, 
> @@ -5906,18 +5922,6 @@ mips_gdbarch_init (struct gdbarch_info i
>        set_gdbarch_long_bit (gdbarch, 64);
>        set_gdbarch_ptr_bit (gdbarch, 64);
>        set_gdbarch_long_long_bit (gdbarch, 64);
> -
> -      /* Set up the disassembler info, so that we get the right
> -	 register names from libopcodes.  */
> -      tm_print_insn_info.flavour = bfd_target_elf_flavour;
> -      tm_print_insn_info.arch = bfd_arch_mips;
> -      if (info.bfd_arch_info != NULL
> -	  && info.bfd_arch_info->arch == bfd_arch_mips
> -	  && info.bfd_arch_info->mach)
> -	tm_print_insn_info.mach = info.bfd_arch_info->mach;
> -      else
> -	tm_print_insn_info.mach = bfd_mach_mips8000;
> -
>        set_gdbarch_use_struct_convention (gdbarch, 
>  					 mips_n32n64_use_struct_convention);
>        set_gdbarch_reg_struct_has_addr (gdbarch, 
> 
> 
> 
> 
> -- Alexandre Oliva Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/ Red Hat GCC Developer aoliva@{redhat.com, gcc.gnu.org} CS PhD student at IC-Unicamp oliva@{lsd.ic.unicamp.br, gnu.org} Free Software Evangelist Professional serial bug killer 

Re: use MIPS NewABI register names when disassembling NewABI code

[Alexandre Oliva]

On Apr 11, 2003, Andrew Cagney <ac131313@redhat.com> wrote:

> Anything with the word `new' in it should set of alarm bells.  "New"
> (in edit case) doublely so :-)

Well, that's in the name of the ABI, unfortunately.

>> Index: gdb/ChangeLog
>> from  Alexandre Oliva  <aoliva@redhat.com>
>> * Makefile.in (libbfd_h): Added missing setting.

> Obvious, thanks (Is the use of libbfd.h permitted though?).

I don't see why not.

>> * mips-tdep.c (mips_gdbarch_init): Set disassembler_options
>> according to the selected ABI.

> Yes!!

Wheee!  At last!  Thanks for shepherding me through the whole process
:-)

> Can you please create a bug report
> (http://www.gnu.org/software/gdb/bugs/, pointing out that the MIPS
> needs a ``set mips disassembler' command.

Done.

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: use MIPS NewABI register names when disassembling NewABI code

[Andrew Cagney]

> I don't see why not.
> 
> 
>>> * mips-tdep.c (mips_gdbarch_init): Set disassembler_options
>>> according to the selected ABI.
> 
> 
>> Yes!!
> 
> 
> Wheee!  At last!  Thanks for shepherding me through the whole process
> :-)

Um, does it work though?  I can see it working for:

	x/i $pc

but what about:

	disassemble

The former uses the global [about to be deprecated] variable 
tm_print_insn_info and would pick up the settings.  The latter, which 
was recently rewritten to not rely on that global, would not.

(the power of deprecation :-),

Andrew