Re: [RFC] mi: add -a option to the "-data-disassemble" command

Pedro Alves
2018-08-14T13:57:09Z

On 08/14/2018 10:15 AM, Jan Vrany wrote:
> A CLI command "disassemble" allows use to specify a single
> address - in that case function surrounding that address is
> disassembled.
>
> This commit adds this feature to MI command "-data-disassemble".
>

Thanks. I've pushed in the patch as at the end of this message,
after fixing a few nits I'll mention below.

> gdb/ChangeLog:
>
> * mi/mi-cmd-disas.c (mi_cmd_disassemble): Added -a option.

Write present tense.

> If used, use find_pc_partial_function to find address range
> to disassemble.
> * mi/mi-main.c (mi/mi-main.c): Add new feature

Should be function name instead of "(mi/mi-main.c)".

> data-disassemble-a-option to indicate that -data-disassemble
> supports -a.
> * NEWS: Mention new -data-disassemble option -a

Missing period.

>
> gdb/doc/ChangeLog:
>
> * gdb.texinfo (GDB/MI Data Manipulation): Document
> "-data-disassemble -a addr".
> (GDB/MI Support Commands): Document data-disassemble-a-option
> feature

Missing period.

>
> gdb/testsuite/ChangeLog:
>
> * gdb.mi/mi-disassemble.exp (test_disassembly_only): Added

Present tense.

> tests for -data-disassemble -a.
> (test_disassembly_bogus_args): Likewise.

> * aarch64-linux-tdep.c (aarch64_linux_supply_sve_regset): New function.
> diff --git a/gdb/NEWS b/gdb/NEWS
> index f275031345..3110210bb9 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,9 @@
>
> *** Changes since GDB 8.2
>
> +* The '-data-disassemble' MI command now accepts the '-a' option to disassemble
> + the whole function surrounding given program counter value or function name.

I moved this into a new "MI changes" section below the CLI commands, and
also added a new sentence for the -list-features addition (borrowed from
earlier MI changes).

> +
> * GDB and GDBserver now support IPv6 connections. IPv6 addresses
> can be passed using the '[ADDRESS]:PORT' notation, or the regular
> 'ADDRESS:PORT' method.
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index ceb4043e32..e1e6a31c95 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,10 @@
> +2018-07-05 Jan Vrany <jan.vrany@fit.cvut.cz>
> +
> + * gdb.texinfo (GDB/MI Data Manipulation): Document
> + "-data-disassemble -a addr".
> + (GDB/MI Support Commands): Document data-disassemble-a-option
> + feature
> +
> 2018-08-07 Simon Marchi <simon.marchi@ericsson.com>
>
> * gdb.texinfo (Index Files Speed Up GDB): Add section about
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index b931834400..c6452220a4 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -31208,6 +31208,7 @@ For details about what an addressable memory unit is,
> @smallexample
> -data-disassemble
> [ -s @var{start-addr} -e @var{end-addr} ]
> + | [ -a @var{addr} ]
> | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
> -- @var{mode}
> @end smallexample
> @@ -31220,6 +31221,11 @@ Where:
> is the beginning address (or @code{$pc})
> @item @var{end-addr}
> is the end address
> +@item @var{addr}
> +is the address anywhere in function code (or function name). If @code{-a}
> +@var{addr} is used, the whole function surrounding that address will be
> +disassembled. If @var{addr} is a function name, the whole function with that
> +name will be disassembled.

It took me a bit to understand what "address anywhere in function code"
meant. I took the liberty of rewording the sentence a bit.

> @item @var{filename}
> is the name of the file to disassemble
> @item @var{linenum}
> @@ -33356,6 +33362,9 @@ records, produced when trying to execute an undefined @sc{gdb/mi} command
> @item exec-run-start-option
> Indicates that the @code{-exec-run} command supports the @option{--start}
> option (@pxref{GDB/MI Program Execution}).
> +@item data-disassemble-a-option
> +Indicates that the @code{-data-disassemble} command supports the @option{-a}
> +option (@pxref{GDB/MI Data Manipulation}).
> @end ftable
>
> @subheading The @code{-list-target-features} Command

