This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH, RFC] Add support for choosing disassembler cpu in GDB for POWER.
- From: Peter Bergner <bergner at vnet dot ibm dot com>
- To: Ulrich Weigand <uweigand at de dot ibm dot com>
- Cc: Pedro Alves <palves at redhat dot com>, Alan Modra <amodra at gmail dot com>, gdb-patches at sourceware dot org, binutils <binutils at sourceware dot org>
- Date: Fri, 7 Oct 2016 16:01:25 -0500
- Subject: Re: [PATCH, RFC] Add support for choosing disassembler cpu in GDB for POWER.
- Authentication-results: sourceware.org; auth=none
- References: <20161007192106.D516E10C1FE@oc8523832656.ibm.com>
On 10/7/16 2:21 PM, Ulrich Weigand wrote:
>> * gdbarch.sh (target_disassemble_init): New.
>> * gdbarch.c: Regenerate.
>> * gdbarch.h: Likewise.
>> * disasm.c (gdb_disassemble_info): Call gdbarch_target_disassemble_init.
>> * rs6000-tdep.c: Include "opcode/ppc.h".
>> (gdb_disassembler_cpu): New static declaration.
>> (prospective_cpu): Likewise.
>> (gdb_rs6000_init_disassembly): New function.
>> (set_disassembler_cpu): Likewise.
>> (show_disassembler_cpu): Likewise.
>> (rs6000_gdbarch_init): Setup callback for gdb_rs6000_init_disassembly.
>> (_initialize_rs6000_tdep): Initialize gdb_disassembler_cpu and
>> target_init_disassembly. Setup callbacks for set_disassembler_cpu()
>> and show_disassembler_cpu().
> I like this a lot better than the original patch :-)
Heh, well, I'm not a gdb developer, so I don't know the gdb code that
well. I just hacked in stuff that worked with bfd. Now you guys
are forcing me to learn the code and to work hard! :-)
I agree it's looking better too.
> Still some comments:
>> @@ -785,6 +785,8 @@ gdb_disassemble_info (struct gdbarch *gdbarch, struct ui_file *file)
>> di.endian = gdbarch_byte_order (gdbarch);
>> di.endian_code = gdbarch_byte_order_for_code (gdbarch);
>> di.application_data = gdbarch;
>> + if (gdbarch_target_disassemble_init_p (gdbarch))
>> + gdbarch_target_disassemble_init (gdbarch, &di);
>> disassemble_init_for_target (&di);
>> return di;
> There's a second use of disassemble_init_for_target, which probably needs
> the same treatment. In fact, maybe the nicest way would be to call the
> callback "gdbarch_disassemble_init_for_target", with a default of simply
> disassemble_init_for_target, but which targets can override to do extra
> stuff before (or after) calling disassemble_init_for_target in there.
> GDB common code would then just call gdbarch_disassemble_init_for_target
> everywhere it currently calls disassemble_init_for_target.
Sure, I can give that a try.
>> + gdb_disassembler_cpu = strdup (PPC_DEFAULT_CPU",any");
> Why do we have to know about this here in GDB? Can't the GDB default just
> be the empty (or NULL) string, and then opcodes does the defaulting
> (as it does today)?
I thought it would be useful if the user does a "show powerpc disassembler"
without ever doing a "set powerpc disassembler <cpu>" to show them what
default cpu they're actually using. If you want this hunk removed, I
can spit out 'default' in the case gdb_disassembler_cpu is NULL.