gdb: improve disassembler styling when Pygments raises an exception

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 4cbe4ca5da
  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).
This commit is contained in:
Andrew Burgess 2022-08-26 21:19:14 +01:00
parent 5b0e2b48ed
commit f22c50c22c
3 changed files with 130 additions and 24 deletions

View file

@ -255,6 +255,9 @@ private:
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
@ -262,9 +265,6 @@ private:
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.
@ -284,6 +284,21 @@ private:
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. */