Patchwork GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'

login
register
mail settings
Submitter Maciej W. Rozycki
Date June 5, 2018, 1:45 p.m.
Message ID <alpine.DEB.2.00.1806042237580.6942@tp.orcam.me.uk>
Download mbox | patch
Permalink /patch/27626/
State Superseded
Headers show

Comments

Maciej W. Rozycki - June 5, 2018, 1:45 p.m.
Implement MIPS target support for passing options to the disassembler, 
complementing commit 65b48a81404c ("GDB: Add support for the new 
set/show disassembler-options commands.").

This includes options that expect an argument, so adjust the generic 
code and data structures used so as to handle such options.  So as to 
give backends syntax flexibility no specific delimiter has been defined 
to separate options from their respective arguments, so it has to be 
included as the last character of the option name.  Completion code 
however has not been adjusted and consequently option arguments cannot 
be completed at this time.

Also the MIPS target has non-empty defaults for the options, so that ABI 
names for the general-purpose registers respect our `set mips abi ...' 
setting rather than always being determined from the ELF headers of the 
binary file selected.  Handle these defaults as implicit options, never 
shown to the user and always prepended to the user-specified options, so 
that the latters can override the defaults.

The resulting output for the MIPS target is as follows:

(gdb) show disassembler-options
The current disassembler options are ''

The following disassembler options are supported for use with the
'set disassembler-options <option>[,<option>...]' command:

  no-aliases      Use canonical instruction forms.

  msa             Recognize MSA instructions.

  virt            Recognize the virtualization ASE instructions.

  xpa             Recognize the eXtended Physical Address (XPA) ASE
                  instructions.

  gpr-names=ABI   Print GPR names according to specified ABI.
                  Default: based on binary being disassembled.

  fpr-names=ABI   Print FPR names according to specified ABI.
                  Default: numeric.

  cp0-names=ARCH  Print CP0 register names according to specified architecture.
                  Default: based on binary being disassembled.

  hwr-names=ARCH  Print HWR names according to specified architecture.
                  Default: based on binary being disassembled.

  reg-names=ABI   Print GPR and FPR names according to specified ABI.

  reg-names=ARCH  Print CP0 register and HWR names according to specified
                  architecture.

  For the options above, the following values are supported for "ABI":
    numeric 32 n32 64

  For the options above, the following values are supported for "ARCH":
    numeric r3000 r3900 r4000 r4010 vr4100 vr4111 vr4120 r4300 r4400 r4600
    r4650 r5000 vr5400 vr5500 r5900 r6000 rm7000 rm9000 r8000 r10000 r12000
    r14000 r16000 mips5 mips32 mips32r2 mips32r3 mips32r5 mips32r6 mips64
    mips64r2 mips64r3 mips64r5 mips64r6 interaptiv-mr2 sb1 loongson2e
    loongson2f loongson3a octeon octeon+ octeon2 octeon3 xlr xlp
(gdb) 

which corresponds to what `objdump --help' used to print for the MIPS 
target, with minor formatting changes, most notably option argument 
lists being wrapped, but also the amount of white space separating 
options from the respective descriptions.  The relevant part the new 
code is now also used by `objdump --help', which means these formatting 
changes apply to both outputs, except for argument list wrapping, which 
is GDB-specific.

This also adds a separating new line between the heading and option 
lists where descriptions are provided, hence:

(gdb) set architecture s390:31-bit
(gdb) show disassembler-options
The current disassembler options are ''

The following disassembler options are supported for use with the
'set disassembler-options <option>[,<option>...]' command:

  esa         Disassemble in ESA architecture mode
  zarch       Disassemble in z/Architecture mode
  insnlength  Print unknown instructions according to length from first two bits
(gdb)

but:

(gdb) set architecture powerpc:common
(gdb) show disassembler-options
The current disassembler options are ''

The following disassembler options are supported for use with the
'set disassembler-options <option>[,<option>...]' command:
  403, 405, 440, 464, 476, 601, 603, 604, 620, 7400, 7410, 7450, 7455, 750cl,
  821, 850, 860, a2, altivec, any, booke, booke32, cell, com, e200z4, e300,
  e500, e500mc, e500mc64, e5500, e6500, e500x2, efs, efs2, power4, power5,
  power6, power7, power8, power9, ppc, ppc32, 32, ppc64, 64, ppc64bridge,
  ppcps, pwr, pwr2, pwr4, pwr5, pwr5x, pwr6, pwr7, pwr8, pwr9, pwrx, raw, spe,
  spe2, titan, vle, vsx
(gdb)

Existing affected target backends have been adjusted accordingly.  The 
only other backend (besides MIPS, now modernized) setting disassembler 
options in its own specific way turned out to be the i386 one.  As its 
`i386_print_insn' handler is always passed a NULL `disassembler_options' 
value, assert that and clear the pointer after use, so that code in 
`gdb_buffered_insn_length' doesn't choke on `free' being called on a 
static data pointer.

This has been verified manually with:

(gdb) set architecture arm
(gdb) set architecture powerpc:common
(gdb) set architecture s390:31-bit

to cause no issues with the `show disassembler-options' and `set 
disassembler-options' commands.  A test case for the MIPS target has 
also been provided, covering the default settings with ABI overrides as 
well as disassembler option overrides.

	include/
	PR tdep/8282
	* dis-asm.h (disasm_option_arg_t): New typedef.
	(disasm_options_and_args_t): Likewise.
	(disasm_options_t): Add `arg' member, document members.
	(disassembler_options_mips): New prototype.
	(disassembler_options_arm, disassembler_options_powerpc)
	(disassembler_options_s390): Update prototypes.

	opcodes/
	PR tdep/8282
	* mips-dis.c (mips_option_arg_t): New enumeration.
	(mips_options): New variable.
	(disassembler_options_mips): New function.
	(print_mips_disassembler_options): Reimplement in terms of
	`disassembler_options_mips'.
	* arm-dis.c (disassembler_options_arm): Adapt to using the
	`disasm_options_and_args_t' structure.
	* ppc-dis.c (disassembler_options_powerpc): Likewise.
	* s390-dis.c (disassembler_options_s390): Likewise.

	gdb/
	PR tdep/8282
	* disasm.h (gdb_disassembler): Add destructor.
	* disasm.c (get_all_disassembler_options): New function.
	(gdb_disassembler::gdb_disassembler): Use it.
	(gdb_disassembler::print_insn): Likewise.
	(gdb_disassembler::~gdb_disassembler): New destructor.
	(gdb_buffered_insn_length): Free `disassembler_options'.
	(set_disassembler_options): Handle options with arguments.
	(show_disassembler_options_sfunc): Likewise.  Add a leading new 
	line if showing options with descriptions.
	(disassembler_options_completer): Adapt to using the 
	`disasm_options_and_args_t' structure.
	* mips-tdep.c (mips_disassembler_options): New variable.
	(mips_disassembler_options_o32): Likewise.
	(mips_disassembler_options_n32): Likewise.
	(mips_disassembler_options_n64): Likewise.
	(gdb_print_insn_mips): Don't set `disassembler_options'.
	(gdb_print_insn_mips_n32, gdb_print_insn_mips_n64): Remove 
	functions.
	(mips_gdbarch_init): Always set `gdbarch_print_insn' to 
	`gdb_print_insn_mips'.  Set `gdbarch_disassembler_options',
	`gdbarch_disassembler_options_implicit' and 
	`gdbarch_valid_disassembler_options'.
	* arm-tdep.c (_initialize_arm_tdep): Adapt to using the 
	`disasm_options_and_args_t' structure.
	* i386-tdep.c (i386_print_insn): Assert that 
	`disassembler_options' is NULL upon entry, reset it to NULL 
	before returning.
	* gdbarch.sh (disassembler_options_implicit): New `gdbarch' 
	method.
	(valid_disassembler_options): Switch from `disasm_options_t' to
	the `disasm_options_and_args_t' structure.
	* NEWS: Document `set disassembler-options' support for the MIPS
	target.
	* gdbarch.h: Regenerate.
	* gdbarch.c: Regenerate.

	gdb/doc/
	PR tdep/8282
	* gdb.texinfo (Source and Machine Code): Document `set 
	disassembler-options' support for the MIPS target.

	gdb/testsuite/
	PR tdep/8282
	* gdb.arch/mips-disassembler-options.exp: New test.
	* gdb.arch/mips-disassembler-options.s: New test source.
---
Hi,

 This is functionally complete as far as the MIPS backend is concerned, 
however as I noted in the commit description completion code has not been 
updated (beyond adapting to the new data structures).  This means that we 
have this situation:

(gdb) set disassembler-options <Tab><Tab>
cp0-names=  gpr-names=  msa         reg-names=  xpa
fpr-names=  hwr-names=  no-aliases  virt
(gdb) set disassembler-options n<Tab>
(gdb) set disassembler-options no-aliases 

and then:

(gdb) set disassembler-options no-aliases,g<Tab>
(gdb) set disassembler-options no-aliases,gpr-names= 

Note that completion add a space after the `=' character and is unable to 
continue from here.  Instead it should stop immediately after `=' and then
offer the completion of the relevant argument.  For the MIPS backend this 
is further complicated by the presence of a duplicate option `reg-names=' 
accepting either an ABI or an ARCH argument, so completion would have to
handle both variants at once.

 Our completion code is unchartered territory for me and a risk has been 
that if I dive into it, then I may run out of time I can afford to handle 
this PR and never complete it.  I have decided therefore that the benefit 
from having disassembler option support for the MIPS target outweighs the 
minor inconvenience of having no completion support for option arguments.
It can be added anytime in the future anyway.

 I found having a line separating the heading from options presented in 
their described form more visually pleasing than having no such line; this 
is what `objdump --help' has been doing for the MIPS target since time 
immemorial.  I hope this change is acceptable for GDB too.  It's up to the 
backend to decide whether option descriptions add further new lines or 
not; these are passed as is.

 Also no automatic wrapping of option descriptions has been added, so 
multi-line ones have to be formatted manually.  I decided it was not worth 
the hassle at this point; if the lack of such a feature turns out 
problematic, wrapping can be added in the future.

 This has been verified to cause no regressions in `mips-linux-gnu' 
testing.  The issue with the i386 backend was discovered with the 
`gdb.base/all-architectures-2.exp' test case in MIPS testing, so I am 
pretty confident that non-MIPS backends have been sufficiently covered to 
conclude that this change does not cause a functional regression for them.

 OK to apply then?

  Maciej
---
 gdb/NEWS                                             |    3 
 gdb/arm-tdep.c                                       |    3 
 gdb/disasm.c                                         |  127 ++++++++++--
 gdb/disasm.h                                         |    2 
 gdb/doc/gdb.texinfo                                  |    2 
 gdb/gdbarch.c                                        |   28 ++
 gdb/gdbarch.h                                        |    7 
 gdb/gdbarch.sh                                       |    3 
 gdb/i386-tdep.c                                      |   10 
 gdb/mips-tdep.c                                      |   60 ++---
 gdb/testsuite/gdb.arch/mips-disassembler-options.exp |   58 +++++
 gdb/testsuite/gdb.arch/mips-disassembler-options.s   |   30 ++
 include/dis-asm.h                                    |   47 +++-
 opcodes/arm-dis.c                                    |   16 +
 opcodes/mips-dis.c                                   |  201 ++++++++++++++-----
 opcodes/ppc-dis.c                                    |   18 +
 opcodes/s390-dis.c                                   |   16 +
 17 files changed, 504 insertions(+), 127 deletions(-)

gdb-mips-disassembler-options.diff
Simon Marchi - June 5, 2018, 3:16 p.m.
Hi Maciej,

I just had a few minutes to look at it, so I looked at the API in 
dis-asm.h, I have one small comment.

> Index: gdb/include/dis-asm.h
> ===================================================================
> --- gdb.orig/include/dis-asm.h	2018-06-04 16:53:56.536062303 +0100
> +++ gdb/include/dis-asm.h	2018-06-05 00:20:26.008674269 +0100
> @@ -222,16 +222,50 @@ typedef struct disassemble_info
> 
>  } disassemble_info;
> 
> -/* This struct is used to pass information about valid disassembler 
> options
> -   and their descriptions from the target to the generic GDB functions 
> that
> -   set and display them.  */
> +/* This struct is used to pass information about valid disassembler
> +   option arguments from the target to the generic GDB functions
> +   that set and display them.  */
> +
> +typedef struct
> +{
> +  /* Option argument name to use in descriptions.  */
> +  const char *name;
> +
> +  /* Vector of acceptable option argument values, NULL-terminated.  */
> +  const char **values;
> +} disasm_option_arg_t;
> +
> +/* This struct is used to pass information about valid disassembler
> +   options, their descriptions and arguments from the target to the
> +   generic GDB functions that set and display them.  Options are
> +   defined by tuples of vector entries at each index.  */
> 
>  typedef struct
>  {
> +  /* Vector of option names, NULL-terminated.  */
>    const char **name;
> +
> +  /* Vector of option descriptions or NULL if none to be shown.  */
>    const char **description;
> +
> +  /* Vector of option argument information pointers or NULL if no
> +     option accepts an argument.  NULL entries denote individual
> +     options that accept no argument.  */
> +  const disasm_option_arg_t **arg;
>  } disasm_options_t;
> 
> +/* This struct is used to pass information about valid disassembler
> +   options and arguments from the target to the generic GDB functions
> +   that set and display them.  */
> +
> +typedef struct
> +{
> +  /* Valid disassembler options.  */
> +  disasm_options_t options;
> +
> +  /* Vector of acceptable option arguments, NULL-terminated.  */
> +  disasm_option_arg_t *args;
> +} disasm_options_and_args_t;

It took me a bit of time why we have a vector of disasm_option_arg_t 
here and in disasm_options_t.  I finally understood that 
disasm_options_and_args_t::args owns the disasm_option_arg_t objects, 
and the pointers in disasm_options_t::arg point to these instances.  
Maybe that could be clarified in both comments?

Do we really need a new struct, or could the "args" field be part of the 
disasm_options_t struct directly?

Simon
Maciej W. Rozycki - June 5, 2018, 4:39 p.m.
Hi Simon,

> I just had a few minutes to look at it, so I looked at the API in dis-asm.h, I
> have one small comment.
> 
> > Index: gdb/include/dis-asm.h
> > ===================================================================
> > --- gdb.orig/include/dis-asm.h	2018-06-04 16:53:56.536062303 +0100
> > +++ gdb/include/dis-asm.h	2018-06-05 00:20:26.008674269 +0100
> > @@ -222,16 +222,50 @@ typedef struct disassemble_info
> > 
> >  } disassemble_info;
> > 
> > -/* This struct is used to pass information about valid disassembler options
> > -   and their descriptions from the target to the generic GDB functions that
> > -   set and display them.  */
> > +/* This struct is used to pass information about valid disassembler
> > +   option arguments from the target to the generic GDB functions
> > +   that set and display them.  */
> > +
> > +typedef struct
> > +{
> > +  /* Option argument name to use in descriptions.  */
> > +  const char *name;
> > +
> > +  /* Vector of acceptable option argument values, NULL-terminated.  */
> > +  const char **values;
> > +} disasm_option_arg_t;
> > +
> > +/* This struct is used to pass information about valid disassembler
> > +   options, their descriptions and arguments from the target to the
> > +   generic GDB functions that set and display them.  Options are
> > +   defined by tuples of vector entries at each index.  */
> > 
> >  typedef struct
> >  {
> > +  /* Vector of option names, NULL-terminated.  */
> >    const char **name;
> > +
> > +  /* Vector of option descriptions or NULL if none to be shown.  */
> >    const char **description;
> > +
> > +  /* Vector of option argument information pointers or NULL if no
> > +     option accepts an argument.  NULL entries denote individual
> > +     options that accept no argument.  */
> > +  const disasm_option_arg_t **arg;
> >  } disasm_options_t;
> > 
> > +/* This struct is used to pass information about valid disassembler
> > +   options and arguments from the target to the generic GDB functions
> > +   that set and display them.  */
> > +
> > +typedef struct
> > +{
> > +  /* Valid disassembler options.  */
> > +  disasm_options_t options;
> > +
> > +  /* Vector of acceptable option arguments, NULL-terminated.  */
> > +  disasm_option_arg_t *args;
> > +} disasm_options_and_args_t;
> 
> It took me a bit of time why we have a vector of disasm_option_arg_t here and
> in disasm_options_t.  I finally understood that
> disasm_options_and_args_t::args owns the disasm_option_arg_t objects, and the
> pointers in disasm_options_t::arg point to these instances.  Maybe that could
> be clarified in both comments?

 Sure, I can add some explanatory notes.  I'll collect comments other 
people may make an post an update then.

> Do we really need a new struct, or could the "args" field be part of the
> disasm_options_t struct directly?

 Possibly.  I thought of a notational separation between the two kinds of 
objects.  I'm not sure if there is a clear advantage either way though, 
so I'm inclined to leave it as I wrote it.  Also having `args' and `arg' 
both as members of the same structure is surely asking for confusion.

 Actually I think `disasm_options_t' could instead be redefined as:

typedef struct
{
  const char *name;
  const char *description;
  const disasm_option_arg_t *arg;
} disasm_options_t;

and then:

typedef struct
{
  disasm_options_t *options;
  disasm_option_arg_t *args;
} disasm_options_and_args_t;

so that static initialisers could be reasonably used rather than current 
run-time setup, at least for the `options' part.  The current solution has 
the advantage of saving a small amount of memory when no descriptions (and 
now also no arguments) are used.  However I think this is all lost with 
the extra code required for run-time setup.  I propose to consider this a 
separate matter though.  Perhaps we could keep the current data structures
as they stand and write static initialisers with the aid of some macros 
(so that option names are bundled with the respective descriptions and 
arguments).

 Thanks for your review.

  Maciej

Patch

Index: gdb/gdb/NEWS
===================================================================
--- gdb.orig/gdb/NEWS	2018-05-31 16:13:14.000000000 +0100
+++ gdb/gdb/NEWS	2018-06-05 12:10:33.210762158 +0100
@@ -3,6 +3,9 @@ 
 
 *** Changes since GDB 8.1
 
+* The 'set disassembler-options' command now supports specifying options
+  for the MIPS target.
+
 * The endianness used with the 'set endian auto' mode in the absence of
   an executable selected for debugging is now the last endianness chosen
   either by one of the 'set endian big' and 'set endian little' commands
Index: gdb/gdb/arm-tdep.c
===================================================================
--- gdb.orig/gdb/arm-tdep.c	2018-06-04 16:53:56.378919250 +0100
+++ gdb/gdb/arm-tdep.c	2018-06-05 00:20:25.808969854 +0100
@@ -9572,7 +9572,8 @@  _initialize_arm_tdep (void)
 
 
   arm_disassembler_options = xstrdup ("reg-names-std");
-  const disasm_options_t *disasm_options = disassembler_options_arm ();
+  const disasm_options_t *disasm_options
+    = &disassembler_options_arm ()->options;
   int num_disassembly_styles = 0;
   for (i = 0; disasm_options->name[i] != NULL; i++)
     if (CONST_STRNEQ (disasm_options->name[i], "reg-names-"))
Index: gdb/gdb/disasm.c
===================================================================
--- gdb.orig/gdb/disasm.c	2018-06-04 16:53:56.393012927 +0100
+++ gdb/gdb/disasm.c	2018-06-05 00:20:25.825087057 +0100
@@ -727,6 +727,35 @@  fprintf_disasm (void *stream, const char
   return 0;
 }
 
+/* Combine implicit and user disassembler options and return them
+   in a newly-allocated buffer.  Return NULL if the string would be
+   empty.  */
+
+static const char *
+get_all_disassembler_options (struct gdbarch *gdbarch)
+{
+  const char *implicit_options;
+  const char *options;
+  char *all_options;
+  size_t len;
+
+  implicit_options = gdbarch_disassembler_options_implicit (gdbarch);
+  options = get_disassembler_options (gdbarch);
+  len = ((implicit_options != NULL ? strlen (implicit_options) : 0) + 1
+	 + (options != NULL ? strlen (options) : 0) + 1);
+  all_options = (char *) xmalloc (len);
+  sprintf (all_options, "%s,%s",
+	   implicit_options != NULL ? implicit_options : "",
+	   options != NULL ? options : "");
+  if (remove_whitespace_and_extra_commas (all_options) == NULL)
+    {
+      xfree (all_options);
+      return NULL;
+    }
+  else
+    return all_options;
+}
+
 gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 				    struct ui_file *file,
 				    di_read_memory_ftype read_memory_func)
