public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb: add new base class to gdb_disassembler
@ 2022-06-15 9:03 Andrew Burgess
0 siblings, 0 replies; only message in thread
From: Andrew Burgess @ 2022-06-15 9:03 UTC (permalink / raw)
To: gdb-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f0c2e3e020d350b410e1bbe4ed636f2ea228d555
commit f0c2e3e020d350b410e1bbe4ed636f2ea228d555
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Wed Oct 27 10:07:56 2021 +0100
gdb: add new base class to gdb_disassembler
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.
Now, my new Python disassembler sub-class will still want to print
things to an output stream, and so we will want access to the
dis_asm_fprintf functionality for printing.
However, rather than move this printing code into the
gdb_disassemble_info base class, I have added yet another level of
hierarchy, a gdb_printing_disassembler, thus the class structure is
now:
struct gdb_disassemble_info {};
struct gdb_printing_disassembler : public gdb_disassemble_info {};
struct gdb_disassembler : public gdb_printing_disassembler {};
In a later commit my new Python disassembler will inherit from
gdb_printing_disassembler.
The reason for adding the additional layer to the class hierarchy is
that in yet another commit I intend to rewrite the function
gdb_buffered_insn_length, and to do this I will be creating yet more
disassembler like classes, however, these will not print anything,
thus I will add a gdb_non_printing_disassembler class that also
inherits from gdb_disassemble_info. Knowing that that change is
coming, I've gone with the above class hierarchy now.
There should be no user visible changes after this commit.
Diff:
---
gdb/arm-tdep.c | 4 +-
gdb/disasm.c | 58 +++++++++++++++--------
gdb/disasm.h | 140 ++++++++++++++++++++++++++++++++++++++++++++------------
gdb/mips-tdep.c | 4 +-
4 files changed, 154 insertions(+), 52 deletions(-)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 456649afdaa..fe62617d4bf 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8290,8 +8290,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 f2df5ef7bc5..6ac84388cc3 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -166,7 +166,8 @@ 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_printing_disassembler::fprintf_func (void *stream,
+ const char *format, ...)
{
va_list args;
@@ -180,9 +181,9 @@ gdb_disassembler::dis_asm_fprintf (void *stream, const char *format, ...)
/* See disasm.h. */
int
-gdb_disassembler::dis_asm_styled_fprintf (void *stream,
- enum disassembler_style style,
- const char *format, ...)
+gdb_printing_disassembler::fprintf_styled_func (void *stream,
+ enum disassembler_style style,
+ const char *format, ...)
{
va_list args;
@@ -797,26 +798,41 @@ 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_printing_disassembler (gdbarch, &m_buffer, func,
+ dis_asm_memory_error, dis_asm_print_address),
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,
+ fprintf_styled_ftype fprintf_styled_func)
+ : m_gdbarch (gdbarch)
{
- init_disassemble_info (&m_di, &m_buffer, dis_asm_fprintf,
- dis_asm_styled_fprintf);
+ gdb_assert (fprintf_func != nullptr);
+ gdb_assert (fprintf_styled_func != nullptr);
+ init_disassemble_info (&m_di, stream, fprintf_func,
+ fprintf_styled_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.read_memory_func = read_memory_func;
+
+ /* The memory_error_func, print_address_func, and read_memory_func are
+ all initialized to a default (non-nullptr) value by the call to
+ init_disassemble_info above. If the user is overriding these fields
+ (by passing non-nullptr values) then do that now, otherwise, leave
+ these fields as the defaults. */
+ if (memory_error_func != nullptr)
+ m_di.memory_error_func = memory_error_func;
+ if (print_address_func != nullptr)
+ m_di.print_address_func = print_address_func;
+ if (read_memory_func != nullptr)
+ 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;
m_di.endian = gdbarch_byte_order (gdbarch);
@@ -828,7 +844,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 7efab7db46c..f31ca92b038 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -26,43 +26,137 @@ 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)
- {}
+/* 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.
- ~gdb_disassembler ();
+ The constructor of this class is protected, you should not create
+ instances of this class directly, instead create an instance of an
+ appropriate sub-class. */
- DISABLE_COPY_AND_ASSIGN (gdb_disassembler);
-
- int print_insn (CORE_ADDR memaddr, int *branch_delay_insns = NULL);
+struct gdb_disassemble_info
+{
+ DISABLE_COPY_AND_ASSIGN (gdb_disassemble_info);
- /* Return the gdbarch of gdb_disassembler. */
+ /* Return the gdbarch we are disassembling for. */
struct gdbarch *arch ()
{ return m_gdbarch; }
+ /* 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:
- gdb_disassembler (struct gdbarch *gdbarch, struct ui_file *file,
- di_read_memory_ftype func);
+ /* Types for the function callbacks within m_di. */
+ 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);
+ 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). */
+ 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,
+ fprintf_styled_ftype fprintf_styled_func);
+
+ /* 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; }
-private:
- struct gdbarch *m_gdbarch;
-
/* Stores data required for disassembling instructions in
opcodes. */
struct disassemble_info m_di;
+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 wrapper around gdb_disassemble_info. This class adds default
+ print functions that are supplied to the disassemble_info within the
+ parent class. These default print functions write to the stream, which
+ is also contained in the parent class.
+
+ As with the parent class, the constructor for this class is protected,
+ you should not create instances of this class, but create an
+ appropriate sub-class instead. */
+struct gdb_printing_disassembler : public gdb_disassemble_info
+{
+ DISABLE_COPY_AND_ASSIGN (gdb_printing_disassembler);
+
+protected:
+
+ /* 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
+ arguments are handled. */
+ gdb_printing_disassembler (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)
+ : gdb_disassemble_info (gdbarch, stream, 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, ...)
+ 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,
+ enum disassembler_style style,
+ const char *format, ...)
+ ATTRIBUTE_PRINTF(3,4);
+};
+
+/* A dissassembler class that provides 'print_insn', a method for
+ disassembling a single instruction to the output stream. */
+
+struct gdb_disassembler : public gdb_printing_disassembler
+{
+ 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
@@ -95,16 +189,6 @@ private:
(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);
-
- /* Print formatted message to STREAM, the content can be styled based on
- STYLE if desired. */
- static int dis_asm_styled_fprintf (void *stream,
- enum disassembler_style style,
- const char *format, ...)
- ATTRIBUTE_PRINTF(3,4);
-
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 805c5beba59..65aa86dd98d 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -7021,8 +7021,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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-06-15 9:03 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-15 9:03 [binutils-gdb] gdb: add new base class to gdb_disassembler Andrew Burgess
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).