From 81384924cdcc9eb2676dd9084b76845d7d0e0759 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Tue, 5 Apr 2022 11:06:16 +0100 Subject: [PATCH] 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. --- 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 (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 */, -- 2.30.2