[RFC] mi: add -a option to the "-data-disassemble" command
Commit Message
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".
gdb/ChangeLog:
* mi/mi-cmd-disas.c (mi_cmd_disassemble): Added -a option.
If used, use find_pc_partial_function to find address range
to disassemble.
* mi/mi-main.c (mi/mi-main.c): Add new feature
data-disassemble-a-option to indicate that -data-disassemble
supports -a.
* NEWS: Mention new -data-disassemble option -a
gdb/doc/ChangeLog:
* gdb.texinfo (GDB/MI Data Manipulation): Document
"-data-disassemble -a addr".
(GDB/MI Support Commands): Document data-disassemble-a-option
feature
gdb/testsuite/ChangeLog:
* gdb.mi/mi-disassemble.exp (test_disassembly_only): Added
tests for -data-disassemble -a.
(test_disassembly_bogus_args): Likewise.
---
gdb/ChangeLog | 10 +++++++++
gdb/NEWS | 3 +++
gdb/doc/ChangeLog | 7 +++++++
gdb/doc/gdb.texinfo | 9 ++++++++
gdb/mi/mi-cmd-disas.c | 28 ++++++++++++++++++-------
gdb/mi/mi-main.c | 1 +
gdb/testsuite/ChangeLog | 6 ++++++
gdb/testsuite/gdb.mi/mi-disassemble.exp | 20 ++++++++++++++++--
8 files changed, 75 insertions(+), 9 deletions(-)
Comments
>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
Jan> A CLI command "disassemble" allows use to specify a single
Jan> address - in that case function surrounding that address is
Jan> disassembled.
Jan> high = parse_and_eval_address (oarg);
Jan> end_seen = 1;
Jan> break;
Jan> + case ADDR_OPT:
Jan> + addr = parse_and_eval_address (oarg);
Jan> + addr_seen = 1;
Jan> + break;
The indentation looked slightly off here.
It could just be how the patch looked here, but could you double-check
just in case?
Jan> + if (!((line_seen && file_seen && num_seen && !start_seen && !end_seen && !addr_seen)
Jan> + || (line_seen && file_seen && !num_seen && !start_seen && !end_seen && !addr_seen)
Jan> + || (!line_seen && !file_seen && !num_seen && start_seen && end_seen && !addr_seen)
Jan> + || (!line_seen && !file_seen && !num_seen && !start_seen && !end_seen && addr_seen)))
I suppose this ought to check for the case where either -s or -e is
given along with -a. That seems like an error.
This "if" made me laugh.
Jan> + mi_gdb_test "112-data-disassemble -a \$pc -- 0" \
Jan> + "112\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
Jan> + "data-disassemble function around pc assembly only"
Jan> +
Jan> + mi_gdb_test "112-data-disassemble -a callee4 -- 0" \
Jan> + "112\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
Jan> + "data-disassemble function callee4 assembly only"
Probably the second "112" should be "113". I don't know if you can
reuse tokens or not, but it seems simple and safe not to.
Thanks for doing this. It seems like a good addition to me.
Tom
On Mon, 2018-07-30 at 09:04 -0600, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
>
> Jan> A CLI command "disassemble" allows use to specify a single
> Jan> address - in that case function surrounding that address is
> Jan> disassembled.
>
> Jan> high = parse_and_eval_address (oarg);
> Jan> end_seen = 1;
> Jan> break;
> Jan> + case ADDR_OPT:
> Jan> + addr = parse_and_eval_address (oarg);
> Jan> + addr_seen = 1;
> Jan> + break;
>
> The indentation looked slightly off here.
> It could just be how the patch looked here, but could you double-check
> just in case?
My fault. My editor put 8 spaces instead of tab (\t) at the beginning of
my lines. Will fix.
>
> Jan> + if (!((line_seen && file_seen && num_seen && !start_seen && !end_seen && !addr_seen)
> Jan> + || (line_seen && file_seen && !num_seen && !start_seen && !end_seen && !addr_seen)
> Jan> + || (!line_seen && !file_seen && !num_seen && start_seen && end_seen && !addr_seen)
> Jan> + || (!line_seen && !file_seen && !num_seen && !start_seen && !end_seen && addr_seen)))
>
> I suppose this ought to check for the case where either -s or -e is
> given along with -a. That seems like an error.
>
I'm sorry I'm confused. Let me try to explain. There are (now) four forms:
1a) -f filename -l linenum
1b) -f filename -l linenum -n lines
2) -s start-addr -e end-addr
3) -a addr
Command must have one of the above four forms, otherwise it's invalid. Each
"line" in the code above checks one of the above form (in order as written here).
Note, that there's negation at the very beginning, so the condition holds
if the command has none of the four forms. Seems "good" to me. Makes sense?
Perhaps it's easier to see with little reformatting (hope email won't screw it):
if (! (( line_seen && file_seen && num_seen && !start_seen && !end_seen && !addr_seen)
|| ( line_seen && file_seen && !num_seen && !start_seen && !end_seen && !addr_seen)
|| (!line_seen && !file_seen && !num_seen && start_seen && end_seen && !addr_seen)
|| (!line_seen && !file_seen && !num_seen && !start_seen && !end_seen && addr_seen)))
error (_("-data-disassemble: Usage: ( [-f filename -l linenum [-n "
"howmany]] | [-s startaddr -e endaddr] | [-a addr] ) [--] mode."));
Now, it appears to me that first two lines can be merged as forms 1a and 1b differ only
in presence of lines. Also, I forgot to update the comment above. So, what about:
/* 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 && 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)))
+ if (!( ( line_seen && file_seen && !start_seen && !end_seen && !addr_seen)
+ || (!line_seen && !file_seen && !num_seen && start_seen && end_seen && !addr_seen)
+ || (!line_seen && !file_seen && !num_seen && !start_seen && !end_seen && addr_seen)))
this passes mi-disassemble.exp on my machine.
> This "if" made me laugh.
:-) I cannot say I was exactly laughing. But followed the crowed anyway.
>
> Jan> + mi_gdb_test "112-data-disassemble -a \$pc -- 0" \
> Jan> + "112\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
> Jan> + "data-disassemble function around pc assembly only"
> Jan> +
> Jan> + mi_gdb_test "112-data-disassemble -a callee4 -- 0" \
> Jan> + "112\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\}.*\]"
> \
> Jan> + "data-disassemble function callee4 assembly only"
>
> Probably the second "112" should be "113". I don't know if you can
> reuse tokens or not, but it seems simple and safe not to.
Sure, will fix and send a patch once we agree on the "if".
Thanks, Jan
>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
>> The indentation looked slightly off here.
>> It could just be how the patch looked here, but could you double-check
>> just in case?
Jan> My fault. My editor put 8 spaces instead of tab (\t) at the beginning of
Jan> my lines. Will fix.
Thanks. According to .dir-locals.el, tabs are fine, assuming things
line up properly. (But there's been talk over the years of switching
purely to spaces, and IIRC using only spaces is also fine.)
Jan> I'm sorry I'm confused. Let me try to explain. There are (now) four forms:
Jan> 1a) -f filename -l linenum
Jan> 1b) -f filename -l linenum -n lines
Jan> 2) -s start-addr -e end-addr
Jan> 3) -a addr
Jan> Command must have one of the above four forms, otherwise it's invalid. Each
Jan> "line" in the code above checks one of the above form (in order as written here).
Jan> Note, that there's negation at the very beginning, so the condition holds
Jan> if the command has none of the four forms. Seems "good" to me. Makes sense?
Yes, thanks for explaining this. I was misreading the new code here.
Jan> Now, it appears to me that first two lines can be merged as forms 1a and 1b differ only
Jan> in presence of lines. Also, I forgot to update the comment above. So, what about:
[...]
Thank you, that seems like a good improvement.
Tom
@@ -1,3 +1,13 @@
+2018-07-05 Jan Vrany <jan.vrany@fit.cvut.cz>
+
+ * mi/mi-cmd-disas.c (mi_cmd_disassemble): Added -a option.
+ If used, use find_pc_partial_function to find address range
+ to disassemble.
+ * mi/mi-main.c (mi/mi-main.c): Add new feature
+ data-disassemble-a-option to indicate that -data-disassemble
+ supports -a.
+ * NEWS: Mention new -data-disassemble option -a
+
2018-07-02 Sebastian Huber <sebastian.huber@embedded-brains.de>
* riscv-tdep.c (riscv_register_aliases): Swap "fp" and "s0" entries.
@@ -3,6 +3,9 @@
*** Changes since GDB 8.1
+* The '-data-disassemble' MI command now accepts the '-a' option to disassemble
+ the whole function surrounding given program counter value or function name.
+
* The 'symbol-file' command now accepts an '-o' option to add a relative
offset to all sections.
@@ -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-06-28 Petr Tesarik <ptesarik@suse.cz>
* gdb.texinfo (Files): Document "add-symbol-file -o offset".
@@ -30947,6 +30947,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
@@ -30959,6 +30960,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.
@item @var{filename}
is the name of the file to disassemble
@item @var{linenum}
@@ -33095,6 +33101,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
@@ -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,6 +125,10 @@ 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;
@@ -130,15 +137,16 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
/* Allow only filename + linenum (with how_many which is not
required) OR start_addr + end_addr. */
- 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)))
+ if (!((line_seen && file_seen && num_seen && !start_seen && !end_seen && !addr_seen)
+ || (line_seen && file_seen && !num_seen && !start_seen && !end_seen && !addr_seen)
+ || (!line_seen && !file_seen && !num_seen && start_seen && end_seen && !addr_seen)
+ || (!line_seen && !file_seen && !num_seen && !start_seen && !end_seen && addr_seen)))
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."));
mode = atoi (argv[0]);
if (mode < 0 || mode > 5)
@@ -184,6 +192,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,
@@ -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");
@@ -1,3 +1,9 @@
+2018-07-05 Jan Vrany <jan.vrany@fit.cvut.cz>
+
+ * gdb.mi/mi-disassemble.exp (test_disassembly_only): Added
+ tests for -data-disassemble -a.
+ (test_disassembly_bogus_args): Likewise.
+
2018-06-29 Pedro Alves <palves@redhat.com>
* gdb.threads/names.exp: Adjust expected "info threads" output.
@@ -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 "112-data-disassemble -a callee4 -- 0" \
+ "112\\^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" \