[PATCH 0/9] Disassembler opcode display and text alignment

[PATCH 0/9] Disassembler opcode display and text alignment

[Andrew Burgess]

This series makes two related changes to GDB's disassembler.  Both
changes flow naturally from having GDB make use of libopcodes
bytes_per_line and bytes_per_chunk presentation hints that are set on
a per-architecure basis with each call into the disassembler.

The end result of this change is that GDB will now display instruction
opcodes in the same way that objdump does.

For x86-64 there's no change in the way that opcode bytes are
presented.  Now due to some special case, it's just that how GDB lays
out instruction opcodes just happens to match how libopcodes requests
that the opcodes be laid out.

For other architectures, risc-v, powerpc, arm, aarch64, etc, this is
not the case, and I (personally) think objdump does a better job of
presenting the information that GDB does; the instruction opcodes will
be grouped together based on the instruction size, and potentially
byte-swapped so they appear in the instruction's natural order.

Making use of the display hints also allows for better alignment of
the disassembly text when opcodes are being printed.

I have proposed that this new behaviour become the default for
'disassemble /r', and I've added a new flag 'disassemble /b' which
allows the user to access the old behaviour.

But, if people feel strongly that the old 'disassemble /r' should not
be changed, I can place the new behaviour under a new flag.

Let me know your thoughts,

Thanks,
Andrew

---

Andrew Burgess (9):
  gdb/doc: improve description of --data-disassemble opcodes output
  gdb/testsuite: new test for -data-disassemble opcodes format
  gdb/disasm: read opcodes bytes with a single read_code call
  gdb: disassembler opcode display formatting
  gdb: make gdb_disassembly_flag unsigned
  gdb/doc: fix column widths in MI compatibility table
  gdb/doc: update syntax of -data-disassemble command arguments
  gdb/mi: some int to bool conversion
  gdb/mi: new options for -data-disassemble command

 gdb/NEWS                                |  12 ++
 gdb/cli/cli-cmds.c                      |   6 +
 gdb/disasm-flags.h                      |   3 +-
 gdb/disasm.c                            |  55 +++++++--
 gdb/disasm.h                            |   3 +
 gdb/doc/gdb.texinfo                     | 137 ++++++++++++++++-----
 gdb/mi/mi-cmd-disas.c                   | 105 ++++++++++++----
 gdb/record.c                            |   3 +
 gdb/testsuite/gdb.mi/mi-disassemble.exp | 153 ++++++++++++++++++++----
 gdb/testsuite/lib/mi-support.exp        |  27 +++++
 10 files changed, 417 insertions(+), 87 deletions(-)

-- 
2.25.4

[PATCH 1/9] gdb/doc: improve description of --data-disassemble opcodes output

[Andrew Burgess]

Extend the description of the MI command --data-disassemble.
Specifically, expand the description of the 'opcodes' field to explain
how the bytes are formatted.
---
 gdb/doc/gdb.texinfo | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2178b476f53..d046ce5891e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34422,8 +34422,10 @@
 The text disassembly for this @samp{address}.
 
 @item opcodes
-This field is only present for modes 2, 3 and 5.  This contains the raw opcode
-bytes for the @samp{inst} field.
+This field is only present for modes 2, 3 and 5.  This contains the
+raw opcode bytes for the @samp{inst} field.  The bytes are formatted
+as single bytes, in hex, in ascending address order, with a single
+space between each byte.
 
 @end table
 
-- 
2.25.4

[PATCH 2/9] gdb/testsuite: new test for -data-disassemble opcodes format

[Andrew Burgess]

Add another test for the output of MI command -data-disassemble.  The
new check validates the format of the 'opcodes' field, specifically,
this test checks that the field contains a series of bytes, separated
by a single space.

We also check that the bytes are in the correct order, that is, the
first byte is from the lowest address, and subsequent bytes are from
increasing addresses.

The motivation for this test (besides more tests being generally good)
is that I plan to make changes to how opcode bytes are displayed in
the disassembler output, and I want to ensure that I don't break any
existing MI behaviour.

There should be no user visible changes to GDB after this commit.
---
 gdb/testsuite/gdb.mi/mi-disassemble.exp | 83 +++++++++++++++++++++++++
 gdb/testsuite/lib/mi-support.exp        | 27 ++++++++
 2 files changed, 110 insertions(+)

diff --git a/gdb/testsuite/gdb.mi/mi-disassemble.exp b/gdb/testsuite/gdb.mi/mi-disassemble.exp
index 2d3e8e216b1..b7c52472c84 100644
--- a/gdb/testsuite/gdb.mi/mi-disassemble.exp
+++ b/gdb/testsuite/gdb.mi/mi-disassemble.exp
@@ -239,6 +239,88 @@ proc test_disassembly_bogus_args {} {
 
 }
 
+# Check the format of the opcode bytes.
+proc test_disassembly_opcode_format {} {
+    # First, we need to find a multi-byte instruction that we can
+    # then disassemble using the MI command.
+    set longest_insn_bytes ""
+    set longest_insn_addr ""
+    gdb_test_multiple "disassemble /r main" "" {
+	-re "^disassemble /r main\r\n" {
+	    exp_continue
+	}
+
+	-re "^&\"disassemble /r main.n\"\r\n" {
+	    exp_continue
+	}
+
+	-re "^~\"Dump of assembler code for function \[^\r\n\]+\r\n" {
+	    exp_continue
+	}
+
+	-re "^~\".. ($::hex) <\[^>\]+>:\\\\t(\[^\\\\\]+)\\\\t\[^\r\n\]+\r\n" {
+	    set addr $expect_out(1,string)
+	    set bytes $expect_out(2,string)
+	    if { [string length $bytes] > [string length $longest_insn_bytes] } {
+		set longest_insn_addr $addr
+		set longest_insn_bytes $bytes
+	    }
+	    exp_continue
+	}
+
+	-re "^~\"End of assembler dump\[^\r\n\]+\r\n" {
+	    exp_continue
+	}
+
+	-re "^\\^done\r\n$::mi_gdb_prompt$" {
+	    gdb_assert { ![string equal $longest_insn_bytes ""] } \
+		"found the bytes string for a longest instruction"
+	    gdb_assert { ![string equal $longest_insn_addr ""] } \
+		"found the address for a longest instruction"
+	}
+    }
+
+    verbose -log "Longest instruction at ${longest_insn_addr} with bytes '${longest_insn_bytes}'"
+
+    # Check that the instruction bytes that we found above consists of
+    # a series of individual bytes separated by a whitespace.  Also,
+    # we check that the bytes reported match what can be found in the
+    # inferior memory.
+    set split_bytes [split $longest_insn_bytes " "]
+    set is_bad false
+    set addr $longest_insn_addr
+    set idx 0
+    foreach b $split_bytes {
+	if { [string length $b] != 2 } {
+	    set is_bad true
+	}
+
+	# Load the actual byte value from memory, and check it matches
+	# the opcode byte reported in the disassembler output.
+	set addr 0x[format %x [expr $longest_insn_addr + $idx]]
+	set actual [format %02x [mi_get_valueof "/x" "*((unsigned char *) $addr)" "XX"]]
+	gdb_assert [string equal $actual "$b"] \
+	    "byte at $addr matches"
+
+	incr idx
+    }
+    gdb_assert { !$is_bad } "check length of each byte"
+    set check_bytes [join $split_bytes " "]
+    gdb_assert { [string equal $check_bytes $longest_insn_bytes] } \
+	"bytes are separated by a single space"
+
+    # Figure out an end address at which to stop the disassembly.
+    set byte_count [llength $split_bytes]
+    set end_addr 0x[format %x [expr $longest_insn_addr + $byte_count]]
+    set start_addr $longest_insn_addr
+
+    verbose -log "Instruction is ${byte_count} bytes, end address ${end_addr}"
+
+    mi_gdb_test "321-data-disassemble -s $start_addr -e $end_addr -- 2" \
+	"321\\^done,asm_insns=\\\[\{address=\"$start_addr\",func-name=\"main\",offset=\"$::decimal\",opcodes=\"$longest_insn_bytes\",inst=\".*\"\}\]" \
+	"data-disassemble checking the opcodes bytes format"
+}
+
 mi_clean_restart $binfile
 mi_runto_main
 test_disassembly_only
@@ -248,6 +330,7 @@ test_disassembly_mixed_with_opcodes
 test_disassembly_bogus_args
 test_disassembly_lines_limit
 test_disassembly_mixed_lines_limit
+test_disassembly_opcode_format
 
 mi_gdb_exit
 return 0
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index e578a7e6f9b..2af12461ec5 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2783,3 +2783,30 @@ proc mi_is_target_remote {} {
 
     return [gdb_is_target_remote_prompt "$mi_gdb_prompt"]
 }
+
+# Retrieve the value of EXP in the inferior, represented in format
+# specified in FMT (using "printFMT").  DEFAULT is used as fallback if
+# print fails.  TEST is the test message to use.  It can be omitted,
+# in which case a test message is built from EXP.
+#
+# This is an MI version of gdb_valueof.
+
+proc mi_get_valueof { fmt exp default {test ""} } {
+    global mi_gdb_prompt
+
+    if {$test == "" } {
+	set test "get valueof \"${exp}\""
+    }
+
+    set val ${default}
+    gdb_test_multiple "print${fmt} ${exp}" "$test" {
+	-re "~\"\\$\[0-9\]* = (\[^\r\n\]*)\\\\n\"\r\n\\^done\r\n$mi_gdb_prompt$" {
+	    set val $expect_out(1,string)
+	    pass "$test"
+	}
+	timeout {
+	    fail "$test (timeout)"
+	}
+    }
+    return ${val}
+}
-- 
2.25.4

[PATCH 3/9] gdb/disasm: read opcodes bytes with a single read_code call

[Andrew Burgess]

This commit reduces the number of times we call read_code when
printing the instruction opcode bytes during disassembly.

I've added a new gdb::byte_vector within the
gdb_pretty_print_disassembler class, in line with all the other
buffers that gdb_pretty_print_disassembler needs.  This byte_vector is
then resized as needed, and filled with a single read_code call for
each instruction.

