[1/8] Refactor disassembly code

Message ID 1484051178-16013-2-git-send-email-yao.qi@linaro.org
State New, archived
Headers

Commit Message

Yao Qi Jan. 10, 2017, 12:26 p.m. UTC
  This patch addes class gdb_disassembler, and refactor
code to use it.  The gdb_disassembler object is saved
in disassember_info.application_data.  However,
disassember_info.application_data is already used by
gdb for arm, mips spu, and scm-disasm.  In arm and mips,
.application_data is gdbarch, but we can still get gdbarch
from gdb_disassember.

The use of application_data in spu is a little bit
complicated.  It creates its own disassemble_info, and
save spu_dis_asm_data in .application_data.  This will
overwrite the pointer to gdb_disassembler, so we need
to find another place to save spu_dis_asm_data.  I
extend disassemble_info, and put "id" there.

gdb:

2017-01-10  Pedro Alves  <palves@redhat.com>
	    Yao Qi  <yao.qi@linaro.org>

	* arm-tdep.c: Include "disasm.h".
	(gdb_print_insn_arm): Update code to get gdbarch.
	* disasm.c (dis_asm_read_memory): Change it to
	gdb_disassembler::dis_asm_read_memory.
	(dis_asm_memory_error): Likewise.
	(dis_asm_print_address): Likewise.
	(gdb_pretty_print_insn): Change it to
	gdb_disassembler::pretty_print_insn.
	(dump_insns): Add one argument gdb_disassemlber.  All
	callers updated.
	(do_mixed_source_and_assembly_deprecated): Likewise.
	(do_mixed_source_and_assembly): Likewise.
	(do_assembly_only): Likewise.
	(gdb_disassembler::gdb_disassembler): New.
	(gdb_disassembler::print_insn): New.
	* disasm.h (class gdb_disassembler): New.
	(gdb_pretty_print_insn): Remove declaration.
	(gdb_disassemble_info): Likewise.
	* guile/scm-disasm.c (class gdbscm_disassembler): New.
	(gdbscm_disasm_read_memory_worker): Update.
	(gdbscm_disasm_read_memory): Update.
	(gdbscm_disasm_memory_error): Remove.
	(gdbscm_disasm_print_address): Remove.
	(gdbscm_disassembler::gdbscm_disassembler): New.
	(gdbscm_print_insn_from_port): Update.
	* mips-tdep.c: Include disasm.h.
	(gdb_print_insn_mips): Update code to get gdbarch.
	* record-btrace.c (btrace_insn_history): Update.
	* spu-tdep.c: Include disasm.h.
	(struct spu_dis_asm_data): Remove.
	(struct spu_dis_asm_info): New.
	(spu_dis_asm_print_address): Use spu_dis_asm_info to get
	SPU id.
	(gdb_print_insn_spu): Cast disassemble_info to
	spu_dis_asm_info.
---
 gdb/arm-tdep.c         |   5 +-
 gdb/disasm.c           | 159 ++++++++++++++++++++++++++++---------------------
 gdb/disasm.h           |  55 +++++++++++++----
 gdb/guile/scm-disasm.c |  77 +++++++-----------------
 gdb/mips-tdep.c        |   5 +-
 gdb/record-btrace.c    |   5 +-
 gdb/spu-tdep.c         |  20 +++----
 7 files changed, 172 insertions(+), 154 deletions(-)
  

Comments

Simon Marchi Jan. 11, 2017, 8:42 p.m. UTC | #1
On 2017-01-10 07:26, Yao Qi wrote:
> @@ -761,15 +768,15 @@ fprintf_disasm (void *stream, const char *format, 
> ...)
>    return 0;
>  }
> 
> -struct disassemble_info
> -gdb_disassemble_info (struct gdbarch *gdbarch, struct ui_file *file)
> +gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
> +				    struct ui_file *file,
> +				    di_read_memory_ftype func)

Perhaps name this parameter "read_memory_func"?