> error (_("-data-disassemble: Usage: ( [-f filename -l linenum [-n "
> - "howmany]] | [-s startaddr -e endaddr]) [--] mode."));
> + "howmany]] | [-s startaddr -e endaddr] | [-a addr] ) [--] mode."));
>
> if (argc != 1)
> - error (_("-data-disassemble: Usage: [-f filename -l linenum "
> - "[-n howmany]] [-s startaddr -e endaddr] [--] mode."));
> + error (_("-data-disassemble: Usage: ( [-f filename -l linenum [-n "
> + "howmany]] | [-s startaddr -e endaddr] | [-a addr] ) [--] mode."));
>

While at it, might as well merge the 'if's instead of changing the same
string twice.

> mode = atoi (argv[0]);
> if (mode < 0 || mode > 5)
> @@ -184,6 +197,12 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
> error (_("-data-disassemble: "
> "No function contains specified address"));
> }
> + else if (addr_seen)

Spurious whitespace after "(addr_seen)".

> + {
> + if (find_pc_partial_function (addr, NULL, &low, &high) == 0)
> + error (_("-data-disassemble: "
> + "No function contains specified address"));
> + }
>
> gdb_disassembly (gdbarch, uiout,
> disasm_flags,
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index 2163d42302..1e31040d1e 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c

> @@ -216,10 +228,14 @@ proc test_disassembly_bogus_args {} {
>
> mi_gdb_test "321-data-disassemble -s foo -e bar -- 0" \
> "321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
> - "data-disassemble bogus address"
> + "data-disassemble bogus address (-s -e)"
> +
> + mi_gdb_test "322-data-disassemble -a foo -- 0" \
> + "322\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
> + "data-disassemble bogus address (-a)"

No tail " (foo)" in test names:

https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

I've switched those to use "," instead.

>
> 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.\\) .--. mode.\"" \
> + "456\\^error,msg=\"-data-disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr. \\| .-a addr. \\) .--. mode.\"" \
> "data-disassemble mix different args"
>
> mi_gdb_test "789-data-disassemble -f basics.c -l $line_main_body -- 9" \
>

Here's what I merged.

From 26fb3983d7062c6f6bc43028ae2074cc29d22c7e Mon Sep 17 00:00:00 2001
From: Jan Vrany <jan.vrany@fit.cvut.cz>
Date: Tue, 14 Aug 2018 14:13:28 +0100
Subject: [PATCH] MI: Add -a option to the "-data-disassemble" command

The CLI "disassemble" command allows specifying a single address - in
that case the function surrounding that address is disassembled.

This commit adds this feature to the equivalent MI command
"-data-disassemble".

gdb/ChangeLog:
2018-08-14 Jan Vrany <jan.vrany@fit.cvut.cz>

* mi/mi-cmd-disas.c (mi_cmd_disassemble): Add -a option.
If used, use find_pc_partial_function to find address range
to disassemble.
* mi/mi-main.c (mi_cmd_list_features): Report
"data-disassemble-a-option" feature.
* NEWS: Mention new -data-disassemble option -a.

gdb/doc/ChangeLog:
2018-08-14 Jan Vrany <jan.vrany@fit.cvut.cz>

* gdb.texinfo (GDB/MI Data Manipulation): Document
"-data-disassemble -a addr".
(GDB/MI Support Commands): Document "data-disassemble-a-option"
feature.

gdb/testsuite/ChangeLog:
2018-08-14 Jan Vrany <jan.vrany@fit.cvut.cz>

* gdb.mi/mi-disassemble.exp (test_disassembly_only): Add tests for
-data-disassemble -a.
(test_disassembly_bogus_args): Likewise.
---
gdb/ChangeLog | 9 +++++++++
gdb/doc/ChangeLog | 7 +++++++
gdb/testsuite/ChangeLog | 6 ++++++
gdb/doc/gdb.texinfo | 9 +++++++++
gdb/NEWS | 8 ++++++++
gdb/mi/mi-cmd-disas.c | 36 ++++++++++++++++++++++++---------
gdb/mi/mi-main.c | 1 +
gdb/testsuite/gdb.mi/mi-disassemble.exp | 20 ++++++++++++++++--
8 files changed, 84 insertions(+), 12 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 37ab81b0a93..d16920d80e9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2018-08-14 Jan Vrany <jan.vrany@fit.cvut.cz>
+
+ * mi/mi-cmd-disas.c (mi_cmd_disassemble): Add -a option.
+ If used, use find_pc_partial_function to find address range
+ to disassemble.
+ * mi/mi-main.c (mi_cmd_list_features): Report
+ "data-disassemble-a-option" feature.
+ * NEWS: Mention new -data-disassemble option -a.
+
2018-08-13 Tom Tromey <tom@tromey.com>

* common/common-defs.h (_FORTIFY_SOURCE): Define.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index ceb4043e326..8688b7fdd93 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,10 @@
+2018-08-14 Jan Vrany <jan.vrany@fit.cvut.cz>
+
+ * gdb.texinfo (GDB/MI Data Manipulation): Document
+ "-data-disassemble -a addr".
+ (GDB/MI Support Commands): Document "data-disassemble-a-option"
+ feature.
+
2018-08-07 Simon Marchi <simon.marchi@ericsson.com>

* gdb.texinfo (Index Files Speed Up GDB): Add section about
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 33749aa6763..1b4dde59bb6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2018-08-14 Jan Vrany <jan.vrany@fit.cvut.cz>
+
+ * gdb.mi/mi-disassemble.exp (test_disassembly_only): Add tests for
+ -data-disassemble -a.
+ (test_disassembly_bogus_args): Likewise.
+
2018-08-14 Andrew Burgess <andrew.burgess@embecosm.com>

* gdb.mi/list-thread-groups-available.exp: Update test regexp.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b9318344002..433a2698a92 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -31208,6 +31208,7 @@ For details about what an addressable memory unit is,
@smallexample
-data-disassemble
[ -s @var{start-addr} -e @var{end-addr} ]
+ | [ -a @var{addr} ]
| [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
-- @var{mode}
@end smallexample
@@ -31220,6 +31221,11 @@ Where:
is the beginning address (or @code{$pc})
@item @var{end-addr}
is the end address
+@item @var{addr}
+is an address anywhere within (or the name of) the function to
+disassemble. If an address is specified, the whole function
+surrounding that address will be disassembled. If a name is
+specified, the whole function with that name will be disassembled.
@item @var{filename}
is the name of the file to disassemble
@item @var{linenum}
@@ -33356,6 +33362,9 @@ records, produced when trying to execute an undefined @sc{gdb/mi} command
@item exec-run-start-option
Indicates that the @code{-exec-run} command supports the @option{--start}
option (@pxref{GDB/MI Program Execution}).
+@item data-disassemble-a-option
+Indicates that the @code{-data-disassemble} command supports the @option{-a}
+option (@pxref{GDB/MI Data Manipulation}).
@end ftable

@subheading The @code{-list-target-features} Command
diff --git a/gdb/NEWS b/gdb/NEWS
index f2750313454..16d3d725897 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -41,6 +41,14 @@ thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
FLAG arguments allow to control what output to produce and how to handle
errors raised when applying COMMAND to a thread.

+* MI changes
+
+ ** The '-data-disassemble' MI command now accepts an '-a' option to
+ disassemble the whole function surrounding the given program
+ counter value or function name. Support for this feature can be
+ verified by using the "-list-features" command, which should
+ contain "data-disassemble-a-option".
+
* New native configurations

GNU/Linux/RISC-V riscv*-*-linux*
diff --git a/gdb/mi/mi-cmd-disas.c b/gdb/mi/mi-cmd-disas.c
index e704e35b34b..f0f26356c2d 100644
--- a/gdb/mi/mi-cmd-disas.c
+++ b/gdb/mi/mi-cmd-disas.c
@@ -67,6 +67,7 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
int num_seen = 0;
int start_seen = 0;
int end_seen = 0;
+ int addr_seen = 0;

/* ... and their corresponding value. */
char *file_string = NULL;
@@ -74,13 +75,14 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
int how_many = -1;
CORE_ADDR low = 0;
CORE_ADDR high = 0;
+ CORE_ADDR addr = 0;

/* Options processing stuff. */
int oind = 0;
char *oarg;
enum opt
{
- FILE_OPT, LINE_OPT, NUM_OPT, START_OPT, END_OPT
+ FILE_OPT, LINE_OPT, NUM_OPT, START_OPT, END_OPT, ADDR_OPT
};
static const struct mi_opt opts[] =
{
@@ -89,6 +91,7 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
{"n", NUM_OPT, 1},
{"s", START_OPT, 1},
{"e", END_OPT, 1},
+ {"a", ADDR_OPT, 1},
{ 0, 0, 0 }
};

@@ -122,23 +125,30 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
high = parse_and_eval_address (oarg);
end_seen = 1;
break;
+ case ADDR_OPT:
+ addr = parse_and_eval_address (oarg);
+ addr_seen = 1;
+ break;
}
}
argv += oind;
argc -= oind;

