* [PATCH 0/2] Use libopcodes disassembler styling with GDB
@ 2022-06-17 10:36 Andrew Burgess
2022-06-17 10:36 ` [PATCH 1/2] gdb: have gdb_disassemble_info carry 'this' in its stream pointer Andrew Burgess
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Andrew Burgess @ 2022-06-17 10:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
libopcodes is slowly gaining the ability to provide styling
information for its disassembler output.
This series extends GDB to make use of this information where possible
(currently only i386 based architectures and RISC-V).
---
Andrew Burgess (2):
gdb: have gdb_disassemble_info carry 'this' in its stream pointer
gdb: add support for disassembler styling using libopcodes
gdb/NEWS | 28 ++++++
gdb/cli/cli-style.c | 107 ++++++++++++++++++++--
gdb/cli/cli-style.h | 28 ++++--
gdb/disasm.c | 217 +++++++++++++++++++++++++++++++++++++++++---
gdb/disasm.h | 81 ++++++++++++-----
gdb/doc/gdb.texinfo | 100 +++++++++++++++++++-
6 files changed, 503 insertions(+), 58 deletions(-)
--
2.25.4
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/2] gdb: have gdb_disassemble_info carry 'this' in its stream pointer 2022-06-17 10:36 [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess @ 2022-06-17 10:36 ` Andrew Burgess 2022-06-17 10:36 ` [PATCH 2/2] gdb: add support for disassembler styling using libopcodes Andrew Burgess 2022-07-04 10:17 ` [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess 2 siblings, 0 replies; 15+ messages in thread From: Andrew Burgess @ 2022-06-17 10:36 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess The gdb_disassemble_info class is a wrapper around the libopcodes disassemble_info struct. The 'stream' field of disassemble_info is passed as an argument to the fprintf_func and fprintf_styled_func callbacks when the disassembler wants to print anything. Previously, GDB would store a pointer to a ui_file object in the 'stream' field, then, when the disassembler wanted to print anything, the content would be written to the ui_file object. An example of an fprintf_func callback, from gdb/disasm.c is: int gdb_disassembler::dis_asm_fprintf (void *stream, const char *format, ...) { /* Write output to STREAM here. */ } This is fine, but has one limitation, within the print callbacks we only have access to STREAM, we can't access any additional state stored within the gdb_disassemble_info object. Right now this isn't a problem, but in a future commit this will become an issue, how we style the output being written to STREAM will depend on the state of the gdb_disassemble_info object, and this state might need to be updated, depending on what is being printed. In this commit I propose changing the 'stream' field of the disassemble_info to carry a pointer to the gdb_disassemble_info sub-class, rather than the stream itself. We then have the two sub-classes of gdb_disassemble_info to consider, the gdb_non_printing_disassembler class never cared about the stream, previously, for this class, the stream was nullptr. With the change to make stream be a gdb_disassemble_info pointer, no further updates are needed for gdb_non_printing_disassembler. The other sub-class is gdb_printing_disassembler. In this case the sub-class now carries around a pointer to the stream object. The print callbacks are updated to cast the incoming stream object back to a gdb_printing_disassembler, and then extract the stream. This is purely a refactoring commit. A later commit will add additional state to the gdb_printing_disassembler, and update the print callbacks to access this state. There should be no user visible changes after this commit. --- gdb/disasm.c | 35 ++++++++++++++++++++-------- gdb/disasm.h | 65 +++++++++++++++++++++++++++++++++------------------- 2 files changed, 67 insertions(+), 33 deletions(-) diff --git a/gdb/disasm.c b/gdb/disasm.c index c6edc92930d..6c64c14feee 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -163,17 +163,33 @@ gdb_disassembler::dis_asm_print_address (bfd_vma addr, print_address (self->arch (), addr, self->stream ()); } +/* See disasm.h. */ + +ui_file * +gdb_printing_disassembler::stream_from_gdb_disassemble_info (void *dis_info) +{ + gdb_disassemble_info *di = (gdb_disassemble_info *) dis_info; + gdb_printing_disassembler *dis + = dynamic_cast<gdb_printing_disassembler *> (di); + gdb_assert (dis != nullptr); + ui_file *stream = dis->stream (); + gdb_assert (stream != nullptr); + return stream; +} + /* Format disassembler output to STREAM. */ int -gdb_printing_disassembler::fprintf_func (void *stream, +gdb_printing_disassembler::fprintf_func (void *dis_info, const char *format, ...) { - va_list args; + ui_file *stream = stream_from_gdb_disassemble_info (dis_info); + va_list args; va_start (args, format); - gdb_vprintf ((struct ui_file *) stream, format, args); + gdb_vprintf (stream, format, args); va_end (args); + /* Something non -ve. */ return 0; } @@ -181,14 +197,15 @@ gdb_printing_disassembler::fprintf_func (void *stream, /* See disasm.h. */ int -gdb_printing_disassembler::fprintf_styled_func (void *stream, +gdb_printing_disassembler::fprintf_styled_func (void *dis_info, enum disassembler_style style, const char *format, ...) { - va_list args; + ui_file *stream = stream_from_gdb_disassemble_info (dis_info); + va_list args; va_start (args, format); - gdb_vprintf ((struct ui_file *) stream, format, args); + gdb_vprintf (stream, format, args); va_end (args); /* Something non -ve. */ return 0; @@ -809,7 +826,7 @@ gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch, /* See disasm.h. */ gdb_disassemble_info::gdb_disassemble_info - (struct gdbarch *gdbarch, struct ui_file *stream, + (struct gdbarch *gdbarch, read_memory_ftype read_memory_func, memory_error_ftype memory_error_func, print_address_ftype print_address_func, fprintf_ftype fprintf_func, fprintf_styled_ftype fprintf_styled_func) @@ -817,7 +834,7 @@ gdb_disassemble_info::gdb_disassemble_info { gdb_assert (fprintf_func != nullptr); gdb_assert (fprintf_styled_func != nullptr); - init_disassemble_info (&m_di, stream, fprintf_func, + init_disassemble_info (&m_di, (void *) this, fprintf_func, fprintf_styled_func); m_di.flavour = bfd_target_unknown_flavour; @@ -930,7 +947,7 @@ gdb_disassembler::print_insn (CORE_ADDR memaddr, /* Push any disassemble output to the real destination stream. We do this even if the disassembler reported failure (-1) as the disassembler may have printed something to its output stream. */ - m_di.fprintf_func (m_dest, "%s", m_buffer.c_str ()); + gdb_printf (m_dest, "%s", m_buffer.c_str ()); /* If the disassembler failed then report an appropriate error. */ if (length < 0) diff --git a/gdb/disasm.h b/gdb/disasm.h index da03e130526..54176eb095a 100644 --- a/gdb/disasm.h +++ b/gdb/disasm.h @@ -58,16 +58,14 @@ struct gdb_disassemble_info using fprintf_ftype = decltype (disassemble_info::fprintf_func); using fprintf_styled_ftype = decltype (disassemble_info::fprintf_styled_func); - /* Constructor, many fields in m_di are initialized from GDBARCH. STREAM - is where the output of the disassembler will be written too, the - remaining arguments are function callbacks that are written into - m_di. Of these function callbacks FPRINTF_FUNC and - FPRINTF_STYLED_FUNC must not be nullptr. If READ_MEMORY_FUNC, - MEMORY_ERROR_FUNC, or PRINT_ADDRESS_FUNC are nullptr, then that field - within m_di is left with its default value (see the libopcodes - function init_disassemble_info for the defaults). */ + /* Constructor, many fields in m_di are initialized from GDBARCH. The + remaining arguments are function callbacks that are written into m_di. + Of these function callbacks FPRINTF_FUNC and FPRINTF_STYLED_FUNC must + not be nullptr. If READ_MEMORY_FUNC, MEMORY_ERROR_FUNC, or + PRINT_ADDRESS_FUNC are nullptr, then that field within m_di is left + with its default value (see the libopcodes function + init_disassemble_info for the defaults). */ gdb_disassemble_info (struct gdbarch *gdbarch, - struct ui_file *stream, read_memory_ftype read_memory_func, memory_error_ftype memory_error_func, print_address_ftype print_address_func, @@ -77,10 +75,6 @@ struct gdb_disassemble_info /* Destructor. */ virtual ~gdb_disassemble_info (); - /* The stream that disassembler output is being written too. */ - struct ui_file *stream () - { return (struct ui_file *) m_di.stream; } - /* Stores data required for disassembling instructions in opcodes. */ struct disassemble_info m_di; @@ -109,6 +103,10 @@ struct gdb_printing_disassembler : public gdb_disassemble_info protected: + /* The stream that disassembler output is being written too. */ + struct ui_file *stream () + { return m_stream; } + /* Constructor. All the arguments are just passed to the parent class. We also add the two print functions to the arguments passed to the parent. See gdb_disassemble_info for a description of how the @@ -118,22 +116,41 @@ struct gdb_printing_disassembler : public gdb_disassemble_info read_memory_ftype read_memory_func, memory_error_ftype memory_error_func, print_address_ftype print_address_func) - : gdb_disassemble_info (gdbarch, stream, read_memory_func, + : gdb_disassemble_info (gdbarch, read_memory_func, memory_error_func, print_address_func, - fprintf_func, fprintf_styled_func) - { /* Nothing. */ } - - /* Callback used as the disassemble_info's fprintf_func callback, this - writes to STREAM, which will be m_di.stream. */ - static int fprintf_func (void *stream, const char *format, ...) + fprintf_func, fprintf_styled_func), + m_stream (stream) + { + gdb_assert (stream != nullptr); + } + + /* Callback used as the disassemble_info's fprintf_func callback. The + DIS_INFO pointer is a pointer to a gdb_printing_disassembler object. + Content is written to the m_stream extracted from DIS_INFO. */ + static int fprintf_func (void *dis_info, const char *format, ...) ATTRIBUTE_PRINTF(2,3); - /* Callback used as the disassemble_info's fprintf_styled_func callback, - this writes to STREAM, which will be m_di.stream. */ - static int fprintf_styled_func (void *stream, + /* Callback used as the disassemble_info's fprintf_styled_func callback. + The DIS_INFO pointer is a pointer to a gdb_printing_disassembler + object. Content is written to the m_stream extracted from DIS_INFO. */ + static int fprintf_styled_func (void *dis_info, enum disassembler_style style, const char *format, ...) ATTRIBUTE_PRINTF(3,4); + +private: + + /* When libopcodes calls the fprintf_func and fprintf_styled_func + callbacks, a 'void *' argument is passed. We arrange, through our + call to init_disassemble_info that this argument will be a pointer to + a gdb_disassemble_info sub-class, specifically, a + gdb_printing_disassembler pointer. This helper function casts + DIS_INFO to the correct type (with some asserts), and then returns the + m_stream member variable. */ + static ui_file *stream_from_gdb_disassemble_info (void *dis_info); + + /* The stream to which output should be sent. */ + struct ui_file *m_stream; }; /* A basic disassembler that doesn't actually print anything. */ @@ -142,7 +159,7 @@ struct gdb_non_printing_disassembler : public gdb_disassemble_info { gdb_non_printing_disassembler (struct gdbarch *gdbarch, read_memory_ftype read_memory_func) - : gdb_disassemble_info (gdbarch, nullptr /* stream */, + : gdb_disassemble_info (gdbarch, read_memory_func, nullptr /* memory_error_func */, nullptr /* print_address_func */, -- 2.25.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] gdb: add support for disassembler styling using libopcodes 2022-06-17 10:36 [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess 2022-06-17 10:36 ` [PATCH 1/2] gdb: have gdb_disassemble_info carry 'this' in its stream pointer Andrew Burgess @ 2022-06-17 10:36 ` Andrew Burgess 2022-06-17 11:16 ` Eli Zaretskii 2022-07-20 2:09 ` Simon Marchi 2022-07-04 10:17 ` [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess 2 siblings, 2 replies; 15+ messages in thread From: Andrew Burgess @ 2022-06-17 10:36 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess This commit extends GDB to make use of libopcodes styling support where available, currently this is just i386 based architectures, and RISC-V. For architectures that don't support styling using libopcodes GDB will fall back to using the Python Pygments package, when the package is available. The new libopcodes based styling has the disassembler identify parts of the disassembled instruction, e.g. registers, immediates, mnemonics, etc, and can style these components differently. Additionally, as the styling is now done in GDB we can add settings to allow the user to configure which colours are used right from the GDB CLI. There's some new maintenance commands: maintenance set libopcodes-styling enabled on|off maintenance show libopcodes-styling These can be used to manually disable use of libopcodes styling. This is a maintenance command as it's not anticipated that a user should need to do this. But, this could be useful for testing, or, in some rare cases, a user might want to override the Python hook used for disassembler styling, and then disable libopcode styling so that GDB falls back to using Python. Right now I would consider this second use case a rare situation, which is why I think a maintenance command is appropriate. When libopcodes is being used for styling then the user can make use of the following new styles: set/show style disassembler comment set/show style disassembler immediate set/show style disassembler mnemonic set/show style disassembler register The disassembler also makes use of the 'address' and 'function' styles to style some parts of the disassembler output. I have also added the following aliases though: set/show style disassembler address set/show style disassembler symbol these are aliases for: set/show style address set/show style function respectively, and exist to make it easier for users to discover disassembler related style settings. The 'address' style is used to style numeric addresses in the disassembler output, while the 'symbol' or 'function' style is used to style the names of symbols in disassembler output. As not every architecture supports libopcodes styling, the maintenance setting 'libopcodes-styling enabled' has an "auto-off" type behaviour. Consider this GDB session: (gdb) show architecture The target architecture is set to "auto" (currently "i386:x86-64"). (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "on". the setting defaults to "on" for architectures that support libopcodes based styling. (gdb) set architecture sparc The target architecture is set to "sparc". (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "off" (not supported on architecture "sparc") the setting will show as "off" if the user switches to an architecture that doesn't support libopcodes styling. The underlying setting is still "on" at this point though, if the user switches back to i386:x86-64 then the setting would go back to being "on". (gdb) maintenance set libopcodes-styling enabled off (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "off". now the setting is "off" for everyone, even if the user switches back to i386:x86-64 the setting will still show as "off". (gdb) maintenance set libopcodes-styling enabled on Use of libopcodes styling not supported on architecture "sparc". (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "off". attempting to switch the setting "on" for an unsupported architecture will give an error, and the setting will remain "off". (gdb) set architecture auto The target architecture is set to "auto" (currently "i386:x86-64"). (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "off". (gdb) maintenance set libopcodes-styling enabled on (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "on". the user will need to switch back to a supported architecture before they can one again turn this setting "on". --- gdb/NEWS | 28 +++++++ gdb/cli/cli-style.c | 107 +++++++++++++++++++++++--- gdb/cli/cli-style.h | 28 +++++-- gdb/disasm.c | 184 ++++++++++++++++++++++++++++++++++++++++++-- gdb/disasm.h | 16 ++++ gdb/doc/gdb.texinfo | 100 +++++++++++++++++++++++- 6 files changed, 437 insertions(+), 26 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index ac9a1aacd34..34a491edd92 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -27,6 +27,12 @@ emit to indicate where a breakpoint should be placed to break in a function past its prologue. +* Disassembler styling using libopcodes. GDB now supports + disassembler styling using libopcodes. This is only available for + some targets (currently x86 and RISC-V). For unsupported targets + Python Pygments is still used. For supported targets, libopcodes + styling is used by default. + * New commands maintenance set ignore-prologue-end-flag on|off @@ -36,6 +42,28 @@ maintenance show ignore-prologue-end-flag used to force GDB to use prologue analyzers if the line-table is constructed from erroneous debug information. +maintenance set libopcodes-styling on|off +maintenance show libopcodes-styling + These can be used to force off libopcodes based styling, the Python + Pygments styling will then be used instead. + +set style disassembler comment +show style disassembler comment +set style disassembler immediate +show style disassembler immediate +set style disassembler mnemonic +show style disassembler mnemonic +set style disassembler register +show style disassembler register +set style disassembler address +show style disassembler address +set style disassembler symbol +show style disassembler symbol + For targets that support libopcodes based styling, these settings + control how various aspects of the disassembler output are styled. + The 'disassembler address' and 'disassembler symbol' styles are + aliases for the 'address' and 'function' styles respectively. + * Changed commands maintenance info line-table diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c index 3fd85f4aa86..2ff1b5815e3 100644 --- a/gdb/cli/cli-style.c +++ b/gdb/cli/cli-style.c @@ -110,6 +110,23 @@ cli_style_option version_style ("version", ui_file_style::MAGENTA, /* See cli-style.h. */ +cli_style_option disasm_mnemonic_style ("mnemonic", ui_file_style::GREEN); + +/* See cli-style.h. */ + +cli_style_option disasm_register_style ("register", ui_file_style::RED); + +/* See cli-style.h. */ + +cli_style_option disasm_immediate_style ("immediate", ui_file_style::BLUE); + +/* See cli-style.h. */ + +cli_style_option disasm_comment_style ("comment", ui_file_style::WHITE, + ui_file_style::DIM); + +/* See cli-style.h. */ + cli_style_option::cli_style_option (const char *name, ui_file_style::basic_color fg, ui_file_style::intensity intensity) @@ -224,15 +241,16 @@ cli_style_option::do_show_intensity (struct ui_file *file, int from_tty, /* See cli-style.h. */ -void +set_show_commands cli_style_option::add_setshow_commands (enum command_class theclass, const char *prefix_doc, struct cmd_list_element **set_list, struct cmd_list_element **show_list, bool skip_intensity) { - add_setshow_prefix_cmd (m_name, theclass, prefix_doc, prefix_doc, - &m_set_list, &m_show_list, set_list, show_list); + set_show_commands prefix_cmds + = add_setshow_prefix_cmd (m_name, theclass, prefix_doc, prefix_doc, + &m_set_list, &m_show_list, set_list, show_list); set_show_commands commands; @@ -274,6 +292,8 @@ cli_style_option::add_setshow_commands (enum command_class theclass, commands.set->set_context (this); commands.show->set_context (this); } + + return prefix_cmds; } static cmd_list_element *style_set_list; @@ -387,11 +407,13 @@ Configure filename colors and display intensity."), &style_set_list, &style_show_list, false); - function_name_style.add_setshow_commands (no_class, _("\ + set_show_commands function_prefix_cmds + = function_name_style.add_setshow_commands (no_class, _("\ Function name display styling.\n\ Configure function name colors and display intensity"), - &style_set_list, &style_show_list, - false); + &style_set_list, + &style_show_list, + false); variable_name_style.add_setshow_commands (no_class, _("\ Variable name display styling.\n\ @@ -399,11 +421,12 @@ Configure variable name colors and display intensity"), &style_set_list, &style_show_list, false); - address_style.add_setshow_commands (no_class, _("\ + set_show_commands address_prefix_cmds + = address_style.add_setshow_commands (no_class, _("\ Address display styling.\n\ Configure address colors and display intensity"), - &style_set_list, &style_show_list, - false); + &style_set_list, &style_show_list, + false); title_style.add_setshow_commands (no_class, _("\ Title display styling.\n\ @@ -451,4 +474,70 @@ Version string display styling.\n\ Configure colors used to display the GDB version string."), &style_set_list, &style_show_list, false); + + disasm_mnemonic_style.add_setshow_commands (no_class, _("\ +Disassembler mnemonic display styling.\n\ +Configure the colors and display intensity for instruction mnemonics\n\ +in the disassembler output. The \"disassembler mnemonic\" style is\n\ +used to displaying instruction mnemonics as well as any assembler\n\ +directives, e.g. \".byte\", \".word\", etc.\n\ +\n\ +This style will only be used for targets that support libopcodes based\n\ +disassembler styling. When Python Pygments based styling is used\n\ +then this style has no effect."), + &style_disasm_set_list, + &style_disasm_show_list, + false); + + disasm_register_style.add_setshow_commands (no_class, _("\ +Disassembler register display styling.\n\ +Configure the colors and display intensity for registers in the\n\ +disassembler output.\n\ +\n\ +This style will only be used for targets that support libopcodes based\n\ +disassembler styling. When Python Pygments based styling is used\n\ +then this style has no effect."), + &style_disasm_set_list, + &style_disasm_show_list, + false); + + disasm_immediate_style.add_setshow_commands (no_class, _("\ +Disassembler immediate display styling.\n\ +Configure the colors and display intensity for immediates in the\n\ +disassembler output. The \"disassembler immediate\" style is used for\n\ +any number that is not an address, this includes constants in arithmetic\n\ +instructions, as well as address offsets in memory access instructions.\n\ +\n\ +This style will only be used for targets that support libopcodes based\n\ +disassembler styling. When Python Pygments based styling is used\n\ +then this style has no effect."), + &style_disasm_set_list, + &style_disasm_show_list, + false); + + disasm_comment_style.add_setshow_commands (no_class, _("\ +Disassembler comment display styling.\n\ +Configure the colors and display intensity for comments in the\n\ +disassembler output. The \"disassembler comment\" style is used for\n\ +the comment character, and everything after the comment character up to\n\ +the end of the line. The comment style overrides any other styling,\n\ +e.g. a register name in a comment will use the comment styling.\n\ +\n\ +This style will only be used for targets that support libopcodes based\n\ +disassembler styling. When Python Pygments based styling is used\n\ +then this style has no effect."), + &style_disasm_set_list, + &style_disasm_show_list, + false); + + /* Setup 'disassembler address' style and 'disassembler symbol' style, + these are aliases for 'address' and 'function' styles respectively. */ + add_alias_cmd ("address", address_prefix_cmds.set, no_class, 0, + &style_disasm_set_list); + add_alias_cmd ("address", address_prefix_cmds.show, no_class, 0, + &style_disasm_show_list); + add_alias_cmd ("symbol", function_prefix_cmds.set, no_class, 0, + &style_disasm_set_list); + add_alias_cmd ("symbol", function_prefix_cmds.show, no_class, 0, + &style_disasm_show_list); } diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h index f69df47098c..4090cf01333 100644 --- a/gdb/cli/cli-style.h +++ b/gdb/cli/cli-style.h @@ -43,12 +43,13 @@ class cli_style_option /* Return the style name. */ const char *name () { return m_name; }; - /* Call once to register this CLI style with the CLI engine. */ - void add_setshow_commands (enum command_class theclass, - const char *prefix_doc, - struct cmd_list_element **set_list, - struct cmd_list_element **show_list, - bool skip_intensity); + /* Call once to register this CLI style with the CLI engine. Returns + the set/show prefix commands for the style. */ + set_show_commands add_setshow_commands (enum command_class theclass, + const char *prefix_doc, + struct cmd_list_element **set_list, + struct cmd_list_element **show_list, + bool skip_intensity); /* Return the 'set style NAME' command list, that can be used to build a lambda DO_SET to call add_setshow_commands. */ @@ -116,6 +117,21 @@ extern cli_style_option title_style; /* The metadata style. */ extern cli_style_option metadata_style; +/* The disassembler style for mnemonics or assembler directives + (e.g. '.byte', etc). */ +extern cli_style_option disasm_mnemonic_style; + +/* The disassembler style for register names. */ +extern cli_style_option disasm_register_style; + +/* The disassembler style for numerical values that are not addresses, this + includes immediate operands (e.g. in, an add instruction), but also + address offsets (e.g. in a load instruction). */ +extern cli_style_option disasm_immediate_style; + +/* The disassembler style for comments. */ +extern cli_style_option disasm_comment_style; + /* The border style of a TUI window that does not have the focus. */ extern cli_style_option tui_border_style; diff --git a/gdb/disasm.c b/gdb/disasm.c index 6c64c14feee..cf27a32fbe7 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -41,6 +41,63 @@ which is set by the "set disassembler_options" command. */ static std::string prospective_options; +/* When this is true we will try to use libopcodes to provide styling to + the disassembler output. */ + +static bool use_libopcodes_styling = true; + +/* To support the set_use_libopcodes_styling function we have a second + variable which is connected to the actual set/show option. */ + +static bool use_libopcodes_styling_option = use_libopcodes_styling; + +/* The "maint show libopcodes-styling enabled" command. */ + +static void +show_use_libopcodes_styling (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) +{ + gdb_non_printing_memory_disassembler dis (target_gdbarch ()); + bool supported = dis.disasm_info ()->created_styled_output; + + if (supported || !use_libopcodes_styling) + gdb_printf (file, _("Use of libopcodes styling support is \"%s\".\n"), + value); + else + { + /* Use of libopcodes styling is not supported, and the user has this + turned on! */ + gdb_printf (file, _("Use of libopcodes styling support is \"off\"" + " (not supported on architecture \"%s\")\n"), + gdbarch_bfd_arch_info (target_gdbarch ())->printable_name); + } +} + +/* The "maint set libopcodes-styling enabled" command. */ + +static void +set_use_libopcodes_styling (const char *args, int from_tty, + struct cmd_list_element *c) +{ + gdb_non_printing_memory_disassembler dis (target_gdbarch ()); + bool supported = dis.disasm_info ()->created_styled_output; + + /* If the current architecture doesn't support libopcodes styling then we + give an error here, but leave the underlying setting enabled. This + means that if the user switches to an architecture that does support + libopcodes styling the setting will be enabled. */ + + if (use_libopcodes_styling_option && !supported) + { + use_libopcodes_styling_option = use_libopcodes_styling; + error (_("Use of libopcodes styling not supported on architecture \"%s\"."), + gdbarch_bfd_arch_info (target_gdbarch ())->printable_name); + } + else + use_libopcodes_styling = use_libopcodes_styling_option; +} + /* This structure is used to store line number information for the deprecated /m option. We need a different sort of line table from the normal one cuz we can't @@ -160,7 +217,26 @@ gdb_disassembler::dis_asm_print_address (bfd_vma addr, gdb_disassembler *self = static_cast<gdb_disassembler *>(info->application_data); - print_address (self->arch (), addr, self->stream ()); + if (self->in_comment_p ()) + { + /* Calling 'print_address' might add styling to the output (based on + the properties of the stream we're writing too). This is usually + fine, but if we are in an assembler comment then we'd prefer to + have the comment style, rather than the default address style. + + Print the address into a temporary buffer which doesn't support + styling, then reprint this unstyled address with the default text + style. + + As we are inside a comment right now, the standard print routine + will ensure that the comment is printed to the user with a + suitable comment style. */ + string_file tmp; + print_address (self->arch (), addr, &tmp); + self->fprintf_styled_func (self, dis_style_text, "%s", tmp.c_str ()); + } + else + print_address (self->arch (), addr, self->stream ()); } /* See disasm.h. */ @@ -202,11 +278,54 @@ gdb_printing_disassembler::fprintf_styled_func (void *dis_info, const char *format, ...) { ui_file *stream = stream_from_gdb_disassemble_info (dis_info); + gdb_printing_disassembler *dis = (gdb_printing_disassembler *) dis_info; va_list args; va_start (args, format); - gdb_vprintf (stream, format, args); + std::string content = string_vprintf (format, args); va_end (args); + + /* Once in a comment then everything should be styled as a comment. */ + if (style == dis_style_comment_start) + dis->set_in_comment (true); + if (dis->in_comment_p ()) + style = dis_style_comment_start; + + /* Now print the content with the correct style. */ + const char *txt = content.c_str (); + switch (style) + { + case dis_style_mnemonic: + case dis_style_assembler_directive: + fputs_styled (txt, disasm_mnemonic_style.style (), stream); + break; + + case dis_style_register: + fputs_styled (txt, disasm_register_style.style (), stream); + break; + + case dis_style_immediate: + case dis_style_address_offset: + fputs_styled (txt, disasm_immediate_style.style (), stream); + break; + + case dis_style_address: + fputs_styled (txt, address_style.style (), stream); + break; + + case dis_style_symbol: + fputs_styled (txt, function_name_style.style (), stream); + break; + + case dis_style_comment_start: + fputs_styled (txt, disasm_comment_style.style (), stream); + break; + + case dis_style_text: + gdb_puts (txt, stream); + break; + } + /* Something non -ve. */ return 0; } @@ -818,7 +937,20 @@ gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch, read_memory_ftype func) : gdb_printing_disassembler (gdbarch, &m_buffer, func, dis_asm_memory_error, dis_asm_print_address), - m_buffer (!use_ext_lang_colorization_p && disassembler_styling + /* The use of m_di.created_styled_output here is a bit of a cheat, but + it works fine for now. Currently, for all targets that support + libopcodes styling, this field is set during the call to + disassemble_init_for_target, which was called as part of the + initialization of gdb_printing_disassembler. And so, we are able to + spot if a target supports libopcodes styling, and create m_buffer in + the correct styling mode. + + If there's ever a target that only sets created_styled_output during + the actual disassemble phase, then the logic here will have to + change. */ + m_buffer ((!use_ext_lang_colorization_p + || (use_libopcodes_styling && m_di.created_styled_output)) + && disassembler_styling && file->can_emit_style_escape ()), m_dest (file) { /* Nothing. */ } @@ -903,14 +1035,17 @@ gdb_disassembler::print_insn (CORE_ADDR memaddr, { m_err_memaddr.reset (); m_buffer.clear (); + this->set_in_comment (false); int length = gdb_print_insn_1 (arch (), memaddr, &m_di); - /* If we have successfully disassembled an instruction, styling is on, we - think that the extension language might be able to perform styling for - us, and the destination can support styling, then lets call into the - extension languages in order to style this output. */ + /* If we have successfully disassembled an instruction, disassembler + styling using the extension language is on, and libopcodes hasn't + already styled the output for us, and, if the destination can support + styling, then lets call into the extension languages in order to style + this output. */ if (length > 0 && disassembler_styling + && (!m_di.created_styled_output || !use_libopcodes_styling) && use_ext_lang_colorization_p && m_dest->can_emit_style_escape ()) { @@ -1311,4 +1446,39 @@ Show the disassembler options."), NULL, show_disassembler_options_sfunc, &setlist, &showlist); set_cmd_completer (set_show_disas_opts.set, disassembler_options_completer); + + + /* All the 'maint set|show libopcodes-styling' sub-commands. */ + static struct cmd_list_element *maint_set_libopcodes_styling_cmdlist; + static struct cmd_list_element *maint_show_libopcodes_styling_cmdlist; + + /* Adds 'maint set|show libopcodes-styling'. */ + add_setshow_prefix_cmd ("libopcodes-styling", class_maintenance, + _("Set libopcodes-styling specific variables."), + _("Show libopcodes-styling specific variables."), + &maint_set_libopcodes_styling_cmdlist, + &maint_show_libopcodes_styling_cmdlist, + &maintenance_set_cmdlist, + &maintenance_show_cmdlist); + + /* Adds 'maint set|show gnu-source-highlight enabled'. */ + add_setshow_boolean_cmd ("enabled", class_maintenance, + &use_libopcodes_styling_option, _("\ +Set whether the libopcodes styling support should be used."), _("\ +Show whether the libopcodes styling support should be used."),_("\ +When enabled, GDB will try to make use of the builtin libopcodes styling\n\ +support, to style the disassembler output. Not every architecture has\n\ +styling support within libopcodes, so enabling this is not a guarantee\n\ +that libopcodes styling will be available.\n\ +\n\ +When this option is disabled, GDB will make use of the Python Pygments\n\ +package (if available) to style the disassembler output.\n\ +\n\ +All disassembler styling can be disabled with:\n\ +\n\ + set style disassembler enabled off"), + set_use_libopcodes_styling, + show_use_libopcodes_styling, + &maint_set_libopcodes_styling_cmdlist, + &maint_show_libopcodes_styling_cmdlist); } diff --git a/gdb/disasm.h b/gdb/disasm.h index 54176eb095a..2921d537e0a 100644 --- a/gdb/disasm.h +++ b/gdb/disasm.h @@ -138,6 +138,16 @@ struct gdb_printing_disassembler : public gdb_disassemble_info const char *format, ...) ATTRIBUTE_PRINTF(3,4); + /* Return true if the disassembler is considered inside a comment, false + otherwise. */ + bool in_comment_p () const + { return m_in_comment; } + + /* Set whether the disassembler should be considered as within comment + text or not. */ + void set_in_comment (bool c) + { m_in_comment = c; } + private: /* When libopcodes calls the fprintf_func and fprintf_styled_func @@ -151,6 +161,12 @@ struct gdb_printing_disassembler : public gdb_disassemble_info /* The stream to which output should be sent. */ struct ui_file *m_stream; + + /* Are we inside a comment? This will be set true if the disassembler + uses styled output and emits a start of comment character. It is up + to the code that uses this disassembler class to reset this flag back + to false at a suitable time (e.g. at the end of every line). */ + bool m_in_comment; }; /* A basic disassembler that doesn't actually print anything. */ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 2178b476f53..e9714459e52 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -26478,6 +26478,7 @@ @item show style sources Show the current state of source code styling. +@anchor{style_disassembler_enabled} @item set style disassembler enabled @samp{on|off} Enable or disable disassembler styling. This affects whether disassembler output, such as the output of the @code{disassemble} @@ -26485,13 +26486,30 @@ general is enabled (with @code{set style enabled on}), and if a source highlighting library is available to @value{GDBN}. -To highlight disassembler output, @value{GDBN} must be compiled with -Python support, and the Python Pygments package must be available. If -these requirements are not met then @value{GDBN} will not highlight -disassembler output, even when this option is @samp{on}. +The two source highlighting libraries that @value{GDBN} could use are; +@value{GDBN}'s builtin disassembler, or the Python Pygments package. + +@value{GDBN}'s first choice will be to use the builtin disassembler +for styling, this usually provides better results, being able to style +different types of instruction operands differently. However, the +builtin disassembler is not able to style all architectures. + +For architectures that the builtin disassembler is unable to style, +@value{GDBN} will fall back to use the Python Pygments package where +possible. In order to use the Python Pygments package, @value{GDBN} +must be built with Python support, and the Pygments package must be +installed. + +If neither of these options are available then @value{GDBN} will +produce unstyled output, even when this setting is @samp{on}. + +To discover if the current architecture supports styling using the +builtin disassembler library see @ref{maint_libopcodes_styling,,maint +show libopcodes-styling enabled}. @item show style disassembler enabled Show the current state of disassembler styling. + @end table Subcommands of @code{set style} control specific forms of styling. @@ -26536,6 +26554,11 @@ @code{set style function} family of commands. By default, this style's foreground color is yellow. +This style is also used for symbol names in styled disassembler output +if @value{GDBN} is using its builtin disassembler library for styling +(@pxref{style_disassembler_enabled,,@kbd{set style disassembler +enabled}}). + @item variable Control the styling of variable names. These are managed with the @code{set style variable} family of commands. By default, this style's @@ -26546,6 +26569,11 @@ @code{set style address} family of commands. By default, this style's foreground color is blue. +This style is also used for addresses in styled disassembler output +if @value{GDBN} is using its builtin disassembler library for styling +(@pxref{style_disassembler_enabled,,@kbd{set style disassembler +enabled}}). + @item version Control the styling of @value{GDBN}'s version number text. By default, this style's foreground color is magenta and it has bold @@ -26594,6 +26622,47 @@ Control the styling of the active TUI border; that is, the TUI window that has the focus. +@item disassembler comment +Control the styling of comments in the disassembler output. These are +managed with the @code{set style disassembler comment} family of +commands. This style is only used when @value{GDBN} is styling using +its builtin disassembler library +(@pxref{style_disassembler_enabled,,@kbd{set style disassembler +enabled}}). By default, this style's intensity is dim, and its +foreground color is white. + +@item disassembler immediate +Control the styling of numeric operands in the disassembler output. +These are managed with the @code{set style disassembler immediate} +family of commands. This style is not used for instruction operands +that represent addresses, in that case the @samp{disassembler address} +style is used. This style is only used when @value{GDBN} is styling +using its builtin disassembler library. By default, this style's +foreground color is blue. + +@item disassembler address +Control the styling of address operands in the disassembler output. +This is an alias for the @samp{address} style. + +@item disassembler symbol +Control the styling of symbol names in the disassembler output. This +is an alias for the @samp{function} style. + +@item disassembler mnemonic +Control the styling of instruction mnemonics in the disassembler +output. These are managed with the @code{set style disassembler +mnemonic} family of commands. This style is also used for assembler +directives, e.g.@: @code{.byte}, @code{.word}, etc. This style is +only used when @value{GDBN} is styling using its builtin disassembler +library. By default, this style's foreground color is green. + +@item disassembler register +Control the styling of register operands in the disassembler output. +These are managed with the @code{set style disassembler register} +family of commands. This style is only used when @value{GDBN} is +styling using its builtin disassembler library. By default, this style's +foreground color is red. + @end table @node Numbers @@ -40365,6 +40434,29 @@ library when @value{GDBN} is linked against the GNU Source Highlight library. +@anchor{maint_libopcodes_styling} +@kindex maint set libopcodes-styling enabled +@kindex maint show libopcodes-styling enabled +@item maint set libopcodes-styling enabled @r{[}on|off@r{]} +@itemx maint show libopcodes-styling enabled +Control whether @value{GDBN} should use its builtin disassembler +(libopcodes) to style disassembler output (@pxref{Output Styling}). +The builtin disassembler does not support styling for all +architectures. + +When this option is @samp{off} the builtin disassembler will not be +used for styling, @value{GDBN} will fall back to using the Python +Pygments package if possible. + +Trying to set this option @samp{on} for an architecture that the +builtin disassembler is unable to style will give an error, otherwise, +the builtin disassembler will be used to style disassembler output. + +This option is @samp{on} by default for supported architectures. + +This option is useful for debugging @value{GDBN}'s use of the Pygments +library when @value{GDBN} is built for an architecture that supports +styling with the builtin disassembler @kindex maint space @cindex memory used by commands @item maint space @var{value} -- 2.25.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gdb: add support for disassembler styling using libopcodes 2022-06-17 10:36 ` [PATCH 2/2] gdb: add support for disassembler styling using libopcodes Andrew Burgess @ 2022-06-17 11:16 ` Eli Zaretskii 2022-06-30 12:22 ` Andrew Burgess 2022-07-20 2:09 ` Simon Marchi 1 sibling, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2022-06-17 11:16 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches > Date: Fri, 17 Jun 2022 11:36:02 +0100 > From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> > Cc: Andrew Burgess <aburgess@redhat.com> > > gdb/NEWS | 28 +++++++ > gdb/cli/cli-style.c | 107 +++++++++++++++++++++++--- > gdb/cli/cli-style.h | 28 +++++-- > gdb/disasm.c | 184 ++++++++++++++++++++++++++++++++++++++++++-- > gdb/disasm.h | 16 ++++ > gdb/doc/gdb.texinfo | 100 +++++++++++++++++++++++- > 6 files changed, 437 insertions(+), 26 deletions(-) Thanks, the documentation parts are approved, with these minor nits: > @@ -26485,13 +26486,30 @@ > general is enabled (with @code{set style enabled on}), and if a source > highlighting library is available to @value{GDBN}. > > -To highlight disassembler output, @value{GDBN} must be compiled with > -Python support, and the Python Pygments package must be available. If > -these requirements are not met then @value{GDBN} will not highlight > -disassembler output, even when this option is @samp{on}. > +The two source highlighting libraries that @value{GDBN} could use are; > +@value{GDBN}'s builtin disassembler, or the Python Pygments package. The original text explicitly talked about styling disassembler output, whereas the new text omits that qualification, and sounds as if it is talking about styling in general. Although this is in a section about disassembly, I'd prefer to keep the original qualification. > +If neither of these options are available then @value{GDBN} will > +produce unstyled output, even when this setting is @samp{on}. Same here. > +@item disassembler immediate > +Control the styling of numeric operands in the disassembler output. > +These are managed with the @code{set style disassembler immediate} > +family of commands. This style is not used for instruction operands > +that represent addresses, in that case the @samp{disassembler address} > +style is used. This style is only used when @value{GDBN} is styling ^^ Two spaces between sentences. > +@anchor{maint_libopcodes_styling} > +@kindex maint set libopcodes-styling enabled > +@kindex maint show libopcodes-styling enabled > +@item maint set libopcodes-styling enabled @r{[}on|off@r{]} > +@itemx maint show libopcodes-styling enabled > +Control whether @value{GDBN} should use its builtin disassembler > +(libopcodes) to style disassembler output (@pxref{Output Styling}). ^^^^^^^^^^ "libopcodes" should have the @file markup. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gdb: add support for disassembler styling using libopcodes 2022-06-17 11:16 ` Eli Zaretskii @ 2022-06-30 12:22 ` Andrew Burgess 2022-06-30 13:46 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Andrew Burgess @ 2022-06-30 12:22 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches Eli Zaretskii <eliz@gnu.org> writes: >> Date: Fri, 17 Jun 2022 11:36:02 +0100 >> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> >> Cc: Andrew Burgess <aburgess@redhat.com> >> >> gdb/NEWS | 28 +++++++ >> gdb/cli/cli-style.c | 107 +++++++++++++++++++++++--- >> gdb/cli/cli-style.h | 28 +++++-- >> gdb/disasm.c | 184 ++++++++++++++++++++++++++++++++++++++++++-- >> gdb/disasm.h | 16 ++++ >> gdb/doc/gdb.texinfo | 100 +++++++++++++++++++++++- >> 6 files changed, 437 insertions(+), 26 deletions(-) > > Thanks, the documentation parts are approved, with these minor nits: > >> @@ -26485,13 +26486,30 @@ >> general is enabled (with @code{set style enabled on}), and if a source >> highlighting library is available to @value{GDBN}. >> >> -To highlight disassembler output, @value{GDBN} must be compiled with >> -Python support, and the Python Pygments package must be available. If >> -these requirements are not met then @value{GDBN} will not highlight >> -disassembler output, even when this option is @samp{on}. >> +The two source highlighting libraries that @value{GDBN} could use are; >> +@value{GDBN}'s builtin disassembler, or the Python Pygments package. > > The original text explicitly talked about styling disassembler output, > whereas the new text omits that qualification, and sounds as if it is > talking about styling in general. Although this is in a section about > disassembly, I'd prefer to keep the original qualification. > >> +If neither of these options are available then @value{GDBN} will >> +produce unstyled output, even when this setting is @samp{on}. > > Same here. > >> +@item disassembler immediate >> +Control the styling of numeric operands in the disassembler output. >> +These are managed with the @code{set style disassembler immediate} >> +family of commands. This style is not used for instruction operands >> +that represent addresses, in that case the @samp{disassembler address} >> +style is used. This style is only used when @value{GDBN} is styling > ^^ > Two spaces between sentences. > >> +@anchor{maint_libopcodes_styling} >> +@kindex maint set libopcodes-styling enabled >> +@kindex maint show libopcodes-styling enabled >> +@item maint set libopcodes-styling enabled @r{[}on|off@r{]} >> +@itemx maint show libopcodes-styling enabled >> +Control whether @value{GDBN} should use its builtin disassembler >> +(libopcodes) to style disassembler output (@pxref{Output Styling}). > ^^^^^^^^^^ > "libopcodes" should have the @file markup. Thanks for your feedback. The updated patch below I believe addresses all these issues. Thanks, Andrew --- commit acb5bc58e73cc6e544def0b5ef212a679de1497b Author: Andrew Burgess <aburgess@redhat.com> Date: Mon Feb 14 14:40:52 2022 +0000 gdb: add support for disassembler styling using libopcodes This commit extends GDB to make use of libopcodes styling support where available, currently this is just i386 based architectures, and RISC-V. For architectures that don't support styling using libopcodes GDB will fall back to using the Python Pygments package, when the package is available. The new libopcodes based styling has the disassembler identify parts of the disassembled instruction, e.g. registers, immediates, mnemonics, etc, and can style these components differently. Additionally, as the styling is now done in GDB we can add settings to allow the user to configure which colours are used right from the GDB CLI. There's some new maintenance commands: maintenance set libopcodes-styling enabled on|off maintenance show libopcodes-styling These can be used to manually disable use of libopcodes styling. This is a maintenance command as it's not anticipated that a user should need to do this. But, this could be useful for testing, or, in some rare cases, a user might want to override the Python hook used for disassembler styling, and then disable libopcode styling so that GDB falls back to using Python. Right now I would consider this second use case a rare situation, which is why I think a maintenance command is appropriate. When libopcodes is being used for styling then the user can make use of the following new styles: set/show style disassembler comment set/show style disassembler immediate set/show style disassembler mnemonic set/show style disassembler register The disassembler also makes use of the 'address' and 'function' styles to style some parts of the disassembler output. I have also added the following aliases though: set/show style disassembler address set/show style disassembler symbol these are aliases for: set/show style address set/show style function respectively, and exist to make it easier for users to discover disassembler related style settings. The 'address' style is used to style numeric addresses in the disassembler output, while the 'symbol' or 'function' style is used to style the names of symbols in disassembler output. As not every architecture supports libopcodes styling, the maintenance setting 'libopcodes-styling enabled' has an "auto-off" type behaviour. Consider this GDB session: (gdb) show architecture The target architecture is set to "auto" (currently "i386:x86-64"). (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "on". the setting defaults to "on" for architectures that support libopcodes based styling. (gdb) set architecture sparc The target architecture is set to "sparc". (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "off" (not supported on architecture "sparc") the setting will show as "off" if the user switches to an architecture that doesn't support libopcodes styling. The underlying setting is still "on" at this point though, if the user switches back to i386:x86-64 then the setting would go back to being "on". (gdb) maintenance set libopcodes-styling enabled off (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "off". now the setting is "off" for everyone, even if the user switches back to i386:x86-64 the setting will still show as "off". (gdb) maintenance set libopcodes-styling enabled on Use of libopcodes styling not supported on architecture "sparc". (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "off". attempting to switch the setting "on" for an unsupported architecture will give an error, and the setting will remain "off". (gdb) set architecture auto The target architecture is set to "auto" (currently "i386:x86-64"). (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "off". (gdb) maintenance set libopcodes-styling enabled on (gdb) maintenance show libopcodes-styling enabled Use of libopcodes styling support is "on". the user will need to switch back to a supported architecture before they can one again turn this setting "on". diff --git a/gdb/NEWS b/gdb/NEWS index ac9a1aacd34..34a491edd92 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -27,6 +27,12 @@ emit to indicate where a breakpoint should be placed to break in a function past its prologue. +* Disassembler styling using libopcodes. GDB now supports + disassembler styling using libopcodes. This is only available for + some targets (currently x86 and RISC-V). For unsupported targets + Python Pygments is still used. For supported targets, libopcodes + styling is used by default. + * New commands maintenance set ignore-prologue-end-flag on|off @@ -36,6 +42,28 @@ maintenance show ignore-prologue-end-flag used to force GDB to use prologue analyzers if the line-table is constructed from erroneous debug information. +maintenance set libopcodes-styling on|off +maintenance show libopcodes-styling + These can be used to force off libopcodes based styling, the Python + Pygments styling will then be used instead. + +set style disassembler comment +show style disassembler comment +set style disassembler immediate +show style disassembler immediate +set style disassembler mnemonic +show style disassembler mnemonic +set style disassembler register +show style disassembler register +set style disassembler address +show style disassembler address +set style disassembler symbol +show style disassembler symbol + For targets that support libopcodes based styling, these settings + control how various aspects of the disassembler output are styled. + The 'disassembler address' and 'disassembler symbol' styles are + aliases for the 'address' and 'function' styles respectively. + * Changed commands maintenance info line-table diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c index 3fd85f4aa86..2ff1b5815e3 100644 --- a/gdb/cli/cli-style.c +++ b/gdb/cli/cli-style.c @@ -110,6 +110,23 @@ cli_style_option version_style ("version", ui_file_style::MAGENTA, /* See cli-style.h. */ +cli_style_option disasm_mnemonic_style ("mnemonic", ui_file_style::GREEN); + +/* See cli-style.h. */ + +cli_style_option disasm_register_style ("register", ui_file_style::RED); + +/* See cli-style.h. */ + +cli_style_option disasm_immediate_style ("immediate", ui_file_style::BLUE); + +/* See cli-style.h. */ + +cli_style_option disasm_comment_style ("comment", ui_file_style::WHITE, + ui_file_style::DIM); + +/* See cli-style.h. */ + cli_style_option::cli_style_option (const char *name, ui_file_style::basic_color fg, ui_file_style::intensity intensity) @@ -224,15 +241,16 @@ cli_style_option::do_show_intensity (struct ui_file *file, int from_tty, /* See cli-style.h. */ -void +set_show_commands cli_style_option::add_setshow_commands (enum command_class theclass, const char *prefix_doc, struct cmd_list_element **set_list, struct cmd_list_element **show_list, bool skip_intensity) { - add_setshow_prefix_cmd (m_name, theclass, prefix_doc, prefix_doc, - &m_set_list, &m_show_list, set_list, show_list); + set_show_commands prefix_cmds + = add_setshow_prefix_cmd (m_name, theclass, prefix_doc, prefix_doc, + &m_set_list, &m_show_list, set_list, show_list); set_show_commands commands; @@ -274,6 +292,8 @@ cli_style_option::add_setshow_commands (enum command_class theclass, commands.set->set_context (this); commands.show->set_context (this); } + + return prefix_cmds; } static cmd_list_element *style_set_list; @@ -387,11 +407,13 @@ Configure filename colors and display intensity."), &style_set_list, &style_show_list, false); - function_name_style.add_setshow_commands (no_class, _("\ + set_show_commands function_prefix_cmds + = function_name_style.add_setshow_commands (no_class, _("\ Function name display styling.\n\ Configure function name colors and display intensity"), - &style_set_list, &style_show_list, - false); + &style_set_list, + &style_show_list, + false); variable_name_style.add_setshow_commands (no_class, _("\ Variable name display styling.\n\ @@ -399,11 +421,12 @@ Configure variable name colors and display intensity"), &style_set_list, &style_show_list, false); - address_style.add_setshow_commands (no_class, _("\ + set_show_commands address_prefix_cmds + = address_style.add_setshow_commands (no_class, _("\ Address display styling.\n\ Configure address colors and display intensity"), - &style_set_list, &style_show_list, - false); + &style_set_list, &style_show_list, + false); title_style.add_setshow_commands (no_class, _("\ Title display styling.\n\ @@ -451,4 +474,70 @@ Version string display styling.\n\ Configure colors used to display the GDB version string."), &style_set_list, &style_show_list, false); + + disasm_mnemonic_style.add_setshow_commands (no_class, _("\ +Disassembler mnemonic display styling.\n\ +Configure the colors and display intensity for instruction mnemonics\n\ +in the disassembler output. The \"disassembler mnemonic\" style is\n\ +used to displaying instruction mnemonics as well as any assembler\n\ +directives, e.g. \".byte\", \".word\", etc.\n\ +\n\ +This style will only be used for targets that support libopcodes based\n\ +disassembler styling. When Python Pygments based styling is used\n\ +then this style has no effect."), + &style_disasm_set_list, + &style_disasm_show_list, + false); + + disasm_register_style.add_setshow_commands (no_class, _("\ +Disassembler register display styling.\n\ +Configure the colors and display intensity for registers in the\n\ +disassembler output.\n\ +\n\ +This style will only be used for targets that support libopcodes based\n\ +disassembler styling. When Python Pygments based styling is used\n\ +then this style has no effect."), + &style_disasm_set_list, + &style_disasm_show_list, + false); + + disasm_immediate_style.add_setshow_commands (no_class, _("\ +Disassembler immediate display styling.\n\ +Configure the colors and display intensity for immediates in the\n\ +disassembler output. The \"disassembler immediate\" style is used for\n\ +any number that is not an address, this includes constants in arithmetic\n\ +instructions, as well as address offsets in memory access instructions.\n\ +\n\ +This style will only be used for targets that support libopcodes based\n\ +disassembler styling. When Python Pygments based styling is used\n\ +then this style has no effect."), + &style_disasm_set_list, + &style_disasm_show_list, + false); + + disasm_comment_style.add_setshow_commands (no_class, _("\ +Disassembler comment display styling.\n\ +Configure the colors and display intensity for comments in the\n\ +disassembler output. The \"disassembler comment\" style is used for\n\ +the comment character, and everything after the comment character up to\n\ +the end of the line. The comment style overrides any other styling,\n\ +e.g. a register name in a comment will use the comment styling.\n\ +\n\ +This style will only be used for targets that support libopcodes based\n\ +disassembler styling. When Python Pygments based styling is used\n\ +then this style has no effect."), + &style_disasm_set_list, + &style_disasm_show_list, + false); + + /* Setup 'disassembler address' style and 'disassembler symbol' style, + these are aliases for 'address' and 'function' styles respectively. */ + add_alias_cmd ("address", address_prefix_cmds.set, no_class, 0, + &style_disasm_set_list); + add_alias_cmd ("address", address_prefix_cmds.show, no_class, 0, + &style_disasm_show_list); + add_alias_cmd ("symbol", function_prefix_cmds.set, no_class, 0, + &style_disasm_set_list); + add_alias_cmd ("symbol", function_prefix_cmds.show, no_class, 0, + &style_disasm_show_list); } diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h index f69df47098c..4090cf01333 100644 --- a/gdb/cli/cli-style.h +++ b/gdb/cli/cli-style.h @@ -43,12 +43,13 @@ class cli_style_option /* Return the style name. */ const char *name () { return m_name; }; - /* Call once to register this CLI style with the CLI engine. */ - void add_setshow_commands (enum command_class theclass, - const char *prefix_doc, - struct cmd_list_element **set_list, - struct cmd_list_element **show_list, - bool skip_intensity); + /* Call once to register this CLI style with the CLI engine. Returns + the set/show prefix commands for the style. */ + set_show_commands add_setshow_commands (enum command_class theclass, + const char *prefix_doc, + struct cmd_list_element **set_list, + struct cmd_list_element **show_list, + bool skip_intensity); /* Return the 'set style NAME' command list, that can be used to build a lambda DO_SET to call add_setshow_commands. */ @@ -116,6 +117,21 @@ extern cli_style_option title_style; /* The metadata style. */ extern cli_style_option metadata_style; +/* The disassembler style for mnemonics or assembler directives + (e.g. '.byte', etc). */ +extern cli_style_option disasm_mnemonic_style; + +/* The disassembler style for register names. */ +extern cli_style_option disasm_register_style; + +/* The disassembler style for numerical values that are not addresses, this + includes immediate operands (e.g. in, an add instruction), but also + address offsets (e.g. in a load instruction). */ +extern cli_style_option disasm_immediate_style; + +/* The disassembler style for comments. */ +extern cli_style_option disasm_comment_style; + /* The border style of a TUI window that does not have the focus. */ extern cli_style_option tui_border_style; diff --git a/gdb/disasm.c b/gdb/disasm.c index 6c64c14feee..cf27a32fbe7 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -41,6 +41,63 @@ which is set by the "set disassembler_options" command. */ static std::string prospective_options; +/* When this is true we will try to use libopcodes to provide styling to + the disassembler output. */ + +static bool use_libopcodes_styling = true; + +/* To support the set_use_libopcodes_styling function we have a second + variable which is connected to the actual set/show option. */ + +static bool use_libopcodes_styling_option = use_libopcodes_styling; + +/* The "maint show libopcodes-styling enabled" command. */ + +static void +show_use_libopcodes_styling (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) +{ + gdb_non_printing_memory_disassembler dis (target_gdbarch ()); + bool supported = dis.disasm_info ()->created_styled_output; + + if (supported || !use_libopcodes_styling) + gdb_printf (file, _("Use of libopcodes styling support is \"%s\".\n"), + value); + else + { + /* Use of libopcodes styling is not supported, and the user has this + turned on! */ + gdb_printf (file, _("Use of libopcodes styling support is \"off\"" + " (not supported on architecture \"%s\")\n"), + gdbarch_bfd_arch_info (target_gdbarch ())->printable_name); + } +} + +/* The "maint set libopcodes-styling enabled" command. */ + +static void +set_use_libopcodes_styling (const char *args, int from_tty, + struct cmd_list_element *c) +{ + gdb_non_printing_memory_disassembler dis (target_gdbarch ()); + bool supported = dis.disasm_info ()->created_styled_output; + + /* If the current architecture doesn't support libopcodes styling then we + give an error here, but leave the underlying setting enabled. This + means that if the user switches to an architecture that does support + libopcodes styling the setting will be enabled. */ + + if (use_libopcodes_styling_option && !supported) + { + use_libopcodes_styling_option = use_libopcodes_styling; + error (_("Use of libopcodes styling not supported on architecture \"%s\"."), + gdbarch_bfd_arch_info (target_gdbarch ())->printable_name); + } + else + use_libopcodes_styling = use_libopcodes_styling_option; +} + /* This structure is used to store line number information for the deprecated /m option. We need a different sort of line table from the normal one cuz we can't @@ -160,7 +217,26 @@ gdb_disassembler::dis_asm_print_address (bfd_vma addr, gdb_disassembler *self = static_cast<gdb_disassembler *>(info->application_data); - print_address (self->arch (), addr, self->stream ()); + if (self->in_comment_p ()) + { + /* Calling 'print_address' might add styling to the output (based on + the properties of the stream we're writing too). This is usually + fine, but if we are in an assembler comment then we'd prefer to + have the comment style, rather than the default address style. + + Print the address into a temporary buffer which doesn't support + styling, then reprint this unstyled address with the default text + style. + + As we are inside a comment right now, the standard print routine + will ensure that the comment is printed to the user with a + suitable comment style. */ + string_file tmp; + print_address (self->arch (), addr, &tmp); + self->fprintf_styled_func (self, dis_style_text, "%s", tmp.c_str ()); + } + else + print_address (self->arch (), addr, self->stream ()); } /* See disasm.h. */ @@ -202,11 +278,54 @@ gdb_printing_disassembler::fprintf_styled_func (void *dis_info, const char *format, ...) { ui_file *stream = stream_from_gdb_disassemble_info (dis_info); + gdb_printing_disassembler *dis = (gdb_printing_disassembler *) dis_info; va_list args; va_start (args, format); - gdb_vprintf (stream, format, args); + std::string content = string_vprintf (format, args); va_end (args); + + /* Once in a comment then everything should be styled as a comment. */ + if (style == dis_style_comment_start) + dis->set_in_comment (true); + if (dis->in_comment_p ()) + style = dis_style_comment_start; + + /* Now print the content with the correct style. */ + const char *txt = content.c_str (); + switch (style) + { + case dis_style_mnemonic: + case dis_style_assembler_directive: + fputs_styled (txt, disasm_mnemonic_style.style (), stream); + break; + + case dis_style_register: + fputs_styled (txt, disasm_register_style.style (), stream); + break; + + case dis_style_immediate: + case dis_style_address_offset: + fputs_styled (txt, disasm_immediate_style.style (), stream); + break; + + case dis_style_address: + fputs_styled (txt, address_style.style (), stream); + break; + + case dis_style_symbol: + fputs_styled (txt, function_name_style.style (), stream); + break; + + case dis_style_comment_start: + fputs_styled (txt, disasm_comment_style.style (), stream); + break; + + case dis_style_text: + gdb_puts (txt, stream); + break; + } + /* Something non -ve. */ return 0; } @@ -818,7 +937,20 @@ gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch, read_memory_ftype func) : gdb_printing_disassembler (gdbarch, &m_buffer, func, dis_asm_memory_error, dis_asm_print_address), - m_buffer (!use_ext_lang_colorization_p && disassembler_styling + /* The use of m_di.created_styled_output here is a bit of a cheat, but + it works fine for now. Currently, for all targets that support + libopcodes styling, this field is set during the call to + disassemble_init_for_target, which was called as part of the + initialization of gdb_printing_disassembler. And so, we are able to + spot if a target supports libopcodes styling, and create m_buffer in + the correct styling mode. + + If there's ever a target that only sets created_styled_output during + the actual disassemble phase, then the logic here will have to + change. */ + m_buffer ((!use_ext_lang_colorization_p + || (use_libopcodes_styling && m_di.created_styled_output)) + && disassembler_styling && file->can_emit_style_escape ()), m_dest (file) { /* Nothing. */ } @@ -903,14 +1035,17 @@ gdb_disassembler::print_insn (CORE_ADDR memaddr, { m_err_memaddr.reset (); m_buffer.clear (); + this->set_in_comment (false); int length = gdb_print_insn_1 (arch (), memaddr, &m_di); - /* If we have successfully disassembled an instruction, styling is on, we - think that the extension language might be able to perform styling for - us, and the destination can support styling, then lets call into the - extension languages in order to style this output. */ + /* If we have successfully disassembled an instruction, disassembler + styling using the extension language is on, and libopcodes hasn't + already styled the output for us, and, if the destination can support + styling, then lets call into the extension languages in order to style + this output. */ if (length > 0 && disassembler_styling + && (!m_di.created_styled_output || !use_libopcodes_styling) && use_ext_lang_colorization_p && m_dest->can_emit_style_escape ()) { @@ -1311,4 +1446,39 @@ Show the disassembler options."), NULL, show_disassembler_options_sfunc, &setlist, &showlist); set_cmd_completer (set_show_disas_opts.set, disassembler_options_completer); + + + /* All the 'maint set|show libopcodes-styling' sub-commands. */ + static struct cmd_list_element *maint_set_libopcodes_styling_cmdlist; + static struct cmd_list_element *maint_show_libopcodes_styling_cmdlist; + + /* Adds 'maint set|show libopcodes-styling'. */ + add_setshow_prefix_cmd ("libopcodes-styling", class_maintenance, + _("Set libopcodes-styling specific variables."), + _("Show libopcodes-styling specific variables."), + &maint_set_libopcodes_styling_cmdlist, + &maint_show_libopcodes_styling_cmdlist, + &maintenance_set_cmdlist, + &maintenance_show_cmdlist); + + /* Adds 'maint set|show gnu-source-highlight enabled'. */ + add_setshow_boolean_cmd ("enabled", class_maintenance, + &use_libopcodes_styling_option, _("\ +Set whether the libopcodes styling support should be used."), _("\ +Show whether the libopcodes styling support should be used."),_("\ +When enabled, GDB will try to make use of the builtin libopcodes styling\n\ +support, to style the disassembler output. Not every architecture has\n\ +styling support within libopcodes, so enabling this is not a guarantee\n\ +that libopcodes styling will be available.\n\ +\n\ +When this option is disabled, GDB will make use of the Python Pygments\n\ +package (if available) to style the disassembler output.\n\ +\n\ +All disassembler styling can be disabled with:\n\ +\n\ + set style disassembler enabled off"), + set_use_libopcodes_styling, + show_use_libopcodes_styling, + &maint_set_libopcodes_styling_cmdlist, + &maint_show_libopcodes_styling_cmdlist); } diff --git a/gdb/disasm.h b/gdb/disasm.h index 54176eb095a..2921d537e0a 100644 --- a/gdb/disasm.h +++ b/gdb/disasm.h @@ -138,6 +138,16 @@ struct gdb_printing_disassembler : public gdb_disassemble_info const char *format, ...) ATTRIBUTE_PRINTF(3,4); + /* Return true if the disassembler is considered inside a comment, false + otherwise. */ + bool in_comment_p () const + { return m_in_comment; } + + /* Set whether the disassembler should be considered as within comment + text or not. */ + void set_in_comment (bool c) + { m_in_comment = c; } + private: /* When libopcodes calls the fprintf_func and fprintf_styled_func @@ -151,6 +161,12 @@ struct gdb_printing_disassembler : public gdb_disassemble_info /* The stream to which output should be sent. */ struct ui_file *m_stream; + + /* Are we inside a comment? This will be set true if the disassembler + uses styled output and emits a start of comment character. It is up + to the code that uses this disassembler class to reset this flag back + to false at a suitable time (e.g. at the end of every line). */ + bool m_in_comment; }; /* A basic disassembler that doesn't actually print anything. */ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 2178b476f53..ab838bc41b3 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -26478,6 +26478,7 @@ @item show style sources Show the current state of source code styling. +@anchor{style_disassembler_enabled} @item set style disassembler enabled @samp{on|off} Enable or disable disassembler styling. This affects whether disassembler output, such as the output of the @code{disassemble} @@ -26485,13 +26486,32 @@ general is enabled (with @code{set style enabled on}), and if a source highlighting library is available to @value{GDBN}. -To highlight disassembler output, @value{GDBN} must be compiled with -Python support, and the Python Pygments package must be available. If -these requirements are not met then @value{GDBN} will not highlight -disassembler output, even when this option is @samp{on}. +The two source highlighting libraries that @value{GDBN} could use to +style disassembler output are; @value{GDBN}'s builtin disassembler, or +the Python Pygments package. + +@value{GDBN}'s first choice will be to use the builtin disassembler +for styling, this usually provides better results, being able to style +different types of instruction operands differently. However, the +builtin disassembler is not able to style all architectures. + +For architectures that the builtin disassembler is unable to style, +@value{GDBN} will fall back to use the Python Pygments package where +possible. In order to use the Python Pygments package, @value{GDBN} +must be built with Python support, and the Pygments package must be +installed. + +If neither of these options are available then @value{GDBN} will +produce unstyled disassembler output, even when this setting is +@samp{on}. + +To discover if the current architecture supports styling using the +builtin disassembler library see @ref{maint_libopcodes_styling,,@kbd{maint +show libopcodes-styling enabled}}. @item show style disassembler enabled Show the current state of disassembler styling. + @end table Subcommands of @code{set style} control specific forms of styling. @@ -26536,6 +26556,11 @@ @code{set style function} family of commands. By default, this style's foreground color is yellow. +This style is also used for symbol names in styled disassembler output +if @value{GDBN} is using its builtin disassembler library for styling +(@pxref{style_disassembler_enabled,,@kbd{set style disassembler +enabled}}). + @item variable Control the styling of variable names. These are managed with the @code{set style variable} family of commands. By default, this style's @@ -26546,6 +26571,11 @@ @code{set style address} family of commands. By default, this style's foreground color is blue. +This style is also used for addresses in styled disassembler output +if @value{GDBN} is using its builtin disassembler library for styling +(@pxref{style_disassembler_enabled,,@kbd{set style disassembler +enabled}}). + @item version Control the styling of @value{GDBN}'s version number text. By default, this style's foreground color is magenta and it has bold @@ -26594,6 +26624,47 @@ Control the styling of the active TUI border; that is, the TUI window that has the focus. +@item disassembler comment +Control the styling of comments in the disassembler output. These are +managed with the @code{set style disassembler comment} family of +commands. This style is only used when @value{GDBN} is styling using +its builtin disassembler library +(@pxref{style_disassembler_enabled,,@kbd{set style disassembler +enabled}}). By default, this style's intensity is dim, and its +foreground color is white. + +@item disassembler immediate +Control the styling of numeric operands in the disassembler output. +These are managed with the @code{set style disassembler immediate} +family of commands. This style is not used for instruction operands +that represent addresses, in that case the @samp{disassembler address} +style is used. This style is only used when @value{GDBN} is styling +using its builtin disassembler library. By default, this style's +foreground color is blue. + +@item disassembler address +Control the styling of address operands in the disassembler output. +This is an alias for the @samp{address} style. + +@item disassembler symbol +Control the styling of symbol names in the disassembler output. This +is an alias for the @samp{function} style. + +@item disassembler mnemonic +Control the styling of instruction mnemonics in the disassembler +output. These are managed with the @code{set style disassembler +mnemonic} family of commands. This style is also used for assembler +directives, e.g.@: @code{.byte}, @code{.word}, etc. This style is +only used when @value{GDBN} is styling using its builtin disassembler +library. By default, this style's foreground color is green. + +@item disassembler register +Control the styling of register operands in the disassembler output. +These are managed with the @code{set style disassembler register} +family of commands. This style is only used when @value{GDBN} is +styling using its builtin disassembler library. By default, this style's +foreground color is red. + @end table @node Numbers @@ -40365,6 +40436,29 @@ library when @value{GDBN} is linked against the GNU Source Highlight library. +@anchor{maint_libopcodes_styling} +@kindex maint set libopcodes-styling enabled +@kindex maint show libopcodes-styling enabled +@item maint set libopcodes-styling enabled @r{[}on|off@r{]} +@itemx maint show libopcodes-styling enabled +Control whether @value{GDBN} should use its builtin disassembler +(@file{libopcodes}) to style disassembler output (@pxref{Output +Styling}). The builtin disassembler does not support styling for all +architectures. + +When this option is @samp{off} the builtin disassembler will not be +used for styling, @value{GDBN} will fall back to using the Python +Pygments package if possible. + +Trying to set this option @samp{on} for an architecture that the +builtin disassembler is unable to style will give an error, otherwise, +the builtin disassembler will be used to style disassembler output. + +This option is @samp{on} by default for supported architectures. + +This option is useful for debugging @value{GDBN}'s use of the Pygments +library when @value{GDBN} is built for an architecture that supports +styling with the builtin disassembler @kindex maint space @cindex memory used by commands @item maint space @var{value} ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gdb: add support for disassembler styling using libopcodes 2022-06-30 12:22 ` Andrew Burgess @ 2022-06-30 13:46 ` Eli Zaretskii 2022-07-04 10:15 ` Andrew Burgess 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2022-06-30 13:46 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches > From: Andrew Burgess <aburgess@redhat.com> > Cc: gdb-patches@sourceware.org > Date: Thu, 30 Jun 2022 13:22:27 +0100 > > Thanks for your feedback. The updated patch below I believe addresses > all these issues. It does, thanks. One last nit: > + disasm_mnemonic_style.add_setshow_commands (no_class, _("\ > +Disassembler mnemonic display styling.\n\ > +Configure the colors and display intensity for instruction mnemonics\n\ > +in the disassembler output. The \"disassembler mnemonic\" style is\n\ > +used to displaying instruction mnemonics as well as any assembler\n\ ^^^^^^^^^^^^^ Either "to display" or "for displaying". ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gdb: add support for disassembler styling using libopcodes 2022-06-30 13:46 ` Eli Zaretskii @ 2022-07-04 10:15 ` Andrew Burgess 0 siblings, 0 replies; 15+ messages in thread From: Andrew Burgess @ 2022-07-04 10:15 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes: >> From: Andrew Burgess <aburgess@redhat.com> >> Cc: gdb-patches@sourceware.org >> Date: Thu, 30 Jun 2022 13:22:27 +0100 >> >> Thanks for your feedback. The updated patch below I believe addresses >> all these issues. > > It does, thanks. One last nit: > >> + disasm_mnemonic_style.add_setshow_commands (no_class, _("\ >> +Disassembler mnemonic display styling.\n\ >> +Configure the colors and display intensity for instruction mnemonics\n\ >> +in the disassembler output. The \"disassembler mnemonic\" style is\n\ >> +used to displaying instruction mnemonics as well as any assembler\n\ > ^^^^^^^^^^^^^ > Either "to display" or "for displaying". Thanks. This fix will be included in the final version of this patch. Andrew ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gdb: add support for disassembler styling using libopcodes 2022-06-17 10:36 ` [PATCH 2/2] gdb: add support for disassembler styling using libopcodes Andrew Burgess 2022-06-17 11:16 ` Eli Zaretskii @ 2022-07-20 2:09 ` Simon Marchi 2022-07-20 13:14 ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess 1 sibling, 1 reply; 15+ messages in thread From: Simon Marchi @ 2022-07-20 2:09 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On 2022-06-17 06:36, Andrew Burgess via Gdb-patches wrote: > This commit extends GDB to make use of libopcodes styling support > where available, currently this is just i386 based architectures, and > RISC-V. > > For architectures that don't support styling using libopcodes GDB will > fall back to using the Python Pygments package, when the package is > available. > > The new libopcodes based styling has the disassembler identify parts > of the disassembled instruction, e.g. registers, immediates, > mnemonics, etc, and can style these components differently. > Additionally, as the styling is now done in GDB we can add settings to > allow the user to configure which colours are used right from the GDB > CLI. > > There's some new maintenance commands: > > maintenance set libopcodes-styling enabled on|off > maintenance show libopcodes-styling > > These can be used to manually disable use of libopcodes styling. This > is a maintenance command as it's not anticipated that a user should > need to do this. But, this could be useful for testing, or, in some > rare cases, a user might want to override the Python hook used for > disassembler styling, and then disable libopcode styling so that GDB > falls back to using Python. Right now I would consider this second > use case a rare situation, which is why I think a maintenance command > is appropriate. > > When libopcodes is being used for styling then the user can make use > of the following new styles: > > set/show style disassembler comment > set/show style disassembler immediate > set/show style disassembler mnemonic > set/show style disassembler register > > The disassembler also makes use of the 'address' and 'function' > styles to style some parts of the disassembler output. I have also > added the following aliases though: > > set/show style disassembler address > set/show style disassembler symbol > > these are aliases for: > > set/show style address > set/show style function > > respectively, and exist to make it easier for users to discover > disassembler related style settings. The 'address' style is used to > style numeric addresses in the disassembler output, while the 'symbol' > or 'function' style is used to style the names of symbols in > disassembler output. > > As not every architecture supports libopcodes styling, the maintenance > setting 'libopcodes-styling enabled' has an "auto-off" type behaviour. > Consider this GDB session: > > (gdb) show architecture > The target architecture is set to "auto" (currently "i386:x86-64"). > (gdb) maintenance show libopcodes-styling enabled > Use of libopcodes styling support is "on". > > the setting defaults to "on" for architectures that support libopcodes > based styling. > > (gdb) set architecture sparc > The target architecture is set to "sparc". > (gdb) maintenance show libopcodes-styling enabled > Use of libopcodes styling support is "off" (not supported on architecture "sparc") > > the setting will show as "off" if the user switches to an architecture > that doesn't support libopcodes styling. The underlying setting is > still "on" at this point though, if the user switches back to > i386:x86-64 then the setting would go back to being "on". > > (gdb) maintenance set libopcodes-styling enabled off > (gdb) maintenance show libopcodes-styling enabled > Use of libopcodes styling support is "off". > > now the setting is "off" for everyone, even if the user switches back > to i386:x86-64 the setting will still show as "off". > > (gdb) maintenance set libopcodes-styling enabled on > Use of libopcodes styling not supported on architecture "sparc". > (gdb) maintenance show libopcodes-styling enabled > Use of libopcodes styling support is "off". > > attempting to switch the setting "on" for an unsupported architecture > will give an error, and the setting will remain "off". > > (gdb) set architecture auto > The target architecture is set to "auto" (currently "i386:x86-64"). > (gdb) maintenance show libopcodes-styling enabled > Use of libopcodes styling support is "off". > (gdb) maintenance set libopcodes-styling enabled on > (gdb) maintenance show libopcodes-styling enabled > Use of libopcodes styling support is "on". > > the user will need to switch back to a supported architecture before > they can one again turn this setting "on". > --- > gdb/NEWS | 28 +++++++ > gdb/cli/cli-style.c | 107 +++++++++++++++++++++++--- > gdb/cli/cli-style.h | 28 +++++-- > gdb/disasm.c | 184 ++++++++++++++++++++++++++++++++++++++++++-- > gdb/disasm.h | 16 ++++ > gdb/doc/gdb.texinfo | 100 +++++++++++++++++++++++- > 6 files changed, 437 insertions(+), 26 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index ac9a1aacd34..34a491edd92 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -27,6 +27,12 @@ > emit to indicate where a breakpoint should be placed to break in a function > past its prologue. > > +* Disassembler styling using libopcodes. GDB now supports > + disassembler styling using libopcodes. This is only available for > + some targets (currently x86 and RISC-V). For unsupported targets > + Python Pygments is still used. For supported targets, libopcodes > + styling is used by default. > + > * New commands > > maintenance set ignore-prologue-end-flag on|off > @@ -36,6 +42,28 @@ maintenance show ignore-prologue-end-flag > used to force GDB to use prologue analyzers if the line-table is constructed > from erroneous debug information. > > +maintenance set libopcodes-styling on|off > +maintenance show libopcodes-styling > + These can be used to force off libopcodes based styling, the Python > + Pygments styling will then be used instead. > + > +set style disassembler comment > +show style disassembler comment > +set style disassembler immediate > +show style disassembler immediate > +set style disassembler mnemonic > +show style disassembler mnemonic > +set style disassembler register > +show style disassembler register > +set style disassembler address > +show style disassembler address > +set style disassembler symbol > +show style disassembler symbol > + For targets that support libopcodes based styling, these settings > + control how various aspects of the disassembler output are styled. > + The 'disassembler address' and 'disassembler symbol' styles are > + aliases for the 'address' and 'function' styles respectively. > + > * Changed commands > > maintenance info line-table > diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c > index 3fd85f4aa86..2ff1b5815e3 100644 > --- a/gdb/cli/cli-style.c > +++ b/gdb/cli/cli-style.c > @@ -110,6 +110,23 @@ cli_style_option version_style ("version", ui_file_style::MAGENTA, > > /* See cli-style.h. */ > > +cli_style_option disasm_mnemonic_style ("mnemonic", ui_file_style::GREEN); > + > +/* See cli-style.h. */ > + > +cli_style_option disasm_register_style ("register", ui_file_style::RED); > + > +/* See cli-style.h. */ > + > +cli_style_option disasm_immediate_style ("immediate", ui_file_style::BLUE); > + > +/* See cli-style.h. */ > + > +cli_style_option disasm_comment_style ("comment", ui_file_style::WHITE, > + ui_file_style::DIM); > + > +/* See cli-style.h. */ > + > cli_style_option::cli_style_option (const char *name, > ui_file_style::basic_color fg, > ui_file_style::intensity intensity) > @@ -224,15 +241,16 @@ cli_style_option::do_show_intensity (struct ui_file *file, int from_tty, > > /* See cli-style.h. */ > > -void > +set_show_commands > cli_style_option::add_setshow_commands (enum command_class theclass, > const char *prefix_doc, > struct cmd_list_element **set_list, > struct cmd_list_element **show_list, > bool skip_intensity) > { > - add_setshow_prefix_cmd (m_name, theclass, prefix_doc, prefix_doc, > - &m_set_list, &m_show_list, set_list, show_list); > + set_show_commands prefix_cmds > + = add_setshow_prefix_cmd (m_name, theclass, prefix_doc, prefix_doc, > + &m_set_list, &m_show_list, set_list, show_list); > > set_show_commands commands; > > @@ -274,6 +292,8 @@ cli_style_option::add_setshow_commands (enum command_class theclass, > commands.set->set_context (this); > commands.show->set_context (this); > } > + > + return prefix_cmds; > } > > static cmd_list_element *style_set_list; > @@ -387,11 +407,13 @@ Configure filename colors and display intensity."), > &style_set_list, &style_show_list, > false); > > - function_name_style.add_setshow_commands (no_class, _("\ > + set_show_commands function_prefix_cmds > + = function_name_style.add_setshow_commands (no_class, _("\ > Function name display styling.\n\ > Configure function name colors and display intensity"), > - &style_set_list, &style_show_list, > - false); > + &style_set_list, > + &style_show_list, > + false); > > variable_name_style.add_setshow_commands (no_class, _("\ > Variable name display styling.\n\ > @@ -399,11 +421,12 @@ Configure variable name colors and display intensity"), > &style_set_list, &style_show_list, > false); > > - address_style.add_setshow_commands (no_class, _("\ > + set_show_commands address_prefix_cmds > + = address_style.add_setshow_commands (no_class, _("\ > Address display styling.\n\ > Configure address colors and display intensity"), > - &style_set_list, &style_show_list, > - false); > + &style_set_list, &style_show_list, > + false); > > title_style.add_setshow_commands (no_class, _("\ > Title display styling.\n\ > @@ -451,4 +474,70 @@ Version string display styling.\n\ > Configure colors used to display the GDB version string."), > &style_set_list, &style_show_list, > false); > + > + disasm_mnemonic_style.add_setshow_commands (no_class, _("\ > +Disassembler mnemonic display styling.\n\ > +Configure the colors and display intensity for instruction mnemonics\n\ > +in the disassembler output. The \"disassembler mnemonic\" style is\n\ > +used to displaying instruction mnemonics as well as any assembler\n\ > +directives, e.g. \".byte\", \".word\", etc.\n\ > +\n\ > +This style will only be used for targets that support libopcodes based\n\ > +disassembler styling. When Python Pygments based styling is used\n\ > +then this style has no effect."), > + &style_disasm_set_list, > + &style_disasm_show_list, > + false); > + > + disasm_register_style.add_setshow_commands (no_class, _("\ > +Disassembler register display styling.\n\ > +Configure the colors and display intensity for registers in the\n\ > +disassembler output.\n\ > +\n\ > +This style will only be used for targets that support libopcodes based\n\ > +disassembler styling. When Python Pygments based styling is used\n\ > +then this style has no effect."), > + &style_disasm_set_list, > + &style_disasm_show_list, > + false); > + > + disasm_immediate_style.add_setshow_commands (no_class, _("\ > +Disassembler immediate display styling.\n\ > +Configure the colors and display intensity for immediates in the\n\ > +disassembler output. The \"disassembler immediate\" style is used for\n\ > +any number that is not an address, this includes constants in arithmetic\n\ > +instructions, as well as address offsets in memory access instructions.\n\ > +\n\ > +This style will only be used for targets that support libopcodes based\n\ > +disassembler styling. When Python Pygments based styling is used\n\ > +then this style has no effect."), > + &style_disasm_set_list, > + &style_disasm_show_list, > + false); > + > + disasm_comment_style.add_setshow_commands (no_class, _("\ > +Disassembler comment display styling.\n\ > +Configure the colors and display intensity for comments in the\n\ > +disassembler output. The \"disassembler comment\" style is used for\n\ > +the comment character, and everything after the comment character up to\n\ > +the end of the line. The comment style overrides any other styling,\n\ > +e.g. a register name in a comment will use the comment styling.\n\ > +\n\ > +This style will only be used for targets that support libopcodes based\n\ > +disassembler styling. When Python Pygments based styling is used\n\ > +then this style has no effect."), > + &style_disasm_set_list, > + &style_disasm_show_list, > + false); > + > + /* Setup 'disassembler address' style and 'disassembler symbol' style, > + these are aliases for 'address' and 'function' styles respectively. */ > + add_alias_cmd ("address", address_prefix_cmds.set, no_class, 0, > + &style_disasm_set_list); > + add_alias_cmd ("address", address_prefix_cmds.show, no_class, 0, > + &style_disasm_show_list); > + add_alias_cmd ("symbol", function_prefix_cmds.set, no_class, 0, > + &style_disasm_set_list); > + add_alias_cmd ("symbol", function_prefix_cmds.show, no_class, 0, > + &style_disasm_show_list); > } > diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h > index f69df47098c..4090cf01333 100644 > --- a/gdb/cli/cli-style.h > +++ b/gdb/cli/cli-style.h > @@ -43,12 +43,13 @@ class cli_style_option > /* Return the style name. */ > const char *name () { return m_name; }; > > - /* Call once to register this CLI style with the CLI engine. */ > - void add_setshow_commands (enum command_class theclass, > - const char *prefix_doc, > - struct cmd_list_element **set_list, > - struct cmd_list_element **show_list, > - bool skip_intensity); > + /* Call once to register this CLI style with the CLI engine. Returns > + the set/show prefix commands for the style. */ > + set_show_commands add_setshow_commands (enum command_class theclass, > + const char *prefix_doc, > + struct cmd_list_element **set_list, > + struct cmd_list_element **show_list, > + bool skip_intensity); > > /* Return the 'set style NAME' command list, that can be used > to build a lambda DO_SET to call add_setshow_commands. */ > @@ -116,6 +117,21 @@ extern cli_style_option title_style; > /* The metadata style. */ > extern cli_style_option metadata_style; > > +/* The disassembler style for mnemonics or assembler directives > + (e.g. '.byte', etc). */ > +extern cli_style_option disasm_mnemonic_style; > + > +/* The disassembler style for register names. */ > +extern cli_style_option disasm_register_style; > + > +/* The disassembler style for numerical values that are not addresses, this > + includes immediate operands (e.g. in, an add instruction), but also > + address offsets (e.g. in a load instruction). */ > +extern cli_style_option disasm_immediate_style; > + > +/* The disassembler style for comments. */ > +extern cli_style_option disasm_comment_style; > + > /* The border style of a TUI window that does not have the focus. */ > extern cli_style_option tui_border_style; > > diff --git a/gdb/disasm.c b/gdb/disasm.c > index 6c64c14feee..cf27a32fbe7 100644 > --- a/gdb/disasm.c > +++ b/gdb/disasm.c > @@ -41,6 +41,63 @@ > which is set by the "set disassembler_options" command. */ > static std::string prospective_options; > > +/* When this is true we will try to use libopcodes to provide styling to > + the disassembler output. */ > + > +static bool use_libopcodes_styling = true; > + > +/* To support the set_use_libopcodes_styling function we have a second > + variable which is connected to the actual set/show option. */ > + > +static bool use_libopcodes_styling_option = use_libopcodes_styling; > + > +/* The "maint show libopcodes-styling enabled" command. */ > + > +static void > +show_use_libopcodes_styling (struct ui_file *file, int from_tty, > + struct cmd_list_element *c, > + const char *value) > +{ > + gdb_non_printing_memory_disassembler dis (target_gdbarch ()); > + bool supported = dis.disasm_info ()->created_styled_output; > + > + if (supported || !use_libopcodes_styling) > + gdb_printf (file, _("Use of libopcodes styling support is \"%s\".\n"), > + value); > + else > + { > + /* Use of libopcodes styling is not supported, and the user has this > + turned on! */ > + gdb_printf (file, _("Use of libopcodes styling support is \"off\"" > + " (not supported on architecture \"%s\")\n"), > + gdbarch_bfd_arch_info (target_gdbarch ())->printable_name); > + } > +} > + > +/* The "maint set libopcodes-styling enabled" command. */ > + > +static void > +set_use_libopcodes_styling (const char *args, int from_tty, > + struct cmd_list_element *c) > +{ > + gdb_non_printing_memory_disassembler dis (target_gdbarch ()); > + bool supported = dis.disasm_info ()->created_styled_output; > + > + /* If the current architecture doesn't support libopcodes styling then we > + give an error here, but leave the underlying setting enabled. This > + means that if the user switches to an architecture that does support > + libopcodes styling the setting will be enabled. */ > + > + if (use_libopcodes_styling_option && !supported) > + { > + use_libopcodes_styling_option = use_libopcodes_styling; > + error (_("Use of libopcodes styling not supported on architecture \"%s\"."), > + gdbarch_bfd_arch_info (target_gdbarch ())->printable_name); > + } > + else > + use_libopcodes_styling = use_libopcodes_styling_option; > +} > + > /* This structure is used to store line number information for the > deprecated /m option. > We need a different sort of line table from the normal one cuz we can't > @@ -160,7 +217,26 @@ gdb_disassembler::dis_asm_print_address (bfd_vma addr, > gdb_disassembler *self > = static_cast<gdb_disassembler *>(info->application_data); > > - print_address (self->arch (), addr, self->stream ()); > + if (self->in_comment_p ()) > + { > + /* Calling 'print_address' might add styling to the output (based on > + the properties of the stream we're writing too). This is usually > + fine, but if we are in an assembler comment then we'd prefer to > + have the comment style, rather than the default address style. > + > + Print the address into a temporary buffer which doesn't support > + styling, then reprint this unstyled address with the default text > + style. > + > + As we are inside a comment right now, the standard print routine > + will ensure that the comment is printed to the user with a > + suitable comment style. */ > + string_file tmp; > + print_address (self->arch (), addr, &tmp); > + self->fprintf_styled_func (self, dis_style_text, "%s", tmp.c_str ()); > + } > + else > + print_address (self->arch (), addr, self->stream ()); > } > > /* See disasm.h. */ > @@ -202,11 +278,54 @@ gdb_printing_disassembler::fprintf_styled_func (void *dis_info, > const char *format, ...) > { > ui_file *stream = stream_from_gdb_disassemble_info (dis_info); > + gdb_printing_disassembler *dis = (gdb_printing_disassembler *) dis_info; > > va_list args; > va_start (args, format); > - gdb_vprintf (stream, format, args); > + std::string content = string_vprintf (format, args); > va_end (args); > + > + /* Once in a comment then everything should be styled as a comment. */ > + if (style == dis_style_comment_start) > + dis->set_in_comment (true); > + if (dis->in_comment_p ()) > + style = dis_style_comment_start; > + > + /* Now print the content with the correct style. */ > + const char *txt = content.c_str (); > + switch (style) > + { > + case dis_style_mnemonic: > + case dis_style_assembler_directive: > + fputs_styled (txt, disasm_mnemonic_style.style (), stream); > + break; > + > + case dis_style_register: > + fputs_styled (txt, disasm_register_style.style (), stream); > + break; > + > + case dis_style_immediate: > + case dis_style_address_offset: > + fputs_styled (txt, disasm_immediate_style.style (), stream); > + break; > + > + case dis_style_address: > + fputs_styled (txt, address_style.style (), stream); > + break; > + > + case dis_style_symbol: > + fputs_styled (txt, function_name_style.style (), stream); > + break; > + > + case dis_style_comment_start: > + fputs_styled (txt, disasm_comment_style.style (), stream); > + break; > + > + case dis_style_text: > + gdb_puts (txt, stream); > + break; > + } > + > /* Something non -ve. */ > return 0; > } > @@ -818,7 +937,20 @@ gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch, > read_memory_ftype func) > : gdb_printing_disassembler (gdbarch, &m_buffer, func, > dis_asm_memory_error, dis_asm_print_address), > - m_buffer (!use_ext_lang_colorization_p && disassembler_styling > + /* The use of m_di.created_styled_output here is a bit of a cheat, but > + it works fine for now. Currently, for all targets that support > + libopcodes styling, this field is set during the call to > + disassemble_init_for_target, which was called as part of the > + initialization of gdb_printing_disassembler. And so, we are able to > + spot if a target supports libopcodes styling, and create m_buffer in > + the correct styling mode. > + > + If there's ever a target that only sets created_styled_output during > + the actual disassemble phase, then the logic here will have to > + change. */ > + m_buffer ((!use_ext_lang_colorization_p > + || (use_libopcodes_styling && m_di.created_styled_output)) > + && disassembler_styling > && file->can_emit_style_escape ()), > m_dest (file) > { /* Nothing. */ } > @@ -903,14 +1035,17 @@ gdb_disassembler::print_insn (CORE_ADDR memaddr, > { > m_err_memaddr.reset (); > m_buffer.clear (); > + this->set_in_comment (false); > > int length = gdb_print_insn_1 (arch (), memaddr, &m_di); > > - /* If we have successfully disassembled an instruction, styling is on, we > - think that the extension language might be able to perform styling for > - us, and the destination can support styling, then lets call into the > - extension languages in order to style this output. */ > + /* If we have successfully disassembled an instruction, disassembler > + styling using the extension language is on, and libopcodes hasn't > + already styled the output for us, and, if the destination can support > + styling, then lets call into the extension languages in order to style > + this output. */ > if (length > 0 && disassembler_styling > + && (!m_di.created_styled_output || !use_libopcodes_styling) > && use_ext_lang_colorization_p > && m_dest->can_emit_style_escape ()) > { > @@ -1311,4 +1446,39 @@ Show the disassembler options."), NULL, > show_disassembler_options_sfunc, > &setlist, &showlist); > set_cmd_completer (set_show_disas_opts.set, disassembler_options_completer); > + > + > + /* All the 'maint set|show libopcodes-styling' sub-commands. */ > + static struct cmd_list_element *maint_set_libopcodes_styling_cmdlist; > + static struct cmd_list_element *maint_show_libopcodes_styling_cmdlist; > + > + /* Adds 'maint set|show libopcodes-styling'. */ > + add_setshow_prefix_cmd ("libopcodes-styling", class_maintenance, > + _("Set libopcodes-styling specific variables."), > + _("Show libopcodes-styling specific variables."), > + &maint_set_libopcodes_styling_cmdlist, > + &maint_show_libopcodes_styling_cmdlist, > + &maintenance_set_cmdlist, > + &maintenance_show_cmdlist); > + > + /* Adds 'maint set|show gnu-source-highlight enabled'. */ > + add_setshow_boolean_cmd ("enabled", class_maintenance, > + &use_libopcodes_styling_option, _("\ > +Set whether the libopcodes styling support should be used."), _("\ > +Show whether the libopcodes styling support should be used."),_("\ > +When enabled, GDB will try to make use of the builtin libopcodes styling\n\ > +support, to style the disassembler output. Not every architecture has\n\ > +styling support within libopcodes, so enabling this is not a guarantee\n\ > +that libopcodes styling will be available.\n\ > +\n\ > +When this option is disabled, GDB will make use of the Python Pygments\n\ > +package (if available) to style the disassembler output.\n\ > +\n\ > +All disassembler styling can be disabled with:\n\ > +\n\ > + set style disassembler enabled off"), > + set_use_libopcodes_styling, > + show_use_libopcodes_styling, > + &maint_set_libopcodes_styling_cmdlist, > + &maint_show_libopcodes_styling_cmdlist); > } > diff --git a/gdb/disasm.h b/gdb/disasm.h > index 54176eb095a..2921d537e0a 100644 > --- a/gdb/disasm.h > +++ b/gdb/disasm.h > @@ -138,6 +138,16 @@ struct gdb_printing_disassembler : public gdb_disassemble_info > const char *format, ...) > ATTRIBUTE_PRINTF(3,4); > > + /* Return true if the disassembler is considered inside a comment, false > + otherwise. */ > + bool in_comment_p () const > + { return m_in_comment; } > + > + /* Set whether the disassembler should be considered as within comment > + text or not. */ > + void set_in_comment (bool c) > + { m_in_comment = c; } My GDB built with UBSan fails gdb.python/py-disasm.exp with: (gdb) PASS: gdb.python/py-disasm.exp: global_disassembler=GlobalPreInfoDisassembler: python add_global_disassembler(GlobalPreInfoDisassembler) disassemble main Dump of assembler code for function main: 0x0000555555555119 <+0>: push %rbp 0x000055555555511a <+1>: mov %rsp,%rbp 0x000055555555511d <+4>: nop /home/simark/src/binutils-gdb/gdb/disasm.h:144:12: runtime error: load of value 118, which is not a valid value for type 'bool' I tracked it to m_in_comment not being initialized, and some code calling in_comment_p without anything setting m_in_comment. The backtrace is: #0 __sanitizer::Die () at /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_termination.cpp:50 #1 0x00007f9a2220e8ad in __ubsan::__ubsan_handle_load_invalid_value_abort (Data=<optimized out>, Val=<optimized out>) at /usr/src/debug/gcc/libsanitizer/ubsan/ubsan_handlers.cpp:551 #2 0x000055d15b4fcb51 in gdb_printing_disassembler::in_comment_p (this=0x7ffde7aadc20) at /home/simark/src/binutils-gdb/gdb/disasm.h:144 #3 0x000055d15b4e9366 in gdb_printing_disassembler::fprintf_styled_func (dis_info=0x7ffde7aadc20, style=dis_style_mnemonic, format=0x55d15f8c4760 "%.*s") at /home/simark/src/binutils-gdb/gdb/disasm.c:291 #4 0x000055d15d6366d7 in i386_dis_printf (ins=0x7ffde7aad580, style=dis_style_mnemonic, fmt=0x55d15f8c4dc0 "%s%*s") at /home/simark/src/binutils-gdb/opcodes/i386-dis.c:9302 #5 0x000055d15d63b83e in print_insn (pc=93824992235806, info=0x7ffde7aadc28, intel_syntax=-1) at /home/simark/src/binutils-gdb/opcodes/i386-dis.c:9818 #6 0x000055d15d63c9aa in print_insn_i386 (pc=93824992235806, info=0x7ffde7aadc28) at /home/simark/src/binutils-gdb/opcodes/i386-dis.c:9924 #7 0x000055d15ab99628 in default_print_insn (memaddr=93824992235806, info=0x7ffde7aadc28) at /home/simark/src/binutils-gdb/gdb/arch-utils.c:1041 #8 0x000055d15bde7f1b in i386_print_insn (pc=93824992235806, info=0x7ffde7aadc28) at /home/simark/src/binutils-gdb/gdb/i386-tdep.c:4073 #9 0x000055d15abba930 in gdbarch_print_insn (gdbarch=0x621000192910, vma=93824992235806, info=0x7ffde7aadc28) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:3348 #10 0x000055d15c73ba57 in disasmpy_builtin_disassemble (self=0x7f9a14df3ab0, args=0x7f9a14d281c0, kw=0x0) at /home/simark/src/binutils-gdb/gdb/python/py-disasm.c:294 #11 0x00007f9a23155511 in ?? () from /usr/lib/libpython3.10.so.1.0 #12 0x00007f9a2314edeb in _PyObject_MakeTpCall () from /usr/lib/libpython3.10.so.1.0 #13 0x00007f9a2314a19a in _PyEval_EvalFrameDefault () from /usr/lib/libpython3.10.so.1.0 #14 0x00007f9a231559c9 in _PyFunction_Vectorcall () from /usr/lib/libpython3.10.so.1.0 I presume we just need to initialize it to false. It makes the test case pass here at least. Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API 2022-07-20 2:09 ` Simon Marchi @ 2022-07-20 13:14 ` Andrew Burgess 2022-07-20 13:14 ` [PATCH 1/2] gdb: fix use of uninitialised gdb_printing_disassembler::m_in_comment Andrew Burgess ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Andrew Burgess @ 2022-07-20 13:14 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess Simon, Thanks for finding this problem. The first patch here fixes the problem you found. While testing this first patch I found another issue, which is what the second patch fixes. I'll probably push these in a few days unless I get some objections. Let me know if you have any thoughts, Thanks, Andrew --- Andrew Burgess (2): gdb: fix use of uninitialised gdb_printing_disassembler::m_in_comment gdb/python: fix invalid use disassemble_info::stream gdb/disasm.h | 2 +- gdb/python/py-disasm.c | 2 +- gdb/testsuite/gdb.python/py-disasm.c | 8 +++++++- gdb/testsuite/gdb.python/py-disasm.exp | 22 ++++++++++++++-------- gdb/testsuite/gdb.python/py-disasm.py | 3 --- 5 files changed, 23 insertions(+), 14 deletions(-) -- 2.25.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] gdb: fix use of uninitialised gdb_printing_disassembler::m_in_comment 2022-07-20 13:14 ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess @ 2022-07-20 13:14 ` Andrew Burgess 2022-07-20 14:21 ` Simon Marchi 2022-07-20 13:14 ` [PATCH 2/2] gdb/python: fix invalid use disassemble_info::stream Andrew Burgess 2022-07-25 18:27 ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess 2 siblings, 1 reply; 15+ messages in thread From: Andrew Burgess @ 2022-07-20 13:14 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess Simon pointed out that gdb_printing_disassembler::m_in_comment can be used uninitialised by the Python disassembler API code. This issue was spotted when GDB was built with the undefined behaviour sanitizer, and causes the gdb.python/py-disasm.exp test to fail like this: (gdb) PASS: gdb.python/py-disasm.exp: global_disassembler=GlobalPreInfoDisassembler: python add_global_disassembler(GlobalPreInfoDisassembler) disassemble main Dump of assembler code for function main: 0x0000555555555119 <+0>: push %rbp 0x000055555555511a <+1>: mov %rsp,%rbp 0x000055555555511d <+4>: nop /home/user/src/binutils-gdb/gdb/disasm.h:144:12: runtime error: load of value 118, which is not a valid value for type 'bool' The problem is that in disasmpy_builtin_disassemble we create a new instance of gdbpy_disassembler, which is a sub-class of gdb_printing_disassembler, however, the m_in_comment field is never initialised. This commit fixes the issue by providing a default initialisation value for m_in_comment in disasm.h. As we only ever disassemble a single instruction in disasmpy_builtin_disassemble then we don't need to worry about reseting m_in_comment back to false after the single instruction has been disassembled. With this commit the above issue is resolved and gdb.python/py-disasm.exp now passes. --- gdb/disasm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdb/disasm.h b/gdb/disasm.h index 2921d537e0a..09cb3921767 100644 --- a/gdb/disasm.h +++ b/gdb/disasm.h @@ -166,7 +166,7 @@ struct gdb_printing_disassembler : public gdb_disassemble_info uses styled output and emits a start of comment character. It is up to the code that uses this disassembler class to reset this flag back to false at a suitable time (e.g. at the end of every line). */ - bool m_in_comment; + bool m_in_comment = false; }; /* A basic disassembler that doesn't actually print anything. */ -- 2.25.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] gdb: fix use of uninitialised gdb_printing_disassembler::m_in_comment 2022-07-20 13:14 ` [PATCH 1/2] gdb: fix use of uninitialised gdb_printing_disassembler::m_in_comment Andrew Burgess @ 2022-07-20 14:21 ` Simon Marchi 0 siblings, 0 replies; 15+ messages in thread From: Simon Marchi @ 2022-07-20 14:21 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On 2022-07-20 09:14, Andrew Burgess via Gdb-patches wrote: > Simon pointed out that gdb_printing_disassembler::m_in_comment can be > used uninitialised by the Python disassembler API code. This issue > was spotted when GDB was built with the undefined behaviour sanitizer, > and causes the gdb.python/py-disasm.exp test to fail like this: > > (gdb) PASS: gdb.python/py-disasm.exp: global_disassembler=GlobalPreInfoDisassembler: python add_global_disassembler(GlobalPreInfoDisassembler) > disassemble main > Dump of assembler code for function main: > 0x0000555555555119 <+0>: push %rbp > 0x000055555555511a <+1>: mov %rsp,%rbp > 0x000055555555511d <+4>: nop > /home/user/src/binutils-gdb/gdb/disasm.h:144:12: runtime error: load of value 118, which is not a valid value for type 'bool' > > The problem is that in disasmpy_builtin_disassemble we create a new > instance of gdbpy_disassembler, which is a sub-class of > gdb_printing_disassembler, however, the m_in_comment field is never > initialised. > > This commit fixes the issue by providing a default initialisation > value for m_in_comment in disasm.h. As we only ever disassemble a > single instruction in disasmpy_builtin_disassemble then we don't need > to worry about reseting m_in_comment back to false after the single > instruction has been disassembled. > > With this commit the above issue is resolved and > gdb.python/py-disasm.exp now passes. > --- > gdb/disasm.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gdb/disasm.h b/gdb/disasm.h > index 2921d537e0a..09cb3921767 100644 > --- a/gdb/disasm.h > +++ b/gdb/disasm.h > @@ -166,7 +166,7 @@ struct gdb_printing_disassembler : public gdb_disassemble_info > uses styled output and emits a start of comment character. It is up > to the code that uses this disassembler class to reset this flag back > to false at a suitable time (e.g. at the end of every line). */ > - bool m_in_comment; > + bool m_in_comment = false; > }; > > /* A basic disassembler that doesn't actually print anything. */ Thanks, LGTM. Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] gdb/python: fix invalid use disassemble_info::stream 2022-07-20 13:14 ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess 2022-07-20 13:14 ` [PATCH 1/2] gdb: fix use of uninitialised gdb_printing_disassembler::m_in_comment Andrew Burgess @ 2022-07-20 13:14 ` Andrew Burgess 2022-07-25 18:27 ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess 2 siblings, 0 replies; 15+ messages in thread From: Andrew Burgess @ 2022-07-20 13:14 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess After this commit: commit 81384924cdcc9eb2676dd9084b76845d7d0e0759 Date: Tue Apr 5 11:06:16 2022 +0100 gdb: have gdb_disassemble_info carry 'this' in its stream pointer The disassemble_info::stream field will no longer be a ui_file*. That commit failed to update one location in py-disasm.c though. While running some tests using the Python disassembler API, I triggered a call to gdbpy_disassembler::print_address_func, and, as I had compiled GDB with the undefined behaviour sanitizer, GDB crashed as the code currently (incorrectly) casts the stream field to be a ui_file*. In this commit I fix this error. In order to test this case I had to tweak the existing test case a little. I also spotted some debug printf statements in py-disasm.py, which I have removed. --- gdb/python/py-disasm.c | 2 +- gdb/testsuite/gdb.python/py-disasm.c | 8 +++++++- gdb/testsuite/gdb.python/py-disasm.exp | 22 ++++++++++++++-------- gdb/testsuite/gdb.python/py-disasm.py | 3 --- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c index 4c78ca350c2..c37452fcf72 100644 --- a/gdb/python/py-disasm.c +++ b/gdb/python/py-disasm.c @@ -626,7 +626,7 @@ gdbpy_disassembler::print_address_func (bfd_vma addr, { gdbpy_disassembler *dis = static_cast<gdbpy_disassembler *> (info->application_data); - print_address (dis->arch (), addr, (struct ui_file *) info->stream); + print_address (dis->arch (), addr, dis->stream ()); } /* constructor. */ diff --git a/gdb/testsuite/gdb.python/py-disasm.c b/gdb/testsuite/gdb.python/py-disasm.c index ee0bb157f4d..e5c4d2f1d0e 100644 --- a/gdb/testsuite/gdb.python/py-disasm.c +++ b/gdb/testsuite/gdb.python/py-disasm.c @@ -16,10 +16,16 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */ int -main () +test () { asm ("nop"); asm ("nop"); /* Break here. */ asm ("nop"); return 0; } + +int +main () +{ + return test (); +} diff --git a/gdb/testsuite/gdb.python/py-disasm.exp b/gdb/testsuite/gdb.python/py-disasm.exp index 1b9cd4465ac..1f94d3e60f3 100644 --- a/gdb/testsuite/gdb.python/py-disasm.exp +++ b/gdb/testsuite/gdb.python/py-disasm.exp @@ -110,8 +110,8 @@ foreach plan $test_plans { gdb_test_no_output "python add_global_disassembler($global_disassembler_name)" } - # Disassemble main, and check the disassembler output. - gdb_test "disassemble main" $expected_pattern + # Disassemble test, and check the disassembler output. + gdb_test "disassemble test" $expected_pattern } } @@ -138,21 +138,21 @@ with_test_prefix "DisassemblerResult errors" { with_test_prefix "GLOBAL tagging disassembler" { py_remove_all_disassemblers gdb_test_no_output "python gdb.disassembler.register_disassembler(TaggingDisassembler(\"GLOBAL\"), None)" - gdb_test "disassemble main" "${base_pattern}\\s+## tag = GLOBAL\r\n.*" + gdb_test "disassemble test" "${base_pattern}\\s+## tag = GLOBAL\r\n.*" } # Now register an architecture specific disassembler, and check it # overrides the global disassembler. with_test_prefix "LOCAL tagging disassembler" { gdb_test_no_output "python gdb.disassembler.register_disassembler(TaggingDisassembler(\"LOCAL\"), \"${curr_arch}\")" - gdb_test "disassemble main" "${base_pattern}\\s+## tag = LOCAL\r\n.*" + gdb_test "disassemble test" "${base_pattern}\\s+## tag = LOCAL\r\n.*" } # Now remove the architecture specific disassembler, and check that # the global disassembler kicks back in. with_test_prefix "GLOBAL tagging disassembler again" { gdb_test_no_output "python gdb.disassembler.register_disassembler(None, \"${curr_arch}\")" - gdb_test "disassemble main" "${base_pattern}\\s+## tag = GLOBAL\r\n.*" + gdb_test "disassemble test" "${base_pattern}\\s+## tag = GLOBAL\r\n.*" } # Check that a DisassembleInfo becomes invalid after the call into the @@ -160,7 +160,7 @@ with_test_prefix "GLOBAL tagging disassembler again" { with_test_prefix "DisassembleInfo becomes invalid" { py_remove_all_disassemblers gdb_test_no_output "python add_global_disassembler(GlobalCachingDisassembler)" - gdb_test "disassemble main" "${base_pattern}\\s+## CACHED\r\n.*" + gdb_test "disassemble test" "${base_pattern}\\s+## CACHED\r\n.*" gdb_test "python GlobalCachingDisassembler.check()" "PASS" } @@ -168,10 +168,10 @@ with_test_prefix "DisassembleInfo becomes invalid" { with_test_prefix "memory source api" { py_remove_all_disassemblers gdb_test_no_output "python analyzing_disassembler = add_global_disassembler(AnalyzingDisassembler)" - gdb_test "disassemble main" "${base_pattern}\r\n.*" + gdb_test "disassemble test" "${base_pattern}\r\n.*" gdb_test "python analyzing_disassembler.find_replacement_candidate()" \ "Replace from $hex to $hex with NOP" - gdb_test "disassemble main" "${base_pattern}\r\n.*" \ + gdb_test "disassemble test" "${base_pattern}\r\n.*" \ "second disassembler pass" gdb_test "python analyzing_disassembler.check()" \ "PASS" @@ -196,6 +196,12 @@ with_test_prefix "maint info python-disassemblers" { "\[^\r\n\]+BuiltinDisassembler\\s+\\(Matches current architecture\\)" \ "GLOBAL\\s+BuiltinDisassembler"] \ "list disassemblers, multiple disassemblers registered" + + # Check that disassembling main (with the BuiltinDisassembler in + # place) doesn't cause GDB to crash. The hope is that + # disassembling main will result in a call to print_address, which + # is where the problem was. + gdb_test "disassemble main" ".*" } # Check the attempt to create a "new" DisassembleInfo object fails. diff --git a/gdb/testsuite/gdb.python/py-disasm.py b/gdb/testsuite/gdb.python/py-disasm.py index ff7ffdb97d9..3b9008b1c54 100644 --- a/gdb/testsuite/gdb.python/py-disasm.py +++ b/gdb/testsuite/gdb.python/py-disasm.py @@ -584,7 +584,6 @@ class AnalyzingDisassembler(Disassembler): if self._nop_index is None and result.string == "nop": self._nop_index = len(self._pass_1_length) # The offset in the following read_memory call defaults to 0. - print("APB: Reading nop bytes") self._nop_bytes = info.read_memory(result.length) # Record information about each instruction that is disassembled. @@ -661,8 +660,6 @@ class AnalyzingDisassembler(Disassembler): def check(self): """Call this after the second disassembler pass to validate the output.""" if self._check != self._pass_2_insn: - print("APB, Check : %s" % self._check) - print("APB, Result: %s" % self._pass_2_insn) raise gdb.GdbError("mismatch") print("PASS") -- 2.25.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API 2022-07-20 13:14 ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess 2022-07-20 13:14 ` [PATCH 1/2] gdb: fix use of uninitialised gdb_printing_disassembler::m_in_comment Andrew Burgess 2022-07-20 13:14 ` [PATCH 2/2] gdb/python: fix invalid use disassemble_info::stream Andrew Burgess @ 2022-07-25 18:27 ` Andrew Burgess 2 siblings, 0 replies; 15+ messages in thread From: Andrew Burgess @ 2022-07-25 18:27 UTC (permalink / raw) To: gdb-patches Andrew Burgess <aburgess@redhat.com> writes: > Simon, > > Thanks for finding this problem. The first patch here fixes the > problem you found. While testing this first patch I found another > issue, which is what the second patch fixes. > > I'll probably push these in a few days unless I get some objections. I've pushed both these patches now. Thanks, Andrew ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Use libopcodes disassembler styling with GDB 2022-06-17 10:36 [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess 2022-06-17 10:36 ` [PATCH 1/2] gdb: have gdb_disassemble_info carry 'this' in its stream pointer Andrew Burgess 2022-06-17 10:36 ` [PATCH 2/2] gdb: add support for disassembler styling using libopcodes Andrew Burgess @ 2022-07-04 10:17 ` Andrew Burgess 2022-07-11 14:18 ` Andrew Burgess 2 siblings, 1 reply; 15+ messages in thread From: Andrew Burgess @ 2022-07-04 10:17 UTC (permalink / raw) To: gdb-patches Ping! Unless somebody objects, I plan to push this series later this week. The libopcodes styling support is already in-tree (via binutils), and I feel the changes in these patches are a fairly obvious extension to GDB to make use of this additional information. As always, feedback is always welcome. Thanks, Andrew Andrew Burgess <aburgess@redhat.com> writes: > libopcodes is slowly gaining the ability to provide styling > information for its disassembler output. > > This series extends GDB to make use of this information where possible > (currently only i386 based architectures and RISC-V). > > --- > > Andrew Burgess (2): > gdb: have gdb_disassemble_info carry 'this' in its stream pointer > gdb: add support for disassembler styling using libopcodes > > gdb/NEWS | 28 ++++++ > gdb/cli/cli-style.c | 107 ++++++++++++++++++++-- > gdb/cli/cli-style.h | 28 ++++-- > gdb/disasm.c | 217 +++++++++++++++++++++++++++++++++++++++++--- > gdb/disasm.h | 81 ++++++++++++----- > gdb/doc/gdb.texinfo | 100 +++++++++++++++++++- > 6 files changed, 503 insertions(+), 58 deletions(-) > > -- > 2.25.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Use libopcodes disassembler styling with GDB 2022-07-04 10:17 ` [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess @ 2022-07-11 14:18 ` Andrew Burgess 0 siblings, 0 replies; 15+ messages in thread From: Andrew Burgess @ 2022-07-11 14:18 UTC (permalink / raw) To: gdb-patches Andrew Burgess <aburgess@redhat.com> writes: > Ping! > > Unless somebody objects, I plan to push this series later this week. > The libopcodes styling support is already in-tree (via binutils), and I > feel the changes in these patches are a fairly obvious extension to GDB > to make use of this additional information. I've now checked this in. Thanks, Andrew ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-07-25 18:27 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-17 10:36 [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess 2022-06-17 10:36 ` [PATCH 1/2] gdb: have gdb_disassemble_info carry 'this' in its stream pointer Andrew Burgess 2022-06-17 10:36 ` [PATCH 2/2] gdb: add support for disassembler styling using libopcodes Andrew Burgess 2022-06-17 11:16 ` Eli Zaretskii 2022-06-30 12:22 ` Andrew Burgess 2022-06-30 13:46 ` Eli Zaretskii 2022-07-04 10:15 ` Andrew Burgess 2022-07-20 2:09 ` Simon Marchi 2022-07-20 13:14 ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess 2022-07-20 13:14 ` [PATCH 1/2] gdb: fix use of uninitialised gdb_printing_disassembler::m_in_comment Andrew Burgess 2022-07-20 14:21 ` Simon Marchi 2022-07-20 13:14 ` [PATCH 2/2] gdb/python: fix invalid use disassemble_info::stream Andrew Burgess 2022-07-25 18:27 ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess 2022-07-04 10:17 ` [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess 2022-07-11 14:18 ` Andrew Burgess
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).