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

Peter Bergner bergner@vnet.ibm.com
Thu Oct 6 19:26:00 GMT 2016


On 10/6/16 4:52 AM, Pedro Alves wrote:
> How do you envision this command working if you consider
> --enable-targets=all?  How would we make it work with other architectures?

Ummmm, I didn't think of that. :-)



> But, I suspect that if need to explicitly set one disassembler cpu
> when debugging a frame of $arch1, and explicitly set another disassembler
> cpu when debugging a frame of $arch2, you'll end up with lots of
> juggling.  And that juggling is only possible if you only consider
> explicit disassembling.  Things like "set disassemble-next-line" makes
> that impossible, since it's gdb that decides when to disassemble.

Ok, I changed it to "{set,show} powerpc disassembler" so as not to
conflict with defaults for other arches.



> Maybe we should instead make the command be "set powerpc disassembler-cpu", or
> generally, "set $arch disassembler-cpu".  We have "set arm disassembler"
> already.

Done.  I also renamed it to "disassembler" to match arm.


> But if we could have a single generic command, that'd be of course better.
> It's worth it to think about how it'd work at the user-interface level,
> even if we don't make any other arch use it right away.

I'm not sure what you mean by a generic command, but how about the
patch below.  It sets up a gdbarch callback function that each arch
can setup.  It seems like it should allow one to set/change disassembler
defaults for multiple arches...if an arch supports it.  Thoughts?

I didn't try and convert any other arches over to use it, since the
design is still in flux. :-)

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/
	* 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().


diff --git a/gdb/disasm.c b/gdb/disasm.c
index 07c3abe..0231229 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -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;
 }
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 4d8ef18..0dcc98d 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -337,6 +337,7 @@ struct gdbarch
   gdbarch_gcc_target_options_ftype *gcc_target_options;
   gdbarch_gnu_triplet_regexp_ftype *gnu_triplet_regexp;
   gdbarch_addressable_memory_unit_size_ftype *addressable_memory_unit_size;
+  gdbarch_target_disassemble_init_ftype *target_disassemble_init;
 };
 
 /* Create a new ``struct gdbarch'' based on information provided by
@@ -691,6 +692,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of gcc_target_options, invalid_p == 0 */
   /* Skip verify of gnu_triplet_regexp, invalid_p == 0 */
   /* Skip verify of addressable_memory_unit_size, invalid_p == 0 */
+  /* Skip verify of target_disassemble_init, has predicate.  */
   buf = ui_file_xstrdup (log, &length);
   make_cleanup (xfree, buf);
   if (length > 0)
@@ -1405,6 +1407,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: target_desc = %s\n",
                       host_address_to_string (gdbarch->target_desc));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_target_disassemble_init_p() = %d\n",
+                      gdbarch_target_disassemble_init_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: target_disassemble_init = <%s>\n",
+                      host_address_to_string (gdbarch->target_disassemble_init));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_unwind_pc_p() = %d\n",
                       gdbarch_unwind_pc_p (gdbarch));
   fprintf_unfiltered (file,
@@ -4918,6 +4926,30 @@ set_gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch,
   gdbarch->addressable_memory_unit_size = addressable_memory_unit_size;
 }
 
+int
+gdbarch_target_disassemble_init_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->target_disassemble_init != NULL;
+}
+
+void
+gdbarch_target_disassemble_init (struct gdbarch *gdbarch, struct disassemble_info *info)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->target_disassemble_init != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_target_disassemble_init called\n");
+  gdbarch->target_disassemble_init (info);
+}
+
+void
+set_gdbarch_target_disassemble_init (struct gdbarch *gdbarch,
+                                     gdbarch_target_disassemble_init_ftype target_disassemble_init)
+{
+  gdbarch->target_disassemble_init = target_disassemble_init;
+}
+
 
 /* Keep a registry of per-architecture data-pointers required by GDB
    modules.  */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index cd01718..c954263 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1530,6 +1530,14 @@ typedef int (gdbarch_addressable_memory_unit_size_ftype) (struct gdbarch *gdbarc
 extern int gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch);
 extern void set_gdbarch_addressable_memory_unit_size (struct gdbarch *gdbarch, gdbarch_addressable_memory_unit_size_ftype *addressable_memory_unit_size);
 
