Re: [PATCH, RFC] Add support for choosing disassembler cpu in GDB for POWER.

[Peter Bergner <bergner@vnet.ibm.com>]

On 10/7/16 2:21 PM, Ulrich Weigand wrote:
>> gdb/
>> 	* 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.

Peter