[binutils-gdb] gdb: have gdb_disassemble_info carry 'this' in its stream pointer

[binutils-gdb] gdb: have gdb_disassemble_info carry 'this' in its stream pointer

[Andrew Burgess]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=81384924cdcc9eb2676dd9084b76845d7d0e0759

commit 81384924cdcc9eb2676dd9084b76845d7d0e0759
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Apr 5 11:06:16 2022 +0100

    gdb: have gdb_disassemble_info carry 'this' in its stream pointer
    
    The gdb_disassemble_info class is a wrapper around the libopcodes
    disassemble_info struct.  The 'stream' field of disassemble_info is
    passed as an argument to the fprintf_func and fprintf_styled_func
    callbacks when the disassembler wants to print anything.
    
    Previously, GDB would store a pointer to a ui_file object in the
    'stream' field, then, when the disassembler wanted to print anything,
    the content would be written to the ui_file object.  An example of an
    fprintf_func callback, from gdb/disasm.c is:
    
      int
      gdb_disassembler::dis_asm_fprintf (void *stream, const char *format, ...)
      {
        /* Write output to STREAM here.  */
      }
    
    This is fine, but has one limitation, within the print callbacks we
    only have access to STREAM, we can't access any additional state
    stored within the gdb_disassemble_info object.
    
    Right now this isn't a problem, but in a future commit this will
    become an issue, how we style the output being written to STREAM will
    depend on the state of the gdb_disassemble_info object, and this state
    might need to be updated, depending on what is being printed.
    
    In this commit I propose changing the 'stream' field of the
    disassemble_info to carry a pointer to the gdb_disassemble_info
    sub-class, rather than the stream itself.
    
    We then have the two sub-classes of gdb_disassemble_info to consider,
    the gdb_non_printing_disassembler class never cared about the stream,
    previously, for this class, the stream was nullptr.  With the change
    to make stream be a gdb_disassemble_info pointer, no further updates
    are needed for gdb_non_printing_disassembler.
    
    The other sub-class is gdb_printing_disassembler.  In this case the
    sub-class now carries around a pointer to the stream object.  The
    print callbacks are updated to cast the incoming stream object back to
    a gdb_printing_disassembler, and then extract the stream.
    
    This is purely a refactoring commit.  A later commit will add
    additional state to the gdb_printing_disassembler, and update the
    print callbacks to access this state.
    
    There should be no user visible changes after this commit.

Diff:
---
 gdb/disasm.c | 35 +++++++++++++++++++++++---------
 gdb/disasm.h | 65 ++++++++++++++++++++++++++++++++++++++----------------------
 2 files changed, 67 insertions(+), 33 deletions(-)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index c6edc92930d..6c64c14feee 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -163,17 +163,33 @@ gdb_disassembler::dis_asm_print_address (bfd_vma addr,
   print_address (self->arch (), addr, self->stream ());
 }
 
+/* See disasm.h.  */
+
+ui_file *
+gdb_printing_disassembler::stream_from_gdb_disassemble_info (void *dis_info)
+{
+  gdb_disassemble_info *di = (gdb_disassemble_info *) dis_info;
+  gdb_printing_disassembler *dis
+    = dynamic_cast<gdb_printing_disassembler *> (di);
+  gdb_assert (dis != nullptr);
+  ui_file *stream = dis->stream ();
+  gdb_assert (stream != nullptr);
+  return stream;
+}
+
 /* Format disassembler output to STREAM.  */
 
 int
