From 8b39b1e7ab20609ced6a224cae440f19e6ae02c1 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Mon, 4 Apr 2022 15:48:19 +0100 Subject: gdb: refactor the non-printing disassemblers This commit started from an observation I made while working on some other disassembler patches, that is, that the function gdb_buffered_insn_length, is broken ... sort of. I noticed that the gdb_buffered_insn_length function doesn't set up the application data field if the disassemble_info structure. Further, I noticed that some architectures, for example, ARM, require that the application_data field be set, see gdb_print_insn_arm in arm-tdep.c. And so, if we ever use gdb_buffered_insn_length for ARM, then GDB will likely crash. Which is why I said only "sort of" broken. Right now we don't use gdb_buffered_insn_length with ARM, so maybe it isn't broken yet? Anyway to prove to myself that there was a problem here I extended the disassembler self tests in disasm-selftests.c to include a test of gdb_buffered_insn_length. As I run the test for all architectures, I do indeed see GDB crash for ARM. To fix this we need gdb_buffered_insn_length to create a disassembler that inherits from gdb_disassemble_info, but we also need this new disassembler to not print anything. And so, I introduce a new gdb_non_printing_disassembler class, this is a disassembler that doesn't print anything to the output stream. I then observed that both ARC and S12Z also create non-printing disassemblers, but these are slightly different. While the disassembler in gdb_non_printing_disassembler reads the instruction from a buffer, the ARC and S12Z disassemblers read from target memory using target_read_code. And so, I further split gdb_non_printing_disassembler into two sub-classes, gdb_non_printing_memory_disassembler and gdb_non_printing_buffer_disassembler. The new selftests now pass, but otherwise, there should be no user visible changes after this commit. --- gdb/disasm.c | 88 +++++++++++++++++++++++++----------------------------------- 1 file changed, 37 insertions(+), 51 deletions(-) (limited to 'gdb/disasm.c') diff --git a/gdb/disasm.c b/gdb/disasm.c index 4af40c9..53cd6f5 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -1003,66 +1003,56 @@ gdb_insn_length (struct gdbarch *gdbarch, CORE_ADDR addr) return gdb_print_insn (gdbarch, addr, &null_stream, NULL); } -/* An fprintf-function for use by the disassembler when we know we don't - want to print anything. Always returns success. */ +/* See disasm.h. */ -static int ATTRIBUTE_PRINTF (2, 3) -gdb_disasm_null_printf (void *stream, const char *format, ...) +int +gdb_non_printing_disassembler::null_fprintf_func (void *stream, + const char *format, ...) { return 0; } -/* An fprintf-function for use by the disassembler when we know we don't - want to print anything, and the disassembler is using style. Always - returns success. */ +/* See disasm.h. */ -static int ATTRIBUTE_PRINTF (3, 4) -gdb_disasm_null_styled_printf (void *stream, - enum disassembler_style style, - const char *format, ...) +int +gdb_non_printing_disassembler::null_fprintf_styled_func + (void *stream, enum disassembler_style style, const char *format, ...) { return 0; } /* See disasm.h. */ -void -init_disassemble_info_for_no_printing (struct disassemble_info *dinfo) +int +gdb_non_printing_memory_disassembler::dis_asm_read_memory + (bfd_vma memaddr, bfd_byte *myaddr, unsigned int length, + struct disassemble_info *dinfo) { - init_disassemble_info (dinfo, nullptr, gdb_disasm_null_printf, - gdb_disasm_null_styled_printf); + return target_read_code (memaddr, myaddr, length); } -/* Initialize a struct disassemble_info for gdb_buffered_insn_length. - Upon return, *DISASSEMBLER_OPTIONS_HOLDER owns the string pointed - to by DI.DISASSEMBLER_OPTIONS. */ +/* A non-printing disassemble_info management class. The disassemble_info + setup by this class will not print anything to the output stream (there + is no output stream), and the instruction to be disassembled will be + read from a buffer passed to the constructor. */ -static void -gdb_buffered_insn_length_init_dis (struct gdbarch *gdbarch, - struct disassemble_info *di, - const gdb_byte *insn, int max_len, - CORE_ADDR addr, - std::string *disassembler_options_holder) +struct gdb_non_printing_buffer_disassembler + : public gdb_non_printing_disassembler { - init_disassemble_info_for_no_printing (di); - - /* init_disassemble_info installs buffer_read_memory, etc. - so we don't need to do that here. - The cast is necessary until disassemble_info is const-ified. */ - di->buffer = (gdb_byte *) insn; - di->buffer_length = max_len; - di->buffer_vma = addr; - - di->arch = gdbarch_bfd_arch_info (gdbarch)->arch; - di->mach = gdbarch_bfd_arch_info (gdbarch)->mach; - di->endian = gdbarch_byte_order (gdbarch); - di->endian_code = gdbarch_byte_order_for_code (gdbarch); - - *disassembler_options_holder = get_all_disassembler_options (gdbarch); - if (!disassembler_options_holder->empty ()) - di->disassembler_options = disassembler_options_holder->c_str (); - disassemble_init_for_target (di); -} + /* Constructor. GDBARCH is the architecture to disassemble for, BUFFER + contains the instruction to disassemble, and INSN_ADDRESS is the + address (in target memory) of the instruction to disassemble. */ + gdb_non_printing_buffer_disassembler (struct gdbarch *gdbarch, + gdb::array_view buffer, + CORE_ADDR insn_address) + : gdb_non_printing_disassembler (gdbarch, nullptr) + { + /* The cast is necessary until disassemble_info is const-ified. */ + m_di.buffer = (gdb_byte *) buffer.data (); + m_di.buffer_length = buffer.size (); + m_di.buffer_vma = insn_address; + } +}; /* Return the length in bytes of INSN. MAX_LEN is the size of the buffer containing INSN. */ @@ -1071,14 +1061,10 @@ int gdb_buffered_insn_length (struct gdbarch *gdbarch, const gdb_byte *insn, int max_len, CORE_ADDR addr) { - struct disassemble_info di; - std::string disassembler_options_holder; - - gdb_buffered_insn_length_init_dis (gdbarch, &di, insn, max_len, addr, - &disassembler_options_holder); - - int result = gdb_print_insn_1 (gdbarch, addr, &di); - disassemble_free_target (&di); + gdb::array_view buffer + = gdb::make_array_view (insn, max_len); + gdb_non_printing_buffer_disassembler dis (gdbarch, buffer, addr); + int result = gdb_print_insn_1 (gdbarch, addr, dis.disasm_info ()); return result; } -- cgit v1.1