* [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
@ 2018-06-21 7:27 Maciej W. Rozycki
2018-06-21 17:52 ` Simon Marchi
0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2018-06-21 7:27 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Joel Brobecker, Fredrik Noring
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.
ginv Recognize the Global INValidate (GINV) 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.
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.
2018-xx-xx Maciej W. Rozycki <macro@mips.com>
Simon Marchi <simon.marchi@polymtl.ca>
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
`m_disassembler_options_holder'. member
* disasm.c (get_all_disassembler_options): New function.
(gdb_disassembler::gdb_disassembler): Use it.
(gdb_buffered_insn_length_init_dis): Likewise.
(gdb_buffered_insn_length): Adjust accordingly.
(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.
* 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,
Here's v4 with your update included and taking your suggestions into
account. As `std::string' cannot be initialised with `nullptr', I have
rewritten code to use an empty string instead; `->disassembler_options' is
still set to NULL in that case, so no change to semantics.
This has been regression tested with the `mips-linux-gnu' target built in
a `--enable-targets=all' configuration, with the o32 ABI and a remote
target board.
OK to apply (for the non-MIPS GDB and binutils parts, respectively)?
Maciej
Changes from v3:
- `get_all_disassembler_options' rewritten to return `std::string' rather
than `char *'.
- `gdb_disassembler' updated with `m_disassembler_options_holder' member
added and destructor removed.
- `gdb_buffered_insn_length_init_dis' and `gdb_buffered_insn_length'
updated accordingly, changes to `i386_print_insn' discarded.
Changes from v2:
- adjusted for `ginv' disassembly option support.
Changes from v1:
- `disasm_options_and_args_t' member comments expanded for clarification.
---
gdb/NEWS | 3
gdb/arm-tdep.c | 3
gdb/disasm.c | 127 ++++++++++-
gdb/disasm.h | 5
gdb/doc/gdb.texinfo | 2
gdb/gdbarch.c | 28 ++
gdb/gdbarch.h | 7
gdb/gdbarch.sh | 3
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 | 50 ++++
opcodes/arm-dis.c | 16 +
opcodes/mips-dis.c | 206 ++++++++++++++-----
opcodes/ppc-dis.c | 18 +
opcodes/s390-dis.c | 16 +
16 files changed, 501 insertions(+), 131 deletions(-)
gdb-mips-disassembler-options.diff
Index: binutils/gdb/NEWS
===================================================================
--- binutils.orig/gdb/NEWS 2018-06-15 21:24:25.964466485 +0100
+++ binutils/gdb/NEWS 2018-06-20 15:58:24.667327435 +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: binutils/gdb/arm-tdep.c
===================================================================
--- binutils.orig/gdb/arm-tdep.c 2018-06-15 21:24:25.997942657 +0100
+++ binutils/gdb/arm-tdep.c 2018-06-20 15:58:24.685475881 +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: binutils/gdb/disasm.c
===================================================================
--- binutils.orig/gdb/disasm.c 2018-06-15 21:24:26.016068748 +0100
+++ binutils/gdb/disasm.c 2018-06-21 03:45:46.333040767 +0100
@@ -722,6 +722,31 @@ fprintf_disasm (void *stream, const char
return 0;
}
+/* Combine implicit and user disassembler options and return them
+ in a newly-created string. */
+
+static std::string
+get_all_disassembler_options (struct gdbarch *gdbarch)
+{
+ const char *implicit = gdbarch_disassembler_options_implicit (gdbarch);
+ const char *options = get_disassembler_options (gdbarch);
+ const char *comma = ",";
+
+ if (implicit == nullptr)
+ {
+ implicit = "";
+ comma = "";
+ }
+
+ if (options == nullptr)
+ {
+ options = "";
+ comma = "";
+ }
+
+ return string_printf ("%s%s%s", implicit, comma, options);
+}
+
gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
struct ui_file *file,
di_read_memory_ftype read_memory_func)
@@ -746,7 +771,9 @@ 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_disassembler_options_holder = get_all_disassembler_options (gdbarch);
+ if (!m_disassembler_options_holder.empty ())
+ m_di.disassembler_options = m_disassembler_options_holder.c_str ();
disassemble_init_for_target (&m_di);
}
@@ -833,13 +860,16 @@ gdb_buffered_insn_length_fprintf (void *
return 0;
}
-/* Initialize a struct disassemble_info for gdb_buffered_insn_length. */
+/* Initialize a struct disassemble_info for gdb_buffered_insn_length.
+ Upon return, *DISASSEMBLER_OPTIONS_HOLDER owns the string pointed
+ to by DI.DISASSEMBLER_OPTIONS. */
static void
gdb_buffered_insn_length_init_dis (struct gdbarch *gdbarch,
struct disassemble_info *di,
const gdb_byte *insn, int max_len,
- CORE_ADDR addr)
+ CORE_ADDR addr,
+ std::string *disassembler_options_holder)
{
init_disassemble_info (di, NULL, gdb_buffered_insn_length_fprintf);
@@ -855,7 +885,9 @@ 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);
+ *disassembler_options_holder = get_all_disassembler_options (gdbarch);
+ if (!disassembler_options_holder->empty ())
+ di->disassembler_options = disassembler_options_holder->c_str ();
disassemble_init_for_target (di);
}
@@ -867,8 +899,10 @@ gdb_buffered_insn_length (struct gdbarch
const gdb_byte *insn, int max_len, CORE_ADDR addr)
{
struct disassemble_info di;
+ std::string disassembler_options_holder;
- gdb_buffered_insn_length_init_dis (gdbarch, &di, insn, max_len, addr);
+ gdb_buffered_insn_length_init_dis (gdbarch, &di, insn, max_len, addr,
+ &disassembler_options_holder);
return gdbarch_print_insn (gdbarch, addr, &di);
}
@@ -887,6 +921,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;
@@ -903,20 +938,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)
{
@@ -943,6 +1000,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);
@@ -952,11 +1011,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"));
@@ -965,10 +1026,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;
}
@@ -976,10 +1042,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");
}
}
@@ -990,12 +1063,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". */
@@ -1006,10 +1100,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: binutils/gdb/disasm.h
===================================================================
--- binutils.orig/gdb/disasm.h 2018-06-15 21:24:26.051668963 +0100
+++ binutils/gdb/disasm.h 2018-06-21 03:45:46.347163161 +0100
@@ -66,6 +66,11 @@ class gdb_disassembler
/* Stores data required for disassembling instructions in
opcodes. */
struct disassemble_info m_di;
+
+ /* If we own the string in `m_di.disassembler_options', we do so
+ using this field. */
+ std::string m_disassembler_options_holder;
+
CORE_ADDR m_err_memaddr;
static int dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr,
Index: binutils/gdb/doc/gdb.texinfo
===================================================================
--- binutils.orig/gdb/doc/gdb.texinfo 2018-06-15 21:24:26.113202229 +0100
+++ binutils/gdb/doc/gdb.texinfo 2018-06-20 15:58:24.760060880 +0100
@@ -8759,7 +8759,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: binutils/gdb/gdbarch.c
===================================================================
--- binutils.orig/gdb/gdbarch.c 2018-06-15 21:24:26.150556003 +0100
+++ binutils/gdb/gdbarch.c 2018-06-20 15:58:24.767165213 +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: binutils/gdb/gdbarch.h
===================================================================
--- binutils.orig/gdb/gdbarch.h 2018-06-15 21:24:26.194158609 +0100
+++ binutils/gdb/gdbarch.h 2018-06-20 15:58:24.783344624 +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: binutils/gdb/gdbarch.sh
===================================================================
--- binutils.orig/gdb/gdbarch.sh 2018-06-15 21:24:26.228507323 +0100
+++ binutils/gdb/gdbarch.sh 2018-06-20 15:58:24.807623590 +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: binutils/gdb/mips-tdep.c
===================================================================
--- binutils.orig/gdb/mips-tdep.c 2018-06-15 21:24:26.301440991 +0100
+++ binutils/gdb/mips-tdep.c 2018-06-20 15:58:24.857592500 +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: binutils/gdb/testsuite/gdb.arch/mips-disassembler-options.exp
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/gdb/testsuite/gdb.arch/mips-disassembler-options.exp 2018-06-20 15:58:24.888391112 +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: binutils/gdb/testsuite/gdb.arch/mips-disassembler-options.s
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/gdb/testsuite/gdb.arch/mips-disassembler-options.s 2018-06-20 15:58:24.912048547 +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: binutils/include/dis-asm.h
===================================================================
--- binutils.orig/include/dis-asm.h 2018-06-15 21:24:26.377270056 +0100
+++ binutils/include/dis-asm.h 2018-06-20 15:58:24.930457954 +0100
@@ -222,16 +222,53 @@ 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. Individual options that support
+ an argument will refer to entries in the ARGS vector. */
+ disasm_options_t options;
+
+ /* Vector of acceptable option arguments, NULL-terminated. This
+ collects all possible option argument choices, some of which
+ may be shared by different options from the OPTIONS member. */
+ disasm_option_arg_t *args;
+} disasm_options_and_args_t;
\f
/* Standard disassemblers. Disassemble one instruction at the given
target address. Return number of octets processed. */
@@ -266,9 +303,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: binutils/opcodes/arm-dis.c
===================================================================
--- binutils.orig/opcodes/arm-dis.c 2018-06-15 21:24:26.396453512 +0100
+++ binutils/opcodes/arm-dis.c 2018-06-20 15:58:24.950803676 +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: binutils/opcodes/mips-dis.c
===================================================================
--- binutils.orig/opcodes/mips-dis.c 2018-06-15 21:24:26.414765089 +0100
+++ binutils/opcodes/mips-dis.c 2018-06-20 15:58:24.968090152 +0100
@@ -2552,72 +2552,178 @@ print_insn_little_mips (bfd_vma memaddr,
return _print_insn_mips (memaddr, info, BFD_ENDIAN_LITTLE);
}
\f
-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 },
+ { "ginv", N_("Recognize the Global INValidate (GINV) ASE "
+ "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\
- ginv Recognize the Global INValidate (GINV) ASe\n\
- instructions.\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\
- gpr-names=ABI Print GPR names according to specified ABI.\n\
- Default: based on binary being disassembled.\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\
- fpr-names=ABI Print FPR names according to specified ABI.\n\
- Default: numeric.\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\
- cp0-names=ARCH Print CP0 register names according to\n\
- specified 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\
- hwr-names=ARCH Print HWR names according to specified \n\
- architecture.\n\
- Default: based on binary being disassembled.\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=ABI Print GPR and FPR names according to\n\
- specified ABI.\n"));
+ return opts_and_args;
+}
- fprintf (stream, _("\n\
- reg-names=ARCH Print CP0 register and HWR names according to\n\
- specified architecture.\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;
- 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"));
+ 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: binutils/opcodes/ppc-dis.c
===================================================================
--- binutils.orig/opcodes/ppc-dis.c 2018-06-15 21:24:26.451364706 +0100
+++ binutils/opcodes/ppc-dis.c 2018-06-20 15:58:24.980194181 +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: binutils/opcodes/s390-dis.c
===================================================================
--- binutils.orig/opcodes/s390-dis.c 2018-06-15 21:24:26.509194605 +0100
+++ binutils/opcodes/s390-dis.c 2018-06-20 15:58:24.992299314 +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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
2018-06-21 7:27 [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options' Maciej W. Rozycki
@ 2018-06-21 17:52 ` Simon Marchi
2018-06-21 18:29 ` Maciej W. Rozycki
0 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2018-06-21 17:52 UTC (permalink / raw)
To: Maciej W. Rozycki, Simon Marchi, gdb-patches, binutils
Cc: Joel Brobecker, Fredrik Noring
Hi Maciej,
This patch looks good from the GDB side (with one nit in the test below), but
somebody from binutils would need to review the bits in opcodes/include.
> Index: binutils/gdb/testsuite/gdb.arch/mips-disassembler-options.exp
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ binutils/gdb/testsuite/gdb.arch/mips-disassembler-options.exp 2018-06-20 15:58:24.888391112 +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)"
Avoid parenthesis at the end of test names:
https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
2018-06-21 17:52 ` Simon Marchi
@ 2018-06-21 18:29 ` Maciej W. Rozycki
2018-06-21 19:19 ` Simon Marchi
0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2018-06-21 18:29 UTC (permalink / raw)
To: Simon Marchi
Cc: Simon Marchi, gdb-patches, binutils, Joel Brobecker, Fredrik Noring
Hi Simon,
> This patch looks good from the GDB side (with one nit in the test below), but
> somebody from binutils would need to review the bits in opcodes/include.
Yes, that's why I requested it separately and posted to both mailing
lists.
> > +# 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)"
>
> Avoid parenthesis at the end of test names:
>
> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages
Thanks for the pointer. Although it makes sense to me at first glance
that's quite a recent change to a long-established practice. Perhaps it
could have been avoided by coding the regression analysis tools referred
more carefully, but I won't be questioning the decision at this point.
The wiki does not indicate a suggested replacement however and I would
rather avoid creating a mess where individual tests would use different
approaches. Offhand I'd be inclined to use brackets, either square or
angled. What has been the new practice then?
Maciej
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
2018-06-21 18:29 ` Maciej W. Rozycki
@ 2018-06-21 19:19 ` Simon Marchi
2018-06-21 19:56 ` Maciej W. Rozycki
0 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2018-06-21 19:19 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Simon Marchi, gdb-patches, binutils, Joel Brobecker, Fredrik Noring
On 2018-06-21 14:28, Maciej W. Rozycki wrote:
> Hi Simon,
>
>> This patch looks good from the GDB side (with one nit in the test
>> below), but
>> somebody from binutils would need to review the bits in
>> opcodes/include.
>
> Yes, that's why I requested it separately and posted to both mailing
> lists.
>
>> > +# 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)"
>>
>> Avoid parenthesis at the end of test names:
>>
>>
>> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages
>
> Thanks for the pointer. Although it makes sense to me at first glance
> that's quite a recent change to a long-established practice. Perhaps
> it
> could have been avoided by coding the regression analysis tools
> referred
> more carefully, but I won't be questioning the decision at this point.
That's because of how DejaGNU formats test messages, for example when
there is a timeout (as shown in the example on the wiki). We don't have
control over that, and we don't want "foo" and "foo (timeout)" to be
considered as two different tests.
> The wiki does not indicate a suggested replacement however and I would
> rather avoid creating a mess where individual tests would use different
> approaches. Offhand I'd be inclined to use brackets, either square or
> angled. What has been the new practice then?
Often, parenthesis would be used when the same series of tests are ran
with different settings. For example:
foo (non-stop)
bar (non-stop)
foo (all-stop)
bar (all-stop)
For this it's useful to use prefixes, with with_test_prefix, for
example.
with_test_prefix "non-stop" {
...
}
with_test_prefix "all-stop" {
...
}
Or even
foreach_with_prefix mode {all-stop non-stop} {
...
}
That does not really apply to your case though. I think here you can
just remove the parenthesis, and maybe add a comma.
mips_disassemble_test bar "move\tv0,a4" "disassemble ABI, n64"
As long as it's clear.
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
2018-06-21 19:19 ` Simon Marchi
@ 2018-06-21 19:56 ` Maciej W. Rozycki
2018-06-21 20:06 ` Simon Marchi
2018-06-29 14:37 ` [PING][PATCH " Maciej W. Rozycki
0 siblings, 2 replies; 10+ messages in thread
From: Maciej W. Rozycki @ 2018-06-21 19:56 UTC (permalink / raw)
To: Simon Marchi
Cc: Simon Marchi, gdb-patches, binutils, Joel Brobecker, Fredrik Noring
On Thu, 21 Jun 2018, Simon Marchi wrote:
> > Thanks for the pointer. Although it makes sense to me at first glance
> > that's quite a recent change to a long-established practice. Perhaps it
> > could have been avoided by coding the regression analysis tools referred
> > more carefully, but I won't be questioning the decision at this point.
>
> That's because of how DejaGNU formats test messages, for example when there is
> a timeout (as shown in the example on the wiki). We don't have control over
> that, and we don't want "foo" and "foo (timeout)" to be considered as two
> different tests.
I think you chose the wrong example, after all what's the problem for
tools to notice that "foo (bar)" and "foo (bar) (timeout)" are the same
test? If the number of parenthesis pairs is different between two
messages, then strip the extra ones along with the text between from the
message that has more of them and only then compare the resulting strings.
What the problem would be are different failures in different runs, e.g.
if we have "foo (bar) (timeout)" in one and "foo (bar) (internal error)"
in the other one, then indeed we have an issue. We could strip
parenthesis pairs one by one from both messages at a time, but then we'd
incorrectly consider "foo (bar)" and "foo (baz)" the same test.
Does it matter in reality? I suppose so, given the outcome of the
discussion referred from the wiki, although I don't see the scenario I
have outlined here actually mentioned there (parts of the discussion seem
to be missing though from the archive).
> That does not really apply to your case though. I think here you can just
> remove the parenthesis, and maybe add a comma.
>
> mips_disassemble_test bar "move\tv0,a4" "disassemble ABI, n64"
>
> As long as it's clear.
OK, works for me. I'll make that adjustment as I commit the change once
the binutils parts have been approved.
Thank you for your review.
Maciej
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
2018-06-21 19:56 ` Maciej W. Rozycki
@ 2018-06-21 20:06 ` Simon Marchi
2018-06-29 14:37 ` [PING][PATCH " Maciej W. Rozycki
1 sibling, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2018-06-21 20:06 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Simon Marchi, gdb-patches, binutils, Joel Brobecker, Fredrik Noring
On 2018-06-21 15:55, Maciej W. Rozycki wrote:
> I think you chose the wrong example, after all what's the problem for
> tools to notice that "foo (bar)" and "foo (bar) (timeout)" are the same
> test? If the number of parenthesis pairs is different between two
> messages, then strip the extra ones along with the text between from
> the
> message that has more of them and only then compare the resulting
> strings.
We can do that if we know that they are supposed to be the different
test. But how do we know whether they are different tests or the same
test with a special outcome in the second case?
> What the problem would be are different failures in different runs,
> e.g.
> if we have "foo (bar) (timeout)" in one and "foo (bar) (internal
> error)"
> in the other one, then indeed we have an issue. We could strip
> parenthesis pairs one by one from both messages at a time, but then
> we'd
> incorrectly consider "foo (bar)" and "foo (baz)" the same test.
>
> Does it matter in reality? I suppose so, given the outcome of the
> discussion referred from the wiki, although I don't see the scenario I
> have outlined here actually mentioned there (parts of the discussion
> seem
> to be missing though from the archive).
The only purpose I am aware of is the script on the buildbot that
analyzes results. It would show a timeout as
new FAIL: foo (timeout)
instead of now:
PASS -> FAIL: foo
So it's not terribly important, but it helps making the analysis output
a bit better.
>> That does not really apply to your case though. I think here you can
>> just
>> remove the parenthesis, and maybe add a comma.
>>
>> mips_disassemble_test bar "move\tv0,a4" "disassemble ABI, n64"
>>
>> As long as it's clear.
>
> OK, works for me. I'll make that adjustment as I commit the change
> once
> the binutils parts have been approved.
>
> Thank you for your review.
>
> Maciej
Thanks,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PING][PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
2018-06-21 19:56 ` Maciej W. Rozycki
2018-06-21 20:06 ` Simon Marchi
@ 2018-06-29 14:37 ` Maciej W. Rozycki
2018-06-29 14:41 ` Maciej W. Rozycki
1 sibling, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2018-06-29 14:37 UTC (permalink / raw)
To: binutils; +Cc: Nick Clifton, gdb-patches
On Thu, 21 Jun 2018, Maciej W. Rozycki wrote:
> OK, works for me. I'll make that adjustment as I commit the change once
> the binutils parts have been approved.
Ping for the binutils side of:
<https://patchwork.sourceware.org/patch/27862/>
Maciej
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PING][PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
2018-06-29 14:37 ` [PING][PATCH " Maciej W. Rozycki
@ 2018-06-29 14:41 ` Maciej W. Rozycki
2018-07-02 15:57 ` Nick Clifton
0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2018-06-29 14:41 UTC (permalink / raw)
To: binutils; +Cc: Nick Clifton, gdb-patches
On Fri, 29 Jun 2018, Maciej W. Rozycki wrote:
> > OK, works for me. I'll make that adjustment as I commit the change once
> > the binutils parts have been approved.
>
> Ping for the binutils side of:
>
> <https://patchwork.sourceware.org/patch/27862/>
Correct version of the patch (paste error, sorry):
<https://patchwork.sourceware.org/patch/27974/>
Maciej
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PING][PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
2018-06-29 14:41 ` Maciej W. Rozycki
@ 2018-07-02 15:57 ` Nick Clifton
2018-07-02 23:01 ` [committed v5] " Maciej W. Rozycki
0 siblings, 1 reply; 10+ messages in thread
From: Nick Clifton @ 2018-07-02 15:57 UTC (permalink / raw)
To: Maciej W. Rozycki, binutils; +Cc: gdb-patches
Hi Maciej,
>>> OK, works for me. I'll make that adjustment as I commit the change once
>>> the binutils parts have been approved.
>>
>> Ping for the binutils side of:
>>
>> <https://patchwork.sourceware.org/patch/27862/>
>
> Correct version of the patch (paste error, sorry):
>
> <https://patchwork.sourceware.org/patch/27974/>
Approved for mainline - please apply.
Cheers
Nick
^ permalink raw reply [flat|nested] 10+ messages in thread
* [committed v5] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
2018-07-02 15:57 ` Nick Clifton
@ 2018-07-02 23:01 ` Maciej W. Rozycki
0 siblings, 0 replies; 10+ messages in thread
From: Maciej W. Rozycki @ 2018-07-02 23:01 UTC (permalink / raw)
To: Simon Marchi, Nick Clifton
Cc: Simon Marchi, Joel Brobecker, Fredrik Noring, binutils, gdb-patches
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.
ginv Recognize the Global INValidate (GINV) 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.
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.
2018-07-02 Maciej W. Rozycki <macro@mips.com>
Simon Marchi <simon.marchi@polymtl.ca>
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
`m_disassembler_options_holder'. member
* disasm.c (get_all_disassembler_options): New function.
(gdb_disassembler::gdb_disassembler): Use it.
(gdb_buffered_insn_length_init_dis): Likewise.
(gdb_buffered_insn_length): Adjust accordingly.
(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.
* 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.
---
Simon, Nick --
On Mon, 2 Jul 2018, Nick Clifton wrote:
> Approved for mainline - please apply.
Thank you for your reviews. This is the final version I have applied.
Maciej
Changes from v4:
- parentheses removed from test case names; replaced with commas,
- `print_mips_disassembler_options' change regenerated to take a typo fix
applied upstream into account,
- NEWS change regenerated.
Changes from v3:
- `get_all_disassembler_options' rewritten to return `std::string' rather
than `char *'.
- `gdb_disassembler' updated with `m_disassembler_options_holder' member
added and destructor removed.
- `gdb_buffered_insn_length_init_dis' and `gdb_buffered_insn_length'
updated accordingly, changes to `i386_print_insn' discarded.
Changes from v2:
- adjusted for `ginv' disassembly option support.
Changes from v1:
- `disasm_options_and_args_t' member comments expanded for clarification.
---
gdb/NEWS | 3
gdb/arm-tdep.c | 3
gdb/disasm.c | 127 ++++++++++-
gdb/disasm.h | 5
gdb/doc/gdb.texinfo | 2
gdb/gdbarch.c | 28 ++
gdb/gdbarch.h | 7
gdb/gdbarch.sh | 3
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 | 50 ++++
opcodes/arm-dis.c | 16 +
opcodes/mips-dis.c | 206 ++++++++++++++-----
opcodes/ppc-dis.c | 18 +
opcodes/s390-dis.c | 16 +
16 files changed, 501 insertions(+), 131 deletions(-)
gdb-mips-disassembler-options.diff
Index: binutils/gdb/NEWS
===================================================================
--- binutils.orig/gdb/NEWS 2018-07-02 15:43:50.979112556 +0100
+++ binutils/gdb/NEWS 2018-07-02 21:43:34.144978161 +0100
@@ -3,6 +3,9 @@
*** Changes since GDB 8.1
+* The 'set disassembler-options' command now supports specifying options
+ for the MIPS target.
+
* The 'symbol-file' command now accepts an '-o' option to add a relative
offset to all sections.
Index: binutils/gdb/arm-tdep.c
===================================================================
--- binutils.orig/gdb/arm-tdep.c 2018-07-02 15:43:51.004741533 +0100
+++ binutils/gdb/arm-tdep.c 2018-07-02 21:43:17.301229905 +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: binutils/gdb/disasm.c
===================================================================
--- binutils.orig/gdb/disasm.c 2018-07-02 15:43:51.017900575 +0100
+++ binutils/gdb/disasm.c 2018-07-02 21:43:17.312339626 +0100
@@ -722,6 +722,31 @@ fprintf_disasm (void *stream, const char
return 0;
}
+/* Combine implicit and user disassembler options and return them
+ in a newly-created string. */
+
+static std::string
+get_all_disassembler_options (struct gdbarch *gdbarch)
+{
+ const char *implicit = gdbarch_disassembler_options_implicit (gdbarch);
+ const char *options = get_disassembler_options (gdbarch);
+ const char *comma = ",";
+
+ if (implicit == nullptr)
+ {
+ implicit = "";
+ comma = "";
+ }
+
+ if (options == nullptr)
+ {
+ options = "";
+ comma = "";
+ }
+
+ return string_printf ("%s%s%s", implicit, comma, options);
+}
+
gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
struct ui_file *file,
di_read_memory_ftype read_memory_func)
@@ -746,7 +771,9 @@ 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_disassembler_options_holder = get_all_disassembler_options (gdbarch);
+ if (!m_disassembler_options_holder.empty ())
+ m_di.disassembler_options = m_disassembler_options_holder.c_str ();
disassemble_init_for_target (&m_di);
}
@@ -833,13 +860,16 @@ gdb_buffered_insn_length_fprintf (void *
return 0;
}
-/* Initialize a struct disassemble_info for gdb_buffered_insn_length. */
+/* Initialize a struct disassemble_info for gdb_buffered_insn_length.
+ Upon return, *DISASSEMBLER_OPTIONS_HOLDER owns the string pointed
+ to by DI.DISASSEMBLER_OPTIONS. */
static void
gdb_buffered_insn_length_init_dis (struct gdbarch *gdbarch,
struct disassemble_info *di,
const gdb_byte *insn, int max_len,
- CORE_ADDR addr)
+ CORE_ADDR addr,
+ std::string *disassembler_options_holder)
{
init_disassemble_info (di, NULL, gdb_buffered_insn_length_fprintf);
@@ -855,7 +885,9 @@ 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);
+ *disassembler_options_holder = get_all_disassembler_options (gdbarch);
+ if (!disassembler_options_holder->empty ())
+ di->disassembler_options = disassembler_options_holder->c_str ();
disassemble_init_for_target (di);
}
@@ -867,8 +899,10 @@ gdb_buffered_insn_length (struct gdbarch
const gdb_byte *insn, int max_len, CORE_ADDR addr)
{
struct disassemble_info di;
+ std::string disassembler_options_holder;
- gdb_buffered_insn_length_init_dis (gdbarch, &di, insn, max_len, addr);
+ gdb_buffered_insn_length_init_dis (gdbarch, &di, insn, max_len, addr,
+ &disassembler_options_holder);
return gdbarch_print_insn (gdbarch, addr, &di);
}
@@ -887,6 +921,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;
@@ -903,20 +938,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)
{
@@ -943,6 +1000,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);
@@ -952,11 +1011,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"));
@@ -965,10 +1026,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;
}
@@ -976,10 +1042,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");
}
}
@@ -990,12 +1063,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". */
@@ -1006,10 +1100,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: binutils/gdb/disasm.h
===================================================================
--- binutils.orig/gdb/disasm.h 2018-07-02 15:43:51.036048109 +0100
+++ binutils/gdb/disasm.h 2018-07-02 21:43:17.328742613 +0100
@@ -66,6 +66,11 @@ class gdb_disassembler
/* Stores data required for disassembling instructions in
opcodes. */
struct disassemble_info m_di;
+
+ /* If we own the string in `m_di.disassembler_options', we do so
+ using this field. */
+ std::string m_disassembler_options_holder;
+
CORE_ADDR m_err_memaddr;
static int dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr,
Index: binutils/gdb/doc/gdb.texinfo
===================================================================
--- binutils.orig/gdb/doc/gdb.texinfo 2018-07-02 15:43:51.049225362 +0100
+++ binutils/gdb/doc/gdb.texinfo 2018-07-02 21:43:17.393431303 +0100
@@ -8759,7 +8759,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: binutils/gdb/gdbarch.c
===================================================================
--- binutils.orig/gdb/gdbarch.c 2018-07-02 15:43:51.066516016 +0100
+++ binutils/gdb/gdbarch.c 2018-07-02 21:43:17.408564790 +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: binutils/gdb/gdbarch.h
===================================================================
--- binutils.orig/gdb/gdbarch.h 2018-07-02 15:43:51.077705580 +0100
+++ binutils/gdb/gdbarch.h 2018-07-02 21:43:17.418678067 +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: binutils/gdb/gdbarch.sh
===================================================================
--- binutils.orig/gdb/gdbarch.sh 2018-07-02 15:43:51.102978282 +0100
+++ binutils/gdb/gdbarch.sh 2018-07-02 21:43:17.427753164 +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: binutils/gdb/mips-tdep.c
===================================================================
--- binutils.orig/gdb/mips-tdep.c 2018-07-02 15:43:51.107040256 +0100
+++ binutils/gdb/mips-tdep.c 2018-07-02 21:43:17.464486024 +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: binutils/gdb/testsuite/gdb.arch/mips-disassembler-options.exp
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/gdb/testsuite/gdb.arch/mips-disassembler-options.exp 2018-07-02 21:43:17.490058219 +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: binutils/gdb/testsuite/gdb.arch/mips-disassembler-options.s
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/gdb/testsuite/gdb.arch/mips-disassembler-options.s 2018-07-02 21:43:17.503421947 +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: binutils/include/dis-asm.h
===================================================================
--- binutils.orig/include/dis-asm.h 2018-07-02 15:43:51.127220726 +0100
+++ binutils/include/dis-asm.h 2018-07-02 21:43:17.517769057 +0100
@@ -222,16 +222,53 @@ 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. Individual options that support
+ an argument will refer to entries in the ARGS vector. */
+ disasm_options_t options;
+
+ /* Vector of acceptable option arguments, NULL-terminated. This
+ collects all possible option argument choices, some of which
+ may be shared by different options from the OPTIONS member. */
+ disasm_option_arg_t *args;
+} disasm_options_and_args_t;
\f
/* Standard disassemblers. Disassemble one instruction at the given
target address. Return number of octets processed. */
@@ -266,9 +303,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: binutils/opcodes/arm-dis.c
===================================================================
--- binutils.orig/opcodes/arm-dis.c 2018-07-02 15:43:51.138327151 +0100
+++ binutils/opcodes/arm-dis.c 2018-07-02 21:43:17.536124809 +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: binutils/opcodes/mips-dis.c
===================================================================
--- binutils.orig/opcodes/mips-dis.c 2018-07-02 15:43:51.150516184 +0100
+++ binutils/opcodes/mips-dis.c 2018-07-02 21:43:17.550417074 +0100
@@ -2552,72 +2552,178 @@ print_insn_little_mips (bfd_vma memaddr,
return _print_insn_mips (memaddr, info, BFD_ENDIAN_LITTLE);
}
\f
-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 },
+ { "ginv", N_("Recognize the Global INValidate (GINV) ASE "
+ "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\
- ginv Recognize the Global INValidate (GINV) ASE\n\
- instructions.\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\
- gpr-names=ABI Print GPR names according to specified ABI.\n\
- Default: based on binary being disassembled.\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\
- fpr-names=ABI Print FPR names according to specified ABI.\n\
- Default: numeric.\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\
- cp0-names=ARCH Print CP0 register names according to\n\
- specified 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\
- hwr-names=ARCH Print HWR names according to specified \n\
- architecture.\n\
- Default: based on binary being disassembled.\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=ABI Print GPR and FPR names according to\n\
- specified ABI.\n"));
+ return opts_and_args;
+}
- fprintf (stream, _("\n\
- reg-names=ARCH Print CP0 register and HWR names according to\n\
- specified architecture.\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;
- 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"));
+ 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: binutils/opcodes/ppc-dis.c
===================================================================
--- binutils.orig/opcodes/ppc-dis.c 2018-07-02 15:43:51.159621521 +0100
+++ binutils/opcodes/ppc-dis.c 2018-07-02 21:43:17.562558409 +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: binutils/opcodes/s390-dis.c
===================================================================
--- binutils.orig/opcodes/s390-dis.c 2018-07-02 15:43:51.170726634 +0100
+++ binutils/opcodes/s390-dis.c 2018-07-02 21:43:17.569646380 +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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-07-02 23:01 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-21 7:27 [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options' Maciej W. Rozycki
2018-06-21 17:52 ` Simon Marchi
2018-06-21 18:29 ` Maciej W. Rozycki
2018-06-21 19:19 ` Simon Marchi
2018-06-21 19:56 ` Maciej W. Rozycki
2018-06-21 20:06 ` Simon Marchi
2018-06-29 14:37 ` [PING][PATCH " Maciej W. Rozycki
2018-06-29 14:41 ` Maciej W. Rozycki
2018-07-02 15:57 ` Nick Clifton
2018-07-02 23:01 ` [committed v5] " Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).