-gdb_printing_disassembler::fprintf_func (void *stream,
+gdb_printing_disassembler::fprintf_func (void *dis_info,
 					 const char *format, ...)
 {
-  va_list args;
+  ui_file *stream = stream_from_gdb_disassemble_info (dis_info);
 
+  va_list args;
   va_start (args, format);
-  gdb_vprintf ((struct ui_file *) stream, format, args);
+  gdb_vprintf (stream, format, args);
   va_end (args);
+
   /* Something non -ve.  */
   return 0;
 }
@@ -181,14 +197,15 @@ gdb_printing_disassembler::fprintf_func (void *stream,
 /* See disasm.h.  */
 
 int
-gdb_printing_disassembler::fprintf_styled_func (void *stream,
+gdb_printing_disassembler::fprintf_styled_func (void *dis_info,
 						enum disassembler_style style,
 						const char *format, ...)
 {
-  va_list args;
+  ui_file *stream = stream_from_gdb_disassemble_info (dis_info);
 
+  va_list args;
   va_start (args, format);
-  gdb_vprintf ((struct ui_file *) stream, format, args);
+  gdb_vprintf (stream, format, args);
   va_end (args);
   /* Something non -ve.  */
   return 0;
@@ -809,7 +826,7 @@ gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 /* See disasm.h.  */
 
 gdb_disassemble_info::gdb_disassemble_info
-  (struct gdbarch *gdbarch, struct ui_file *stream,
+  (struct gdbarch *gdbarch,
    read_memory_ftype read_memory_func, memory_error_ftype memory_error_func,
    print_address_ftype print_address_func, fprintf_ftype fprintf_func,
    fprintf_styled_ftype fprintf_styled_func)
@@ -817,7 +834,7 @@ gdb_disassemble_info::gdb_disassemble_info
 {
   gdb_assert (fprintf_func != nullptr);
   gdb_assert (fprintf_styled_func != nullptr);
-  init_disassemble_info (&m_di, stream, fprintf_func,
+  init_disassemble_info (&m_di, (void *) this, fprintf_func,
 			 fprintf_styled_func);
   m_di.flavour = bfd_target_unknown_flavour;
 
@@ -930,7 +947,7 @@ gdb_disassembler::print_insn (CORE_ADDR memaddr,
   /* Push any disassemble output to the real destination stream.  We do
      this even if the disassembler reported failure (-1) as the
      disassembler may have printed something to its output stream.  */
-  m_di.fprintf_func (m_dest, "%s", m_buffer.c_str ());
+  gdb_printf (m_dest, "%s", m_buffer.c_str ());
 
   /* If the disassembler failed then report an appropriate error.  */
   if (length < 0)
diff --git a/gdb/disasm.h b/gdb/disasm.h
index da03e130526..54176eb095a 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -58,16 +58,14 @@ protected:
   using fprintf_ftype = decltype (disassemble_info::fprintf_func);
   using fprintf_styled_ftype = decltype (disassemble_info::fprintf_styled_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.  Of these function callbacks FPRINTF_FUNC and
-     FPRINTF_STYLED_FUNC must not be nullptr.  If READ_MEMORY_FUNC,
-     MEMORY_ERROR_FUNC, or PRINT_ADDRESS_FUNC are nullptr, then that field
-     within m_di is left with its default value (see the libopcodes
-     function init_disassemble_info for the defaults).  */
+  /* Constructor, many fields in m_di are initialized from GDBARCH.  The
+     remaining arguments are function callbacks that are written into m_di.
+     Of these function callbacks FPRINTF_FUNC and FPRINTF_STYLED_FUNC must
+     not be nullptr.  If READ_MEMORY_FUNC, MEMORY_ERROR_FUNC, or
+     PRINT_ADDRESS_FUNC are nullptr, then that field within m_di is left
+     with its default value (see the libopcodes function
+     init_disassemble_info for the defaults).  */
   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,
@@ -77,10 +75,6 @@ protected:
   /* Destructor.  */
   virtual ~gdb_disassemble_info ();
 
-  /* The stream that disassembler output is being written too.  */
-  struct ui_file *stream ()
-  { return (struct ui_file *) m_di.stream; }
-
   /* Stores data required for disassembling instructions in
      opcodes.  */
   struct disassemble_info m_di;
@@ -109,6 +103,10 @@ struct gdb_printing_disassembler : public gdb_disassemble_info
 
 protected:
 
+  /* The stream that disassembler output is being written too.  */
+  struct ui_file *stream ()
+  { return m_stream; }
+
   /* Constructor.  All the arguments are just passed to the parent class.
      We also add the two print functions to the arguments passed to the
      parent.  See gdb_disassemble_info for a description of how the
@@ -118,22 +116,41 @@ protected:
 			     read_memory_ftype read_memory_func,
 			     memory_error_ftype memory_error_func,
 			     print_address_ftype print_address_func)
-    : gdb_disassemble_info (gdbarch, stream, read_memory_func,
+    : gdb_disassemble_info (gdbarch, read_memory_func,
 			    memory_error_func, print_address_func,
-			    fprintf_func, fprintf_styled_func)
-  { /* Nothing.  */ }
-
-  /* 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, ...)
+			    fprintf_func, fprintf_styled_func),
+      m_stream (stream)
+  {
+    gdb_assert (stream != nullptr);
+  }
+
+  /* Callback used as the disassemble_info's fprintf_func callback.  The
+     DIS_INFO pointer is a pointer to a gdb_printing_disassembler object.
+     Content is written to the m_stream extracted from DIS_INFO.  */
+  static int fprintf_func (void *dis_info, const char *format, ...)
     ATTRIBUTE_PRINTF(2,3);
 
-  /* Callback used as the disassemble_info's fprintf_styled_func callback,
-     this writes to STREAM, which will be m_di.stream.  */
-  static int fprintf_styled_func (void *stream,
+  /* Callback used as the disassemble_info's fprintf_styled_func callback.
+     The DIS_INFO pointer is a pointer to a gdb_printing_disassembler
+     object.  Content is written to the m_stream extracted from DIS_INFO.  */
+  static int fprintf_styled_func (void *dis_info,
 				  enum disassembler_style style,
 				  const char *format, ...)
     ATTRIBUTE_PRINTF(3,4);
+
+private:
+
+  /* When libopcodes calls the fprintf_func and fprintf_styled_func
+     callbacks, a 'void *' argument is passed.  We arrange, through our
+     call to init_disassemble_info that this argument will be a pointer to
+     a gdb_disassemble_info sub-class, specifically, a
+     gdb_printing_disassembler pointer.  This helper function casts
+     DIS_INFO to the correct type (with some asserts), and then returns the
+     m_stream member variable.  */
+  static ui_file *stream_from_gdb_disassemble_info (void *dis_info);
+
+  /* The stream to which output should be sent.  */
+  struct ui_file *m_stream;
 };
 
 /* A basic disassembler that doesn't actually print anything.  */
@@ -142,7 +159,7 @@ struct gdb_non_printing_disassembler : public gdb_disassemble_info
 {
   gdb_non_printing_disassembler (struct gdbarch *gdbarch,
 				 read_memory_ftype read_memory_func)
-    : gdb_disassemble_info (gdbarch, nullptr /* stream */,
+    : gdb_disassemble_info (gdbarch,
 			    read_memory_func,
 			    nullptr /* memory_error_func */,
 			    nullptr /* print_address_func */,