While working on another issue relating to GDB's use of the Python
Pygments package for disassembly styling I noticed an issue in the
case where the Pygments package raises an exception.
The intention of the current code is that, should the Pygments package
raise an exception, GDB will disable future attempts to call into the
Pygments code. This was intended to prevent repeated errors during
disassembly if, for some reason, the Pygments code isn't working.
Since the Pygments based styling was added, GDB now supports
disassembly styling using libopcodes, but this is only available for
some architectures. For architectures not covered by libopcodes
Pygments is still the only option.
What I observed is that, if I disable the libopcodes styling, then
setup GDB so that the Pygments based styling code will indicate an
error (by returning None), GDB does, as expected, stop using the
Pygments based styling. However, the libopcodes based styling will
instead be used, despite this feature having been disabled.
The problem is that the disassembler output is produced into a
string_file buffer. When we are using Pygments, this buffer is
created without styling support. However, when Pygments fails, we
recreate the buffer with styling support.
The problem is that we should only recreate the buffer with styling
support only if libopcodes styling is enabled. This was an oversight
in commit:
commit
4cbe4ca5da5cd7e1e6331ce11f024bf3c07b9744
Date: Mon Feb 14 14:40:52 2022 +0000
gdb: add support for disassembler styling using libopcodes
This commit:
1. Factors out some of the condition checking logic into two new
helper functions use_ext_lang_for_styling and
use_libopcodes_for_styling,
2. Reorders gdb_disassembler::m_buffer and gdb_disassembler::m_dest,
this allows these fields to be initialised m_dest first, which means
that the new condition checking functions can rely on m_dest being
set, even when called from the gdb_disassembler constructor,
3. Make use of the new condition checking functions each time
m_buffer is initialised,
4. Add a new test that forces the Python disassembler styling
function to return None, this will cause GDB to disable use of
Pygments for styling, and
5. When we reinitialise m_buffer, and re-disassemble the
instruction, call reset the in-comment flag. If the instruction
being disassembler ends in a comment then the first disassembly pass
will have set the in-comment flag to true. This shouldn't be a
problem as we will only be using Pygments, and thus performing a
re-disassembly pass, if libopcodes is disabled, so the in-comment
flag will never be checked, even if it is set incorrectly. However,
I think that having the flag set correctly is a good thing, even if
we don't check it (you never know what future uses might come up).
read_memory_ftype func)
: gdb_printing_disassembler (gdbarch, &m_buffer, func,
dis_asm_memory_error, dis_asm_print_address),
- /* 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)
+ m_dest (file),
+ m_buffer (!use_ext_lang_for_styling () && use_libopcodes_for_styling ())
{ /* Nothing. */ }
/* See disasm.h. */
+bool
+gdb_disassembler::use_ext_lang_for_styling () const
+{
+ /* The use of m_di.created_styled_output here is a bit of a cheat, but
+ it works fine for now.
+
+ This function is called in situations after m_di has been initialized,
+ but before the instruction has been disassembled.
+
+ Currently, every target that supports libopcodes styling sets the
+ created_styled_output field in disassemble_init_for_target, which was
+ called as part of the initialization of gdb_printing_disassembler.
+
+ This means that we are OK to check the created_styled_output field
+ here.
+
+ If, in the future, there's ever a target that only sets the
+ created_styled_output field during the actual instruction disassembly
+ phase, then we will need to update this code. */
+ return (disassembler_styling
+ && (!m_di.created_styled_output || !use_libopcodes_styling)
+ && use_ext_lang_colorization_p
+ && m_dest->can_emit_style_escape ());
+}
+
+/* See disasm.h. */
+
+bool
+gdb_disassembler::use_libopcodes_for_styling () const
+{
+ /* See the comment on the use of m_di.created_styled_output in the
+ gdb_disassembler::use_ext_lang_for_styling function. */
+ return (disassembler_styling
+ && m_di.created_styled_output
+ && use_libopcodes_styling
+ && m_dest->can_emit_style_escape ());
+}
+
+/* See disasm.h. */
+
gdb_disassemble_info::gdb_disassemble_info
(struct gdbarch *gdbarch,
read_memory_ftype read_memory_func, memory_error_ftype memory_error_func,
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 ())
+ if (length > 0 && use_ext_lang_for_styling ())
{
gdb::optional<std::string> ext_contents;
ext_contents = ext_lang_colorize_disasm (m_buffer.string (), arch ());
extension language for styling. */
use_ext_lang_colorization_p = false;
+ /* We're about to disassemble this instruction again, reset the
+ in-comment state. */
+ this->set_in_comment (false);
+
/* The instruction we just disassembled, and the extension
languages failed to style, might have otherwise had some
minimal styling applied by GDB. To regain that styling we
string_file>::value));
gdb_assert (!m_buffer.term_out ());
m_buffer.~string_file ();
- new (&m_buffer) string_file (true);
+ new (&m_buffer) string_file (use_libopcodes_for_styling ());
length = gdb_print_insn_1 (arch (), memaddr, &m_di);
gdb_assert (length > 0);
}
non-memory error. */
gdb::optional<CORE_ADDR> m_err_memaddr;
+ /* The stream to which disassembler output will be written. */
+ ui_file *m_dest;
+
/* Disassembler output is built up into this buffer. Whether this
string_file is created with styling support or not depends on the
value of use_ext_lang_colorization_p, as well as whether disassembler
styling or not. */
string_file m_buffer;
- /* The stream to which disassembler output will be written. */
- ui_file *m_dest;
-
/* When true, m_buffer will be created without styling support,
otherwise, m_buffer will be created with styling support.
struct disassemble_info *info);
static void dis_asm_print_address (bfd_vma addr,
struct disassemble_info *info);
+
+ /* Return true if we should use the extension language to apply
+ disassembler styling. This requires disassembler styling to be on
+ (i.e. 'set style disassembler enabled on'), the output stream needs to
+ support styling, and libopcode styling needs to be either off, or not
+ supported for the current architecture (libopcodes is used in
+ preference to the extension language method). */
+ bool use_ext_lang_for_styling () const;
+
+ /* Return true if we should use libopcodes to apply disassembler styling.
+ This requires disassembler styling to be on (i.e. 'set style
+ disassembler enabled on'), the output stream needs to support styling,
+ and libopcodes styling needs to be supported for the current
+ architecture, and not disabled by the user. */
+ bool use_libopcodes_for_styling () const;
};
/* An instruction to be disassembled. */
}
}
+# Disassemble a single isntruction at the start of main, then strip
+# off the address and symbol information, returning just the
+# disassembled instruction part.
+proc get_single_disassembled_insn {} {
+ set disasm_line [capture_command_output "x/1i *main" ""]
+ regexp "^\[^:\]+:\\s*(.*)$" $disasm_line whole_match insn
+ return $insn
+}
+
+# Check that, if the user is using Python Pygments for disassembler
+# styling, then the styling correctly switches off when an error is
+# detected in the Python code.
+proc test_disassembler_error_handling { } {
+
+ # This test requires the Python Pygments module to be installed
+ # and used by GDB.
+ if { !$::python_disassembly_styling } {
+ return
+ }
+
+ save_vars { env(TERM) } {
+ # We need an ANSI-capable terminal to get the output.
+ setenv TERM ansi
+
+ # Restart GDB with the correct TERM variable setting, this
+ # means that GDB will enable styling.
+ clean_restart_and_disable "restart 4" $::binfile
+
+ # Disable use of libopcodes for styling. As this function is
+ # only called when Python Pygments module is available, we
+ # should now be using that module to style the disassembler
+ # output.
+ gdb_test_no_output "maint set libopcodes-styling enabled off"
+
+ # Disassemble a single instruction and ensure that the output
+ # has styling markers in it.
+ set insn_before [get_single_disassembled_insn]
+ gdb_assert { [regexp "\033" $insn_before] } \
+ "have style markers when Pygments is working fine"
+
+ # Now replace the standard function that colorizes the
+ # disassembler output, with a new function that always returns
+ # None, this should cause GDB to stop using the Pygments
+ # module for disassembler styling.
+ gdb_py_test_silent_cmd \
+ [multi_line_input \
+ "python" \
+ "def replacement_colorize_disasm(content,gdbarch):" \
+ " return None" \
+ "gdb.styling.colorize_disasm = replacement_colorize_disasm" \
+ "\004"] \
+ "setup replacement colorize_disasm function" \
+ true
+
+ set insn_after [get_single_disassembled_insn]
+ gdb_assert { ![regexp "\033" $insn_after] } \
+ "have no style markers when Pygments is broken"
+ }
+}
+
# A separate test from the above as the styled text this checks can't
# currently be disabled (the text is printed too early in GDB's
# startup process).
# Check that the disassembler styling can be disabled.
test_disable_disassembler_styling
+# Check that GDB handles an error in the Python Pygments disassembly
+# styling code.
+test_disassembler_error_handling
+
# Finally, check the styling of the version string during startup.
test_startup_version_string
projects / binutils-gdb.git / commitdiff