+/* Function for allowing a target to modify its disassembler options. */
+
+extern int gdbarch_target_disassemble_init_p (struct gdbarch *gdbarch);
+
+typedef void (gdbarch_target_disassemble_init_ftype) (struct disassemble_info *info);
+extern void gdbarch_target_disassemble_init (struct gdbarch *gdbarch, struct disassemble_info *info);
+extern void set_gdbarch_target_disassemble_init (struct gdbarch *gdbarch, gdbarch_target_disassemble_init_ftype *target_disassemble_init);
+
 /* Definition for an unknown syscall, used basically in error-cases.  */
 #define UNKNOWN_SYSCALL (-1)
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 1663156..3914129 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1152,6 +1152,9 @@ m:const char *:gnu_triplet_regexp:void:::default_gnu_triplet_regexp::0
 # each address in memory.
 m:int:addressable_memory_unit_size:void:::default_addressable_memory_unit_size::0
 
+# Function for allowing a target to modify its disassembler options.
+F:void:target_disassemble_init:struct disassemble_info *info:info
+
 EOF
 }
 
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index ca4d668..b03587b 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 powerpc disassembler".  */
+static char *gdb_disassembler_cpu;
+static char *prospective_cpu;
+
 /* To be used by skip_prologue.  */
 
 struct rs6000_framedata
@@ -5924,6 +5929,12 @@ UNKNOWN_OP:
   return 0;
 }
 
