[PATCHv2 1/3] gdb: add new base class to gdb_disassembler

[Andrew Burgess <aburgess@redhat.com>]

From: Andrew Burgess <andrew.burgess@embecosm.com>

The motivation for this change is an upcoming Python disassembler API
that I would like to add.  As part of that change I need to create a
new disassembler like class that contains a disassemble_info and a
gdbarch.  The management of these two objects is identical to how we
manage these objects within gdb_disassembler, so it might be tempting
for my new class to inherit from gdb_disassembler.

The problem however, is that gdb_disassembler has a tight connection
between its constructor, and its print_insn method.  In the
constructor the ui_file* that is passed in is replaced with a member
variable string_file*, and then in print_insn, the contents of the
member variable string_file are printed to the original ui_file*.

What this means is that the gdb_disassembler class has a tight
coupling between its constructor and print_insn; the class just isn't
intended to be used in a situation where print_insn is not going to be
called, which is how my (upcoming) sub-class would need to operate.

My solution then, is to separate out the management of the
disassemble_info and gdbarch into a new gdb_disassemble_info class,
and make this class a parent of gdb_disassembler.

In arm-tdep.c and mips-tdep.c, where we used to cast the
disassemble_info->application_data to a gdb_disassembler, we can now
cast to a gdb_disassemble_info as we only need to access the gdbarch
information.

Additionally, I have moved the gdb_disassembler::dis_asm_fprintf
method to gdb_disassemble_info::fprintf_func.  This method only makes
use of the disassemble_info::stream member variable, and will be
useful for my future Python disassembler sub-class.

Now, in my future patch, I can inherit from gdb_disassemble_info
instead of gdb_disassembler, I will then be able to obtain the
disassemble_info and gdbarch management, without having to work around
all the ui_file manipulation that gdb_disassembler performs.

