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

[Peter Bergner <bergner at vnet dot ibm dot com>]

On 10/3/16 5:25 PM, Alan Modra wrote:
> Somewhere in gdb you must be calling disassemble_init_for_target.
> Why can't you call that again after setting the cpu (in struct
> disassemble_info disassembler_options)?

We (rs6000-tdep.c) don't call disassemble_init_for_target().  That is done
by architecture independent gdb code before gdb_print_insn_powerpc() is
ever called.  We could call it from there, but as I mentioned in one of
my previous notes, we'd be doing that before every insn we emit, which
would be very expensive.

The problem is, currently there is no way for the target to catch the
disassembly initialization process to modify the struct disassemble_info
disassembler_options value before we start emitting insns.  That said,
how about the following patch which adds that ability?

Peter


include/
        * dis-asm.h (ppc_verify_disassembler_options): New prototype.
        * opcode/ppc.h (PPC_DEFAULT_CPU): New define.

opcodes/
        * ppc-dis.c (parse_ppc_dis_option): New function.
        (ppc_verify_disassembler_options): Likewise.
        (powerpc_init_dialect): Use parse_ppc_dis_option() and PPC_DEFAULT_CPU.

gdb/
	* target.h (target_init_disassembly): New prototype.
	* disasm.c (target_init_disassembly): New global function pointer.
	(gdb_disassemble_info): Use it.
        * rs6000-tdep.c: Include "opcode/ppc.h".
        (gdb_disassembler_cpu): New static declaration.
        (prospective_cpu): Likewise.
        (set_disassembler_cpu): New function.
        (show_disassembler_cpu): Likewise.
	(rs6000_init_disassembly): Likewise.
        (_initialize_rs6000_tdep): Initialize gdb_disassembler_cpu and
	target_init_disassembly.  Setup callbacks for set_disassembler_cpu()
	and show_disassembler_cpu().

diff --git a/gdb/disasm.c b/gdb/disasm.c
index 07c3abe..03fbd70 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -27,6 +27,8 @@
 #include "source.h"
 #include <algorithm>
 
+void (*target_init_disassembly)(struct disassemble_info *);
+
 /* Disassemble functions.
    FIXME: We should get rid of all the duplicate code in gdb that does
    the same thing: disassemble_command() and the gdbtk variation.  */
@@ -785,6 +787,9 @@ 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;
+  /* Allow the target to set disassembly options if it wishes.  */
+  if (target_init_disassembly)
+    target_init_disassembly (&di);
   disassemble_init_for_target (&di);
   return di;
 }
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index ca4d668..774b94b 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -57,6 +57,7 @@
 #include "ppc-ravenscar-thread.h"
 
 #include "dis-asm.h"
+#include "opcode/ppc.h"
 
 #include "trad-frame.h"
 #include "frame-unwind.h"
@@ -127,6 +128,10 @@ static const char *const powerpc_vector_strings[] =
 static enum powerpc_vector_abi powerpc_vector_abi_global = POWERPC_VEC_AUTO;
 static const char *powerpc_vector_abi_string = "auto";
 
+/* This is the variable that is set with "set disassembler-cpu".  */
+static char *gdb_disassembler_cpu;
+static char *prospective_cpu;
+
 /* To be used by skip_prologue.  */
 
 struct rs6000_framedata
@@ -6676,6 +6681,30 @@ show_powerpc_exact_watchpoints (struct ui_file *file, int from_tty,
   fprintf_filtered (file, _("Use of exact watchpoints is %s.\n"), value);
 }
 
+static void
+set_disassembler_cpu (char *args, int from_tty, struct cmd_list_element *c)
+{
+  char *opt;
+  if ((opt = ppc_verify_disassembler_options (prospective_cpu)) != NULL)
+    {
+      fprintf_filtered (gdb_stdlog,
+			_("Invalid disasembler-cpu value: '%s'.\n"), opt);
+      return;
+    }
+  free (gdb_disassembler_cpu);
+  gdb_disassembler_cpu = strdup (prospective_cpu);
+}
+
+static void
+show_disassembler_cpu (struct ui_file *file, int from_tty,
+		       struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("\
+The current disassembler cpu is '%s'\n\n\
+See objdump's -M<cpu> option for the list of supported PPC specific\n\
+disassembler options.\n"), gdb_disassembler_cpu);
+}
+
 /* Read a PPC instruction from memory.  */
 
 static unsigned int
@@ -6747,6 +6776,12 @@ ppc_insn_ds_field (unsigned int insn)
 
 /* Initialization code.  */
 