+static void
+gdb_rs6000_init_disassembly (disassemble_info *info)
+{
+  info->disassembler_options = gdb_disassembler_cpu;
+}
+
 /* Initialize the current architecture based on INFO.  If possible, re-use an
    architecture from ARCHES, which is a list of architectures already created
    during this debugging session.
@@ -6597,6 +6608,8 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   else
     register_ppc_ravenscar_ops (gdbarch);
 
+  set_gdbarch_target_disassemble_init (gdbarch, gdb_rs6000_init_disassembly);
+
   return gdbarch;
 }
 
@@ -6676,6 +6689,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
@@ -6777,6 +6814,8 @@ _initialize_rs6000_tdep (void)
   initialize_tdesc_powerpc_e500 ();
   initialize_tdesc_rs6000 ();
 
+  gdb_disassembler_cpu = strdup (PPC_DEFAULT_CPU",any");
+
   /* Add root prefix command for all "set powerpc"/"show powerpc"
      commands.  */
   add_prefix_cmd ("powerpc", no_class, set_powerpc_command,
@@ -6796,6 +6835,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", no_class,
+			  &prospective_cpu, _("\
+Set the disassembler cpu.\n\
+Usage: set powerpc disassembler <cpu>[,<cpu>]*\n\
+See 'show powerpc disassembler' for the valid <cpu> names."), _("\
+Show the disassembler cpu."), _("\
+The default value is '" PPC_DEFAULT_CPU ",any'."),
+			set_disassembler_cpu,
+			show_disassembler_cpu,
+			&setpowerpccmdlist, &showpowerpccmdlist);
+
   add_setshow_enum_cmd ("vector-abi", class_support, powerpc_vector_strings,
 			&powerpc_vector_abi_string,
 			_("Set the vector ABI."),
diff --git a/gdb/testsuite/gdb.arch/powerpc-altivec.exp b/gdb/testsuite/gdb.arch/powerpc-altivec.exp
index acbb113..52c8040 100644
--- a/gdb/testsuite/gdb.arch/powerpc-altivec.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-altivec.exp
@@ -33,7 +33,7 @@ clean_restart ${objfile}
 
 # Disassemble the function.
 
-gdb_test "set disassembler-cpu altivec"
+gdb_test "set powerpc disassembler altivec"
 set test "disass func"
 gdb_test_multiple $test $test {
     -re "\r\nDump of assembler code for function func:(\r\n.*\r\n)End of assembler dump.\r\n$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.arch/powerpc-altivec2.exp b/gdb/testsuite/gdb.arch/powerpc-altivec2.exp
index 3e76157..220b408 100644
--- a/gdb/testsuite/gdb.arch/powerpc-altivec2.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-altivec2.exp
@@ -33,7 +33,7 @@ clean_restart ${objfile}
 
 # Disassemble the function.
 
-gdb_test "set disassembler-cpu altivec"
+gdb_test "set powerpc disassembler altivec"
 set test "disass func"
 gdb_test_multiple $test $test {
     -re "\r\nDump of assembler code for function func:(\r\n.*\r\n)End of assembler dump.\r\n$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.arch/powerpc-altivec3.exp b/gdb/testsuite/gdb.arch/powerpc-altivec3.exp
index a6d04e0..c8255ae 100644
--- a/gdb/testsuite/gdb.arch/powerpc-altivec3.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-altivec3.exp
@@ -33,7 +33,7 @@ clean_restart ${objfile}
 
 # Disassemble the function.
 
-gdb_test "set disassembler-cpu altivec"
+gdb_test "set powerpc disassembler altivec"
 set test "disass func"
 gdb_test_multiple $test $test {
     -re "\r\nDump of assembler code for function func:(\r\n.*\r\n)End of assembler dump.\r\n$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.arch/powerpc-power7.exp b/gdb/testsuite/gdb.arch/powerpc-power7.exp
index 68004f9..87e79f3 100644
--- a/gdb/testsuite/gdb.arch/powerpc-power7.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-power7.exp
@@ -33,7 +33,7 @@ clean_restart ${objfile}
 
 # Disassemble the function.
 
-gdb_test "set disassembler-cpu power7"
+gdb_test "set powerpc disassembler power7"
 set test "disass func"
 gdb_test_multiple $test $test {
     -re "\r\nDump of assembler code for function func:(\r\n.*\r\n)End of assembler dump.\r\n$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.arch/powerpc-power8.exp b/gdb/testsuite/gdb.arch/powerpc-power8.exp
index 9b7950a..2900f3c 100644
--- a/gdb/testsuite/gdb.arch/powerpc-power8.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-power8.exp
@@ -33,7 +33,7 @@ clean_restart ${objfile}
 
 # Disassemble the function.
 
-gdb_test "set disassembler-cpu power8"
+gdb_test "set powerpc disassembler power8"
 set test "disass func"
 gdb_test_multiple $test $test {
     -re "\r\nDump of assembler code for function func:(\r\n.*\r\n)End of assembler dump.\r\n$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.arch/powerpc-power9.exp b/gdb/testsuite/gdb.arch/powerpc-power9.exp
index e2b3407..aa30317 100644
--- a/gdb/testsuite/gdb.arch/powerpc-power9.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-power9.exp
@@ -33,7 +33,7 @@ clean_restart ${objfile}
 
 # Disassemble the function.
 
-gdb_test "set disassembler-cpu power9"
+gdb_test "set powerpc disassembler power9"
 set test "disass func"
 gdb_test_multiple $test $test {
     -re "\r\nDump of assembler code for function func:(\r\n.*\r\n)End of assembler dump.\r\n$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.arch/powerpc-vsx.exp b/gdb/testsuite/gdb.arch/powerpc-vsx.exp
index 41b06ce..fefa246 100644
--- a/gdb/testsuite/gdb.arch/powerpc-vsx.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-vsx.exp
@@ -33,7 +33,7 @@ clean_restart ${objfile}
 
 # Disassemble the function.
 
-gdb_test "set disassembler-cpu vsx"
+gdb_test "set powerpc disassembler vsx"
 set test "disass func"
 gdb_test_multiple $test $test {
     -re "\r\nDump of assembler code for function func:(\r\n.*\r\n)End of assembler dump.\r\n$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.arch/powerpc-vsx2.exp b/gdb/testsuite/gdb.arch/powerpc-vsx2.exp
index 8f84eee..9d2b140 100644
--- a/gdb/testsuite/gdb.arch/powerpc-vsx2.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-vsx2.exp
@@ -33,7 +33,7 @@ clean_restart ${objfile}
 
 # Disassemble the function.
 
-gdb_test "set disassembler-cpu vsx"
+gdb_test "set powerpc disassembler vsx"
 set test "disass func"
 gdb_test_multiple $test $test {
     -re "\r\nDump of assembler code for function func:(\r\n.*\r\n)End of assembler dump.\r\n$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.arch/powerpc-vsx3.exp b/gdb/testsuite/gdb.arch/powerpc-vsx3.exp
index 24905d1..47fce8d 100644
--- a/gdb/testsuite/gdb.arch/powerpc-vsx3.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-vsx3.exp
@@ -33,7 +33,7 @@ clean_restart ${objfile}
 
 # Disassemble the function.
 
-gdb_test "set disassembler-cpu vsx"
+gdb_test "set powerpc disassembler vsx"
 set test "disass func"
 gdb_test_multiple $test $test {
     -re "\r\nDump of assembler code for function func:(\r\n.*\r\n)End of assembler dump.\r\n$gdb_prompt $" {
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;



More information about the Binutils mailing list