There should be no user visible changes after this commit.
---
 gdb/disasm.c | 16 +++++++---------
 gdb/disasm.h |  3 +++
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index c6edc92930d..42351c735d3 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -323,21 +323,19 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
 
     if (flags & DISASSEMBLY_RAW_INSN)
       {
-	CORE_ADDR end_pc;
-	bfd_byte data;
-	const char *spacer = "";
-
 	/* Build the opcodes using a temporary stream so we can
 	   write them out in a single go for the MI.  */
 	m_opcode_stb.clear ();
 
-	end_pc = pc + size;
+	/* Read the instruction opcode data.  */
+	m_opcode_data.resize (size);
+	read_code (pc, m_opcode_data.data (), size);
 
-	for (;pc < end_pc; ++pc)
+	for (int i = 0; i < size; ++i)
 	  {
-	    read_code (pc, &data, 1);
-	    m_opcode_stb.printf ("%s%02x", spacer, (unsigned) data);
-	    spacer = " ";
+	    if (i > 0)
+	      m_opcode_stb.puts (" ");
+	    m_opcode_stb.printf ("%02x", (unsigned) m_opcode_data[i]);
 	  }
 
 	m_uiout->field_stream ("opcodes", m_opcode_stb);
diff --git a/gdb/disasm.h b/gdb/disasm.h
index da03e130526..13403042709 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -311,6 +311,9 @@ class gdb_pretty_print_disassembler
 
   /* The buffer used to build the raw opcodes string.  */
   string_file m_opcode_stb;
+
+  /* The buffer used to hold the opcode bytes (if required).  */
+  gdb::byte_vector m_opcode_data;
 };
 
 /* Return the length in bytes of the instruction at address MEMADDR in
-- 
2.25.4

[PATCH 4/9] gdb: disassembler opcode display formatting

[Andrew Burgess]

This commit changes the format of 'disassemble /r' to match GNU
objdump.  Specifically, GDB will now display the instruction bytes in
as 'objdump --wide --disassemble' does.

Here is an example for RISC-V before this patch:

  (gdb) disassemble /r 0x0001018e,0x0001019e
  Dump of assembler code from 0x1018e to 0x1019e:
     0x0001018e <call_me+66>:     03 26 84 fe     lw      a2,-24(s0)
     0x00010192 <call_me+70>:     83 25 c4 fe     lw      a1,-20(s0)
     0x00010196 <call_me+74>:     61 65   lui     a0,0x18
     0x00010198 <call_me+76>:     13 05 85 6a     addi    a0,a0,1704
     0x0001019c <call_me+80>:     f1 22   jal     0x10368 <printf>
  End of assembler dump.

And here's an example after this patch:

  (gdb) disassemble /r 0x0001018e,0x0001019e
  Dump of assembler code from 0x1018e to 0x1019e:
     0x0001018e <call_me+66>:     fe842603                lw      a2,-24(s0)
     0x00010192 <call_me+70>:     fec42583                lw      a1,-20(s0)
     0x00010196 <call_me+74>:     6561                    lui     a0,0x18
     0x00010198 <call_me+76>:     6a850513                addi    a0,a0,1704
     0x0001019c <call_me+80>:     22f1                    jal     0x10368 <printf>
  End of assembler dump.

There are two differences here.  First, the instruction bytes after
the patch are grouped based on the size of the instruction, and are
byte-swapped to little-endian order.

Second, after the patch, GDB now uses the bytes-per-line hint from
libopcodes to add whitespace padding after the opcode bytes, this
means that in most cases the instructions are nicely aligned.

It is still possible for a very long instruction to intrude into the
disassembled text space.  The next example is x86-64, before the
patch:

  (gdb) disassemble /r main
  Dump of assembler code for function main:
     0x0000000000401106 <+0>:     55      push   %rbp
     0x0000000000401107 <+1>:     48 89 e5        mov    %rsp,%rbp
     0x000000000040110a <+4>:     c7 87 d8 00 00 00 01 00 00 00   movl   $0x1,0xd8(%rdi)
     0x0000000000401114 <+14>:    b8 00 00 00 00  mov    $0x0,%eax
     0x0000000000401119 <+19>:    5d      pop    %rbp
     0x000000000040111a <+20>:    c3      ret
  End of assembler dump.

And after the patch:

  (gdb) disassemble /r main
  Dump of assembler code for function main:
     0x0000000000401106 <+0>:     55                      push   %rbp
     0x0000000000401107 <+1>:     48 89 e5                mov    %rsp,%rbp
     0x000000000040110a <+4>:     c7 87 d8 00 00 00 01 00 00 00   movl   $0x1,0xd8(%rdi)
     0x0000000000401114 <+14>:    b8 00 00 00 00          mov    $0x0,%eax
     0x0000000000401119 <+19>:    5d                      pop    %rbp
     0x000000000040111a <+20>:    c3                      ret
  End of assembler dump.

Most instructions are aligned, except for the very long instruction.
Notice too that for x86-64 libopcodes doesn't request that GDB group
the instruction bytes.  This matches the behaviour of objdump.

In case the user really wants the old behaviour, I have added a new
modifier 'disassemble /b', this displays the instruction byte at a
time.  For x86-64, which never groups instruction bytes, /b and /r are
equivalent, but for RISC-V, using /b gets the old layout back (except
that the whitespace for alignment is still present).  Consider our
original RISC-V example, this time using /b:

  (gdb) disassemble /b 0x0001018e,0x0001019e
  Dump of assembler code from 0x1018e to 0x1019e:
     0x0001018e <call_me+66>:     03 26 84 fe             lw      a2,-24(s0)
     0x00010192 <call_me+70>:     83 25 c4 fe             lw      a1,-20(s0)
     0x00010196 <call_me+74>:     61 65                   lui     a0,0x18
     0x00010198 <call_me+76>:     13 05 85 6a             addi    a0,a0,1704
     0x0001019c <call_me+80>:     f1 22                   jal     0x10368 <printf>
  End of assembler dump.

Obviously, this patch is a potentially significant change to the
behaviour or /r.  I could have added /b with the new behaviour and
left /r alone.  However, personally, I feel the new behaviour is
significantly better than the old, hence, I made /r be what I consider
the "better" behaviour.

The reason I prefer the new behaviour is that, when I use /r, I almost
always want to manually decode the instruction for some reason, and
having the bytes displayed in "instruction order" rather than memory
order, just makes this easier.

The 'record instruction-history' command also takes a /r modifier, and
has been modified in the same way as disassemble; /r gets the new
behaviour, and /b has been added to retain the old behaviour.

Finally, the MI command -data-disassemble, is unchanged in behaviour,
this command now requests the raw bytes of the instruction, which is
equivalent to the /b modifier.  This means that the MI output will
remain backward compatible.
---
 gdb/NEWS                                | 12 +++++++
 gdb/cli/cli-cmds.c                      |  6 ++++
 gdb/disasm-flags.h                      |  1 +
 gdb/disasm.c                            | 43 ++++++++++++++++++++--
 gdb/doc/gdb.texinfo                     | 48 +++++++++++++++++++++----
 gdb/mi/mi-cmd-disas.c                   |  6 ++--
 gdb/record.c                            |  3 ++
 gdb/testsuite/gdb.mi/mi-disassemble.exp |  6 ++--
 8 files changed, 109 insertions(+), 16 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index ac9a1aacd34..3e8853505eb 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -27,6 +27,18 @@
   emit to indicate where a breakpoint should be placed to break in a function
   past its prologue.
 
+* The format of 'disassemble /r' and 'record instruction-history /r'
+  has changed.  The instruction bytes could now be grouped together,
+  and displayed in the endianness of the instruction.  This is the
+  same layout as used by GNU objdump when disassembling.
+
+  There is now 'disassemble /b' and 'record instruction-history /b'
+  which will always display the instructions bytes one at a time in
+  memory order, that is, the byte at the lowest address first.
+
+  For both /r and /b GDB is now better at using whitespace in order to
+  align the disassembled instruction text.
+
 * New commands
 
 maintenance set ignore-prologue-end-flag on|off
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 31911ebe61f..4950ee376a1 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1508,6 +1508,9 @@ disassemble_current_function (gdb_disassembly_flags flags)
 
    A /r modifier will include raw instructions in hex with the assembly.
 
+   A /b modifier is similar to /r except the instruction bytes are printed
+   as separate bytes with no grouping, or endian switching.
+
    A /s modifier will include source code with the assembly, like /m, with
    two important differences:
    1) The output is still in pc address order.
@@ -1546,6 +1549,9 @@ disassemble_command (const char *arg, int from_tty)
 	    case 'r':
 	      flags |= DISASSEMBLY_RAW_INSN;
 	      break;
+	    case 'b':
+	      flags |= DISASSEMBLY_RAW_BYTES;
+	      break;
 	    case 's':
 	      flags |= DISASSEMBLY_SOURCE;
 	      break;
diff --git a/gdb/disasm-flags.h b/gdb/disasm-flags.h
index 025b6893941..5a7371b0a39 100644
--- a/gdb/disasm-flags.h
+++ b/gdb/disasm-flags.h
@@ -33,6 +33,7 @@ enum gdb_disassembly_flag
     DISASSEMBLY_OMIT_PC = (0x1 << 4),
     DISASSEMBLY_SOURCE = (0x1 << 5),
     DISASSEMBLY_SPECULATIVE = (0x1 << 6),
+    DISASSEMBLY_RAW_BYTES = (0x1 << 7),
   };
 DEF_ENUM_FLAGS_TYPE (enum gdb_disassembly_flag, gdb_disassembly_flags);
 
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 42351c735d3..946c235e7ef 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -321,7 +321,7 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
 	throw ex;
       }
 
-    if (flags & DISASSEMBLY_RAW_INSN)
+    if ((flags & (DISASSEMBLY_RAW_INSN | DISASSEMBLY_RAW_BYTES)) != 0)
       {
 	/* Build the opcodes using a temporary stream so we can
 	   write them out in a single go for the MI.  */
@@ -331,14 +331,51 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
 	m_opcode_data.resize (size);
 	read_code (pc, m_opcode_data.data (), size);
 
-	for (int i = 0; i < size; ++i)
+	/* The disassembler provides information about the best way to
+	   display the instruction bytes to the user.  We provide some sane
+	   defaults in case the disassembler gets it wrong.  */
+	const struct disassemble_info *di = m_di.disasm_info ();
+	int bytes_per_line = std::max (di->bytes_per_line, size);
+	int bytes_per_chunk = std::max (di->bytes_per_chunk, 1);
+
+	/* If the user has requested the instruction bytes be displayed
+	   byte at a time, then handle that here.  Also, if the instruction
+	   is not a multiple of the chunk size (which probably indicates a
+	   disassembler problem) then avoid that causing display problems
+	   by switching to byte at a time mode.  */
+	if ((flags & DISASSEMBLY_RAW_BYTES) != 0
+	    || (size % bytes_per_chunk) != 0)
+	  bytes_per_chunk = 1;
+
+	/* Print the instruction opcodes bytes, grouped into chunks.  */
+	for (int i = 0; i < size; i += bytes_per_chunk)
 	  {
 	    if (i > 0)
 	      m_opcode_stb.puts (" ");
-	    m_opcode_stb.printf ("%02x", (unsigned) m_opcode_data[i]);
+
+	    if (di->display_endian == BFD_ENDIAN_LITTLE)
+	      {
+		for (int k = bytes_per_chunk; k-- != 0; )
+		  m_opcode_stb.printf ("%02x", (unsigned) m_opcode_data[i + k]);
+	      }
+	    else
+	      {
+		for (int k = 0; k < bytes_per_chunk; k++)
+		  m_opcode_stb.printf ("%02x", (unsigned) m_opcode_data[i + k]);
+	      }
+	  }
+
+	/* Calculate required padding.  */
+	int nspaces = 0;
+	for (int i = size; i < bytes_per_line; i += bytes_per_chunk)
+	  {
+	    if (i > size)
+	      nspaces++;
+	    nspaces += bytes_per_chunk * 2;
 	  }
 
 	m_uiout->field_stream ("opcodes", m_opcode_stb);
+	m_uiout->spaces (nspaces);
 	m_uiout->text ("\t");
       }
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d046ce5891e..af559370db0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7901,7 +7901,10 @@
 
 It can also print mixed source+disassembly if you specify the the
 @code{/m} or @code{/s} modifier, and print the raw instructions in hex