@@ -751,10 +780,15 @@  gdb_disassembler::gdb_disassembler (stru
   m_di.endian = gdbarch_byte_order (gdbarch);
   m_di.endian_code = gdbarch_byte_order_for_code (gdbarch);
   m_di.application_data = this;
-  m_di.disassembler_options = get_disassembler_options (gdbarch);
+  m_di.disassembler_options = get_all_disassembler_options (gdbarch);
   disassemble_init_for_target (&m_di);
 }
 
+gdb_disassembler::~gdb_disassembler ()
+{
+  xfree (const_cast<char *> (m_di.disassembler_options));
+}
+
 int
 gdb_disassembler::print_insn (CORE_ADDR memaddr,
 			      int *branch_delay_insns)
@@ -860,7 +894,7 @@  gdb_buffered_insn_length_init_dis (struc
   di->endian = gdbarch_byte_order (gdbarch);
   di->endian_code = gdbarch_byte_order_for_code (gdbarch);
 
-  di->disassembler_options = get_disassembler_options (gdbarch);
+  di->disassembler_options = get_all_disassembler_options (gdbarch);
   disassemble_init_for_target (di);
 }
 
@@ -872,10 +906,14 @@  gdb_buffered_insn_length (struct gdbarch
 			  const gdb_byte *insn, int max_len, CORE_ADDR addr)
 {
   struct disassemble_info di;
+  int len;
 
   gdb_buffered_insn_length_init_dis (gdbarch, &di, insn, max_len, addr);
 
-  return gdbarch_print_insn (gdbarch, addr, &di);
+  len = gdbarch_print_insn (gdbarch, addr, &di);
+
+  xfree (const_cast<char *> (di.disassembler_options));
+  return len;
 }
 
 char *
