public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 1/2] gdb: have gdb_disassemble_info carry 'this' in its stream pointer
Date: Fri, 17 Jun 2022 11:36:01 +0100	[thread overview]
Message-ID: <1faa12184e140a51e8c0af2435ac890465fac845.1655462053.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1655462053.git.aburgess@redhat.com>

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.
---
 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 @@ struct gdb_disassemble_info
   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 @@ struct gdb_disassemble_info
   /* 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 @@ struct gdb_printing_disassembler : public gdb_disassemble_info
 			     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 */,
-- 
2.25.4


  reply	other threads:[~2022-06-17 10:36 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-17 10:36 [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess
2022-06-17 10:36 ` Andrew Burgess [this message]
2022-06-17 10:36 ` [PATCH 2/2] gdb: add support for disassembler styling using libopcodes Andrew Burgess
2022-06-17 11:16   ` Eli Zaretskii
2022-06-30 12:22     ` Andrew Burgess
2022-06-30 13:46       ` Eli Zaretskii
2022-07-04 10:15         ` Andrew Burgess
2022-07-20  2:09   ` Simon Marchi
2022-07-20 13:14     ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess
2022-07-20 13:14       ` [PATCH 1/2] gdb: fix use of uninitialised gdb_printing_disassembler::m_in_comment Andrew Burgess
2022-07-20 14:21         ` Simon Marchi
2022-07-20 13:14       ` [PATCH 2/2] gdb/python: fix invalid use disassemble_info::stream Andrew Burgess
2022-07-25 18:27       ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess
2022-07-04 10:17 ` [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess
2022-07-11 14:18   ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1faa12184e140a51e8c0af2435ac890465fac845.1655462053.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).