-as well as in symbolic form by specifying the @code{/r} modifier.
+as well as in symbolic form by specifying the @code{/r} or @code{/b}
+modifier.  The behaviour of the @code{/m}, @code{/s}, @code{/r}, and
+@code{/b} modifiers are the same as for the @kbd{disassemble} command
+(@pxref{disassemble,,@kbd{disassemble}}).
 
 The current position marker is printed for the instruction at the
 current program counter value.  This instruction can appear multiple
@@ -9815,6 +9818,7 @@
 specifying a location will display information about the next source
 line.
 
+@anchor{disassemble}
 @table @code
 @kindex disassemble
 @cindex assembly instructions
@@ -9825,16 +9829,17 @@
 @itemx disassemble /m
 @itemx disassemble /s
 @itemx disassemble /r
+@itemx disassemble /b
 This specialized command dumps a range of memory as machine
 instructions.  It can also print mixed source+disassembly by specifying
-the @code{/m} or @code{/s} modifier and print the raw instructions in hex
-as well as in symbolic form by specifying the @code{/r} modifier.
-The default memory range is the function surrounding the
+the @code{/m} or @code{/s} modifier and print the raw instructions in
+hex as well as in symbolic form by specifying the @code{/r} or @code{/b}
+modifier.  The default memory range is the function surrounding the
 program counter of the selected frame.  A single argument to this
 command is a program counter value; @value{GDBN} dumps the function
-surrounding this value.  When two arguments are given, they should
-be separated by a comma, possibly surrounded by whitespace.  The
-arguments specify a range of addresses to dump, in one of two forms:
+surrounding this value.  When two arguments are given, they should be
+separated by a comma, possibly surrounded by whitespace.  The arguments
+specify a range of addresses to dump, in one of two forms:
 
 @table @code
 @item @var{start},@var{end}
@@ -9872,6 +9877,35 @@
 End of assembler dump.
 @end smallexample
 
+The following two examples are for RISC-V, and demonstrates the
+difference between the @code{/r} and @code{/b} modifiers.  First with
+@code{/b}, the bytes of the instruction are printed, in hex, in memory
+order:
+
+@smallexample
+(@value{GDBP}) disassemble /b 0x00010150,0x0001015c
+Dump of assembler code from 0x10150 to 0x1015c:
+   0x00010150 <call_me+4>:      22 dc                 	sw	s0,56(sp)
+   0x00010152 <call_me+6>:      80 00                 	addi	s0,sp,64
+   0x00010154 <call_me+8>:      23 26 a4 fe           	sw	a0,-20(s0)
+   0x00010158 <call_me+12>:     23 24 b4 fe           	sw	a1,-24(s0)
+End of assembler dump.
+@end smallexample
+
+In contrast, with @code{/r} the bytes of the instruction are displayed
+in the instruction order, for RISC-V this means that the bytes have been
+swapped to little-endian order:
+
+@smallexample
+(@value{GDBP}) disassemble /r 0x00010150,0x0001015c
+Dump of assembler code from 0x10150 to 0x1015c:
+   0x00010150 <call_me+4>:      dc22              	sw	s0,56(sp)
+   0x00010152 <call_me+6>:      0080              	addi	s0,sp,64
+   0x00010154 <call_me+8>:      fea42623        	sw	a0,-20(s0)
+   0x00010158 <call_me+12>:     feb42423        	sw	a1,-24(s0)
+End of assembler dump.
+@end smallexample
+
 Here is an example showing mixed source+assembly for Intel x86
 with @code{/m} or @code{/s}, when the program is stopped just after
 function prologue in a non-optimized function with no inline code.
diff --git a/gdb/mi/mi-cmd-disas.c b/gdb/mi/mi-cmd-disas.c
index 387c4900150..c8e06cd940a 100644
--- a/gdb/mi/mi-cmd-disas.c
+++ b/gdb/mi/mi-cmd-disas.c
@@ -165,16 +165,16 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
       disasm_flags |= DISASSEMBLY_SOURCE_DEPRECATED;
       break;
     case 2:
-      disasm_flags |= DISASSEMBLY_RAW_INSN;
+      disasm_flags |= DISASSEMBLY_RAW_BYTES;
       break;
     case 3:
-      disasm_flags |= DISASSEMBLY_SOURCE_DEPRECATED | DISASSEMBLY_RAW_INSN;
+      disasm_flags |= DISASSEMBLY_SOURCE_DEPRECATED | DISASSEMBLY_RAW_BYTES;
       break;
     case 4:
       disasm_flags |= DISASSEMBLY_SOURCE;
       break;
     case 5:
-      disasm_flags |= DISASSEMBLY_SOURCE | DISASSEMBLY_RAW_INSN;
+      disasm_flags |= DISASSEMBLY_SOURCE | DISASSEMBLY_RAW_BYTES;
       break;
     default:
       gdb_assert_not_reached ("bad disassembly mode");
diff --git a/gdb/record.c b/gdb/record.c
index 17a5df262bd..2390a58f9c0 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -494,6 +494,9 @@ get_insn_history_modifiers (const char **arg)
 	    case 'r':
 	      modifiers |= DISASSEMBLY_RAW_INSN;
 	      break;
+	    case 'b':
+	      modifiers |= DISASSEMBLY_RAW_BYTES;
+	      break;
 	    case 'f':
 	      modifiers |= DISASSEMBLY_OMIT_FNAME;
 	      break;
