[PATCH v7 0/3] disass: Add /x modifier

[PATCH v7 0/3] disass: Add /x modifier

[fam]

From: Fam Zheng <famzheng@amazon.com>

v7: Fix texinfo node name in ChangeLog. [Eli]

v6: Add texinfo section name in ChangeLog. [Eli]

v5: Add function names in ChangeLog. [Eli]

v4: Add ChangeLog entries. [Eli]

v3: Add a test, more doc, and drop tmp buffer. [Andreas, Simon, Pedro]

This is a new modifier for disass command, to print offsets in hex. This
reduces pain for low level debugging (Linux kernel, Xen, etc.) when the
backtraces from dmesg are in hex - no manual converting needed to do a cross
matching when looking at both sides at the same time.

Fam Zheng (3):
  disass: Add /x modifier to print offsets in hex
  disass: Add test for /x mode
  disass: Add texinfo doc for the /x modifier

 gdb/NEWS                                |  3 ++
 gdb/cli/cli-cmds.c                      | 17 +++++++----
 gdb/disasm.c                            |  5 +++-
 gdb/disasm.h                            |  1 +
 gdb/doc/gdb.texinfo                     | 38 +++++++++++++++++++------
 gdb/record.c                            |  3 ++
 gdb/testsuite/gdb.base/disasm-optim.exp |  7 +++++
 7 files changed, 59 insertions(+), 15 deletions(-)

-- 
2.25.1

[PATCH v7 1/3] disass: Add /x modifier to print offsets in hex

[fam]

From: Fam Zheng <famzheng@amazon.com>

Backtrace messages printed by Linux kernel and Xen have hex offsets,
e.g.:

(XEN) Xen call trace:
(XEN)    [<ffff82d0402eefbb>] R guest_walk_tables_2_levels+0x189/0x66d
(XEN)    [<ffff82d0402edbbd>] F hap_p2m_ga_to_gfn_2_levels+0x112/0x25b
(XEN)    [<ffff82d0402edd22>] F hap_gva_to_gfn_2_levels+0x1c/0x1e
(XEN)    [<ffff82d0402f832e>] F paging_gva_to_gfn+0x14a/0x167

Having this modifier saves converting between hex values from the
backtrace log and offsets in gdb disass output.

gdb/ChangeLog:

2020-09-25  Fam Zheng  <famzheng@amazon.com>

	* NEWS: Mention the /x modifier of disass.
	* cli/cli-cmds.c (disassemble_current_function): Add usage text
	for /x modifier.
	(disassemble_command): Parse /x modifier for disass.
	* disasm.c (gdb_pretty_print_disassembler::pretty_print_insn):
	Handle /x modifier.
	* disasm.h (enum gdb_disassembly_flag): Add a new flag for /x
	modifier.
	* record.c (get_insn_history_modifiers): Parse /x modifier.
---
 gdb/NEWS           |  3 +++
 gdb/cli/cli-cmds.c | 17 ++++++++++++-----
 gdb/disasm.c       |  5 ++++-
 gdb/disasm.h       |  1 +
 gdb/record.c       |  3 +++
 5 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index f30d718331..ba1cf76b36 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 10
 
+* A new modifier '/x' is added to disassemble command, to print instruction
+  offsets in hex.
+
 * MI changes
 
  ** '-break-insert --qualified' and '-dprintf-insert --qualified'
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index e3965fea07..e10544a734 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1456,12 +1456,12 @@ disassemble_current_function (gdb_disassembly_flags flags)
 /* Dump a specified section of assembly code.
 
    Usage:
-     disassemble [/mrs]
+     disassemble [/mrsx]
        - dump the assembly code for the function of the current pc
-     disassemble [/mrs] addr
+     disassemble [/mrsx] addr
        - dump the assembly code for the function at ADDR
-     disassemble [/mrs] low,high
-     disassemble [/mrs] low,+length
+     disassemble [/mrsx] low,high
+     disassemble [/mrsx] low,+length
        - dump the assembly code in the range [LOW,HIGH), or [LOW,LOW+length)
 
    A /m modifier will include source code with the assembly in a
@@ -1472,6 +1472,8 @@ disassemble_current_function (gdb_disassembly_flags flags)
 
    A /r modifier will include raw instructions in hex with the assembly.
 
+   A /x modifier will print offsets in hex.
+
    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.
@@ -1510,6 +1512,9 @@ disassemble_command (const char *arg, int from_tty)
 	    case 'r':
 	      flags |= DISASSEMBLY_RAW_INSN;
 	      break;
+	    case 'x':
+	      flags |= DISASSEMBLY_HEX_OFFSET;
+	      break;
 	    case 's':
 	      flags |= DISASSEMBLY_SOURCE;
 	      break;
@@ -2535,7 +2540,7 @@ can be shown using \"show listsize\"."));
 
   c = add_com ("disassemble", class_vars, disassemble_command, _("\
 Disassemble a specified section of memory.\n\
-Usage: disassemble[/m|/r|/s] START [, END]\n\
+Usage: disassemble[/m|/r|/s|/x] START [, END]\n\
 Default is the function surrounding the pc of the selected frame.\n\
 \n\
 With a /s modifier, source lines are included (if available).\n\
@@ -2551,6 +2556,8 @@ in favor of /s.\n\
 \n\
 With a /r modifier, raw instructions in hex are included.\n\
 \n\
+With a /x modifier, offsets are printed in hex.\n\
+\n\
 With a single argument, the function surrounding that address is dumped.\n\
 Two arguments (separated by a comma) are taken as a range of memory to dump,\n\
   in the form of \"start,end\", or \"start,+length\".\n\
diff --git a/gdb/disasm.c b/gdb/disasm.c
index e45c840068..c681d8b1e9 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -250,7 +250,10 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
 	   the offset takes the place of the "+" here.  */
 	if (offset >= 0)
 	  m_uiout->text ("+");