> diff --git a/gdb/disasm.h b/gdb/disasm.h
> index 4c6fd54..5592cdb 100644
> --- a/gdb/disasm.h
> +++ b/gdb/disasm.h
> @@ -33,6 +33,48 @@ struct gdbarch;
>  struct ui_out;
>  struct ui_file;
> 
> +class gdb_disassembler
> +{
> +  using di_read_memory_ftype = decltype 
> (disassemble_info::read_memory_func);
> +
> +public:
> +  gdb_disassembler (struct gdbarch *gdbarch, struct ui_file *file)
> +    : gdb_disassembler (gdbarch, file, dis_asm_read_memory)
> +  {}
> +
> +  int print_insn (CORE_ADDR memaddr);
> +  int print_insn (CORE_ADDR memaddr, int *branch_delay_insns);

Not very important, but since print_insn(CORE_ADDR) is trivial, you 
could merge those two methods and provide a default parameter value of 
NULL for branch_delay_insns.

> +
> +  /* Prints the instruction INSN into UIOUT and returns the length of
> +     the printed instruction in bytes.  */
> +  int pretty_print_insn (struct ui_out *uiout,
> +			 const struct disasm_insn *insn, int flags);

It could be a good time to modernize the flags parameter and make it an 
enum flag.
  
Yao Qi Jan. 12, 2017, 12:19 p.m. UTC | #2
On 17-01-11 15:42:58, Simon Marchi wrote:
> On 2017-01-10 07:26, Yao Qi wrote:
> >@@ -761,15 +768,15 @@ fprintf_disasm (void *stream, const char
> >*format, ...)
> >   return 0;
> > }
> >
> >-struct disassemble_info
> >-gdb_disassemble_info (struct gdbarch *gdbarch, struct ui_file *file)
> >+gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
> >+				    struct ui_file *file,
> >+				    di_read_memory_ftype func)
> 
> Perhaps name this parameter "read_memory_func"?
> 

OK.