diff --git a/gdb/testsuite/gdb.mi/mi-disassemble.exp b/gdb/testsuite/gdb.mi/mi-disassemble.exp
index b7c52472c84..ef3337d4062 100644
--- a/gdb/testsuite/gdb.mi/mi-disassemble.exp
+++ b/gdb/testsuite/gdb.mi/mi-disassemble.exp
@@ -245,12 +245,12 @@ proc test_disassembly_opcode_format {} {
     # then disassemble using the MI command.
     set longest_insn_bytes ""
     set longest_insn_addr ""
-    gdb_test_multiple "disassemble /r main" "" {
-	-re "^disassemble /r main\r\n" {
+    gdb_test_multiple "disassemble /b main" "" {
+	-re "^disassemble /b main\r\n" {
 	    exp_continue
 	}
 
-	-re "^&\"disassemble /r main.n\"\r\n" {
+	-re "^&\"disassemble /b main.n\"\r\n" {
 	    exp_continue
 	}
 
-- 
2.25.4

[PATCH 5/9] gdb: make gdb_disassembly_flag unsigned

[Andrew Burgess]

In a later commit I want to use operator~ on a gdb_disassembly_flag
flag value.  This is currently not possible as gdb_disassembly_flag
is, by default, signed.

This commit just makes this enum unsigned.

There should be no user visible changes after this commit.
---
 gdb/disasm-flags.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/disasm-flags.h b/gdb/disasm-flags.h
index 5a7371b0a39..9420e7c478c 100644
--- a/gdb/disasm-flags.h
+++ b/gdb/disasm-flags.h
@@ -24,7 +24,7 @@
 
 /* Flags used to control how GDB's disassembler behaves.  */
 
-enum gdb_disassembly_flag
+enum gdb_disassembly_flag : unsigned
   {
     DISASSEMBLY_SOURCE_DEPRECATED = (0x1 << 0),
     DISASSEMBLY_RAW_INSN = (0x1 << 1),
-- 
2.25.4

[PATCH 6/9] gdb/doc: fix column widths in MI compatibility table

[Andrew Burgess]

In passing I noticed that the column headings for the table of MI
compatibility and breaking changes, were overlapping, at least when
the PDF is generated on my machine.

I propose giving slightly more space to the two version number
columns, this prevents the headers overlapping for me.
---
 gdb/doc/gdb.texinfo | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index af559370db0..b0624afce2f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -30308,7 +30308,7 @@
 interface: the version number, the version of GDB in which it first appeared
 and the breaking changes compared to the previous version.
 
-@multitable @columnfractions .05 .05 .9
+@multitable @columnfractions .1 .1 .8
 @headitem MI version @tab GDB version @tab Breaking changes
 
 @item
-- 
2.25.4

[PATCH 7/9] gdb/doc: update syntax of -data-disassemble command arguments

[Andrew Burgess]

The argument documentation looks like this:

   -data-disassemble
      [ -s @var{start-addr} -e @var{end-addr} ]
    | [ -a @var{addr} ]
    | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
    -- @var{mode}

However, I believe, according to the 'Notation and Terminology'
section, this means that the there are 3 optional location
specification argument groups for the command, followed by a
non-optional '-- mode'.

However, this is not true, one of the location specification must be
given, i.e. we can't choose to give NO location specification, which
is what the above implies.

I propose that we change this to instead be:

   -data-disassemble
    ( -s @var{start-addr} -e @var{end-addr}
    | -a @var{addr}
    | -f @var{filename} -l @var{linenum} [ -n @var{lines} ] )
    -- @var{mode}

By placing all the location specifications within '( ... )' we
indication that these are a group, from which one of the options,
separated by '|', must be selected.

This change is important because, in a later commit, I want to add
additional optional arguments to the -data-disassemble command, and
things start to get confusing with the original syntax.
---
 gdb/doc/gdb.texinfo | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b0624afce2f..ca9d29bd364 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34384,9 +34384,9 @@
 
 @smallexample
  -data-disassemble
-    [ -s @var{start-addr} -e @var{end-addr} ]
-  | [ -a @var{addr} ]
-  | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
+  ( -s @var{start-addr} -e @var{end-addr}
+  | -a @var{addr}
+  | -f @var{filename} -l @var{linenum} [ -n @var{lines} ] )
   -- @var{mode}
 @end smallexample
 
-- 
2.25.4

[PATCH 8/9] gdb/mi: some int to bool conversion

[Andrew Burgess]

Just some simple int to bool conversion in mi_cmd_disassemble.  There
should be no user visible changes after this commit.
---
 gdb/mi/mi-cmd-disas.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/gdb/mi/mi-cmd-disas.c b/gdb/mi/mi-cmd-disas.c
index c8e06cd940a..4f6e5613b9d 100644
--- a/gdb/mi/mi-cmd-disas.c
+++ b/gdb/mi/mi-cmd-disas.c
@@ -62,12 +62,12 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
   struct symtab *s;
 
   /* Which options have we processed ... */
-  int file_seen = 0;
-  int line_seen = 0;
-  int num_seen = 0;
-  int start_seen = 0;
-  int end_seen = 0;
-  int addr_seen = 0;
+  bool file_seen = false;
+  bool line_seen = false;
+  bool num_seen = false;
+  bool start_seen = false;
+  bool end_seen = false;
+  bool addr_seen = false;
 
   /* ... and their corresponding value. */
   char *file_string = NULL;
@@ -107,27 +107,27 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
 	{
 	case FILE_OPT:
 	  file_string = oarg;
-	  file_seen = 1;
+	  file_seen = true;
 	  break;
 	case LINE_OPT:
 	  line_num = atoi (oarg);
-	  line_seen = 1;
+	  line_seen = true;
 	  break;
 	case NUM_OPT:
 	  how_many = atoi (oarg);
-	  num_seen = 1;
+	  num_seen = true;
 	  break;
 	case START_OPT:
 	  low = parse_and_eval_address (oarg);
-	  start_seen = 1;
+	  start_seen = true;
 	  break;
 	case END_OPT:
 	  high = parse_and_eval_address (oarg);
-	  end_seen = 1;
+	  end_seen = true;
 	  break;
 	case ADDR_OPT:
 	  addr = parse_and_eval_address (oarg);
-	  addr_seen = 1;
+	  addr_seen = true;
 	  break;
 	}
     }
-- 
2.25.4

[PATCH 9/9] gdb/mi: new options for -data-disassemble command

[Andrew Burgess]

Now that the disassembler has two different strategies for laying out
the opcode bytes of an instruction (see /r vs /b for the disassemble
command), I wanted to add support for this to the MI disassemble
command.

Currently the -data-disassemble command takes a single 'mode' value,
which currently has 6 different values (0 -> 5), 3 of these modes
relate to opcode display.

So, clearly I should just add an additional 3 modes to handle the new
opcode format, right?

No, I didn't think that was a great idea either.

So, I wonder, could I adjust the -data-disassemble command in a
backward compatible way, that would allow GDB to move away from using
the mode value altogether?

I think we can.

In this commit, I propose adding two new options to -data-disassemble,
these are:

  --opcodes none|bytes|display
  --source

Additionally, I will make the mode optional, and default to mode 0 if
no mode value is given.  Mode 0 is the simplest, no source code, no
opcodes disassembly mode.

The two new options are only valid for mode 0, if they are used with
any other mode then an error is thrown.

The --opcodes option can add opcodes to the result, with 'bytes' being
equivalent to 'disassemble /b' and 'display' being 'disassemble /r'.

The --source option will enable the /s style source code display, this
is equivalent to modes 4 and 5.  There is no way, using the new
command options to get the now deprecated /m style source code
display, that is mode 1 and 3.

My hope is that new users of the MI will not use the mode at all, and
instead will just use the new options to achieve the output they need.
Existing MI users can continue to use the mode, and will not need to
be updated to use the new options.
---
 gdb/doc/gdb.texinfo                     | 83 ++++++++++++++++++-------
 gdb/mi/mi-cmd-disas.c                   | 75 +++++++++++++++++++---
 gdb/testsuite/gdb.mi/mi-disassemble.exp | 66 +++++++++++++-------
 3 files changed, 174 insertions(+), 50 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index ca9d29bd364..456d362a109 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34387,7 +34387,9 @@
   ( -s @var{start-addr} -e @var{end-addr}
   | -a @var{addr}
   | -f @var{filename} -l @var{linenum} [ -n @var{lines} ] )
-  -- @var{mode}
+  [ --opcodes @var{opcodes-mode} ]
+  [ --source ]
+  [ -- @var{mode} ]
 @end smallexample
 
 @noindent
@@ -34416,31 +34418,56 @@
 displayed; if @var{lines} is higher than the number of lines between
 @var{start-addr} and @var{end-addr}, only the lines up to @var{end-addr}
 are displayed.
+@item @var{opcodes-mode}
+is one of @samp{none}, @samp{bytes}, or @samp{display}.  This setting
+can only be used with @var{mode} 0.  Passing @samp{none} here will
+prevent opcodes being included in the result; passing @samp{bytes} here
+will include opcodes in the result, the opcodes will be formatted as for
+@kbd{disassemble /b}; and passing @samp{display} here will include the
+opcodes in the result, the opcodes will be formatted as for
+@kbd{disassemble /r}.
 @item @var{mode}
-is one of:
+the use of @var{mode} is deprecated in favour of using the
+@code{--opcodes} and @code{--source} options.  When no @var{mode} is
+given, @var{mode} 0 will be assumed.  However, the @var{mode} is still
+available for backward compatibility.  The @var{mode} should be one of:
 @itemize @bullet
-@item 0 disassembly only
-@item 1 mixed source and disassembly (deprecated)
-@item 2 disassembly with raw opcodes
-@item 3 mixed source and disassembly with raw opcodes (deprecated)
-@item 4 mixed source and disassembly
-@item 5 mixed source and disassembly with raw opcodes
+@item 0 disassembly only:
+This is the default mode if no mode is specified.
+@item 1 mixed source and disassembly (deprecated):
+It is not possible to recreate this mode using @code{--opcodes} and
+@code{--source} options.
+@item 2 disassembly with raw opcodes:
+This mode is equivalent to using @var{mode} 0 and passing
+@code{--opcodes bytes} to the command.
+@item 3 mixed source and disassembly with raw opcodes (deprecated):
+It is not possible to recreate this mode using @code{--opcodes} and
+@code{--source} options.
+@item 4 mixed source and disassembly:
+This mode is equivalent to using @var{mode} 0 and passing
+@code{--source} to the command.
+@item 5 mixed source and disassembly with raw opcodes:
+This mode is equivalent to using @var{mode} 0 and passing
+@code{--opcodes bytes} and @code{--source} to the command.
 @end itemize
-
 Modes 1 and 3 are deprecated.  The output is ``source centric''
 which hasn't proved useful in practice.
 @xref{Machine Code}, for a discussion of the difference between
 @code{/m} and @code{/s} output of the @code{disassemble} command.
 @end table
 
+The @code{--source} can only be used with @var{mode} 0.  Passing this
+option will include the source code in the disassembly result as if
+@var{mode} 4 or 5 had been used.
+
 @subsubheading Result
 
 The result of the @code{-data-disassemble} command will be a list named
-@samp{asm_insns}, the contents of this list depend on the @var{mode}
-used with the @code{-data-disassemble} command.
+@samp{asm_insns}, the contents of this list depend on the options used
+with the @code{-data-disassemble} command.
 
-For modes 0 and 2 the @samp{asm_insns} list contains tuples with the
-following fields:
+For modes 0 and 2, and when the @code{--source} option is not used the
+@samp{asm_insns} list contains tuples with the following fields:
 
 @table @code
 @item address
@@ -34456,15 +34483,27 @@
 The text disassembly for this @samp{address}.
 
 @item opcodes
-This field is only present for modes 2, 3 and 5.  This contains the
-raw opcode bytes for the @samp{inst} field.  The bytes are formatted
-as single bytes, in hex, in ascending address order, with a single
-space between each byte.
-
-@end table
-
-For modes 1, 3, 4 and 5 the @samp{asm_insns} list contains tuples named
-@samp{src_and_asm_line}, each of which has the following fields:
+This field is only present for modes 2, 3 and 5, or when the
+@code{--opcodes} option @samp{bytes} or @samp{display} is used.  This
+contains the raw opcode bytes for the @samp{inst} field.
+
+When the @samp{--opcodes} option is not passed to
+@code{-data-disassemble}, or the @samp{bytes} value is passed to
+@samp{--opcodes}, then the bytes are formatted as a series of single
+bytes, in hex, in ascending address order, with a single space between
+each byte.  This format is equivalent to the @samp{/b} option being used
+with the @kbd{disassemble} command.
+
+When @samp{--opcodes} is passed the value @samp{display} then the bytes
+are formatted in the natural instruction display order.  This means
+multiple bytes can be grouped together, and the bytes might be
+byte-swapped.  This format is equivalent to the @samp{/r} option being
+used with the @kbd{disassemble} command.
+@end table
+
+For modes 1, 3, 4 and 5, or when the @code{--source} option is used, the
+@samp{asm_insns} list contains tuples named @samp{src_and_asm_line},
+each of which has the following fields:
 
 @table @code
 @item line
diff --git a/gdb/mi/mi-cmd-disas.c b/gdb/mi/mi-cmd-disas.c
index 4f6e5613b9d..db5a78827fc 100644
--- a/gdb/mi/mi-cmd-disas.c
+++ b/gdb/mi/mi-cmd-disas.c
@@ -68,6 +68,8 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
   bool start_seen = false;
   bool end_seen = false;
   bool addr_seen = false;
+  bool opcodes_seen = false;
+  bool source_seen = false;
 
   /* ... and their corresponding value. */
   char *file_string = NULL;
@@ -77,12 +79,23 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
   CORE_ADDR high = 0;
   CORE_ADDR addr = 0;
 
+  /* Flags to handle the --opcodes option.  */
+  enum opcodes_mode
+  {
+    OPCODES_DEFAULT, OPCODES_NONE, OPCODES_DISPLAY, OPCODES_BYTES
+  };
+  enum opcodes_mode opcodes_mode = OPCODES_DEFAULT;
+
+  /* Handle the -source option.  */
+  bool show_source = false;
+
   /* Options processing stuff.  */
   int oind = 0;
   char *oarg;
   enum opt
   {
-    FILE_OPT, LINE_OPT, NUM_OPT, START_OPT, END_OPT, ADDR_OPT
+    FILE_OPT, LINE_OPT, NUM_OPT, START_OPT, END_OPT, ADDR_OPT, OPCODES_OPT,
+    SHOW_SRC_OPT
   };
   static const struct mi_opt opts[] =
     {
@@ -92,6 +105,8 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
       {"s", START_OPT, 1},
       {"e", END_OPT, 1},
       {"a", ADDR_OPT, 1},
+      {"-opcodes", OPCODES_OPT, 1},
+      {"-source", SHOW_SRC_OPT, 0},
       { 0, 0, 0 }
     };
 
@@ -129,6 +144,21 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
 	  addr = parse_and_eval_address (oarg);
 	  addr_seen = true;
 	  break;
+	case OPCODES_OPT:
+	  opcodes_seen = true;
+	  if (strcmp (oarg, "none") == 0)
+	    opcodes_mode = OPCODES_NONE;
+	  else if (strcmp (oarg, "display") == 0)
+	    opcodes_mode = OPCODES_DISPLAY;
+	  else if (strcmp (oarg, "bytes") == 0)
+	    opcodes_mode = OPCODES_BYTES;
+	  else
+	    error (_("-data-disassemble: unknown value for -opcodes argument"));
+	  break;
+	case SHOW_SRC_OPT:
+	  source_seen = true;
+	  show_source = true;
+	  break;
 	}
     }
   argv += oind;
@@ -146,13 +176,23 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
 
        || (!line_seen && !file_seen && !num_seen && !start_seen && !end_seen
 								&&  addr_seen))
-      || argc != 1)
-    error (_("-data-disassemble: Usage: ( [-f filename -l linenum "
-	     "[-n howmany]] | [-s startaddr -e endaddr] | [-a addr] ) [--] mode."));
+      || argc > 1)
+    error (_("-data-disassemble: Usage: "
+	     "( -f filename -l linenum [-n howmany] |"
+	     " -s startaddr -e endaddr | -a addr ) "
+	     "[ --opcodes mode ] [ --source ] [ [--] mode ]."));
+
+  if (argc == 1)
+    {
+      mode = atoi (argv[0]);
+      if (mode < 0 || mode > 5)
+	error (_("-data-disassemble: Mode argument must be in the range 0-5."));
+    }
+  else
+    mode = 0;
 
-  mode = atoi (argv[0]);
-  if (mode < 0 || mode > 5)
-    error (_("-data-disassemble: Mode argument must be in the range 0-5."));
+  if (mode != 0 && (source_seen || opcodes_seen))
+    error (_("-data-disassemble: --opcodes and --source can only be used with mode 0"));
 
   /* Convert the mode into a set of disassembly flags.  */
 
@@ -180,6 +220,27 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
       gdb_assert_not_reached ("bad disassembly mode");
     }
 