-	m_uiout->field_signed ("offset", offset);
+	m_uiout->field_fmt ("offset",
+			    ((flags & DISASSEMBLY_HEX_OFFSET) != 0
+			     ? "0x%x" : "%d"),
+			    offset);
 	m_uiout->text (">:\t");
       }
     else
diff --git a/gdb/disasm.h b/gdb/disasm.h
index b0f535eaa2..fb293650de 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -31,6 +31,7 @@ enum gdb_disassembly_flag
     DISASSEMBLY_OMIT_PC = (0x1 << 4),
     DISASSEMBLY_SOURCE = (0x1 << 5),
     DISASSEMBLY_SPECULATIVE = (0x1 << 6),
+    DISASSEMBLY_HEX_OFFSET = (0x1 << 7),
   };
 DEF_ENUM_FLAGS_TYPE (enum gdb_disassembly_flag, gdb_disassembly_flags);
 
diff --git a/gdb/record.c b/gdb/record.c
index 759395d5bc..44591e8cf0 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 'x':
+	      modifiers |= DISASSEMBLY_HEX_OFFSET;
+	      break;
 	    case 'f':
 	      modifiers |= DISASSEMBLY_OMIT_FNAME;
 	      break;
-- 
2.25.1

Re: [PATCH v7 1/3] disass: Add /x modifier to print offsets in hex

[Fam Zheng]

Ping?

On Sat, 26 Sep 2020, 09:51 , <fam@euphon.net> wrote:

> From: Fam Zheng <famzheng@amazon.com>
>
> Backtrace messages printed by Linux kernel and Xen have hex offsets,
> e.g.:
>
> (XEN) Xen call trace:
> (XEN)    [<ffff82d0402eefbb>] R guest_walk_tables_2_levels+0x189/0x66d
> (XEN)    [<ffff82d0402edbbd>] F hap_p2m_ga_to_gfn_2_levels+0x112/0x25b
> (XEN)    [<ffff82d0402edd22>] F hap_gva_to_gfn_2_levels+0x1c/0x1e
> (XEN)    [<ffff82d0402f832e>] F paging_gva_to_gfn+0x14a/0x167
>
> Having this modifier saves converting between hex values from the
> backtrace log and offsets in gdb disass output.
>
> gdb/ChangeLog:
>
> 2020-09-25  Fam Zheng  <famzheng@amazon.com>
>
>         * NEWS: Mention the /x modifier of disass.
>         * cli/cli-cmds.c (disassemble_current_function): Add usage text
>         for /x modifier.
>         (disassemble_command): Parse /x modifier for disass.
>         * disasm.c (gdb_pretty_print_disassembler::pretty_print_insn):
>         Handle /x modifier.
>         * disasm.h (enum gdb_disassembly_flag): Add a new flag for /x
>         modifier.
>         * record.c (get_insn_history_modifiers): Parse /x modifier.
> ---
>  gdb/NEWS           |  3 +++
>  gdb/cli/cli-cmds.c | 17 ++++++++++++-----
>  gdb/disasm.c       |  5 ++++-
>  gdb/disasm.h       |  1 +
>  gdb/record.c       |  3 +++
>  5 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index f30d718331..ba1cf76b36 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,9 @@
>
>  *** Changes since GDB 10
>
> +* A new modifier '/x' is added to disassemble command, to print
> instruction
> +  offsets in hex.
> +
>  * MI changes
>
>   ** '-break-insert --qualified' and '-dprintf-insert --qualified'
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index e3965fea07..e10544a734 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -1456,12 +1456,12 @@ disassemble_current_function
> (gdb_disassembly_flags flags)
>  /* Dump a specified section of assembly code.
>
>     Usage:
> -     disassemble [/mrs]
> +     disassemble [/mrsx]
>         - dump the assembly code for the function of the current pc
> -     disassemble [/mrs] addr
> +     disassemble [/mrsx] addr
>         - dump the assembly code for the function at ADDR
> -     disassemble [/mrs] low,high
> -     disassemble [/mrs] low,+length
> +     disassemble [/mrsx] low,high
> +     disassemble [/mrsx] low,+length
>         - dump the assembly code in the range [LOW,HIGH), or
> [LOW,LOW+length)
>
>     A /m modifier will include source code with the assembly in a
> @@ -1472,6 +1472,8 @@ disassemble_current_function (gdb_disassembly_flags
> flags)
>
>     A /r modifier will include raw instructions in hex with the assembly.
>
> +   A /x modifier will print offsets in hex.
> +
>     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.
> @@ -1510,6 +1512,9 @@ disassemble_command (const char *arg, int from_tty)
>             case 'r':
>               flags |= DISASSEMBLY_RAW_INSN;
>               break;
> +           case 'x':
> +             flags |= DISASSEMBLY_HEX_OFFSET;
> +             break;
>             case 's':
>               flags |= DISASSEMBLY_SOURCE;
>               break;
> @@ -2535,7 +2540,7 @@ can be shown using \"show listsize\"."));
>
>    c = add_com ("disassemble", class_vars, disassemble_command, _("\
>  Disassemble a specified section of memory.\n\
> -Usage: disassemble[/m|/r|/s] START [, END]\n\
> +Usage: disassemble[/m|/r|/s|/x] START [, END]\n\
>  Default is the function surrounding the pc of the selected frame.\n\
>  \n\
>  With a /s modifier, source lines are included (if available).\n\
> @@ -2551,6 +2556,8 @@ in favor of /s.\n\
>  \n\
>  With a /r modifier, raw instructions in hex are included.\n\
>  \n\
> +With a /x modifier, offsets are printed in hex.\n\
> +\n\
>  With a single argument, the function surrounding that address is
> dumped.\n\
>  Two arguments (separated by a comma) are taken as a range of memory to
> dump,\n\
>    in the form of \"start,end\", or \"start,+length\".\n\
> diff --git a/gdb/disasm.c b/gdb/disasm.c
> index e45c840068..c681d8b1e9 100644
> --- a/gdb/disasm.c
> +++ b/gdb/disasm.c
> @@ -250,7 +250,10 @@ gdb_pretty_print_disassembler::pretty_print_insn
> (const struct disasm_insn *insn
>            the offset takes the place of the "+" here.  */
>         if (offset >= 0)
>           m_uiout->text ("+");
> -       m_uiout->field_signed ("offset", offset);
> +       m_uiout->field_fmt ("offset",
> +                           ((flags & DISASSEMBLY_HEX_OFFSET) != 0
> +                            ? "0x%x" : "%d"),
> +                           offset);
>         m_uiout->text (">:\t");
>        }
>      else
> diff --git a/gdb/disasm.h b/gdb/disasm.h
> index b0f535eaa2..fb293650de 100644
> --- a/gdb/disasm.h
> +++ b/gdb/disasm.h
> @@ -31,6 +31,7 @@ enum gdb_disassembly_flag
>      DISASSEMBLY_OMIT_PC = (0x1 << 4),
>      DISASSEMBLY_SOURCE = (0x1 << 5),
>      DISASSEMBLY_SPECULATIVE = (0x1 << 6),
> +    DISASSEMBLY_HEX_OFFSET = (0x1 << 7),
>    };
>  DEF_ENUM_FLAGS_TYPE (enum gdb_disassembly_flag, gdb_disassembly_flags);
>
> diff --git a/gdb/record.c b/gdb/record.c
> index 759395d5bc..44591e8cf0 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 'x':
> +             modifiers |= DISASSEMBLY_HEX_OFFSET;
> +             break;
>             case 'f':
>               modifiers |= DISASSEMBLY_OMIT_FNAME;
>               break;
> --
> 2.25.1
>
>
>

Re: [PATCH v7 1/3] disass: Add /x modifier to print offsets in hex

[Eli Zaretskii]

> From: Fam Zheng <fam@euphon.net>
> Date: Mon, 12 Oct 2020 10:56:57 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, Andreas Schwab <schwab@linux-m68k.org>, Pedro Alves <pedro@palves.net>, 
> 	Andrew Burgess <andrew.burgess@embecosm.com>, Simon Marchi <simark@simark.ca>
> 
> Ping?