> >diff --git a/gdb/disasm.h b/gdb/disasm.h
> >index 4c6fd54..5592cdb 100644
> >--- a/gdb/disasm.h
> >+++ b/gdb/disasm.h
> >@@ -33,6 +33,48 @@ struct gdbarch;
> > struct ui_out;
> > struct ui_file;
> >
> >+class gdb_disassembler
> >+{
> >+  using di_read_memory_ftype = decltype
> >(disassemble_info::read_memory_func);
> >+
> >+public:
> >+  gdb_disassembler (struct gdbarch *gdbarch, struct ui_file *file)
> >+    : gdb_disassembler (gdbarch, file, dis_asm_read_memory)
> >+  {}
> >+
> >+  int print_insn (CORE_ADDR memaddr);
> >+  int print_insn (CORE_ADDR memaddr, int *branch_delay_insns);
> 
> Not very important, but since print_insn(CORE_ADDR) is trivial, you
> could merge those two methods and provide a default parameter value
> of NULL for branch_delay_insns.

OK.  Fixed them locally.

> 
> >+
> >+  /* Prints the instruction INSN into UIOUT and returns the length of
> >+     the printed instruction in bytes.  */
> >+  int pretty_print_insn (struct ui_out *uiout,
> >+			 const struct disasm_insn *insn, int flags);
> 
> It could be a good time to modernize the flags parameter and make it
> an enum flag.
> 

This can be done later.
  
Pedro Alves Jan. 12, 2017, 12:36 p.m. UTC | #3
On 01/12/2017 12:19 PM, Yao Qi wrote:

>>> diff --git a/gdb/disasm.h b/gdb/disasm.h
>>> index 4c6fd54..5592cdb 100644
>>> --- a/gdb/disasm.h
>>> +++ b/gdb/disasm.h
>>> @@ -33,6 +33,48 @@ struct gdbarch;
>>> struct ui_out;
>>> struct ui_file;
>>>
>>> +class gdb_disassembler
>>> +{
>>> +  using di_read_memory_ftype = decltype
>>> (disassemble_info::read_memory_func);
>>> +
>>> +public:
>>> +  gdb_disassembler (struct gdbarch *gdbarch, struct ui_file *file)
>>> +    : gdb_disassembler (gdbarch, file, dis_asm_read_memory)
>>> +  {}
>>> +
>>> +  int print_insn (CORE_ADDR memaddr);
>>> +  int print_insn (CORE_ADDR memaddr, int *branch_delay_insns);
>>
>> Not very important, but since print_insn(CORE_ADDR) is trivial, you
>> could merge those two methods and provide a default parameter value
>> of NULL for branch_delay_insns.
> 
> OK.  Fixed them locally.

I had written it that way originally because a default parameter forces
the compiler to pass down an extra parameter (adding to register pressure)
to all call sites, when only a few places actually need the extra
output parameter.  It's like a double-optional -- i.e., the
parameter can be NULL, so merging doesn't simplify that much,
given that the version with the single argument does not need to
check the parameter.  I.e., one function can be built on top of the
other.  I see it as a different case from when a parameter is optional
such that the passed in value always need to be taken in consideration
by the method implementation, like when passing a flags argument, with
the default being some flag value (or zero).

But this is not really performance critical code, so if you
want to change it, I don't mind.

Thanks,
Pedro Alves
  
Simon Marchi Jan. 12, 2017, 3:29 p.m. UTC | #4
On 2017-01-12 07:36, Pedro Alves wrote:
> I had written it that way originally because a default parameter forces
> the compiler to pass down an extra parameter (adding to register 
> pressure)
> to all call sites, when only a few places actually need the extra
> output parameter.  It's like a double-optional -- i.e., the
> parameter can be NULL, so merging doesn't simplify that much,
> given that the version with the single argument does not need to
> check the parameter.  I.e., one function can be built on top of the
> other.  I see it as a different case from when a parameter is optional
> such that the passed in value always need to be taken in consideration
> by the method implementation, like when passing a flags argument, with
> the default being some flag value (or zero).
> 
> But this is not really performance critical code, so if you
> want to change it, I don't mind.

Ok, I hadn't gone that far in my reflexion, I thought it was simply a 
convenience thing.
  

Patch

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 2bdfa57..0ae311f 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -27,6 +27,7 @@ 
 #include "gdbcmd.h"
 #include "gdbcore.h"
 #include "dis-asm.h"		/* For register styles.  */
+#include "disasm.h"
 #include "regcache.h"
 #include "reggroups.h"
 #include "doublest.h"
@@ -7739,7 +7740,9 @@  arm_displaced_step_fixup (struct gdbarch *gdbarch,
 static int
 gdb_print_insn_arm (bfd_vma memaddr, disassemble_info *info)
 {
-  struct gdbarch *gdbarch = (struct gdbarch *) info->application_data;
+  gdb_disassembler *di
+    = static_cast<gdb_disassembler *>(info->application_data);
+  struct gdbarch *gdbarch = di->arch ();
 
   if (arm_pc_is_thumb (gdbarch, memaddr))
     {
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 84a03e9..6a3f3aa 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -120,28 +120,34 @@  line_has_code_p (htab_t table, struct symtab *symtab, int line)
 }
 
 /* Like target_read_memory, but slightly different parameters.  */
-static int
-dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr, unsigned int len,
-		     struct disassemble_info *info)
+
+int
+gdb_disassembler::dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr,
+				       unsigned int len,
+				       struct disassemble_info *info)
 {
   return target_read_code (memaddr, myaddr, len);
 }
 
 /* Like memory_error with slightly different parameters.  */
-static void
-dis_asm_memory_error (int err, bfd_vma memaddr,
-		      struct disassemble_info *info)
+
+void
+gdb_disassembler::dis_asm_memory_error (int err, bfd_vma memaddr,
+					struct disassemble_info *info)
 {
   memory_error (TARGET_XFER_E_IO, memaddr);
 }
 
 /* Like print_address with slightly different parameters.  */
-static void
-dis_asm_print_address (bfd_vma addr, struct disassemble_info *info)
+
+void
+gdb_disassembler::dis_asm_print_address (bfd_vma addr,
+					 struct disassemble_info *info)
 {
-  struct gdbarch *gdbarch = (struct gdbarch *) info->application_data;
+  gdb_disassembler *self
+    = static_cast<gdb_disassembler *>(info->application_data);
 
-  print_address (gdbarch, addr, (struct ui_file *) info->stream);
+  print_address (self->arch (), addr, self->stream ());
 }
 
 static int
@@ -173,10 +179,9 @@  compare_lines (const void *mle1p, const void *mle2p)
 /* See disasm.h.  */
 
 int
-gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
-		       struct disassemble_info * di,
-		       const struct disasm_insn *insn, int flags,
-		       struct ui_file *stb)
+gdb_disassembler::pretty_print_insn (struct ui_out *uiout,
+				     const struct disasm_insn *insn,
+				     int flags)
 {
   /* parts of the symbolic representation of the address */
   int unmapped;
@@ -187,6 +192,8 @@  gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
   char *filename = NULL;
   char *name = NULL;
   CORE_ADDR pc;
+  struct ui_file *stb = stream ();
+  struct gdbarch *gdbarch = arch ();
 
   ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
   pc = insn->addr;
@@ -254,14 +261,14 @@  gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
       struct cleanup *cleanups =
 	make_cleanup_ui_file_delete (opcode_stream);
 
-      size = gdbarch_print_insn (gdbarch, pc, di);
+      size = print_insn (pc);
       end_pc = pc + size;
 
       for (;pc < end_pc; ++pc)
 	{
-	  err = (*di->read_memory_func) (pc, &data, 1, di);
+	  err = m_di.read_memory_func (pc, &data, 1, &m_di);
 	  if (err != 0)
-	    (*di->memory_error_func) (err, pc, di);
+	    m_di.memory_error_func (err, pc, &m_di);
 	  fprintf_filtered (opcode_stream, "%s%02x",
 			    spacer, (unsigned) data);
 	  spacer = " ";
@@ -273,7 +280,7 @@  gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
       do_cleanups (cleanups);
     }
   else
-    size = gdbarch_print_insn (gdbarch, pc, di);
+    size = print_insn (pc);
 
   uiout->field_stream ("inst", stb);
   ui_file_rewind (stb);
@@ -284,10 +291,9 @@  gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
 }
 
 static int
-dump_insns (struct gdbarch *gdbarch, struct ui_out *uiout,
-	    struct disassemble_info * di,
+dump_insns (struct ui_out *uiout, gdb_disassembler *di,
 	    CORE_ADDR low, CORE_ADDR high,
-	    int how_many, int flags, struct ui_file *stb,
+	    int how_many, int flags,
 	    CORE_ADDR *end_pc)
 {
   struct disasm_insn insn;
@@ -300,7 +306,7 @@  dump_insns (struct gdbarch *gdbarch, struct ui_out *uiout,
     {
       int size;
 
-      size = gdb_pretty_print_insn (gdbarch, uiout, di, &insn, flags, stb);
+      size = di->pretty_print_insn (uiout, &insn, flags);
       if (size <= 0)
 	break;
 
@@ -326,10 +332,10 @@  dump_insns (struct gdbarch *gdbarch, struct ui_out *uiout,
 
 static void
 do_mixed_source_and_assembly_deprecated
-  (struct gdbarch *gdbarch, struct ui_out *uiout,
-   struct disassemble_info *di, struct symtab *symtab,
+  (struct ui_out *uiout,
+   gdb_disassembler *di, struct symtab *symtab,
    CORE_ADDR low, CORE_ADDR high,
-   int how_many, int flags, struct ui_file *stb)
+   int how_many, int flags)
 {
   int newlines = 0;
   int nlines;
@@ -462,9 +468,9 @@  do_mixed_source_and_assembly_deprecated
 	    = make_cleanup_ui_out_list_begin_end (uiout, "line_asm_insn");
 	}
 
-      num_displayed += dump_insns (gdbarch, uiout, di,
+      num_displayed += dump_insns (uiout, di,
 				   mle[i].start_pc, mle[i].end_pc,
-				   how_many, flags, stb, NULL);
+				   how_many, flags, NULL);
 
       /* When we've reached the end of the mle array, or we've seen the last
          assembly range for this source line, close out the list/tuple.  */
@@ -488,11 +494,12 @@  do_mixed_source_and_assembly_deprecated
    immediately following.  */
 
 static void
-do_mixed_source_and_assembly (struct gdbarch *gdbarch, struct ui_out *uiout,
-			      struct disassemble_info *di,
+do_mixed_source_and_assembly (struct gdbarch *gdbarch,
+			      struct ui_out *uiout,
+			      gdb_disassembler *di,
 			      struct symtab *main_symtab,
 			      CORE_ADDR low, CORE_ADDR high,
-			      int how_many, int flags, struct ui_file *stb)
+			      int how_many, int flags)
 {
   const struct linetable_entry *le, *first_le;
   int i, nlines;
@@ -716,8 +723,8 @@  do_mixed_source_and_assembly (struct gdbarch *gdbarch, struct ui_out *uiout,
 	end_pc = std::min (sal.end, high);
       else
 	end_pc = pc + 1;
-      num_displayed += dump_insns (gdbarch, uiout, di, pc, end_pc,
-				   how_many, flags, stb, &end_pc);
+      num_displayed += dump_insns (uiout, di, pc, end_pc,
+				   how_many, flags, &end_pc);
       pc = end_pc;
 
       if (how_many >= 0 && num_displayed >= how_many)
@@ -732,16 +739,16 @@  do_mixed_source_and_assembly (struct gdbarch *gdbarch, struct ui_out *uiout,
 }
 
 static void
-do_assembly_only (struct gdbarch *gdbarch, struct ui_out *uiout,
-		  struct disassemble_info * di,
+do_assembly_only (struct ui_out *uiout,
+		  gdb_disassembler *di,
 		  CORE_ADDR low, CORE_ADDR high,
-		  int how_many, int flags, struct ui_file *stb)
+		  int how_many, int flags)
 {
   struct cleanup *ui_out_chain;
 
   ui_out_chain = make_cleanup_ui_out_list_begin_end (uiout, "asm_insns");
 
-  dump_insns (gdbarch, uiout, di, low, high, how_many, flags, stb, NULL);
+  dump_insns (uiout, di, low, high, how_many, flags, NULL);
 
   do_cleanups (ui_out_chain);
 }
@@ -761,15 +768,15 @@  fprintf_disasm (void *stream, const char *format, ...)
   return 0;
 }
 
-struct disassemble_info
-gdb_disassemble_info (struct gdbarch *gdbarch, struct ui_file *file)
+gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
+				    struct ui_file *file,
+				    di_read_memory_ftype func)
+  : m_gdbarch (gdbarch)
 {
-  struct disassemble_info di;
-
-  init_disassemble_info (&di, file, fprintf_disasm);
-  di.flavour = bfd_target_unknown_flavour;
-  di.memory_error_func = dis_asm_memory_error;
-  di.print_address_func = dis_asm_print_address;
+  init_disassemble_info (&m_di, file, fprintf_disasm);
+  m_di.flavour = bfd_target_unknown_flavour;
+  m_di.memory_error_func = dis_asm_memory_error;
+  m_di.print_address_func = dis_asm_print_address;
   /* NOTE: cagney/2003-04-28: The original code, from the old Insight
      disassembler had a local optomization here.  By default it would
      access the executable file, instead of the target memory (there
@@ -778,14 +785,37 @@  gdb_disassemble_info (struct gdbarch *gdbarch, struct ui_file *file)
      didn't work as they relied on the access going to the target.
      Further, it has been supperseeded by trust-read-only-sections
      (although that should be superseeded by target_trust..._p()).  */
-  di.read_memory_func = dis_asm_read_memory;
-  di.arch = gdbarch_bfd_arch_info (gdbarch)->arch;
-  di.mach = gdbarch_bfd_arch_info (gdbarch)->mach;
-  di.endian = gdbarch_byte_order (gdbarch);
-  di.endian_code = gdbarch_byte_order_for_code (gdbarch);
-  di.application_data = gdbarch;
-  disassemble_init_for_target (&di);
-  return di;
+  m_di.read_memory_func = func;
+  m_di.arch = gdbarch_bfd_arch_info (gdbarch)->arch;
+  m_di.mach = gdbarch_bfd_arch_info (gdbarch)->mach;
+  m_di.endian = gdbarch_byte_order (gdbarch);
+  m_di.endian_code = gdbarch_byte_order_for_code (gdbarch);
+  m_di.application_data = this;
+  disassemble_init_for_target (&m_di);
+}
+
+int
+gdb_disassembler::print_insn (CORE_ADDR memaddr)
+{
+  int length = gdbarch_print_insn (arch (), memaddr, &m_di);
+
+  return length;
+}
+
+int
+gdb_disassembler::print_insn (CORE_ADDR memaddr,
+			      int *branch_delay_insns)
+{
+  int length = print_insn (memaddr);
+
+  if (branch_delay_insns != NULL)
+    {
+      if (m_di.insn_info_valid)
+	*branch_delay_insns = m_di.branch_delay_insns;
+      else
+	*branch_delay_insns = 0;
+    }
+  return length;
 }
 
 void
@@ -795,7 +825,7 @@  gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
 {
   struct ui_file *stb = mem_fileopen ();
   struct cleanup *cleanups = make_cleanup_ui_file_delete (stb);
-  struct disassemble_info di = gdb_disassemble_info (gdbarch, stb);
+  gdb_disassembler di (gdbarch, stb);
   struct symtab *symtab;
   int nlines = -1;
 
@@ -807,15 +837,15 @@  gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
 
   if (!(flags & (DISASSEMBLY_SOURCE_DEPRECATED | DISASSEMBLY_SOURCE))
       || nlines <= 0)
-    do_assembly_only (gdbarch, uiout, &di, low, high, how_many, flags, stb);
+    do_assembly_only (uiout, &di, low, high, how_many, flags);
 
   else if (flags & DISASSEMBLY_SOURCE)
     do_mixed_source_and_assembly (gdbarch, uiout, &di, symtab, low, high,
-				  how_many, flags, stb);
+				  how_many, flags);
 
   else if (flags & DISASSEMBLY_SOURCE_DEPRECATED)
-    do_mixed_source_and_assembly_deprecated (gdbarch, uiout, &di, symtab,
-					     low, high, how_many, flags, stb);
+    do_mixed_source_and_assembly_deprecated (uiout, &di, symtab,
+					     low, high, how_many, flags);
 
   do_cleanups (cleanups);
   gdb_flush (gdb_stdout);
@@ -829,19 +859,10 @@  int
 gdb_print_insn (struct gdbarch *gdbarch, CORE_ADDR memaddr,
 		struct ui_file *stream, int *branch_delay_insns)
 {
-  struct disassemble_info di;
-  int length;
 
-  di = gdb_disassemble_info (gdbarch, stream);
-  length = gdbarch_print_insn (gdbarch, memaddr, &di);
-  if (branch_delay_insns)
-    {
-      if (di.insn_info_valid)
-	*branch_delay_insns = di.branch_delay_insns;
-      else
-	*branch_delay_insns = 0;
-    }
-  return length;
+  gdb_disassembler di (gdbarch, stream);
+
+  return di.print_insn (memaddr, branch_delay_insns);
 }
 
 static void
diff --git a/gdb/disasm.h b/gdb/disasm.h
index 4c6fd54..5592cdb 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -33,6 +33,48 @@  struct gdbarch;
 struct ui_out;
 struct ui_file;
 
+class gdb_disassembler
+{
+  using di_read_memory_ftype = decltype (disassemble_info::read_memory_func);
+
+public:
+  gdb_disassembler (struct gdbarch *gdbarch, struct ui_file *file)
+    : gdb_disassembler (gdbarch, file, dis_asm_read_memory)
+  {}
+
+  int print_insn (CORE_ADDR memaddr);
+  int print_insn (CORE_ADDR memaddr, int *branch_delay_insns);
+
+  /* Prints the instruction INSN into UIOUT and returns the length of
+     the printed instruction in bytes.  */
+  int pretty_print_insn (struct ui_out *uiout,
+			 const struct disasm_insn *insn, int flags);
+
+  /* Return the gdbarch of gdb_disassembler.  */
+  struct gdbarch *arch ()
+  { return m_gdbarch; }
+
+protected:
+  gdb_disassembler (struct gdbarch *gdbarch, struct ui_file *file,
+		    di_read_memory_ftype func);
+
+  struct ui_file *stream ()
+  { return (struct ui_file *) m_di.stream; }
+
+  struct disassemble_info m_di;
+
+private:
+  struct gdbarch *m_gdbarch;
+
+  static int dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr,
+				  unsigned int len,
+				  struct disassemble_info *info);
+  static void dis_asm_memory_error (int err, bfd_vma memaddr,
+				    struct disassemble_info *info);
+  static void dis_asm_print_address (bfd_vma addr,
+				     struct disassemble_info *info);
+};
+
 /* An instruction to be disassembled.  */
 
 struct disasm_insn
@@ -47,19 +89,6 @@  struct disasm_insn
   unsigned int is_speculative:1;
 };
 
-/* Prints the instruction INSN into UIOUT and returns the length of the
-   printed instruction in bytes.  */
-
-extern int gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
-				  struct disassemble_info * di,
-				  const struct disasm_insn *insn, int flags,
-				  struct ui_file *stb);
-
-/* Return a filled in disassemble_info object for use by gdb.  */
-
-extern struct disassemble_info gdb_disassemble_info (struct gdbarch *gdbarch,
-						     struct ui_file *file);
-
 extern void gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
 			     char *file_string, int flags, int how_many,
 			     CORE_ADDR low, CORE_ADDR high);
diff --git a/gdb/guile/scm-disasm.c b/gdb/guile/scm-disasm.c
index d06c481..25cae5a 100644
--- a/gdb/guile/scm-disasm.c
+++ b/gdb/guile/scm-disasm.c
@@ -37,11 +37,13 @@  static SCM address_symbol;
 static SCM asm_symbol;
 static SCM length_symbol;
 
-/* Struct used to pass "application data" in disassemble_info.  */
-
-struct gdbscm_disasm_data
+class gdbscm_disassembler : public gdb_disassembler
 {
-  struct gdbarch *gdbarch;
+public:
+  gdbscm_disassembler (struct gdbarch *gdbarch,
+		       struct ui_file *stream,
+		       SCM port, ULONGEST offset);
+
   SCM port;
   /* The offset of the address of the first instruction in PORT.  */
   ULONGEST offset;
@@ -55,7 +57,7 @@  struct gdbscm_disasm_read_data
   bfd_vma memaddr;
   bfd_byte *myaddr;
   unsigned int length;
-  struct disassemble_info *dinfo;
+  gdbscm_disassembler *dinfo;
 };
 
 /* Subroutine of gdbscm_arch_disassemble to simplify it.
@@ -81,13 +83,11 @@  gdbscm_disasm_read_memory_worker (void *datap)
 {
   struct gdbscm_disasm_read_data *data
     = (struct gdbscm_disasm_read_data *) datap;
-  struct disassemble_info *dinfo = data->dinfo;
-  struct gdbscm_disasm_data *disasm_data
-    = (struct gdbscm_disasm_data *) dinfo->application_data;
-  SCM seekto, newpos, port = disasm_data->port;
+  gdbscm_disassembler *dinfo = data->dinfo;
+  SCM seekto, newpos, port = dinfo->port;
   size_t bytes_read;
 
-  seekto = gdbscm_scm_from_ulongest (data->memaddr - disasm_data->offset);
+  seekto = gdbscm_scm_from_ulongest (data->memaddr - dinfo->offset);
   newpos = scm_seek (port, seekto, scm_from_int (SEEK_SET));
   if (!scm_is_eq (seekto, newpos))
     return "seek error";
@@ -108,13 +108,15 @@  gdbscm_disasm_read_memory (bfd_vma memaddr, bfd_byte *myaddr,
 			   unsigned int length,
 			   struct disassemble_info *dinfo)
 {
+  gdbscm_disassembler *self
+    = static_cast<gdbscm_disassembler *> (dinfo->application_data);
   struct gdbscm_disasm_read_data data;
   const char *status;
 
   data.memaddr = memaddr;
   data.myaddr = myaddr;
   data.length = length;
-  data.dinfo = dinfo;
+  data.dinfo = self;
 
   status = gdbscm_with_guile (gdbscm_disasm_read_memory_worker, &data);
 
@@ -123,30 +125,12 @@  gdbscm_disasm_read_memory (bfd_vma memaddr, bfd_byte *myaddr,
   return status != NULL ? -1 : 0;
 }
 
-/* disassemble_info.memory_error_func for gdbscm_print_insn_from_port.
-   Technically speaking, we don't need our own memory_error_func,
-   but to not provide one would leave a subtle dependency in the code.
-   This function exists to keep a clear boundary.  */
-
-static void
-gdbscm_disasm_memory_error (int status, bfd_vma memaddr,
-			    struct disassemble_info *info)
-{
-  memory_error (TARGET_XFER_E_IO, memaddr);
-}
-
-/* disassemble_info.print_address_func for gdbscm_print_insn_from_port.
-   Since we need to use our own application_data value, we need to supply
-   this routine as well.  */
-
-static void
-gdbscm_disasm_print_address (bfd_vma addr, struct disassemble_info *info)
+gdbscm_disassembler::gdbscm_disassembler (struct gdbarch *gdbarch,
+					  struct ui_file *stream,
+					  SCM port_, ULONGEST offset_)
+  : gdb_disassembler (gdbarch, stream, gdbscm_disasm_read_memory),
+    port (port_), offset (offset_)
 {
-  struct gdbscm_disasm_data *data
-    = (struct gdbscm_disasm_data *) info->application_data;
-  struct gdbarch *gdbarch = data->gdbarch;
-
-  print_address (gdbarch, addr, (struct ui_file *) info->stream);
 }
 
 /* Subroutine of gdbscm_arch_disassemble to simplify it.
@@ -164,30 +148,9 @@  gdbscm_print_insn_from_port (struct gdbarch *gdbarch,
 			     SCM port, ULONGEST offset, CORE_ADDR memaddr,
 			     struct ui_file *stream, int *branch_delay_insns)
 {
-  struct disassemble_info di;
-  int length;
-  struct gdbscm_disasm_data data;
-
-  di = gdb_disassemble_info (gdbarch, stream);
-  data.gdbarch = gdbarch;
-  data.port = port;
-  data.offset = offset;
-  di.application_data = &data;
-  di.read_memory_func = gdbscm_disasm_read_memory;
-  di.memory_error_func = gdbscm_disasm_memory_error;
-  di.print_address_func = gdbscm_disasm_print_address;
-
-  length = gdbarch_print_insn (gdbarch, memaddr, &di);
-
-  if (branch_delay_insns)
-    {
-      if (di.insn_info_valid)
-	*branch_delay_insns = di.branch_delay_insns;
-      else
-	*branch_delay_insns = 0;
-    }
+  gdbscm_disassembler di (gdbarch, stream, port, offset);
 
-  return length;
+  return di.print_insn (memaddr, branch_delay_insns);
 }
 
 /* (arch-disassemble <gdb:arch> address
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 637b34e..41cb9d8 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -44,6 +44,7 @@ 
 #include "symcat.h"
 #include "sim-regno.h"
 #include "dis-asm.h"
+#include "disasm.h"
 #include "frame-unwind.h"
 #include "frame-base.h"
 #include "trad-frame.h"
@@ -6982,7 +6983,9 @@  reinit_frame_cache_sfunc (char *args, int from_tty,
 static int
 gdb_print_insn_mips (bfd_vma memaddr, struct disassemble_info *info)
 {
-  struct gdbarch *gdbarch = (struct gdbarch *) info->application_data;
+  gdb_disassembler *di
+    = static_cast<gdb_disassembler *>(info->application_data);
+  struct gdbarch *gdbarch = di->arch ();
 
   /* FIXME: cagney/2003-06-26: Is this even necessary?  The
      disassembler needs to be able to locally determine the ISA, and
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 6cba1d2..8896241 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -698,7 +698,6 @@  btrace_insn_history (struct ui_out *uiout,
 {
   struct ui_file *stb;
   struct cleanup *cleanups, *ui_item_chain;
-  struct disassemble_info di;
   struct gdbarch *gdbarch;
   struct btrace_insn_iterator it;
   struct btrace_line_range last_lines;
@@ -711,7 +710,7 @@  btrace_insn_history (struct ui_out *uiout,
   gdbarch = target_gdbarch ();
   stb = mem_fileopen ();
   cleanups = make_cleanup_ui_file_delete (stb);
-  di = gdb_disassemble_info (gdbarch, stb);
+  gdb_disassembler di (gdbarch, stb);
   last_lines = btrace_mk_line_range (NULL, 0, 0);
 
   make_cleanup_ui_out_list_begin_end (uiout, "asm_insns");
@@ -773,7 +772,7 @@  btrace_insn_history (struct ui_out *uiout,
 	  if ((insn->flags & BTRACE_INSN_FLAG_SPECULATIVE) != 0)
 	    dinsn.is_speculative = 1;
 
-	  gdb_pretty_print_insn (gdbarch, uiout, &di, &dinsn, flags, stb);
+	  di.pretty_print_insn (uiout, &dinsn, flags);
 	}
     }
 
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 8756256..70d7f6f 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -33,6 +33,7 @@ 
 #include "value.h"
 #include "inferior.h"
 #include "dis-asm.h"
+#include "disasm.h"
 #include "objfiles.h"
 #include "language.h"
 #include "regcache.h"
@@ -1693,18 +1694,19 @@  spu_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc)
 
 /* Disassembler.  */
 
-struct spu_dis_asm_data
+struct spu_dis_asm_info : disassemble_info
 {
-  struct gdbarch *gdbarch;
   int id;
 };
 
 static void
 spu_dis_asm_print_address (bfd_vma addr, struct disassemble_info *info)
 {
-  struct spu_dis_asm_data *data
-    = (struct spu_dis_asm_data *) info->application_data;
-  print_address (data->gdbarch, SPUADDR (data->id, addr),
+  struct spu_dis_asm_info *data = (struct spu_dis_asm_info *) info;
+  gdb_disassembler *di
+    = static_cast<gdb_disassembler *>(info->application_data);
+
+  print_address (di->arch (), SPUADDR (data->id, addr),
 		 (struct ui_file *) info->stream);
 }
 
@@ -1714,12 +1716,10 @@  gdb_print_insn_spu (bfd_vma memaddr, struct disassemble_info *info)
   /* The opcodes disassembler does 18-bit address arithmetic.  Make
      sure the SPU ID encoded in the high bits is added back when we
      call print_address.  */
-  struct disassemble_info spu_info = *info;
-  struct spu_dis_asm_data data;
-  data.gdbarch = (struct gdbarch *) info->application_data;
-  data.id = SPUADDR_SPU (memaddr);
+  struct spu_dis_asm_info spu_info;
 
-  spu_info.application_data = &data;
+  memcpy (&spu_info, info, sizeof (*info));
+  spu_info.id = SPUADDR_SPU (memaddr);
   spu_info.print_address_func = spu_dis_asm_print_address;
   return print_insn_spu (memaddr, &spu_info);
 }