+  /* Now handle the (optional) --opcodes argument.  This partially
+     overrides the mode value.  */
+  if (opcodes_mode != OPCODES_DEFAULT)
+    {
+      /* Remove any existing flags related to opcodes display.  */
+      disasm_flags &= ~(DISASSEMBLY_RAW_BYTES | DISASSEMBLY_RAW_INSN);
+
+      /* Add back any required flags.  */
+      if (opcodes_mode == OPCODES_DISPLAY)
+	disasm_flags |= DISASSEMBLY_RAW_INSN;
+      else if (opcodes_mode == OPCODES_BYTES)
+	disasm_flags |= DISASSEMBLY_RAW_BYTES;
+    }
+
+  /* Handle the optional --source argument.  */
+  if (show_source)
+    {
+      disasm_flags &= ~DISASSEMBLY_SOURCE_DEPRECATED;
+      disasm_flags |= DISASSEMBLY_SOURCE;
+    }
+
   /* We must get the function beginning and end where line_num is
      contained.  */
 
diff --git a/gdb/testsuite/gdb.mi/mi-disassemble.exp b/gdb/testsuite/gdb.mi/mi-disassemble.exp
index ef3337d4062..4d7c2f2e6e7 100644
--- a/gdb/testsuite/gdb.mi/mi-disassemble.exp
+++ b/gdb/testsuite/gdb.mi/mi-disassemble.exp
@@ -47,21 +47,27 @@ proc test_disassembly_only {} {
 
 
     mi_gdb_test "print/x \$pc" "" ""
-    mi_gdb_test "111-data-disassemble -s \$pc -e \"\$pc + 12\" -- 0" \
-	    "111\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
-             "data-disassemble from pc to pc+12 assembly only"
 
-    mi_gdb_test "112-data-disassemble -a \$pc -- 0" \
-	    "112\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
-             "data-disassemble function around pc assembly only"
+    foreach { test_name option_string } [list "mode 0" "-- 0" \
+					     "default mode" "" ] {
+	with_test_prefix $test_name {
+	    mi_gdb_test "111-data-disassemble -s \$pc -e \"\$pc + 12\" ${option_string}" \
+		"111\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
+		"data-disassemble from pc to pc+12 assembly only"
 
-    mi_gdb_test "113-data-disassemble -a callee4 -- 0" \
-	    "113\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
-             "data-disassemble function callee4 assembly only"
+	    mi_gdb_test "112-data-disassemble -a \$pc ${option_string}" \
+		"112\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
+		"data-disassemble function around pc assembly only"
 
-    mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -- 0" \
-	    "222\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]" \
-              "data-disassemble file & line, assembly only"
+	    mi_gdb_test "113-data-disassemble -a callee4 ${option_string}" \
+		"113\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
+		"data-disassemble function callee4 assembly only"
+
+	    mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body ${option_string}" \
+		"222\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]" \
+		"data-disassemble file & line, assembly only"
+	}
+    }
 }
 
 proc test_disassembly_with_opcodes {} {
@@ -78,13 +84,20 @@ proc test_disassembly_with_opcodes {} {
     # -data-disassembly -f basics.c -l $line_main_body -- 2
 
     mi_gdb_test "print/x \$pc" "" ""
-    mi_gdb_test "111-data-disassemble -s \$pc -e \"\$pc + 12\" -- 2" \
-           "111\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}.*\]" \
-             "data-disassemble from pc to pc+12 assembly with opcodes"
 
-    mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -- 2" \
-           "222\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",opcodes=\".*\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]" \
-              "data-disassemble file & line, assembly with opcodes"
+    foreach { test_name option_string} [list "mode 2" "-- 2" \
+					    "mode 0 and --opcodes bytes" "--opcodes bytes -- 0" \
+					    "default mode and --opcodes bytes" "--opcodes bytes"] {
+	with_test_prefix $test_name {
+	    mi_gdb_test "111-data-disassemble -s \$pc -e \"\$pc + 12\" ${option_string}" \
+		"111\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}.*\]" \
+		"data-disassemble from pc to pc+12 assembly"
+
+	    mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body ${option_string}" \
+		"222\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",opcodes=\".*\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]" \
+		"data-disassemble file & line, assembly"
+	}
+    }
 }
 
 proc test_disassembly_lines_limit {} {
@@ -230,13 +243,24 @@ proc test_disassembly_bogus_args {} {
              "data-disassemble bogus address, -a"
 
     mi_gdb_test "456-data-disassemble -s \$pc -f basics.c -- 0" \
-             "456\\^error,msg=\"-data-disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr. \\| .-a addr. \\) .--. mode.\"" \
-             "data-disassemble mix different args"
+	"456\\^error,msg=\"-data-disassemble: Usage: \\( -f filename -l linenum .-n howmany. \\| -s startaddr -e endaddr \\| -a addr \\) . --opcodes mode . . --source . . .--. mode .\\.\"" \
+	"data-disassemble mix different args"
 
     mi_gdb_test "789-data-disassemble -f basics.c -l $line_main_body -- 9" \
              "789\\^error,msg=\"-data-disassemble: Mode argument must be in the range 0-5.\"" \
-             "data-disassemble wrong mode arg"
+	"data-disassemble wrong mode arg"
+
+    foreach mode { 1 2 3 4 5 } {
+	foreach opcode_arg { none bytes display } {
+	    mi_gdb_test "801-data-disassemble -s \$pc -e \"\$pc + 12\" --opcodes ${opcode_arg} -- ${mode}" \
+		"801\\^error,msg=\"-data-disassemble: --opcodes and --source can only be used with mode 0\"" \
+		"data-disassemble use --opcode ${opcode_arg} with mode ${mode}"
+	}
 
+	mi_gdb_test "802-data-disassemble -s \$pc -e \"\$pc + 12\" --source -- ${mode}" \
+	    "802\\^error,msg=\"-data-disassemble: --opcodes and --source can only be used with mode 0\"" \
+	    "data-disassemble use --source with mode ${mode}"
+    }
 }
 
 # Check the format of the opcode bytes.
-- 
2.25.4

Re: [PATCH 1/9] gdb/doc: improve description of --data-disassemble opcodes output

[Eli Zaretskii]

> Date: Thu, 23 Jun 2022 17:05:08 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
> Extend the description of the MI command --data-disassemble.
> Specifically, expand the description of the 'opcodes' field to explain
> how the bytes are formatted.
> ---
>  gdb/doc/gdb.texinfo | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Thanks, this is OK.

Re: [PATCH 4/9] gdb: disassembler opcode display formatting

[Eli Zaretskii]

> Date: Thu, 23 Jun 2022 17:05:11 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
>  gdb/NEWS                                | 12 +++++++
>  gdb/cli/cli-cmds.c                      |  6 ++++
>  gdb/disasm-flags.h                      |  1 +
>  gdb/disasm.c                            | 43 ++++++++++++++++++++--
>  gdb/doc/gdb.texinfo                     | 48 +++++++++++++++++++++----
>  gdb/mi/mi-cmd-disas.c                   |  6 ++--
>  gdb/record.c                            |  3 ++
>  gdb/testsuite/gdb.mi/mi-disassemble.exp |  6 ++--
>  8 files changed, 109 insertions(+), 16 deletions(-)

Thanks, the documentation parts are OK.

Re: [PATCH 7/9] gdb/doc: update syntax of -data-disassemble command arguments

[Eli Zaretskii]

> Date: Thu, 23 Jun 2022 17:05:14 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
> The argument documentation looks like this:
> 
>    -data-disassemble
>       [ -s @var{start-addr} -e @var{end-addr} ]
>     | [ -a @var{addr} ]
>     | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
>     -- @var{mode}
> 
> However, I believe, according to the 'Notation and Terminology'
> section, this means that the there are 3 optional location
> specification argument groups for the command, followed by a
> non-optional '-- mode'.
> 
> However, this is not true, one of the location specification must be
> given, i.e. we can't choose to give NO location specification, which
> is what the above implies.

I don't believe we ever used this convention this rigorously.  But I
agree that it is better to be as accurate as possible.

> I propose that we change this to instead be:
> 
>    -data-disassemble
>     ( -s @var{start-addr} -e @var{end-addr}
>     | -a @var{addr}
>     | -f @var{filename} -l @var{linenum} [ -n @var{lines} ] )
>     -- @var{mode}
> 
> By placing all the location specifications within '( ... )' we
> indication that these are a group, from which one of the options,
> separated by '|', must be selected.

According to "Notation and Terminology", the (...) construct should be
followed by either * or +, so I think you should use + here.

Otherwise, this is fine, thanks.

Re: [PATCH 6/9] gdb/doc: fix column widths in MI compatibility table

[Eli Zaretskii]

> Date: Thu, 23 Jun 2022 17:05:13 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
> In passing I noticed that the column headings for the table of MI
> compatibility and breaking changes, were overlapping, at least when
> the PDF is generated on my machine.
> 
> I propose giving slightly more space to the two version number
> columns, this prevents the headers overlapping for me.

This should fall under the "obvious change" rule.

Thanks.

Re: [PATCH 9/9] gdb/mi: new options for -data-disassemble command

[Eli Zaretskii]

> Date: Thu, 23 Jun 2022 17:05:16 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
> @@ -34416,31 +34418,56 @@
>  displayed; if @var{lines} is higher than the number of lines between
>  @var{start-addr} and @var{end-addr}, only the lines up to @var{end-addr}
>  are displayed.
> +@item @var{opcodes-mode}
> +is one of @samp{none}, @samp{bytes}, or @samp{display}.  This setting
> +can only be used with @var{mode} 0.  Passing @samp{none} here will
> +prevent opcodes being included in the result; passing @samp{bytes} here
> +will include opcodes in the result, the opcodes will be formatted as for
> +@kbd{disassemble /b}; and passing @samp{display} here will include the
> +opcodes in the result, the opcodes will be formatted as for
> +@kbd{disassemble /r}.

This begs for an inner @table or @itemize, describing each of the 3
options separately.  It would be also more correct, English-wise,
since starting an @item with a lowercase letter and then writing
several sentences there is not really correct.

>  @item @var{mode}
> -is one of:
> +the use of @var{mode} is deprecated in favour of using the
> +@code{--opcodes} and @code{--source} options.  When no @var{mode} is
> +given, @var{mode} 0 will be assumed.  However, the @var{mode} is still
> +available for backward compatibility.  The @var{mode} should be one of:
>  @itemize @bullet
> -@item 0 disassembly only
> -@item 1 mixed source and disassembly (deprecated)
> -@item 2 disassembly with raw opcodes
> -@item 3 mixed source and disassembly with raw opcodes (deprecated)
> -@item 4 mixed source and disassembly
> -@item 5 mixed source and disassembly with raw opcodes
> +@item 0 disassembly only:
> +This is the default mode if no mode is specified.

This is in @itemize, so @items should appear alone on their lines, and
the text on the line below it.  But the style in which you wrote this
is better suited fore @table, so maybe just replace @itemize by @table.

> +@item 1 mixed source and disassembly (deprecated):
> +It is not possible to recreate this mode using @code{--opcodes} and
> +@code{--source} options.

Then why are we deprecating it (and the MODE argument itself)?

> -For modes 0 and 2 the @samp{asm_insns} list contains tuples with the
> -following fields:
> +For modes 0 and 2, and when the @code{--source} option is not used the
                                                                     ^^
Comma missing there.

> +When the @samp{--opcodes} option is not passed to
> +@code{-data-disassemble}, or the @samp{bytes} value is passed to
> +@samp{--opcodes}, then the bytes are formatted as a series of single
> +bytes, in hex, in ascending address order, with a single space between
> +each byte.  This format is equivalent to the @samp{/b} option being used
> +with the @kbd{disassemble} command.

A cross-reference to the description of 'disassemble' would be good
here.

Re: [PATCH 1/9] gdb/doc: improve description of --data-disassemble opcodes output

[Andrew Burgess]

Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Date: Thu, 23 Jun 2022 17:05:08 +0100
>> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Andrew Burgess <aburgess@redhat.com>
>> 
>> Extend the description of the MI command --data-disassemble.
>> Specifically, expand the description of the 'opcodes' field to explain
>> how the bytes are formatted.
>> ---
>>  gdb/doc/gdb.texinfo | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> Thanks, this is OK.

Thanks, I pushed this patch.

Andrew

Re: [PATCH 6/9] gdb/doc: fix column widths in MI compatibility table

[Andrew Burgess]

Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Date: Thu, 23 Jun 2022 17:05:13 +0100
>> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Andrew Burgess <aburgess@redhat.com>
>> 
>> In passing I noticed that the column headings for the table of MI
>> compatibility and breaking changes, were overlapping, at least when
>> the PDF is generated on my machine.
>> 
>> I propose giving slightly more space to the two version number
>> columns, this prevents the headers overlapping for me.
>
> This should fall under the "obvious change" rule.

Thanks, pushed.

Andrew

Re: [PATCH 7/9] gdb/doc: update syntax of -data-disassemble command arguments

[Andrew Burgess]

Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Date: Thu, 23 Jun 2022 17:05:14 +0100
>> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Andrew Burgess <aburgess@redhat.com>
>> 
>> The argument documentation looks like this:
>> 
>>    -data-disassemble
>>       [ -s @var{start-addr} -e @var{end-addr} ]
>>     | [ -a @var{addr} ]
>>     | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
>>     -- @var{mode}
>> 
>> However, I believe, according to the 'Notation and Terminology'
>> section, this means that the there are 3 optional location
>> specification argument groups for the command, followed by a
>> non-optional '-- mode'.
>> 
>> However, this is not true, one of the location specification must be
>> given, i.e. we can't choose to give NO location specification, which
>> is what the above implies.
>
> I don't believe we ever used this convention this rigorously.  But I
> agree that it is better to be as accurate as possible.

I agree, and I don't plan to go out of my way to "fix" the syntax of
every command, it was just that with my next patch the syntax became
rather muddled unless we were a little more rigorous.

>
>> I propose that we change this to instead be:
>> 
>>    -data-disassemble
>>     ( -s @var{start-addr} -e @var{end-addr}
>>     | -a @var{addr}
>>     | -f @var{filename} -l @var{linenum} [ -n @var{lines} ] )
>>     -- @var{mode}
>> 
>> By placing all the location specifications within '( ... )' we
>> indication that these are a group, from which one of the options,
>> separated by '|', must be selected.
>
> According to "Notation and Terminology", the (...) construct should be
> followed by either * or +, so I think you should use + here.

You are correct that (...) is not mentioned.  Unfortunately (...)+
clearly means one or more times, and this is not correct in this case.
I propose adding (...) to the "Notation and Terminology" section to mean
exactly once.

How's the patch below?

Thanks,
Andrew

---

commit e151b49f5adac4f42bc5df977c5ab646ef7fe83c
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Jun 23 13:57:57 2022 +0100

    gdb/doc: update syntax of -data-disassemble command arguments
    
    The argument documentation for -data-disassemble looks like this:
    
       -data-disassemble
          [ -s @var{start-addr} -e @var{end-addr} ]
        | [ -a @var{addr} ]
        | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
        -- @var{mode}
    
    However, I believe, according to the 'Notation and Terminology'
    section, this means that the there are 3 optional location
    specification argument groups for the command, followed by a
    non-optional '-- mode'.
    
    However, this is not true, one of the location specifications must be
    given, i.e. we can't choose to give NO location specification, which
    is what the above implies.
    
    I propose that we change this to instead be:
    
       -data-disassemble
        ( -s @var{start-addr} -e @var{end-addr}
        | -a @var{addr}
        | -f @var{filename} -l @var{linenum} [ -n @var{lines} ] )
        -- @var{mode}
    
    By placing all the location specifications within '( ... )' we
    indication that these are a group, from which one of the options,
    separated by '|', must be selected.
    
    However, the 'Notation and Terminology' section only describes two
    uses for parenthesis: '( GROUP )*' and '( GROUP )+', in the first case
    GROUP is repeated zero or more times, and in the second GROUP is
    repeated 1 or more times.
    
    Neither of those exactly describe what I want, which is GROUP must
    appear exactly once.  I propose to extend 'Notation and Terminology'
    to include '( GROUP )' which means that GROUP should appear exactly
    once.
    
    This change is important because, in a later commit, I want to add
    additional optional arguments to the -data-disassemble command, and
    things start to get confusing with the original syntax.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9ae88ee03df..984ad9c1ed1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -29766,6 +29766,10 @@
 @code{( @var{group} )+} means that @var{group} inside the parentheses
 may repeat one or more times.
 
+@item
+@code{( @var{group} )} means that @var{group} inside the parentheses
+occurs exactly once.
+
 @item
 @code{"@var{string}"} means a literal @var{string}.
 @end itemize
@@ -34416,9 +34420,9 @@
 
 @smallexample
  -data-disassemble
-    [ -s @var{start-addr} -e @var{end-addr} ]
-  | [ -a @var{addr} ]
-  | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
+  ( -s @var{start-addr} -e @var{end-addr}
+  | -a @var{addr}
+  | -f @var{filename} -l @var{linenum} [ -n @var{lines} ] )
   -- @var{mode}
 @end smallexample
 

Re: [PATCH 7/9] gdb/doc: update syntax of -data-disassemble command arguments

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Thu, 30 Jun 2022 11:18:35 +0100
> 
> > According to "Notation and Terminology", the (...) construct should be
> > followed by either * or +, so I think you should use + here.
> 
> You are correct that (...) is not mentioned.  Unfortunately (...)+
> clearly means one or more times, and this is not correct in this case.
> I propose adding (...) to the "Notation and Terminology" section to mean
> exactly once.
> 
> How's the patch below?

LGTM, thanks.

Re: [PATCH 9/9] gdb/mi: new options for -data-disassemble command

[Andrew Burgess]

Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Date: Thu, 23 Jun 2022 17:05:16 +0100
>> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Andrew Burgess <aburgess@redhat.com>
>> 
>> @@ -34416,31 +34418,56 @@
>>  displayed; if @var{lines} is higher than the number of lines between
>>  @var{start-addr} and @var{end-addr}, only the lines up to @var{end-addr}
>>  are displayed.
>> +@item @var{opcodes-mode}
>> +is one of @samp{none}, @samp{bytes}, or @samp{display}.  This setting
>> +can only be used with @var{mode} 0.  Passing @samp{none} here will
>> +prevent opcodes being included in the result; passing @samp{bytes} here
>> +will include opcodes in the result, the opcodes will be formatted as for
>> +@kbd{disassemble /b}; and passing @samp{display} here will include the
>> +opcodes in the result, the opcodes will be formatted as for
>> +@kbd{disassemble /r}.
>
> This begs for an inner @table or @itemize, describing each of the 3
> options separately.  It would be also more correct, English-wise,
> since starting an @item with a lowercase letter and then writing
> several sentences there is not really correct.
>
>>  @item @var{mode}
>> -is one of:
>> +the use of @var{mode} is deprecated in favour of using the
>> +@code{--opcodes} and @code{--source} options.  When no @var{mode} is
>> +given, @var{mode} 0 will be assumed.  However, the @var{mode} is still
>> +available for backward compatibility.  The @var{mode} should be one of:
>>  @itemize @bullet
>> -@item 0 disassembly only
>> -@item 1 mixed source and disassembly (deprecated)
>> -@item 2 disassembly with raw opcodes
>> -@item 3 mixed source and disassembly with raw opcodes (deprecated)
>> -@item 4 mixed source and disassembly
>> -@item 5 mixed source and disassembly with raw opcodes
>> +@item 0 disassembly only:
>> +This is the default mode if no mode is specified.
>
> This is in @itemize, so @items should appear alone on their lines, and
> the text on the line below it.  But the style in which you wrote this
> is better suited fore @table, so maybe just replace @itemize by @table.
>
>> +@item 1 mixed source and disassembly (deprecated):
>> +It is not possible to recreate this mode using @code{--opcodes} and
>> +@code{--source} options.
>
> Then why are we deprecating it (and the MODE argument itself)?

Mode 1 and 3 have been deprecated for a while now, and were marked as
suched before this patch.  The only change I'm making is to also
deprecate the mode value.

The problem with the mode is that is just doesn't scale very well, we
already have 6 modes, and if I were to add my new options this would
take us to 9 modes.

Instead, by adding some new (optional) command flags, the user can now
access all the old (non-deprecated) modes, without having to specify a
mode value.

Note that, as part of this patch, there is some new behaviour that is
only available when using the command flags (i.e. '--opcodes display'),
there's no mode value that gives access to this functionality.

My hope would be that, if we ever add more functionality to the
-data-disassemble command, this would be added as a command flag rather
than a mode value.  Over time MI users will be motivated to switch to
the command flag system rather than the mode value system in order to
access the new features.

Obviously, I've left the mode value system in place so all existing MI
users will continue to function just as they did before.

Hopefully that answers your question.

>
>> -For modes 0 and 2 the @samp{asm_insns} list contains tuples with the
>> -following fields:
>> +For modes 0 and 2, and when the @code{--source} option is not used the
>                                                                      ^^
> Comma missing there.
>
>> +When the @samp{--opcodes} option is not passed to
>> +@code{-data-disassemble}, or the @samp{bytes} value is passed to
>> +@samp{--opcodes}, then the bytes are formatted as a series of single
>> +bytes, in hex, in ascending address order, with a single space between
>> +each byte.  This format is equivalent to the @samp{/b} option being used
>> +with the @kbd{disassemble} command.
>
> A cross-reference to the description of 'disassemble' would be good
> here.

Thanks for your feedback.  I've updated the docs to (I hope) address all
the points you raised, please let me know what you think.

Thanks,
Andrew

---

commit f7175f62ae260b1b2e7b7fb3a89208582a4540b7
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Jun 23 14:49:55 2022 +0100

    gdb/mi: new options for -data-disassemble command
    
    Now that the disassembler has two different strategies for laying out
    the opcode bytes of an instruction (see /r vs /b for the disassemble
    command), I wanted to add support for this to the MI disassemble
    command.
    
    Currently the -data-disassemble command takes a single 'mode' value,
    which currently has 6 different values (0 -> 5), 3 of these modes
    relate to opcode display.
    
    So, clearly I should just add an additional 3 modes to handle the new
    opcode format, right?
    
    No, I didn't think that was a great idea either.
    
    So, I wonder, could I adjust the -data-disassemble command in a
    backward compatible way, that would allow GDB to move away from using
    the mode value altogether?
    
    I think we can.
    
    In this commit, I propose adding two new options to -data-disassemble,
    these are:
    
      --opcodes none|bytes|display
      --source
    
    Additionally, I will make the mode optional, and default to mode 0 if
    no mode value is given.  Mode 0 is the simplest, no source code, no
    opcodes disassembly mode.
    
    The two new options are only valid for mode 0, if they are used with
    any other mode then an error is thrown.
    
    The --opcodes option can add opcodes to the result, with 'bytes' being
    equivalent to 'disassemble /b' and 'display' being 'disassemble /r'.
    
    The --source option will enable the /s style source code display, this
    is equivalent to modes 4 and 5.  There is no way, using the new
    command options to get the now deprecated /m style source code
    display, that is mode 1 and 3.
    
    My hope is that new users of the MI will not use the mode at all, and
    instead will just use the new options to achieve the output they need.
    Existing MI users can continue to use the mode, and will not need to
    be updated to use the new options.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 984ad9c1ed1..9d2cedd8b8b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34423,7 +34423,9 @@
   ( -s @var{start-addr} -e @var{end-addr}
   | -a @var{addr}
   | -f @var{filename} -l @var{linenum} [ -n @var{lines} ] )
-  -- @var{mode}
+  [ --opcodes @var{opcodes-mode} ]
+  [ --source ]
+  [ -- @var{mode} ]
 @end smallexample
 
 @noindent
@@ -34452,31 +34454,71 @@
 displayed; if @var{lines} is higher than the number of lines between
 @var{start-addr} and @var{end-addr}, only the lines up to @var{end-addr}
 are displayed.
+@item @var{opcodes-mode}
+can only be used with @var{mode} 0, and should be one of the following:
+@table @samp
+@item none
+no opcode information will be included in the result.
+
+@item bytes
+opcodes will be included in the result, the opcodes will be formatted
+as for @kbd{disassemble /b}.
+
+@item display
+opcodes will be included in the result, the opcodes will be formatted
+as for @kbd{disassemble /r}.
+@end table
 @item @var{mode}
-is one of:
-@itemize @bullet
-@item 0 disassembly only
-@item 1 mixed source and disassembly (deprecated)
-@item 2 disassembly with raw opcodes
-@item 3 mixed source and disassembly with raw opcodes (deprecated)
-@item 4 mixed source and disassembly
-@item 5 mixed source and disassembly with raw opcodes
-@end itemize
+the use of @var{mode} is deprecated in favour of using the
+@code{--opcodes} and @code{--source} options.  When no @var{mode} is
+given, @var{mode} 0 will be assumed.  However, the @var{mode} is still
+available for backward compatibility.  The @var{mode} should be one of:
+@table @samp
+@item 0
+@emph{disassembly only}, this is the default mode if no mode is
+specified.
+
+@item 1
+@emph{mixed source and disassembly (deprecated)}, it is not possible
+to recreate this mode using @code{--opcodes} and @code{--source}
+options.
+
+@item 2
+@emph{disassembly with raw opcodes}, this mode is equivalent to using
+@var{mode} 0 and passing @code{--opcodes bytes} to the command.
+
+@item 3
+@emph{mixed source and disassembly with raw opcodes (deprecated)}, it
+is not possible to recreate this mode using @code{--opcodes} and
+@code{--source} options.
+
+@item 4
+@emph{mixed source and disassembly}, this mode is equivalent to using
+@var{mode} 0 and passing @code{--source} to the command.
 
+@item 5
+@emph{mixed source and disassembly with raw opcodes}, this mode is
+equivalent to using @var{mode} 0 and passing @code{--opcodes bytes}
+and @code{--source} to the command.
+@end table
 Modes 1 and 3 are deprecated.  The output is ``source centric''
 which hasn't proved useful in practice.
 @xref{Machine Code}, for a discussion of the difference between
 @code{/m} and @code{/s} output of the @code{disassemble} command.
 @end table
 
+The @code{--source} can only be used with @var{mode} 0.  Passing this
+option will include the source code in the disassembly result as if
+@var{mode} 4 or 5 had been used.
+
 @subsubheading Result
 
 The result of the @code{-data-disassemble} command will be a list named
-@samp{asm_insns}, the contents of this list depend on the @var{mode}
-used with the @code{-data-disassemble} command.
+@samp{asm_insns}, the contents of this list depend on the options used
+with the @code{-data-disassemble} command.
 
-For modes 0 and 2 the @samp{asm_insns} list contains tuples with the
-following fields:
+For modes 0 and 2, and when the @code{--source} option is not used, the
+@samp{asm_insns} list contains tuples with the following fields:
 
 @table @code
 @item address
@@ -34492,15 +34534,28 @@
 The text disassembly for this @samp{address}.
 
 @item opcodes
-This field is only present for modes 2, 3 and 5.  This contains the
-raw opcode bytes for the @samp{inst} field.  The bytes are formatted
-as single bytes, in hex, in ascending address order, with a single
-space between each byte.
+This field is only present for modes 2, 3 and 5, or when the
+@code{--opcodes} option @samp{bytes} or @samp{display} is used.  This
+contains the raw opcode bytes for the @samp{inst} field.
+
+When the @samp{--opcodes} option is not passed to
+@code{-data-disassemble}, or the @samp{bytes} value is passed to
+@samp{--opcodes}, then the bytes are formatted as a series of single
+bytes, in hex, in ascending address order, with a single space between
+each byte.  This format is equivalent to the @samp{/b} option being
+used with the @kbd{disassemble} command
+(@pxref{disassemble,,@kbd{disassemble}}).
 
+When @samp{--opcodes} is passed the value @samp{display} then the bytes
+are formatted in the natural instruction display order.  This means
+multiple bytes can be grouped together, and the bytes might be
+byte-swapped.  This format is equivalent to the @samp{/r} option being
+used with the @kbd{disassemble} command.
 @end table
 
-For modes 1, 3, 4 and 5 the @samp{asm_insns} list contains tuples named
-@samp{src_and_asm_line}, each of which has the following fields:
+For modes 1, 3, 4 and 5, or when the @code{--source} option is used, the
+@samp{asm_insns} list contains tuples named @samp{src_and_asm_line},
+each of which has the following fields:
 
 @table @code
 @item line
diff --git a/gdb/mi/mi-cmd-disas.c b/gdb/mi/mi-cmd-disas.c
index 4f6e5613b9d..db5a78827fc 100644
--- a/gdb/mi/mi-cmd-disas.c
+++ b/gdb/mi/mi-cmd-disas.c
@@ -68,6 +68,8 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
   bool start_seen = false;
   bool end_seen = false;
   bool addr_seen = false;
+  bool opcodes_seen = false;
+  bool source_seen = false;
 
   /* ... and their corresponding value. */
   char *file_string = NULL;
@@ -77,12 +79,23 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
   CORE_ADDR high = 0;
   CORE_ADDR addr = 0;
 
+  /* Flags to handle the --opcodes option.  */
+  enum opcodes_mode
+  {
+    OPCODES_DEFAULT, OPCODES_NONE, OPCODES_DISPLAY, OPCODES_BYTES
+  };
+  enum opcodes_mode opcodes_mode = OPCODES_DEFAULT;
+
+  /* Handle the -source option.  */
+  bool show_source = false;
+
   /* Options processing stuff.  */
   int oind = 0;
   char *oarg;
   enum opt
   {
-    FILE_OPT, LINE_OPT, NUM_OPT, START_OPT, END_OPT, ADDR_OPT
+    FILE_OPT, LINE_OPT, NUM_OPT, START_OPT, END_OPT, ADDR_OPT, OPCODES_OPT,
+    SHOW_SRC_OPT
   };
   static const struct mi_opt opts[] =
     {
@@ -92,6 +105,8 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
       {"s", START_OPT, 1},
       {"e", END_OPT, 1},
       {"a", ADDR_OPT, 1},
+      {"-opcodes", OPCODES_OPT, 1},
+      {"-source", SHOW_SRC_OPT, 0},
       { 0, 0, 0 }
     };
 
@@ -129,6 +144,21 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
 	  addr = parse_and_eval_address (oarg);
 	  addr_seen = true;
 	  break;
+	case OPCODES_OPT:
+	  opcodes_seen = true;
+	  if (strcmp (oarg, "none") == 0)
+	    opcodes_mode = OPCODES_NONE;
+	  else if (strcmp (oarg, "display") == 0)
+	    opcodes_mode = OPCODES_DISPLAY;
+	  else if (strcmp (oarg, "bytes") == 0)
+	    opcodes_mode = OPCODES_BYTES;
+	  else
+	    error (_("-data-disassemble: unknown value for -opcodes argument"));
+	  break;
+	case SHOW_SRC_OPT:
+	  source_seen = true;
+	  show_source = true;
+	  break;
 	}
     }
   argv += oind;