There should be no user visible changes after this commit.
---
 gdb/arm-tdep.c  |  4 +--
 gdb/disasm.c    | 35 ++++++++++---------
 gdb/disasm.h    | 89 ++++++++++++++++++++++++++++++++++++-------------
 gdb/mips-tdep.c |  4 +--
 4 files changed, 89 insertions(+), 43 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index d216d1daff7..89c0734ebc1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -7759,8 +7759,8 @@ arm_displaced_step_fixup (struct gdbarch *gdbarch,
 static int
 gdb_print_insn_arm (bfd_vma memaddr, disassemble_info *info)
 {
-  gdb_disassembler *di
-    = static_cast<gdb_disassembler *>(info->application_data);
+  gdb_disassemble_info *di
+    = static_cast<gdb_disassemble_info *> (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 b4cde801cb0..128b097a51a 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -166,7 +166,7 @@ gdb_disassembler::dis_asm_print_address (bfd_vma addr,
 /* Format disassembler output to STREAM.  */
 
 int
-gdb_disassembler::dis_asm_fprintf (void *stream, const char *format, ...)
+gdb_disassemble_info::fprintf_func (void *stream, const char *format, ...)
 {
   va_list args;
 
@@ -781,24 +781,27 @@ get_all_disassembler_options (struct gdbarch *gdbarch)
 
 gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 				    struct ui_file *file,
-				    di_read_memory_ftype read_memory_func)
-  : m_gdbarch (gdbarch),
+				    read_memory_ftype func)
+  : gdb_disassemble_info (gdbarch, &m_buffer, func,
+			  dis_asm_memory_error, dis_asm_print_address,
+			  fprintf_func),
     m_buffer (!use_ext_lang_colorization_p && disassembler_styling
 	      && file->can_emit_style_escape ()),
     m_dest (file)
+{ /* Nothing.  */ }
+
+/* See disasm.h.  */
+
+gdb_disassemble_info::gdb_disassemble_info
+  (struct gdbarch *gdbarch, struct ui_file *stream,
+   read_memory_ftype read_memory_func, memory_error_ftype memory_error_func,
+   print_address_ftype print_address_func, fprintf_ftype fprintf_func)
+    : m_gdbarch (gdbarch)
 {
-  init_disassemble_info (&m_di, &m_buffer, dis_asm_fprintf);
+  init_disassemble_info (&m_di, stream, fprintf_func);
   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 optimization here.  By default it would
-     access the executable file, instead of the target memory (there
-     was a growing list of exceptions though).  Unfortunately, the
-     heuristic was flawed.  Commands like "disassemble &variable"
-     didn't work as they relied on the access going to the target.
-     Further, it has been superseeded by trust-read-only-sections
-     (although that should be superseeded by target_trust..._p()).  */
+  m_di.memory_error_func = memory_error_func;
+  m_di.print_address_func = print_address_func;
   m_di.read_memory_func = read_memory_func;
   m_di.arch = gdbarch_bfd_arch_info (gdbarch)->arch;
   m_di.mach = gdbarch_bfd_arch_info (gdbarch)->mach;
@@ -811,7 +814,9 @@ gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
   disassemble_init_for_target (&m_di);
 }
 
-gdb_disassembler::~gdb_disassembler ()
+/* See disasm.h.  */
+
+gdb_disassemble_info::~gdb_disassemble_info ()
 {
   disassemble_free_target (&m_di);
 }
diff --git a/gdb/disasm.h b/gdb/disasm.h
index 399afc5ae71..4499929fe14 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -38,43 +38,87 @@ 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)
-  {}
-
-  ~gdb_disassembler ();
-
-  DISABLE_COPY_AND_ASSIGN (gdb_disassembler);
-
-  int print_insn (CORE_ADDR memaddr, int *branch_delay_insns = NULL);
+/* A wrapper around a disassemble_info and a gdbarch.  This is the core
+   set of data that all disassembler sub-classes will need.  This class
+   doesn't actually implement the disassembling process, that is something
+   that sub-classes will do, with each sub-class doing things slightly
+   differently.  */
 
-  /* Return the gdbarch of gdb_disassembler.  */
+struct gdb_disassemble_info
+{
+  /* Types for the function callbacks within disassemble_info.  */
+  using read_memory_ftype = decltype (disassemble_info::read_memory_func);
+  using memory_error_ftype = decltype (disassemble_info::memory_error_func);
+  using print_address_ftype = decltype (disassemble_info::print_address_func);
+  using fprintf_ftype = decltype (disassemble_info::fprintf_func);
+
+  /* Constructor, many fields in m_di are initialized from GDBARCH.  STREAM
+     is where the output of the disassembler will be written too, the
+     remaining arguments are function callbacks that are written into
+     m_di.  */
+  gdb_disassemble_info (struct gdbarch *gdbarch,
+			 struct ui_file *stream,
+			 read_memory_ftype read_memory_func,
+			 memory_error_ftype memory_error_func,
+			 print_address_ftype print_address_func,
+			 fprintf_ftype fprintf_func);
+
+  /* Destructor.  */
+  ~gdb_disassemble_info ();
+
+  /* Return the gdbarch we are disassembing for.  */
   struct gdbarch *arch ()
   { return m_gdbarch; }
 
-protected:
-  gdb_disassembler (struct gdbarch *gdbarch, struct ui_file *file,
-		    di_read_memory_ftype func);
+  /* Return a pointer to the disassemble_info, this will be needed for
+     passing into the libopcodes disassembler.  */
+  struct disassemble_info *disasm_info ()
+  { return &m_di; }
 
+protected:
+  /* The stream that disassembler output is being written too.  */
   struct ui_file *stream ()
   { return (struct ui_file *) m_di.stream; }
 
-private:
-  struct gdbarch *m_gdbarch;
-
   /* Stores data required for disassembling instructions in
      opcodes.  */
   struct disassemble_info m_di;
 
+  /* Callback used as the disassemble_info's fprintf_func callback, this
+     writes to STREAM, which will be m_di.stream.  */
+  static int fprintf_func (void *stream, const char *format, ...)
+    ATTRIBUTE_PRINTF(2,3);
+
+private:
+  /* The architecture we are disassembling for.  */
+  struct gdbarch *m_gdbarch;
+
   /* If we own the string in `m_di.disassembler_options', we do so
      using this field.  */
   std::string m_disassembler_options_holder;
+};
+
+/* A dissassembler class that provides 'print_insn', a method for
+   disassembling a single instruction to the output stream.  */
 
+struct gdb_disassembler : public gdb_disassemble_info
+{
+  gdb_disassembler (struct gdbarch *gdbarch, struct ui_file *file)
+    : gdb_disassembler (gdbarch, file, dis_asm_read_memory)
+  { /* Nothing.  */ }
+
+  DISABLE_COPY_AND_ASSIGN (gdb_disassembler);
+
+  /* Disassemble a single instruction at MEMADDR to the ui_file* that was
+     passed to the constructor.  If a memory error occurs while
+     disassembling this instruction then an error will be thrown.  */
+  int print_insn (CORE_ADDR memaddr, int *branch_delay_insns = NULL);
+
+protected:
+  gdb_disassembler (struct gdbarch *gdbarch, struct ui_file *file,
+		    read_memory_ftype func);
+
+private:
   /* This member variable is given a value by calling dis_asm_memory_error.
      If after calling into the libopcodes disassembler we get back a
      negative value (which indicates an error), then, if this variable has
@@ -107,9 +151,6 @@ class gdb_disassembler
      (currently just to addresses and symbols) as it goes.  */
   static bool use_ext_lang_colorization_p;
 
-  static int dis_asm_fprintf (void *stream, const char *format, ...)
-    ATTRIBUTE_PRINTF(2,3);
-
   static int dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr,
 				  unsigned int len,
 				  struct disassemble_info *info);
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 5cd72ae2451..dd9b86ee8f5 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -7004,8 +7004,8 @@ reinit_frame_cache_sfunc (const char *args, int from_tty,
 static int
 gdb_print_insn_mips (bfd_vma memaddr, struct disassemble_info *info)
 {
-  gdb_disassembler *di
-    = static_cast<gdb_disassembler *>(info->application_data);
+  gdb_disassemble_info *di
+    = static_cast<gdb_disassemble_info *> (info->application_data);
   struct gdbarch *gdbarch = di->arch ();
 
   /* FIXME: cagney/2003-06-26: Is this even necessary?  The
-- 
2.25.4