/* Allow only filename + linenum (with how_many which is not
- required) OR start_addr + end_addr. */
+ required) OR start_addr + end_addr OR addr. */
+
+ if (!(
+ ( line_seen && file_seen && !start_seen && !end_seen
+ && !addr_seen)

- if (!((line_seen && file_seen && num_seen && !start_seen && !end_seen)
- || (line_seen && file_seen && !num_seen && !start_seen && !end_seen)
- || (!line_seen && !file_seen && !num_seen && start_seen && end_seen)))
- error (_("-data-disassemble: Usage: ( [-f filename -l linenum [-n "
- "howmany]] | [-s startaddr -e endaddr]) [--] mode."));
+ || (!line_seen && !file_seen && !num_seen && start_seen && end_seen
+ && !addr_seen)

- if (argc != 1)
- error (_("-data-disassemble: Usage: [-f filename -l linenum "
- "[-n howmany]] [-s startaddr -e endaddr] [--] mode."));
+ || (!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."));

mode = atoi (argv[0]);
if (mode < 0 || mode > 5)
@@ -184,6 +194,12 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
error (_("-data-disassemble: "
"No function contains specified address"));
}
+ else if (addr_seen)
+ {
+ if (find_pc_partial_function (addr, NULL, &low, &high) == 0)
+ error (_("-data-disassemble: "
+ "No function contains specified address"));
+ }

gdb_disassembly (gdbarch, uiout,
disasm_flags,
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 2163d423028..1e31040d1e2 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1683,6 +1683,7 @@ mi_cmd_list_features (const char *command, char **argv, int argc)
uiout->field_string (NULL, "info-gdb-mi-command");
uiout->field_string (NULL, "undefined-command-error-code");
uiout->field_string (NULL, "exec-run-start-option");
+ uiout->field_string (NULL, "data-disassemble-a-option");

if (ext_lang_initialized_p (get_ext_lang_defn (EXT_LANG_PYTHON)))
uiout->field_string (NULL, "python");
diff --git a/gdb/testsuite/gdb.mi/mi-disassemble.exp b/gdb/testsuite/gdb.mi/mi-disassemble.exp
index 6b1fcf58845..f9399477e5a 100644
--- a/gdb/testsuite/gdb.mi/mi-disassemble.exp
+++ b/gdb/testsuite/gdb.mi/mi-disassemble.exp
@@ -46,13 +46,24 @@ proc test_disassembly_only {} {
# Test disassembly more only for the current function.
# Tests:
# -data-disassemble -s $pc -e "$pc+8" -- 0
+ # -data-disassemble -a $pc -- 0
+ # -data-disassemble -a callee4 -- 0
# -data-disassembly -f basics.c -l $line_main_body -- 0

+
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"
+
+ 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 "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"
@@ -207,6 +218,7 @@ proc test_disassembly_bogus_args {} {
# Tests:
# -data-disassembly -f foo -l abc -n 0 -- 0
# -data-disassembly -s foo -e bar -- 0
+ # -data-disassembly -a foo -- 0
# -data-disassembly -s $pc -f basics.c -- 0
# -data-disassembly -f basics.c -l 32 -- 9

@@ -216,10 +228,14 @@ proc test_disassembly_bogus_args {} {

mi_gdb_test "321-data-disassemble -s foo -e bar -- 0" \
"321\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
- "data-disassemble bogus address"
+ "data-disassemble bogus address, -s -e"
+
+ mi_gdb_test "322-data-disassemble -a foo -- 0" \
+ "322\\^error,msg=\"No symbol \\\\\"foo\\\\\" in current context.\"" \
+ "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.\\) .--. mode.\"" \
+ "456\\^error,msg=\"-data-disassemble: Usage: \\( .-f filename -l linenum .-n howmany.. \\| .-s startaddr -e endaddr. \\| .-a addr. \\) .--. mode.\"" \
"data-disassemble mix different args"

mi_gdb_test "789-data-disassemble -f basics.c -l $line_main_body -- 9" \
--
2.14.4