From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 48C6638485A5 for ; Wed, 25 May 2022 10:50:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 48C6638485A5 Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-574-l9ICodpkPS2xF01MeF4KDQ-1; Wed, 25 May 2022 06:50:06 -0400 X-MC-Unique: l9ICodpkPS2xF01MeF4KDQ-1 Received: by mail-wm1-f71.google.com with SMTP id n18-20020a05600c3b9200b0039746f3d9faso4167721wms.4 for ; Wed, 25 May 2022 03:50:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=IH45gvB78+gWj+Zze6G71vhtqmi1tqE+zfQjtGII+hM=; b=RZUzaL3AsXquhIz9G5NjYQ+OuCMoN425M6UG+fgEPuIYBJj6xikzEr5vuZKD47ayJq xZPlAxixoHxuCZnbCrTSsr3ktWSm+ij1dHQNWYwkzXfeTnYYlczlK0/kmhHmEZzlnOjf c7NfB1IoT4b2UmO/CSv8ywB6QExfqd5JtRglhf5F3D0rhz6xXvXpkev5pWsoYjoRkNtq 2SxBfbCt2K9KUSuhbr08tVqyvMxW8qMWFeD2Y9P93pTUEGk1wBK1IrnBbhLqE1Ijy4m5 O+Yc6O4YSsSWJ15XRoec0mGcecg1z/xU/T80JRXL4DaAoVooXEK/RU/xOzMkbcRJXpKS 2dZA== X-Gm-Message-State: AOAM533fbIJyiNuzPh/P0kht6qoxGr4ycUPs6QLyMmaLjUyJF9mXARFu sECs8xjNrnm0si2B2HZOE6BdFn95Sk/AHFpReBThTQgcFdRFBDwDwiAVa05L1xtJam4u0rS4mQP kiFK98BIXETXfqNSSNxK8hLLnKTc/Fw1qN2S+UfcFWi3YX+h2XSM0VX5o17gInvj6dnUD8I3+0Q == X-Received: by 2002:a5d:5003:0:b0:20f:fe68:1dc4 with SMTP id e3-20020a5d5003000000b0020ffe681dc4mr2096278wrt.405.1653475805155; Wed, 25 May 2022 03:50:05 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxcjJ+S5Z9RboElXOrOq6tRbWuqIgWwrixhuh+yySVLoGg1YQdlsOkyb4BsrTyGoIWi/AzZOQ== X-Received: by 2002:a5d:5003:0:b0:20f:fe68:1dc4 with SMTP id e3-20020a5d5003000000b0020ffe681dc4mr2096244wrt.405.1653475804719; Wed, 25 May 2022 03:50:04 -0700 (PDT) Received: from localhost (host109-154-214-95.range109-154.btcentralplus.com. [109.154.214.95]) by smtp.gmail.com with ESMTPSA id n1-20020adffe01000000b0020d0931b48asm1778584wrr.37.2022.05.25.03.50.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 May 2022 03:50:04 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv6 2/6] gdb: add new base class to gdb_disassembler Date: Wed, 25 May 2022 11:49:51 +0100 Message-Id: <6117d794cc7dfeb099073a62a0c7e89fb926a5c6.1653475113.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 May 2022 10:50:10 -0000 From: Andrew Burgess 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. --- 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 49664093f00..d09e570a619 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -8228,8 +8228,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(info->application_data); + gdb_disassemble_info *di + = static_cast (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 @@ class gdb_disassembler (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(info->application_data); + gdb_disassemble_info *di + = static_cast (info->application_data); struct gdbarch *gdbarch = di->arch (); /* FIXME: cagney/2003-06-26: Is this even necessary? The -- 2.25.4