@@ -892,6 +930,7 @@  set_disassembler_options (char *prospect
 {
   struct gdbarch *gdbarch = get_current_arch ();
   char **disassembler_options = gdbarch_disassembler_options (gdbarch);
+  const disasm_options_and_args_t *valid_options_and_args;
   const disasm_options_t *valid_options;
   char *options = remove_whitespace_and_extra_commas (prospective_options);
   const char *opt;
@@ -908,20 +947,42 @@  set_disassembler_options (char *prospect
       return;
     }
 
-  valid_options = gdbarch_valid_disassembler_options (gdbarch);
-  if (valid_options  == NULL)
+  valid_options_and_args = gdbarch_valid_disassembler_options (gdbarch);
+  if (valid_options_and_args == NULL)
     {
       fprintf_filtered (gdb_stdlog, _("\
 'set disassembler-options ...' is not supported on this architecture.\n"));
       return;
     }
 
+  valid_options = &valid_options_and_args->options;
+
   /* Verify we have valid disassembler options.  */
   FOR_EACH_DISASSEMBLER_OPTION (opt, options)
     {
       size_t i;
       for (i = 0; valid_options->name[i] != NULL; i++)
-	if (disassembler_options_cmp (opt, valid_options->name[i]) == 0)
+	if (valid_options->arg != NULL && valid_options->arg[i] != NULL)
+	  {
+	    size_t len = strlen (valid_options->name[i]);
+	    bool found = false;
+	    const char *arg;
+	    size_t j;
+
+	    if (memcmp (opt, valid_options->name[i], len) != 0)
+	      continue;
+	    arg = opt + len;
+	    for (j = 0; valid_options->arg[i]->values[j] != NULL; j++)
+	      if (disassembler_options_cmp
+		    (arg, valid_options->arg[i]->values[j]) == 0)
+		{
+		  found = true;
+		  break;
+		}
+	    if (found)
+	      break;
+	  }
+	else if (disassembler_options_cmp (opt, valid_options->name[i]) == 0)
 	  break;
       if (valid_options->name[i] == NULL)
 	{
@@ -948,6 +1009,8 @@  show_disassembler_options_sfunc (struct 
 				 struct cmd_list_element *c, const char *value)
 {
   struct gdbarch *gdbarch = get_current_arch ();
+  const disasm_options_and_args_t *valid_options_and_args;
+  const disasm_option_arg_t *valid_args;
   const disasm_options_t *valid_options;
 
   const char *options = get_disassembler_options (gdbarch);
@@ -957,11 +1020,13 @@  show_disassembler_options_sfunc (struct 
   fprintf_filtered (file, _("The current disassembler options are '%s'\n"),
 		    options);
 
-  valid_options = gdbarch_valid_disassembler_options (gdbarch);
+  valid_options_and_args = gdbarch_valid_disassembler_options (gdbarch);
 
-  if (valid_options == NULL)
+  if (valid_options_and_args == NULL)
     return;
 
+  valid_options = &valid_options_and_args->options;
+
   fprintf_filtered (file, _("\n\
 The following disassembler options are supported for use with the\n\
 'set disassembler-options <option>[,<option>...]' command:\n"));
@@ -970,10 +1035,15 @@  The following disassembler options are s
     {
       size_t i, max_len = 0;
 
+      fprintf_filtered (file, "\n");
+
       /* Compute the length of the longest option name.  */
       for (i = 0; valid_options->name[i] != NULL; i++)
 	{
 	  size_t len = strlen (valid_options->name[i]);
+
+	  if (valid_options->arg != NULL && valid_options->arg[i] != NULL)
+	    len += strlen (valid_options->arg[i]->name);
 	  if (max_len < len)
 	    max_len = len;
 	}
@@ -981,10 +1051,17 @@  The following disassembler options are s
       for (i = 0, max_len++; valid_options->name[i] != NULL; i++)
 	{
 	  fprintf_filtered (file, "  %s", valid_options->name[i]);
+	  if (valid_options->arg != NULL && valid_options->arg[i] != NULL)
+	    fprintf_filtered (file, "%s", valid_options->arg[i]->name);
 	  if (valid_options->description[i] != NULL)
-	    fprintf_filtered (file, "%*c %s",
-			      (int)(max_len - strlen (valid_options->name[i])), ' ',
-			      valid_options->description[i]);
+	    {
+	      size_t len = strlen (valid_options->name[i]);
+
+	      if (valid_options->arg != NULL && valid_options->arg[i] != NULL)
+		len += strlen (valid_options->arg[i]->name);
+	      fprintf_filtered (file, "%*c %s", (int) (max_len - len), ' ',
+				valid_options->description[i]);
+	    }
 	  fprintf_filtered (file, "\n");
 	}
     }
@@ -995,12 +1072,33 @@  The following disassembler options are s
       for (i = 0; valid_options->name[i] != NULL; i++)
 	{
 	  fprintf_filtered (file, "%s", valid_options->name[i]);
+	  if (valid_options->arg != NULL && valid_options->arg[i] != NULL)
+	    fprintf_filtered (file, "%s", valid_options->arg[i]->name);
 	  if (valid_options->name[i + 1] != NULL)
 	    fprintf_filtered (file, ", ");
 	  wrap_here ("  ");
 	}
       fprintf_filtered (file, "\n");
     }
+
+  valid_args = valid_options_and_args->args;
+  if (valid_args != NULL)
+    {
+      size_t i, j;
+
+      for (i = 0; valid_args[i].name != NULL; i++)
+	{
+	  fprintf_filtered (file, _("\n\
+  For the options above, the following values are supported for \"%s\":\n   "),
+			    valid_args[i].name);
+	  for (j = 0; valid_args[i].values[j] != NULL; j++)
+	    {
+	      fprintf_filtered (file, " %s", valid_args[i].values[j]);
+	      wrap_here ("   ");
+	    }
+	  fprintf_filtered (file, "\n");
+	}
+    }
 }
 
 /* A completion function for "set disassembler".  */
@@ -1011,10 +1109,13 @@  disassembler_options_completer (struct c
 				const char *text, const char *word)
 {
   struct gdbarch *gdbarch = get_current_arch ();
-  const disasm_options_t *opts = gdbarch_valid_disassembler_options (gdbarch);
+  const disasm_options_and_args_t *opts_and_args
+    = gdbarch_valid_disassembler_options (gdbarch);
 
-  if (opts != NULL)
+  if (opts_and_args != NULL)
     {
+      const disasm_options_t *opts = &opts_and_args->options;
+
       /* Only attempt to complete on the last option text.  */
       const char *separator = strrchr (text, ',');
       if (separator != NULL)
Index: gdb/gdb/disasm.h
===================================================================
--- gdb.orig/gdb/disasm.h	2018-06-04 16:53:56.416609898 +0100
+++ gdb/gdb/disasm.h	2018-06-05 00:20:25.838233800 +0100
@@ -47,6 +47,8 @@  class gdb_disassembler
     : gdb_disassembler (gdbarch, file, dis_asm_read_memory)
   {}
 
+  ~gdb_disassembler ();
+
   int print_insn (CORE_ADDR memaddr, int *branch_delay_insns = NULL);
 
   /* Return the gdbarch of gdb_disassembler.  */
Index: gdb/gdb/doc/gdb.texinfo
===================================================================
--- gdb.orig/gdb/doc/gdb.texinfo	2018-05-31 16:13:15.000000000 +0100
+++ gdb/gdb/doc/gdb.texinfo	2018-06-05 12:07:50.287139499 +0100
@@ -8758,7 +8758,7 @@  The default value is the empty string.
 
 If it is necessary to specify more than one disassembler option, then
 multiple options can be placed together into a comma separated list.
-Currently this command is only supported on targets ARM, PowerPC
+Currently this command is only supported on targets ARM, MIPS, PowerPC
 and S/390.
 
 @kindex show disassembler-options
Index: gdb/gdb/gdbarch.c
===================================================================
--- gdb.orig/gdb/gdbarch.c	2018-06-04 16:53:56.433047025 +0100
+++ gdb/gdb/gdbarch.c	2018-06-05 00:20:25.850483677 +0100
@@ -350,8 +350,9 @@  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;
+  const char * disassembler_options_implicit;
   char ** disassembler_options;
-  const disasm_options_t * valid_disassembler_options;
+  const disasm_options_and_args_t * valid_disassembler_options;
   gdbarch_type_align_ftype *type_align;
 };
 
@@ -707,6 +708,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 disassembler_options_implicit, invalid_p == 0 */
   /* Skip verify of disassembler_options, invalid_p == 0 */
   /* Skip verify of valid_disassembler_options, invalid_p == 0 */
   /* Skip verify of type_align, invalid_p == 0 */
@@ -902,6 +904,9 @@  gdbarch_dump (struct gdbarch *gdbarch, s
                       "gdbarch_dump: disassembler_options = %s\n",
                       pstring_ptr (gdbarch->disassembler_options));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: disassembler_options_implicit = %s\n",
+                      pstring (gdbarch->disassembler_options_implicit));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_displaced_step_copy_insn_p() = %d\n",
                       gdbarch_displaced_step_copy_insn_p (gdbarch));
   fprintf_unfiltered (file,
@@ -5044,6 +5049,23 @@  set_gdbarch_addressable_memory_unit_size
   gdbarch->addressable_memory_unit_size = addressable_memory_unit_size;
 }
 
+const char *
+gdbarch_disassembler_options_implicit (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  /* Skip verify of disassembler_options_implicit, invalid_p == 0 */
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_disassembler_options_implicit called\n");
+  return gdbarch->disassembler_options_implicit;
+}
+
+void
+set_gdbarch_disassembler_options_implicit (struct gdbarch *gdbarch,
+                                           const char * disassembler_options_implicit)
+{
+  gdbarch->disassembler_options_implicit = disassembler_options_implicit;
+}
+
 char **
 gdbarch_disassembler_options (struct gdbarch *gdbarch)
 {
@@ -5061,7 +5083,7 @@  set_gdbarch_disassembler_options (struct
   gdbarch->disassembler_options = disassembler_options;
 }
 
-const disasm_options_t *
+const disasm_options_and_args_t *
 gdbarch_valid_disassembler_options (struct gdbarch *gdbarch)
 {
   gdb_assert (gdbarch != NULL);
@@ -5073,7 +5095,7 @@  gdbarch_valid_disassembler_options (stru
 
 void
 set_gdbarch_valid_disassembler_options (struct gdbarch *gdbarch,
-                                        const disasm_options_t * valid_disassembler_options)
+                                        const disasm_options_and_args_t * valid_disassembler_options)
 {
   gdbarch->valid_disassembler_options = valid_disassembler_options;
 }
Index: gdb/gdb/gdbarch.h
===================================================================
--- gdb.orig/gdb/gdbarch.h	2018-06-04 16:53:56.448201617 +0100
+++ gdb/gdb/gdbarch.h	2018-06-05 00:20:25.860571742 +0100
@@ -1549,11 +1549,14 @@  extern void set_gdbarch_addressable_memo
 
 /* Functions for allowing a target to modify its disassembler options. */
 
+extern const char * gdbarch_disassembler_options_implicit (struct gdbarch *gdbarch);
+extern void set_gdbarch_disassembler_options_implicit (struct gdbarch *gdbarch, const char * disassembler_options_implicit);
+
 extern char ** gdbarch_disassembler_options (struct gdbarch *gdbarch);
 extern void set_gdbarch_disassembler_options (struct gdbarch *gdbarch, char ** disassembler_options);
 
-extern const disasm_options_t * gdbarch_valid_disassembler_options (struct gdbarch *gdbarch);
-extern void set_gdbarch_valid_disassembler_options (struct gdbarch *gdbarch, const disasm_options_t * valid_disassembler_options);
+extern const disasm_options_and_args_t * gdbarch_valid_disassembler_options (struct gdbarch *gdbarch);
+extern void set_gdbarch_valid_disassembler_options (struct gdbarch *gdbarch, const disasm_options_and_args_t * valid_disassembler_options);
 
 /* Type alignment. */
 
Index: gdb/gdb/gdbarch.sh
===================================================================
--- gdb.orig/gdb/gdbarch.sh	2018-06-04 16:53:56.466457230 +0100
+++ gdb/gdb/gdbarch.sh	2018-06-05 00:20:25.874655955 +0100
@@ -1157,8 +1157,9 @@  m;const char *;gnu_triplet_regexp;void;;
 m;int;addressable_memory_unit_size;void;;;default_addressable_memory_unit_size;;0
 
 # Functions for allowing a target to modify its disassembler options.
+v;const char *;disassembler_options_implicit;;;0;0;;0;pstring (gdbarch->disassembler_options_implicit)
 v;char **;disassembler_options;;;0;0;;0;pstring_ptr (gdbarch->disassembler_options)
-v;const disasm_options_t *;valid_disassembler_options;;;0;0;;0;host_address_to_string (gdbarch->valid_disassembler_options)
+v;const disasm_options_and_args_t *;valid_disassembler_options;;;0;0;;0;host_address_to_string (gdbarch->valid_disassembler_options)
 
 # Type alignment.
 m;ULONGEST;type_align;struct type *type;type;;default_type_align;;0
Index: gdb/gdb/i386-tdep.c
===================================================================
--- gdb.orig/gdb/i386-tdep.c	2018-06-04 16:53:56.510847538 +0100
+++ gdb/gdb/i386-tdep.c	2018-06-05 00:20:25.906830510 +0100
@@ -3966,12 +3966,20 @@  i386_sigtramp_p (struct frame_info *this
 static int
 i386_print_insn (bfd_vma pc, struct disassemble_info *info)
 {
+  int result;
+
   gdb_assert (disassembly_flavor == att_flavor
 	      || disassembly_flavor == intel_flavor);
 
+  gdb_assert (info->disassembler_options == NULL);
+
   info->disassembler_options = disassembly_flavor;
 
-  return default_print_insn (pc, info);
+  result = default_print_insn (pc, info);
+
+  info->disassembler_options = NULL;
+
+  return result;
 }
 
 
Index: gdb/gdb/mips-tdep.c
===================================================================
--- gdb.orig/gdb/mips-tdep.c	2018-06-04 16:53:56.487634857 +0100
+++ gdb/gdb/mips-tdep.c	2018-06-05 00:20:25.921981432 +0100
@@ -214,6 +214,18 @@  static unsigned int mips_debug = 0;
 struct target_desc *mips_tdesc_gp32;
 struct target_desc *mips_tdesc_gp64;
 
+/* The current set of options to be passed to the disassembler.  */
+static char *mips_disassembler_options;
+
+/* Implicit disassembler options for individual ABIs.  These tell
+   libopcodes to use general-purpose register names corresponding
+   to the ABI we have selected, perhaps via a `set mips abi ...'
+   override, rather than ones inferred from the ABI set in the ELF
+   headers of the binary file selected for debugging.  */
+static const char mips_disassembler_options_o32[] = "gpr-names=32";
+static const char mips_disassembler_options_n32[] = "gpr-names=n32";
+static const char mips_disassembler_options_n64[] = "gpr-names=64";
+
 const struct mips_regnum *
 mips_regnum (struct gdbarch *gdbarch)
 {
@@ -6990,40 +7002,9 @@  gdb_print_insn_mips (bfd_vma memaddr, st
   memaddr &= (info->mach == bfd_mach_mips16
 	      || info->mach == bfd_mach_mips_micromips) ? ~1 : ~3;
 
-  /* Set the disassembler options.  */
-  if (!info->disassembler_options)
-    /* This string is not recognized explicitly by the disassembler,
-       but it tells the disassembler to not try to guess the ABI from
-       the bfd elf headers, such that, if the user overrides the ABI
-       of a program linked as NewABI, the disassembly will follow the
-       register naming conventions specified by the user.  */
-    info->disassembler_options = "gpr-names=32";
-
   return default_print_insn (memaddr, info);
 }
 
-static int
-gdb_print_insn_mips_n32 (bfd_vma memaddr, struct disassemble_info *info)
-{
-  /* Set up the disassembler info, so that we get the right
-     register names from libopcodes.  */
-  info->disassembler_options = "gpr-names=n32";
-  info->flavour = bfd_target_elf_flavour;
-
-  return gdb_print_insn_mips (memaddr, info);
-}
-
-static int
-gdb_print_insn_mips_n64 (bfd_vma memaddr, struct disassemble_info *info)
-{
-  /* Set up the disassembler info, so that we get the right
-     register names from libopcodes.  */
-  info->disassembler_options = "gpr-names=64";
-  info->flavour = bfd_target_elf_flavour;
-
-  return gdb_print_insn_mips (memaddr, info);
-}
-
 /* Implement the breakpoint_kind_from_pc gdbarch method.  */
 
 static int
@@ -8727,12 +8708,19 @@  mips_gdbarch_init (struct gdbarch_info i
 
   set_gdbarch_print_registers_info (gdbarch, mips_print_registers_info);
 
-  if (mips_abi == MIPS_ABI_N32)
-    set_gdbarch_print_insn (gdbarch, gdb_print_insn_mips_n32);
-  else if (mips_abi == MIPS_ABI_N64)
-    set_gdbarch_print_insn (gdbarch, gdb_print_insn_mips_n64);
+  set_gdbarch_print_insn (gdbarch, gdb_print_insn_mips);
+  if (mips_abi == MIPS_ABI_N64)
+    set_gdbarch_disassembler_options_implicit
+      (gdbarch, (const char *) mips_disassembler_options_n64);
+  else if (mips_abi == MIPS_ABI_N32)
+    set_gdbarch_disassembler_options_implicit
+      (gdbarch, (const char *) mips_disassembler_options_n32);
   else
-    set_gdbarch_print_insn (gdbarch, gdb_print_insn_mips);
+    set_gdbarch_disassembler_options_implicit
+      (gdbarch, (const char *) mips_disassembler_options_o32);
+  set_gdbarch_disassembler_options (gdbarch, &mips_disassembler_options);
+  set_gdbarch_valid_disassembler_options (gdbarch,
+					  disassembler_options_mips ());
 
   /* FIXME: cagney/2003-08-29: The macros target_have_steppable_watchpoint,
      HAVE_NONSTEPPABLE_WATCHPOINT, and target_have_continuable_watchpoint
Index: gdb/gdb/testsuite/gdb.arch/mips-disassembler-options.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/gdb/testsuite/gdb.arch/mips-disassembler-options.exp	2018-06-05 00:20:25.964373253 +0100
@@ -0,0 +1,58 @@ 
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test MIPS disassembler options.
+
+if { ![istarget "mips*-*-*"] } then {
+    verbose "Skipping MIPS disassembler option tests."
+    return
+}
+
+standard_testfile .s
+set objfile [standard_output_file ${testfile}.o]
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${objfile}" object {}] \
+     != "" } {
+    return
+}
+
+clean_restart ${objfile}
+
+proc mips_disassemble_test { func insn mesg } {
+    gdb_test "disassemble $func" \
+	"Dump of assembler code for function\
+	 $func:\r\n\[^:\]+:\t$insn\r\nEnd of assembler dump\." \
+	$mesg
+}
+
+# Verify defaults.
+mips_disassemble_test foo "move\tv0,v1" "disassemble default"
+
+# Verify option overrides.
+gdb_test "set disassembler-options gpr-names=numeric"
+mips_disassemble_test foo "move\t\\\$2,\\\$3" "disassemble numeric (gpr-names)"
+# Check multiple options too.
+gdb_test "set disassembler-options msa,reg-names=numeric,reg-names=r3000"
+mips_disassemble_test foo "move\t\\\$2,\\\$3" "disassemble numeric (reg-names)"
+
+# Verify ABI overrides.
+mips_disassemble_test bar "move\t\\\$2,\\\$8" "disassemble ABI (numeric)"
+gdb_test "set disassembler-options"
+gdb_test "set mips abi o32"
+mips_disassemble_test bar "move\tv0,t0" "disassemble ABI (o32)"
+gdb_test "set mips abi n32"
+mips_disassemble_test bar "move\tv0,a4" "disassemble ABI (n32)"
+gdb_test "set mips abi n64"
+mips_disassemble_test bar "move\tv0,a4" "disassemble ABI (n64)"
Index: gdb/gdb/testsuite/gdb.arch/mips-disassembler-options.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/gdb/testsuite/gdb.arch/mips-disassembler-options.s	2018-06-05 00:20:25.981501516 +0100
@@ -0,0 +1,30 @@ 
+#  This test is part of GDB, the GNU debugger.
+#
+#  Copyright 2018 Free Software Foundation, Inc.
+#
+#  This program is free software; you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 3 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#  GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Note: this is meant to be buildable as MIPS16 or microMIPS code too.
+
+	.globl	foo
+	.ent	foo
+foo:
+	move	$2, $3
+	.end	foo
+
+	.globl	bar
+	.ent	bar
+bar:
+	move	$2, $8
+	.end	bar
Index: gdb/include/dis-asm.h
===================================================================
--- gdb.orig/include/dis-asm.h	2018-06-04 16:53:56.536062303 +0100
+++ gdb/include/dis-asm.h	2018-06-05 00:20:26.008674269 +0100
@@ -222,16 +222,50 @@  typedef struct disassemble_info
 
 } disassemble_info;
 
-/* This struct is used to pass information about valid disassembler options
-   and their descriptions from the target to the generic GDB functions that
-   set and display them.  */
+/* This struct is used to pass information about valid disassembler
+   option arguments from the target to the generic GDB functions
+   that set and display them.  */
+
+typedef struct
+{
+  /* Option argument name to use in descriptions.  */
+  const char *name;
+
+  /* Vector of acceptable option argument values, NULL-terminated.  */
+  const char **values;
+} disasm_option_arg_t;
+
+/* This struct is used to pass information about valid disassembler
+   options, their descriptions and arguments from the target to the
+   generic GDB functions that set and display them.  Options are
+   defined by tuples of vector entries at each index.  */
 
 typedef struct
 {
+  /* Vector of option names, NULL-terminated.  */
   const char **name;
+
+  /* Vector of option descriptions or NULL if none to be shown.  */
   const char **description;
+
+  /* Vector of option argument information pointers or NULL if no
+     option accepts an argument.  NULL entries denote individual
+     options that accept no argument.  */
+  const disasm_option_arg_t **arg;
 } disasm_options_t;
 
+/* This struct is used to pass information about valid disassembler
+   options and arguments from the target to the generic GDB functions
+   that set and display them.  */
+
+typedef struct
+{
+  /* Valid disassembler options.  */
+  disasm_options_t options;
+
+  /* Vector of acceptable option arguments, NULL-terminated.  */
+  disasm_option_arg_t *args;
+} disasm_options_and_args_t;
 
 /* Standard disassemblers.  Disassemble one instruction at the given
    target address.  Return number of octets processed.  */
@@ -266,9 +300,10 @@  extern bfd_boolean arm_symbol_is_valid (
 extern void disassemble_init_powerpc (struct disassemble_info *);
 extern void disassemble_init_s390 (struct disassemble_info *);
 extern void disassemble_init_wasm32 (struct disassemble_info *);
-extern const disasm_options_t *disassembler_options_powerpc (void);
-extern const disasm_options_t *disassembler_options_arm (void);
-extern const disasm_options_t *disassembler_options_s390 (void);
+extern const disasm_options_and_args_t *disassembler_options_arm (void);
+extern const disasm_options_and_args_t *disassembler_options_mips (void);
+extern const disasm_options_and_args_t *disassembler_options_powerpc (void);
+extern const disasm_options_and_args_t *disassembler_options_s390 (void);
 
 /* Fetch the disassembler for a given architecture ARC, endianess (big
    endian if BIG is true), bfd_mach value MACH, and ABFD, if that support
Index: gdb/opcodes/arm-dis.c
===================================================================
--- gdb.orig/opcodes/arm-dis.c	2018-06-04 16:53:56.547135933 +0100
+++ gdb/opcodes/arm-dis.c	2018-06-05 00:20:26.034853389 +0100
@@ -6821,17 +6821,23 @@  print_insn_little_arm (bfd_vma pc, struc
   return print_insn (pc, info, TRUE);
 }
 
-const disasm_options_t *
+const disasm_options_and_args_t *
 disassembler_options_arm (void)
 {
-  static disasm_options_t *opts = NULL;
+  static disasm_options_and_args_t *opts_and_args;
 
-  if (opts == NULL)
+  if (opts_and_args == NULL)
     {
+      disasm_options_t *opts;
       unsigned int i;
-      opts = XNEW (disasm_options_t);
+
+      opts_and_args = XNEW (disasm_options_and_args_t);
+      opts_and_args->args = NULL;
+
+      opts = &opts_and_args->options;
       opts->name = XNEWVEC (const char *, NUM_ARM_OPTIONS + 1);
       opts->description = XNEWVEC (const char *, NUM_ARM_OPTIONS + 1);
+      opts->arg = NULL;
       for (i = 0; i < NUM_ARM_OPTIONS; i++)
 	{
 	  opts->name[i] = regnames[i].name;
@@ -6845,7 +6851,7 @@  disassembler_options_arm (void)
       opts->description[i] = NULL;
     }
 
-  return opts;
+  return opts_and_args;
 }
 
 void
Index: gdb/opcodes/mips-dis.c
===================================================================
--- gdb.orig/opcodes/mips-dis.c	2018-06-04 16:53:56.578614715 +0100
+++ gdb/opcodes/mips-dis.c	2018-06-05 00:20:26.055994137 +0100
@@ -2545,68 +2545,175 @@  print_insn_little_mips (bfd_vma memaddr,
   return _print_insn_mips (memaddr, info, BFD_ENDIAN_LITTLE);
 }
 
-void
-print_mips_disassembler_options (FILE *stream)
+/* Indices into option argument vector for options accepting an argument.
+   Use MIPS_OPTION_ARG_NONE for options accepting no argument.  */
+typedef enum
 {
-  unsigned int i;
+  MIPS_OPTION_ARG_NONE = -1,
+  MIPS_OPTION_ARG_ABI,
+  MIPS_OPTION_ARG_ARCH,
+  MIPS_OPTION_ARG_SIZE
+} mips_option_arg_t;
 
-  fprintf (stream, _("\n\
-The following MIPS specific disassembler options are supported for use\n\
-with the -M switch (multiple options should be separated by commas):\n"));
+/* Valid MIPS disassembler options.  */
+static struct
+{
+  const char *name;
+  const char *description;
+  mips_option_arg_t arg;
+} mips_options[] =
+{
+  { "no-aliases", N_("Use canonical instruction forms.\n"),
+		  MIPS_OPTION_ARG_NONE },
+  { "msa",        N_("Recognize MSA instructions.\n"),
+		  MIPS_OPTION_ARG_NONE },
+  { "virt",       N_("Recognize the virtualization ASE instructions.\n"),
+		  MIPS_OPTION_ARG_NONE },
+  { "xpa",        N_("Recognize the eXtended Physical Address (XPA) ASE\n\
+                  instructions.\n"),
+		  MIPS_OPTION_ARG_NONE },
+  { "gpr-names=", N_("Print GPR names according to specified ABI.\n\
+                  Default: based on binary being disassembled.\n"),
+		  MIPS_OPTION_ARG_ABI },
+  { "fpr-names=", N_("Print FPR names according to specified ABI.\n\
+                  Default: numeric.\n"),
+		  MIPS_OPTION_ARG_ABI },
+  { "cp0-names=", N_("Print CP0 register names according to specified "
+		     "architecture.\n\
+                  Default: based on binary being disassembled.\n"),
+		  MIPS_OPTION_ARG_ARCH },
+  { "hwr-names=", N_("Print HWR names according to specified architecture.\n\
+                  Default: based on binary being disassembled.\n"),
+		  MIPS_OPTION_ARG_ARCH },
+  { "reg-names=", N_("Print GPR and FPR names according to specified ABI.\n"),
+		  MIPS_OPTION_ARG_ABI },
+  { "reg-names=", N_("Print CP0 register and HWR names according to "
+		     "specified\n\
+                  architecture."),
+		  MIPS_OPTION_ARG_ARCH }
+};
 
-  fprintf (stream, _("\n\
-  no-aliases               Use canonical instruction forms.\n"));
+/* Build the structure representing valid MIPS disassembler options.
+   This is done dynamically for maintenance ease purpose; a static
+   initializer would be unreadable.  */
 
-  fprintf (stream, _("\n\
-  msa                      Recognize MSA instructions.\n"));
+const disasm_options_and_args_t *
+disassembler_options_mips (void)
+{
+  static disasm_options_and_args_t *opts_and_args;
 
-  fprintf (stream, _("\n\
-  virt                     Recognize the virtualization ASE instructions.\n"));
+  if (opts_and_args == NULL)
+    {
+      size_t num_options = ARRAY_SIZE (mips_options);
+      size_t num_args = MIPS_OPTION_ARG_SIZE;
+      disasm_option_arg_t *args;
+      disasm_options_t *opts;
+      size_t i;
+      size_t j;
 
-  fprintf (stream, _("\n\
-  xpa                      Recognize the eXtended Physical Address (XPA)\n\
-                           ASE instructions.\n"));
+      args = XNEWVEC (disasm_option_arg_t, num_args + 1);
 
-  fprintf (stream, _("\n\
-  gpr-names=ABI            Print GPR names according to specified ABI.\n\
-                           Default: based on binary being disassembled.\n"));
+      args[MIPS_OPTION_ARG_ABI].name = "ABI";
+      args[MIPS_OPTION_ARG_ABI].values
+	= XNEWVEC (const char *, ARRAY_SIZE (mips_abi_choices) + 1);
+      for (i = 0; i < ARRAY_SIZE (mips_abi_choices); i++)
+	args[MIPS_OPTION_ARG_ABI].values[i] = mips_abi_choices[i].name;
+      /* The array we return must be NULL terminated.  */
+      args[MIPS_OPTION_ARG_ABI].values[i] = NULL;
 
-  fprintf (stream, _("\n\
-  fpr-names=ABI            Print FPR names according to specified ABI.\n\
-                           Default: numeric.\n"));
+      args[MIPS_OPTION_ARG_ARCH].name = "ARCH";
+      args[MIPS_OPTION_ARG_ARCH].values
+	= XNEWVEC (const char *, ARRAY_SIZE (mips_arch_choices) + 1);
+      for (i = 0, j = 0; i < ARRAY_SIZE (mips_arch_choices); i++)
+	if (*mips_arch_choices[i].name != '\0')
+	  args[MIPS_OPTION_ARG_ARCH].values[j++] = mips_arch_choices[i].name;
+      /* The array we return must be NULL terminated.  */
+      args[MIPS_OPTION_ARG_ARCH].values[j] = NULL;
 
-  fprintf (stream, _("\n\
-  cp0-names=ARCH           Print CP0 register names according to\n\
-                           specified architecture.\n\
-                           Default: based on binary being disassembled.\n"));
+      /* The array we return must be NULL terminated.  */
+      args[MIPS_OPTION_ARG_SIZE].name = NULL;
+      args[MIPS_OPTION_ARG_SIZE].values = NULL;
 
-  fprintf (stream, _("\n\
-  hwr-names=ARCH           Print HWR names according to specified \n\
-                           architecture.\n\
-                           Default: based on binary being disassembled.\n"));
+      opts_and_args = XNEW (disasm_options_and_args_t);
+      opts_and_args->args = args;
 
-  fprintf (stream, _("\n\
-  reg-names=ABI            Print GPR and FPR names according to\n\
-                           specified ABI.\n"));
+      opts = &opts_and_args->options;
+      opts->name = XNEWVEC (const char *, num_options + 1);
+      opts->description = XNEWVEC (const char *, num_options + 1);
+      opts->arg = XNEWVEC (const disasm_option_arg_t *, num_options + 1);
+      for (i = 0; i < num_options; i++)
+	{
+	  opts->name[i] = mips_options[i].name;
+	  opts->description[i] = _(mips_options[i].description);
+	  if (mips_options[i].arg != MIPS_OPTION_ARG_NONE)
+	    opts->arg[i] = &args[mips_options[i].arg];
+	  else
+	    opts->arg[i] = NULL;
+	}
+      /* The array we return must be NULL terminated.  */
+      opts->name[i] = NULL;
+      opts->description[i] = NULL;
+      opts->arg[i] = NULL;
+    }
 
-  fprintf (stream, _("\n\
-  reg-names=ARCH           Print CP0 register and HWR names according to\n\
-                           specified architecture.\n"));
+  return opts_and_args;
+}
 
-  fprintf (stream, _("\n\
-  For the options above, the following values are supported for \"ABI\":\n\
-   "));
-  for (i = 0; i < ARRAY_SIZE (mips_abi_choices); i++)
-    fprintf (stream, " %s", mips_abi_choices[i].name);
-  fprintf (stream, _("\n"));
+void
+print_mips_disassembler_options (FILE *stream)
+{
+  const disasm_options_and_args_t *opts_and_args;
+  const disasm_option_arg_t *args;
+  const disasm_options_t *opts;
+  size_t max_len = 0;
+  size_t i;
+  size_t j;
+
+  opts_and_args = disassembler_options_mips ();
+  opts = &opts_and_args->options;
+  args = opts_and_args->args;
 
   fprintf (stream, _("\n\
-  For the options above, The following values are supported for \"ARCH\":\n\
-   "));
-  for (i = 0; i < ARRAY_SIZE (mips_arch_choices); i++)
-    if (*mips_arch_choices[i].name != '\0')
-      fprintf (stream, " %s", mips_arch_choices[i].name);
-  fprintf (stream, _("\n"));
+The following MIPS specific disassembler options are supported for use\n\
+with the -M switch (multiple options should be separated by commas):\n\n"));
+
+  /* Compute the length of the longest option name.  */
+  for (i = 0; opts->name[i] != NULL; i++)
+    {
+      size_t len = strlen (opts->name[i]);
+
+      if (opts->arg[i] != NULL)
+	len += strlen (opts->arg[i]->name);
+      if (max_len < len)
+	max_len = len;
+    }
+
+  for (i = 0, max_len++; opts->name[i] != NULL; i++)
+    {
+      fprintf (stream, "  %s", opts->name[i]);
+      if (opts->arg[i] != NULL)
+	fprintf (stream, "%s", opts->arg[i]->name);
+      if (opts->description[i] != NULL)
+	{
+	  size_t len = strlen (opts->name[i]);
+
+	  if (opts->arg[i] != NULL)
+	    len += strlen (opts->arg[i]->name);
+	  fprintf (stream,
+		   "%*c %s", (int) (max_len - len), ' ', opts->description[i]);
+	}
+      fprintf (stream, _("\n"));
+    }
+
+  for (i = 0; args[i].name != NULL; i++)
+    {
+      fprintf (stream, _("\n\
+  For the options above, the following values are supported for \"%s\":\n   "),
+	       args[i].name);
+      for (j = 0; args[i].values[j] != NULL; j++)
+	fprintf (stream, " %s", args[i].values[j]);
+      fprintf (stream, _("\n"));
+    }
 
   fprintf (stream, _("\n"));
 }
Index: gdb/opcodes/ppc-dis.c
===================================================================
--- gdb.orig/opcodes/ppc-dis.c	2018-06-04 16:53:56.603979422 +0100
+++ gdb/opcodes/ppc-dis.c	2018-06-05 00:20:26.075101428 +0100
@@ -810,24 +810,30 @@  print_insn_powerpc (bfd_vma memaddr,
   return 4;
 }
 
-const disasm_options_t *
+const disasm_options_and_args_t *
 disassembler_options_powerpc (void)
 {
-  static disasm_options_t *opts = NULL;
+  static disasm_options_and_args_t *opts_and_args;
 
-  if (opts == NULL)
+  if (opts_and_args == NULL)
     {
       size_t i, num_options = ARRAY_SIZE (ppc_opts);
-      opts = XNEW (disasm_options_t);
+      disasm_options_t *opts;
+
+      opts_and_args = XNEW (disasm_options_and_args_t);
+      opts_and_args->args = NULL;
+
+      opts = &opts_and_args->options;
       opts->name = XNEWVEC (const char *, num_options + 1);
+      opts->description = NULL;
+      opts->arg = NULL;
       for (i = 0; i < num_options; i++)
 	opts->name[i] = ppc_opts[i].opt;
       /* The array we return must be NULL terminated.  */
       opts->name[i] = NULL;
-      opts->description = NULL;
     }
 
-  return opts;
+  return opts_and_args;
 }
 
 void
Index: gdb/opcodes/s390-dis.c
===================================================================
--- gdb.orig/opcodes/s390-dis.c	2018-06-04 16:53:56.625154305 +0100
+++ gdb/opcodes/s390-dis.c	2018-06-05 00:20:26.084239726 +0100
@@ -379,17 +379,23 @@  print_insn_s390 (bfd_vma memaddr, struct
   return 0;
 }
 
-const disasm_options_t *
+const disasm_options_and_args_t *
 disassembler_options_s390 (void)
 {
-  static disasm_options_t *opts = NULL;
+  static disasm_options_and_args_t *opts_and_args;
 
-  if (opts == NULL)
+  if (opts_and_args == NULL)
     {
       size_t i, num_options = ARRAY_SIZE (options);
-      opts = XNEW (disasm_options_t);
+      disasm_options_t *opts;
+
+      opts_and_args = XNEW (disasm_options_and_args_t);
+      opts_and_args->args = NULL;
+
+      opts = &opts_and_args->options;
       opts->name = XNEWVEC (const char *, num_options + 1);
       opts->description = XNEWVEC (const char *, num_options + 1);
+      opts->arg = NULL;
       for (i = 0; i < num_options; i++)
 	{
 	  opts->name[i] = options[i].name;
@@ -400,7 +406,7 @@  disassembler_options_s390 (void)
       opts->description[i] = NULL;
     }
 
-  return opts;
+  return opts_and_args;
 }
 
 void