+void
+rs6000_init_disassembly (disassemble_info *info)
+{
+  info->disassembler_options = gdb_disassembler_cpu;
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_rs6000_tdep;
 
@@ -6777,6 +6812,9 @@ _initialize_rs6000_tdep (void)
   initialize_tdesc_powerpc_e500 ();
   initialize_tdesc_rs6000 ();
 
+  gdb_disassembler_cpu = strdup (PPC_DEFAULT_CPU",any");
+  target_init_disassembly = rs6000_init_disassembly;
+
   /* Add root prefix command for all "set powerpc"/"show powerpc"
      commands.  */
   add_prefix_cmd ("powerpc", no_class, set_powerpc_command,
@@ -6796,6 +6834,18 @@ _initialize_rs6000_tdep (void)
 				powerpc_set_soft_float, NULL,
 				&setpowerpccmdlist, &showpowerpccmdlist);
 
+  /* Add the command that controls the disassembler cpu.  */
+  add_setshow_string_cmd ("disassembler-cpu", no_class,
+			  &prospective_cpu, _("\
+Set the disassembler cpu.\n\
+Usage: set disassembler-cpu <cpu>[,<cpu>]*\n\
+See 'show disassembler-cpu' for the valid <cpu> names."), _("\
+Show the disassembler cpu."), _("\
+The default value is '" PPC_DEFAULT_CPU ",any'."),
+			set_disassembler_cpu,
+			show_disassembler_cpu,
+			&setlist, &showlist);
+
   add_setshow_enum_cmd ("vector-abi", class_support, powerpc_vector_strings,
 			&powerpc_vector_abi_string,
 			_("Set the vector ABI."),
diff --git a/gdb/target.h b/gdb/target.h
index b458970..507b7ec 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2515,4 +2515,6 @@ extern void target_prepare_to_generate_core (void);
 /* See to_done_generating_core.  */
 extern void target_done_generating_core (void);
 
+extern void (*target_init_disassembly)(struct disassemble_info *);
+
 #endif /* !defined (TARGET_H) */
diff --git a/include/dis-asm.h b/include/dis-asm.h
index 05bfa37..15573d9 100644
--- a/include/dis-asm.h
+++ b/include/dis-asm.h
@@ -337,6 +337,7 @@ extern int  get_arm_regnames (int, const char **, const char **, const char *con
 extern bfd_boolean aarch64_symbol_is_valid (asymbol *, struct disassemble_info *);
 extern bfd_boolean arm_symbol_is_valid (asymbol *, struct disassemble_info *);
 extern void disassemble_init_powerpc (struct disassemble_info *);
+extern char *ppc_verify_disassembler_options (char *);
 
 /* Fetch the disassembler for a given BFD, if that support is available.  */
 extern disassembler_ftype disassembler (bfd *);
diff --git a/include/opcode/ppc.h b/include/opcode/ppc.h
index 66d2ceb..f97c652 100644
--- a/include/opcode/ppc.h
+++ b/include/opcode/ppc.h
@@ -28,6 +28,11 @@
 extern "C" {
 #endif
 
+/* The default cpu type the assembler/disassembler will use if there
+   is no explcit use of -m or -M.  */
+
+#define PPC_DEFAULT_CPU "power9"
+
 typedef uint64_t ppc_cpu_t;
 
 /* The opcode table is an array of struct powerpc_opcode.  */
diff --git a/opcodes/ppc-dis.c b/opcodes/ppc-dis.c
index da1301e..fcbb4c8 100644
--- a/opcodes/ppc-dis.c
+++ b/opcodes/ppc-dis.c
@@ -271,6 +271,50 @@ ppc_parse_cpu (ppc_cpu_t ppc_cpu, ppc_cpu_t *sticky, const char *arg)
   return ppc_cpu;
 }
 
+/* Parse the OPTIONS argument looking for ',' seperated cpu names.
+   The first cpu name is copied into CPU and a pointer to the
+   next name is returned or NULL if there are no more cpu names.
+   CPU must contain enough space to hold the cpu name.  */
+
+static char *
+parse_ppc_dis_option (char *cpu, const char *options)
+{
+  char *next = strchr (options, ',');
+
+  if (next != NULL)
+    {
+      strncpy (cpu, options, (size_t) (next - options));
+      cpu[(size_t) (next - options)] = 0;
+      next++;
+    }
+  else
+    strcpy (cpu, options);
+
+  return next;
+}
+
+/* Parse OPTIONS looking for ',' seperated cpu names and verify each name
+   is valid.  Return NULL if all names are valid.  Otherwise, return a
+   pointer to the first invalid cpu name.  */
+
+char *
+ppc_verify_disassembler_options (char *options)
+{
+  static char opt[32];
+  while (options != NULL)
+    {
+      unsigned int i;
+      options = parse_ppc_dis_option (opt, options);
+
+      for (i = 0; ppc_opts[i].opt; i++)
+	if (strcmp (ppc_opts[i].opt, opt) == 0)
+	  break;
+      if (i >= sizeof (ppc_opts) / sizeof (ppc_opts[0]))
+	return opt;
+    }
+  return NULL;
+}
+
 /* Determine which set of machines to disassemble for.  */
 
 static void
@@ -323,30 +367,26 @@ powerpc_init_dialect (struct disassemble_info *info)
       dialect = ppc_parse_cpu (dialect, &sticky, "vle");
       break;
     default:
-      dialect = ppc_parse_cpu (dialect, &sticky, "power9") | PPC_OPCODE_ANY;
+      dialect = ppc_parse_cpu (dialect, &sticky, PPC_DEFAULT_CPU)
+		| PPC_OPCODE_ANY;
+      break;
     }
 
   arg = info->disassembler_options;
   while (arg != NULL)
     {
       ppc_cpu_t new_cpu = 0;
-      char *end = strchr (arg, ',');
+      char opt[64];
+      arg = parse_ppc_dis_option (opt, arg);
 
-      if (end != NULL)
-	*end = 0;
-
-      if ((new_cpu = ppc_parse_cpu (dialect, &sticky, arg)) != 0)
+      if ((new_cpu = ppc_parse_cpu (dialect, &sticky, opt)) != 0)
 	dialect = new_cpu;
-      else if (strcmp (arg, "32") == 0)
+      else if (strcmp (opt, "32") == 0)
 	dialect &= ~(ppc_cpu_t) PPC_OPCODE_64;
-      else if (strcmp (arg, "64") == 0)
+      else if (strcmp (opt, "64") == 0)
 	dialect |= PPC_OPCODE_64;
       else
-	fprintf (stderr, _("warning: ignoring unknown -M%s option\n"), arg);
-
-      if (end != NULL)
-	*end++ = ',';
-      arg = end;
+	fprintf (stderr, _("warning: ignoring unknown -M%s option\n"), opt);
     }
 
   info->private_data = priv;