@@ -146,13 +176,23 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
 
        || (!line_seen && !file_seen && !num_seen && !start_seen && !end_seen
 								&&  addr_seen))
-      || argc != 1)
-    error (_("-data-disassemble: Usage: ( [-f filename -l linenum "
-	     "[-n howmany]] | [-s startaddr -e endaddr] | [-a addr] ) [--] mode."));
+      || argc > 1)
+    error (_("-data-disassemble: Usage: "
+	     "( -f filename -l linenum [-n howmany] |"
+	     " -s startaddr -e endaddr | -a addr ) "
+	     "[ --opcodes mode ] [ --source ] [ [--] mode ]."));
+
+  if (argc == 1)
+    {
+      mode = atoi (argv[0]);
+      if (mode < 0 || mode > 5)
+	error (_("-data-disassemble: Mode argument must be in the range 0-5."));
+    }
+  else
+    mode = 0;
 
-  mode = atoi (argv[0]);
-  if (mode < 0 || mode > 5)
-    error (_("-data-disassemble: Mode argument must be in the range 0-5."));
+  if (mode != 0 && (source_seen || opcodes_seen))
+    error (_("-data-disassemble: --opcodes and --source can only be used with mode 0"));
 
   /* Convert the mode into a set of disassembly flags.  */
 