OK for the documentation parts.

Thanks.

Re: [PATCH v7 1/3] disass: Add /x modifier to print offsets in hex

[Tom Tromey]

>>>>> ">" == fam  <fam@euphon.net> writes:

>> 2020-09-25  Fam Zheng  <famzheng@amazon.com>

>> 	* NEWS: Mention the /x modifier of disass.
>> 	* cli/cli-cmds.c (disassemble_current_function): Add usage text
>> 	for /x modifier.
>> 	(disassemble_command): Parse /x modifier for disass.
>> 	* disasm.c (gdb_pretty_print_disassembler::pretty_print_insn):
>> 	Handle /x modifier.
>> 	* disasm.h (enum gdb_disassembly_flag): Add a new flag for /x
>> 	modifier.
>> 	* record.c (get_insn_history_modifiers): Parse /x modifier.

Thank you for the patch.  This looks good to me.

Do you have a copyright assignment in place, or in progress?
If not, contact me off-list and we can get things started.
That is a prerequisite for getting this in.

Tom

[PATCH v7 2/3] disass: Add test for /x mode

[fam]

From: Fam Zheng <famzheng@amazon.com>

gdb/testsuite/ChangeLog:

2020-09-25  Fam Zheng  <famzheng@amazon.com>

	* gdb.base/disasm-optim.exp: Add test for /x.
---
 gdb/testsuite/gdb.base/disasm-optim.exp | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gdb/testsuite/gdb.base/disasm-optim.exp b/gdb/testsuite/gdb.base/disasm-optim.exp
index 179e76ad0e..8c28fe10de 100644
--- a/gdb/testsuite/gdb.base/disasm-optim.exp
+++ b/gdb/testsuite/gdb.base/disasm-optim.exp
@@ -38,3 +38,10 @@ gdb_test_sequence "disassemble /s main" \
 	"disasm-optim\\.h:\r\n22"
 	"End of assembler dump\\."
     }
+
+gdb_test_sequence "disassemble /x main" \
+    "Disassemble main with source" {
+	"Dump of assembler code for function main:"
+	"=> 0x[0-9a-f]* \\<\\+0x0>:"
+	"End of assembler dump\\."
+    }
-- 
2.25.1

Re: [PATCH v7 2/3] disass: Add test for /x mode

[Tom Tromey]

>>>>> ">" == fam  <fam@euphon.net> writes:

>> 2020-09-25  Fam Zheng  <famzheng@amazon.com>

>> 	* gdb.base/disasm-optim.exp: Add test for /x.

Looks good.  Normally we'd have the test suite patch in with the main
patch, but I don't think it's a huge deal.

Tom

[PATCH v7 3/3] disass: Add texinfo doc for the /x modifier

[fam]

From: Fam Zheng <famzheng@amazon.com>

gdb/doc/ChangeLog:

2020-09-25  Fam Zheng  <famzheng@amazon.com>

	* gdb.texinfo (Machine Code): Document /x modifier of disass
	command.
---
 gdb/doc/gdb.texinfo | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8bff27c940..4dad72a34f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -9514,16 +9514,20 @@ line.
 @itemx disassemble /m
 @itemx disassemble /s
 @itemx disassemble /r
-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
-program counter of the selected frame.  A single argument to this
+@itemx disassemble /x
+This specialized command dumps a range of memory as machine instructions.
+If the address of an instruction resolves to a symbol, such as a function
+name, the symbol name and the offset are printed following the instruction
+address.  The offset is in decimal by default and in hex if @code{/x} is
+modifier is used.  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 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}
@@ -9561,6 +9565,22 @@ Dump of assembler code from 0x32c4 to 0x32e4:
 End of assembler dump.
 @end smallexample
 
+@code{/x} could be used to show offsets in hex:
+
+@smallexample
+(@value{GDBP}) disas /x 0x32c4, 0x32e4
+Dump of assembler code from 0x32c4 to 0x32e4:
+   0x32c4 <main+0xcc>:      addil 0,dp
+   0x32c8 <main+0xd0>:      ldw 0x22c(sr0,r1),r26
+   0x32cc <main+0xd4>:      ldil 0x3000,r31
+   0x32d0 <main+0xd8>:      ble 0x3f8(sr4,r31)
+   0x32d4 <main+0xdc>:      ldo 0(r31),rp
+   0x32d8 <main+0xe0>:      addil -0x800,dp
+   0x32dc <main+0xe4>:      ldo 0x588(r1),r26
+   0x32e0 <main+0xe8>:      ldil 0x3000,r31
+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.
-- 
2.25.1