@@ -180,6 +220,27 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
       gdb_assert_not_reached ("bad disassembly mode");
     }
 
+  /* Now handle the (optional) --opcodes argument.  This partially
+     overrides the mode value.  */
+  if (opcodes_mode != OPCODES_DEFAULT)
+    {
+      /* Remove any existing flags related to opcodes display.  */
+      disasm_flags &= ~(DISASSEMBLY_RAW_BYTES | DISASSEMBLY_RAW_INSN);
+
+      /* Add back any required flags.  */
+      if (opcodes_mode == OPCODES_DISPLAY)
+	disasm_flags |= DISASSEMBLY_RAW_INSN;
+      else if (opcodes_mode == OPCODES_BYTES)
+	disasm_flags |= DISASSEMBLY_RAW_BYTES;
+    }
+
+  /* Handle the optional --source argument.  */
+  if (show_source)
+    {
+      disasm_flags &= ~DISASSEMBLY_SOURCE_DEPRECATED;
+      disasm_flags |= DISASSEMBLY_SOURCE;
+    }
+
   /* We must get the function beginning and end where line_num is
      contained.  */
 
diff --git a/gdb/testsuite/gdb.mi/mi-disassemble.exp b/gdb/testsuite/gdb.mi/mi-disassemble.exp
index ef3337d4062..4d7c2f2e6e7 100644
--- a/gdb/testsuite/gdb.mi/mi-disassemble.exp
+++ b/gdb/testsuite/gdb.mi/mi-disassemble.exp
@@ -47,21 +47,27 @@ proc test_disassembly_only {} {
 
 
     mi_gdb_test "print/x \$pc" "" ""
-    mi_gdb_test "111-data-disassemble -s \$pc -e \"\$pc + 12\" -- 0" \
-	    "111\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
-             "data-disassemble from pc to pc+12 assembly only"
 
-    mi_gdb_test "112-data-disassemble -a \$pc -- 0" \
-	    "112\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
-             "data-disassemble function around pc assembly only"
+    foreach { test_name option_string } [list "mode 0" "-- 0" \
+					     "default mode" "" ] {
+	with_test_prefix $test_name {
+	    mi_gdb_test "111-data-disassemble -s \$pc -e \"\$pc + 12\" ${option_string}" \
+		"111\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
+		"data-disassemble from pc to pc+12 assembly only"
 
-    mi_gdb_test "113-data-disassemble -a callee4 -- 0" \
-	    "113\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
-             "data-disassemble function callee4 assembly only"
+	    mi_gdb_test "112-data-disassemble -a \$pc ${option_string}" \
+		"112\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
+		"data-disassemble function around pc assembly only"
 
-    mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -- 0" \
-	    "222\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]" \
-              "data-disassemble file & line, assembly only"
+	    mi_gdb_test "113-data-disassemble -a callee4 ${option_string}" \
+		"113\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
+		"data-disassemble function callee4 assembly only"
+
+	    mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body ${option_string}" \
+		"222\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]" \
+		"data-disassemble file & line, assembly only"
+	}
+    }
 }
 
 proc test_disassembly_with_opcodes {} {
@@ -78,13 +84,20 @@ proc test_disassembly_with_opcodes {} {
     # -data-disassembly -f basics.c -l $line_main_body -- 2
 
     mi_gdb_test "print/x \$pc" "" ""
-    mi_gdb_test "111-data-disassemble -s \$pc -e \"\$pc + 12\" -- 2" \
-           "111\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}.*\]" \
-             "data-disassemble from pc to pc+12 assembly with opcodes"
 
-    mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -- 2" \
-           "222\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",opcodes=\".*\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]" \
-              "data-disassemble file & line, assembly with opcodes"
+    foreach { test_name option_string} [list "mode 2" "-- 2" \
+					    "mode 0 and --opcodes bytes" "--opcodes bytes -- 0" \
+					    "default mode and --opcodes bytes" "--opcodes bytes"] {
+	with_test_prefix $test_name {
+	    mi_gdb_test "111-data-disassemble -s \$pc -e \"\$pc + 12\" ${option_string}" \
+		"111\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}.*\]" \
+		"data-disassemble from pc to pc+12 assembly"
+
+	    mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body ${option_string}" \
+		"222\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",opcodes=\".*\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]" \
+		"data-disassemble file & line, assembly"
+	}
+    }
 }
 
 proc test_disassembly_lines_limit {} {
@@ -230,13 +243,24 @@ proc test_disassembly_bogus_args {} {
              "data-disassemble bogus address, -a"
 
     mi_gdb_test "456-data-disassemble -s \$pc -f basics.c -- 0" \
-             "456\\^error,msg=\"-data-disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr. \\| .-a addr. \\) .--. mode.\"" \
-             "data-disassemble mix different args"
+	"456\\^error,msg=\"-data-disassemble: Usage: \\( -f filename -l linenum .-n howmany. \\| -s startaddr -e endaddr \\| -a addr \\) . --opcodes mode . . --source . . .--. mode .\\.\"" \
+	"data-disassemble mix different args"
 
     mi_gdb_test "789-data-disassemble -f basics.c -l $line_main_body -- 9" \
              "789\\^error,msg=\"-data-disassemble: Mode argument must be in the range 0-5.\"" \
-             "data-disassemble wrong mode arg"
+	"data-disassemble wrong mode arg"
+
+    foreach mode { 1 2 3 4 5 } {
+	foreach opcode_arg { none bytes display } {
+	    mi_gdb_test "801-data-disassemble -s \$pc -e \"\$pc + 12\" --opcodes ${opcode_arg} -- ${mode}" \
+		"801\\^error,msg=\"-data-disassemble: --opcodes and --source can only be used with mode 0\"" \
+		"data-disassemble use --opcode ${opcode_arg} with mode ${mode}"
+	}
 
+	mi_gdb_test "802-data-disassemble -s \$pc -e \"\$pc + 12\" --source -- ${mode}" \
+	    "802\\^error,msg=\"-data-disassemble: --opcodes and --source can only be used with mode 0\"" \
+	    "data-disassemble use --source with mode ${mode}"
+    }
 }
 
 # Check the format of the opcode bytes.

Re: [PATCH 9/9] gdb/mi: new options for -data-disassemble command

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Thu, 30 Jun 2022 12:22:28 +0100
> 
> Thanks for your feedback.  I've updated the docs to (I hope) address all
> the points you raised, please let me know what you think.

Thanks, the documentation part looks good.

Re: [PATCH 0/9] Disassembler opcode display and text alignment

[Andrew Burgess]

Ping!

Thanks,
Andrew

Andrew Burgess <aburgess@redhat.com> writes:

> This series makes two related changes to GDB's disassembler.  Both
> changes flow naturally from having GDB make use of libopcodes
> bytes_per_line and bytes_per_chunk presentation hints that are set on
> a per-architecure basis with each call into the disassembler.
>
> The end result of this change is that GDB will now display instruction
> opcodes in the same way that objdump does.
>
> For x86-64 there's no change in the way that opcode bytes are
> presented.  Now due to some special case, it's just that how GDB lays
> out instruction opcodes just happens to match how libopcodes requests
> that the opcodes be laid out.
>
> For other architectures, risc-v, powerpc, arm, aarch64, etc, this is
> not the case, and I (personally) think objdump does a better job of
> presenting the information that GDB does; the instruction opcodes will
> be grouped together based on the instruction size, and potentially
> byte-swapped so they appear in the instruction's natural order.
>
> Making use of the display hints also allows for better alignment of
> the disassembly text when opcodes are being printed.
>
> I have proposed that this new behaviour become the default for
> 'disassemble /r', and I've added a new flag 'disassemble /b' which
> allows the user to access the old behaviour.
>
> But, if people feel strongly that the old 'disassemble /r' should not
> be changed, I can place the new behaviour under a new flag.
>
> Let me know your thoughts,
>
> Thanks,
> Andrew
>
> ---
>
> Andrew Burgess (9):
>   gdb/doc: improve description of --data-disassemble opcodes output
>   gdb/testsuite: new test for -data-disassemble opcodes format
>   gdb/disasm: read opcodes bytes with a single read_code call
>   gdb: disassembler opcode display formatting
>   gdb: make gdb_disassembly_flag unsigned
>   gdb/doc: fix column widths in MI compatibility table
>   gdb/doc: update syntax of -data-disassemble command arguments
>   gdb/mi: some int to bool conversion
>   gdb/mi: new options for -data-disassemble command
>
>  gdb/NEWS                                |  12 ++
>  gdb/cli/cli-cmds.c                      |   6 +
>  gdb/disasm-flags.h                      |   3 +-
>  gdb/disasm.c                            |  55 +++++++--
>  gdb/disasm.h                            |   3 +
>  gdb/doc/gdb.texinfo                     | 137 ++++++++++++++++-----
>  gdb/mi/mi-cmd-disas.c                   | 105 ++++++++++++----
>  gdb/record.c                            |   3 +
>  gdb/testsuite/gdb.mi/mi-disassemble.exp | 153 ++++++++++++++++++++----
>  gdb/testsuite/lib/mi-support.exp        |  27 +++++
>  10 files changed, 417 insertions(+), 87 deletions(-)
>
> -- 
> 2.25.4

Re: [PATCH 0/9] Disassembler opcode display and text alignment

[Andrew Burgess]

Ping!

Thanks,
Andrew

Andrew Burgess <aburgess@redhat.com> writes:

> Ping!
>
> Thanks,
> Andrew
>
> Andrew Burgess <aburgess@redhat.com> writes:
>
>> This series makes two related changes to GDB's disassembler.  Both
>> changes flow naturally from having GDB make use of libopcodes
>> bytes_per_line and bytes_per_chunk presentation hints that are set on
>> a per-architecure basis with each call into the disassembler.
>>
>> The end result of this change is that GDB will now display instruction
>> opcodes in the same way that objdump does.
>>
>> For x86-64 there's no change in the way that opcode bytes are
>> presented.  Now due to some special case, it's just that how GDB lays
>> out instruction opcodes just happens to match how libopcodes requests
>> that the opcodes be laid out.
>>
>> For other architectures, risc-v, powerpc, arm, aarch64, etc, this is
>> not the case, and I (personally) think objdump does a better job of
>> presenting the information that GDB does; the instruction opcodes will
>> be grouped together based on the instruction size, and potentially
>> byte-swapped so they appear in the instruction's natural order.
>>
>> Making use of the display hints also allows for better alignment of
>> the disassembly text when opcodes are being printed.
>>
>> I have proposed that this new behaviour become the default for
>> 'disassemble /r', and I've added a new flag 'disassemble /b' which
>> allows the user to access the old behaviour.
>>
>> But, if people feel strongly that the old 'disassemble /r' should not
>> be changed, I can place the new behaviour under a new flag.
>>
>> Let me know your thoughts,
>>
>> Thanks,
>> Andrew
>>
>> ---
>>
>> Andrew Burgess (9):
>>   gdb/doc: improve description of --data-disassemble opcodes output
>>   gdb/testsuite: new test for -data-disassemble opcodes format
>>   gdb/disasm: read opcodes bytes with a single read_code call
>>   gdb: disassembler opcode display formatting
>>   gdb: make gdb_disassembly_flag unsigned
>>   gdb/doc: fix column widths in MI compatibility table
>>   gdb/doc: update syntax of -data-disassemble command arguments
>>   gdb/mi: some int to bool conversion
>>   gdb/mi: new options for -data-disassemble command
>>
>>  gdb/NEWS                                |  12 ++
>>  gdb/cli/cli-cmds.c                      |   6 +
>>  gdb/disasm-flags.h                      |   3 +-
>>  gdb/disasm.c                            |  55 +++++++--
>>  gdb/disasm.h                            |   3 +
>>  gdb/doc/gdb.texinfo                     | 137 ++++++++++++++++-----
>>  gdb/mi/mi-cmd-disas.c                   | 105 ++++++++++++----
>>  gdb/record.c                            |   3 +
>>  gdb/testsuite/gdb.mi/mi-disassemble.exp | 153 ++++++++++++++++++++----
>>  gdb/testsuite/lib/mi-support.exp        |  27 +++++
>>  10 files changed, 417 insertions(+), 87 deletions(-)
>>
>> -- 
>> 2.25.4

Re: [PATCH 0/9] Disassembler opcode display and text alignment

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> This series makes two related changes to GDB's disassembler.  Both
Andrew> changes flow naturally from having GDB make use of libopcodes
Andrew> bytes_per_line and bytes_per_chunk presentation hints that are set on
Andrew> a per-architecure basis with each call into the disassembler.

I read through this series and it all looked fine to me.
Thank you for doing this.

Tom

Re: [PATCH 0/9] Disassembler opcode display and text alignment

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> This series makes two related changes to GDB's disassembler.  Both
> Andrew> changes flow naturally from having GDB make use of libopcodes
> Andrew> bytes_per_line and bytes_per_chunk presentation hints that are set on
> Andrew> a per-architecure basis with each call into the disassembler.
>
> I read through this series and it all looked fine to me.
> Thank you for doing this.

Thanks, pushed.

Andrew