[PATCH 0/4] Disassembler Output Styling

[PATCH 0/4] Disassembler Output Styling

[Andrew Burgess]

This series replaces some parts of this earlier series:

  https://sourceware.org/pipermail/gdb-patches/2021-October/182526.html

Specifically, all of the disassembler styling related changes are now
replaced by this series.

In the earlier patch I develop a general API for augmenting the
disassembler output.  While I was working on that I realise I could
use that API to provide styled disassembler output, so I did that, and
included it in the earlier series.

However, after reading the feedback, and working with the new API a
little more, I realised that including the disassembler styling within
the general disassembler API was a bad idea.  It meant that when a
user wrote a plugin to augment the disassembler output, they had to
ensure that they also syntax-highlighted the output, this just seemed
unnecessary.

Instead, I think that syntax highlighting should be a separate step
that is performed after the disassembler output has been constructed,
whether that output is from GDB's builtin disassembler, or, in some
future world, where the output originates from a new Python
Disassembler API.

One additional reason I wanted the syntax highlighting to be moved out
of Python was, originally, so I could make use of libsource-highlight,
as we do for source code highlighting, I figured it would be quicker
to use that library than calling into Python, however, as I describe
in patch #3, the output of libsource-highlight on assembler code is
not great (currently), so in the end I stuck with the pure Python
solution.

If this series is accepted, then I plan to rebase the general purpose
disassembler API on to this, with some simplifications made now that
we don't need to support syntax highlighting.

Thanks,
Andrew

---

Andrew Burgess (4):
  gdb/python: make some global variables static
  gdb: rename source_styling_changed observer
  gdb: use python to colorize disassembler output
  gdb/python: move styling support to gdb.styling

 gdb/NEWS                         |   6 ++
 gdb/cli/cli-style.c              |  28 +++++-
 gdb/cli/cli-style.h              |   3 +
 gdb/data-directory/Makefile.in   |   1 +
 gdb/disasm.c                     |  23 ++++-
 gdb/disasm.h                     |   6 ++
 gdb/doc/gdb.texinfo              |  15 ++++
 gdb/extension-priv.h             |   6 ++
 gdb/extension.c                  |  20 +++++
 gdb/extension.h                  |   8 ++
 gdb/observable.c                 |   2 +-
 gdb/observable.h                 |   6 +-
 gdb/python/lib/gdb/__init__.py   |  18 ----
 gdb/python/lib/gdb/styling.py    |  48 ++++++++++
 gdb/python/python.c              | 145 ++++++++++++++++++++++++-------
 gdb/testsuite/gdb.base/style.exp |  46 ++++++++--
 gdb/tui/tui-winsource.c          |   4 +-
 17 files changed, 320 insertions(+), 65 deletions(-)
 create mode 100644 gdb/python/lib/gdb/styling.py

-- 
2.25.4

[PATCH 1/4] gdb/python: make some global variables static

[Andrew Burgess]

Make a couple of global variables static in python/python.c.  To do
this I had to move the definition of extension_language_python to
later in the file.

There should be no user visible changes after this commit.
---
 gdb/python/python.c | 53 ++++++++++++++++++++-------------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/gdb/python/python.c b/gdb/python/python.c
index c81814c557b..c7b5e7faa8e 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -58,33 +58,6 @@ static const char *const python_excp_enums[] =
    the default.  */
 static const char *gdbpy_should_print_stack = python_excp_message;
 
-#ifdef HAVE_PYTHON
-/* Forward decls, these are defined later.  */
-extern const struct extension_language_script_ops python_extension_script_ops;
-extern const struct extension_language_ops python_extension_ops;
-#endif
-
-/* The main struct describing GDB's interface to the Python
-   extension language.  */
-const struct extension_language_defn extension_language_python =
-{
-  EXT_LANG_PYTHON,
-  "python",
-  "Python",
-
-  ".py",
-  "-gdb.py",
-
-  python_control,
-
-#ifdef HAVE_PYTHON
-  &python_extension_script_ops,
-  &python_extension_ops
-#else
-  NULL,
-  NULL
-#endif
-};
 \f
 #ifdef HAVE_PYTHON
 
@@ -151,7 +124,7 @@ static gdb::optional<std::string> gdbpy_colorize
 
 /* The interface between gdb proper and loading of python scripts.  */
 
-const struct extension_language_script_ops python_extension_script_ops =
+static const struct extension_language_script_ops python_extension_script_ops =
 {
   gdbpy_source_script,
   gdbpy_source_objfile_script,
@@ -161,7 +134,7 @@ const struct extension_language_script_ops python_extension_script_ops =
 
 /* The interface between gdb proper and python extensions.  */
 
-const struct extension_language_ops python_extension_ops =
+static const struct extension_language_ops python_extension_ops =
 {
   gdbpy_initialize,
   gdbpy_initialized,
@@ -191,6 +164,28 @@ const struct extension_language_ops python_extension_ops =
   gdbpy_colorize,
 };
 
+/* The main struct describing GDB's interface to the Python
+   extension language.  */
+const struct extension_language_defn extension_language_python =
+{
+  EXT_LANG_PYTHON,
+  "python",
+  "Python",
+
+  ".py",
+  "-gdb.py",
+
+  python_control,
+
+#ifdef HAVE_PYTHON
+  &python_extension_script_ops,
+  &python_extension_ops
+#else
+  NULL,
+  NULL
+#endif
+};
+
 /* Architecture and language to be used in callbacks from
    the Python interpreter.  */
 struct gdbarch *python_gdbarch;
-- 
2.25.4

Re: [PATCH 1/4] gdb/python: make some global variables static

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Make a couple of global variables static in python/python.c.  To do
Andrew> this I had to move the definition of extension_language_python to
Andrew> later in the file.

Andrew> There should be no user visible changes after this commit.

This looks good to me.

Tom

Re: [PATCH 1/4] gdb/python: make some global variables static

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2021-10-27 14:20:01 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> Make a couple of global variables static in python/python.c.  To do
> Andrew> this I had to move the definition of extension_language_python to
> Andrew> later in the file.
> 
> Andrew> There should be no user visible changes after this commit.
> 
> This looks good to me.

Thanks, I've pushed this patch.

Andrew

Re: [PATCH 1/4] gdb/python: make some global variables static

[Enze Li]

Hi Andrew,

After using this patch, I encountered the following error when
compiling.

-----------------------------------------------------------------------
  CXXLD  gdb
/usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
linux/bin/ld: extension.o: in function `ext_lang_before_prompt(char
const*)':
/home/lee/dev/binutils-gdb/gdb/extension.c:914: undefined reference to
`extension_language_python'
/usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
linux/bin/ld: extension.o: in function
`get_ext_lang_defn(extension_language)':
/home/lee/dev/binutils-gdb/gdb/extension.c:107: undefined reference to
`extension_language_python'
/usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
linux/bin/ld: /home/lee/dev/binutils-gdb/gdb/extension.c:105: undefined
reference to `extension_language_python'
/usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
linux/bin/ld: extension.o: in function `get_ext_lang_of_file(char
const*)':
/home/lee/dev/binutils-gdb/gdb/extension.c:132: undefined reference to
`extension_language_python'
/usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
linux/bin/ld: extension.o: in function `ext_lang_initialization()':
/home/lee/dev/binutils-gdb/gdb/extension.c:331: undefined reference to
`extension_language_python'
/usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
linux/bin/ld: extension.o:/home/lee/dev/binutils-
gdb/gdb/extension.c:359: more undefined references to
`extension_language_python' follow
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:1904: gdb] Error 1
make[2]: Leaving directory '/home/lee/dev/binutils-gdb/gdb'
make[1]: *** [Makefile:13458: all-gdb] Error 2
make[1]: Leaving directory '/home/lee/dev/binutils-gdb'
make: *** [Makefile:1000: all] Error 2
----------------------------------------------------------------------

On Thu, 2021-11-25 at 10:12 +0000, Andrew Burgess via Gdb-patches
wrote:
> * Tom Tromey <tom@tromey.com> [2021-10-27 14:20:01 -0600]:
> 
> > > > > > > "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com>
> > > > > > > writes:
> > 
> > Andrew> Make a couple of global variables static in python/python.c. 
> > To do
> > Andrew> this I had to move the definition of
> > extension_language_python to
> > Andrew> later in the file.
> > 
> > Andrew> There should be no user visible changes after this commit.
> > 
> > This looks good to me.
> 
> Thanks, I've pushed this patch.
> 
> Andrew
> 

Here is my system environment,
OpenSuse Tumbleweed (Updated just now)
- gcc 11.2.1 20210816 
[revision 056e324ce46a7924b5cf10f61010cf9dd2ca10e9]

I executed the following command to compile，
# ./configure --prefix=/path/to/gdb-src/build/
# make

I guess the problem may be in the macro definition. I tested with the
following patch and the problem disappeared. Hope this may help to
solve this problem.

-----------------------------------------------------------------------
diff --git a/gdb/python/python.c b/gdb/python/python.c
index d8a6a5978de..bfb691f0eac 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -164,6 +164,8 @@ static const struct extension_language_ops
python_extension_ops =
   gdbpy_colorize,
 };
 
+#endif /* HAVE_PYTHON */
+
 /* The main struct describing GDB's interface to the Python
    extension language.  */
 const struct extension_language_defn extension_language_python =
@@ -186,6 +188,8 @@ const struct extension_language_defn
extension_language_python =
 #endif
 };
 
+#ifdef HAVE_PYTHON
+
 /* Architecture and language to be used in callbacks from
    the Python interpreter.  */
 struct gdbarch *python_gdbarch;
-----------------------------------------------------------------------


Thanks,
Enze

Re: [PATCH 1/4] gdb/python: make some global variables static

[Andrew Burgess]

* Enze Li <lienze2010@hotmail.com> [2021-11-25 23:02:49 +0800]:

> Hi Andrew,
> 
> After using this patch, I encountered the following error when
> compiling.
> 
> -----------------------------------------------------------------------
>   CXXLD  gdb
> /usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
> linux/bin/ld: extension.o: in function `ext_lang_before_prompt(char
> const*)':
> /home/lee/dev/binutils-gdb/gdb/extension.c:914: undefined reference to
> `extension_language_python'
> /usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
> linux/bin/ld: extension.o: in function
> `get_ext_lang_defn(extension_language)':
> /home/lee/dev/binutils-gdb/gdb/extension.c:107: undefined reference to
> `extension_language_python'
> /usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
> linux/bin/ld: /home/lee/dev/binutils-gdb/gdb/extension.c:105: undefined
> reference to `extension_language_python'
> /usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
> linux/bin/ld: extension.o: in function `get_ext_lang_of_file(char
> const*)':
> /home/lee/dev/binutils-gdb/gdb/extension.c:132: undefined reference to
> `extension_language_python'
> /usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
> linux/bin/ld: extension.o: in function `ext_lang_initialization()':
> /home/lee/dev/binutils-gdb/gdb/extension.c:331: undefined reference to
> `extension_language_python'
> /usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-
> linux/bin/ld: extension.o:/home/lee/dev/binutils-
> gdb/gdb/extension.c:359: more undefined references to
> `extension_language_python' follow
> collect2: error: ld returned 1 exit status
> make[2]: *** [Makefile:1904: gdb] Error 1
> make[2]: Leaving directory '/home/lee/dev/binutils-gdb/gdb'
> make[1]: *** [Makefile:13458: all-gdb] Error 2
> make[1]: Leaving directory '/home/lee/dev/binutils-gdb'
> make: *** [Makefile:1000: all] Error 2
> ----------------------------------------------------------------------
> 
> On Thu, 2021-11-25 at 10:12 +0000, Andrew Burgess via Gdb-patches
> wrote:
> > * Tom Tromey <tom@tromey.com> [2021-10-27 14:20:01 -0600]:
> > 
> > > > > > > > "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com>
> > > > > > > > writes:
> > > 
> > > Andrew> Make a couple of global variables static in python/python.c. 
> > > To do
> > > Andrew> this I had to move the definition of
> > > extension_language_python to
> > > Andrew> later in the file.
> > > 
> > > Andrew> There should be no user visible changes after this commit.
> > > 
> > > This looks good to me.
> > 
> > Thanks, I've pushed this patch.
> > 
> > Andrew
> > 
> 
> Here is my system environment,
> OpenSuse Tumbleweed (Updated just now)
> - gcc 11.2.1 20210816 
> [revision 056e324ce46a7924b5cf10f61010cf9dd2ca10e9]
> 
> I executed the following command to compile，
> # ./configure --prefix=/path/to/gdb-src/build/
> # make
> 
> I guess the problem may be in the macro definition. I tested with the
> following patch and the problem disappeared. Hope this may help to
> solve this problem.
> 
> -----------------------------------------------------------------------
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index d8a6a5978de..bfb691f0eac 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -164,6 +164,8 @@ static const struct extension_language_ops
> python_extension_ops =
>    gdbpy_colorize,
>  };
>  
> +#endif /* HAVE_PYTHON */
> +
>  /* The main struct describing GDB's interface to the Python
>     extension language.  */
>  const struct extension_language_defn extension_language_python =
> @@ -186,6 +188,8 @@ const struct extension_language_defn
> extension_language_python =
>  #endif
>  };
>  
> +#ifdef HAVE_PYTHON
> +
>  /* Architecture and language to be used in callbacks from
>     the Python interpreter.  */
>  struct gdbarch *python_gdbarch;
> -----------------------------------------------------------------------

Enze,

Thanks for reporting this, and thanks for looking into the problem and
providing a fix.

I pushed the patch below to resolve this issue.

Thanks,
Andrew

---

commit 9e99facd6cae1a94923166bd02f716d96395c19f
Author: Enze Li <lienze2010@hotmail.com>
Date:   Thu Nov 25 18:05:46 2021 +0000

    gdb: ensure extension_language_python is always defined
    
    In this commit:
    
      commit c6a6aad52d9e839d6a84ac31cabe2b7e1a2a31a0
      Date:   Mon Oct 25 17:25:45 2021 +0100
    
          gdb/python: make some global variables static
    
    building without Python was broken.  The extension_language_python
    global was moved from being always defined, to only being defined when
    the HAVE_PYTHON macro was defined.  As a consequence, building without
    Python support would result in errors like:
    
      /usr/bin/ld: extension.o:(.rodata+0x120): undefined reference to `extension_language_python'
    
    This commit fixes the problem by moving the definition of
    extension_language_python outside of the HAVE_PYTHON macro protection.

diff --git a/gdb/python/python.c b/gdb/python/python.c
index d8a6a5978de..bfb691f0eac 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -164,6 +164,8 @@ static const struct extension_language_ops python_extension_ops =
   gdbpy_colorize,
 };
 
+#endif /* HAVE_PYTHON */
+
 /* The main struct describing GDB's interface to the Python
    extension language.  */
 const struct extension_language_defn extension_language_python =
@@ -186,6 +188,8 @@ const struct extension_language_defn extension_language_python =
 #endif
 };
 
+#ifdef HAVE_PYTHON
+
 /* Architecture and language to be used in callbacks from
    the Python interpreter.  */
 struct gdbarch *python_gdbarch;

[PATCH 2/4] gdb: rename source_styling_changed observer

[Andrew Burgess]

In the next commit I plan to add disassembler styling.  In the same
way that we have a source_styling_changed observer I would need to add
a disassembler_styling_changed observer.

However, currently, these observers would only be notified from
cli-style.c:set_style_enabled, and observed in tui-winsource.c,
tui_source_window::style_changed, as a result, having two observers
seems unnecessary right now, so, in this commit, I plan to rename
source_styling_changed to just styling_changed, then, in the next
commit, I can use the same observer for both source styling, and
disassembler styling.

There should be no user visible changes after this commit.
---
 gdb/cli/cli-style.c     | 2 +-
 gdb/observable.c        | 2 +-
 gdb/observable.h        | 6 +++---
 gdb/tui/tui-winsource.c | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index aca19c51b84..228fa698c13 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -279,7 +279,7 @@ static void
 set_style_enabled  (const char *args, int from_tty, struct cmd_list_element *c)
 {
   g_source_cache.clear ();
-  gdb::observers::source_styling_changed.notify ();
+  gdb::observers::styling_changed.notify ();
 }
 
 static void
diff --git a/gdb/observable.c b/gdb/observable.c
index b020076cf26..d965ec3e52d 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -75,7 +75,7 @@ DEFINE_OBSERVABLE (inferior_call_pre);
 DEFINE_OBSERVABLE (inferior_call_post);
 DEFINE_OBSERVABLE (register_changed);
 DEFINE_OBSERVABLE (user_selected_context_changed);
-DEFINE_OBSERVABLE (source_styling_changed);
+DEFINE_OBSERVABLE (styling_changed);
 DEFINE_OBSERVABLE (current_source_symtab_and_line_changed);
 DEFINE_OBSERVABLE (gdb_exiting);
 
diff --git a/gdb/observable.h b/gdb/observable.h
index f20f532870f..707c214ac66 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -241,9 +241,9 @@ extern observable<struct frame_info */* frame */, int /* regnum */>
 extern observable<user_selected_what /* selection */>
     user_selected_context_changed;
 
-/* This is notified when the source styling setting has changed and
-   should be reconsulted.  */
-extern observable<> source_styling_changed;
+/* This is notified when a styling setting has changed, content may need
+   to be updated based on the new settings.  */
+extern observable<> styling_changed;
 
 /* The CLI's notion of the current source has changed.  This differs
    from user_selected_context_changed in that it is also set by the
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 955b68901fe..e2ab87eb9d4 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -292,14 +292,14 @@ tui_source_window_base::tui_source_window_base ()
   m_start_line_or_addr.loa = LOA_ADDRESS;
   m_start_line_or_addr.u.addr = 0;
 
-  gdb::observers::source_styling_changed.attach
+  gdb::observers::styling_changed.attach
     (std::bind (&tui_source_window::style_changed, this),
      m_observable, "tui-winsource");
 }
 
 tui_source_window_base::~tui_source_window_base ()
 {
-  gdb::observers::source_styling_changed.detach (m_observable);
+  gdb::observers::styling_changed.detach (m_observable);
 }
 
 /* See tui-data.h.  */
-- 
2.25.4

Re: [PATCH 2/4] gdb: rename source_styling_changed observer

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> In the next commit I plan to add disassembler styling.  In the same
Andrew> way that we have a source_styling_changed observer I would need to add
Andrew> a disassembler_styling_changed observer.

Andrew> However, currently, these observers would only be notified from
Andrew> cli-style.c:set_style_enabled, and observed in tui-winsource.c,
Andrew> tui_source_window::style_changed, as a result, having two observers
Andrew> seems unnecessary right now, so, in this commit, I plan to rename
Andrew> source_styling_changed to just styling_changed, then, in the next
Andrew> commit, I can use the same observer for both source styling, and
Andrew> disassembler styling.

Andrew> There should be no user visible changes after this commit.

Looks good.  It seems to me that the observable was misnamed all along.

Tom

Re: [PATCH 2/4] gdb: rename source_styling_changed observer

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2021-10-27 14:22:23 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> In the next commit I plan to add disassembler styling.  In the same
> Andrew> way that we have a source_styling_changed observer I would need to add
> Andrew> a disassembler_styling_changed observer.
> 
> Andrew> However, currently, these observers would only be notified from
> Andrew> cli-style.c:set_style_enabled, and observed in tui-winsource.c,
> Andrew> tui_source_window::style_changed, as a result, having two observers
> Andrew> seems unnecessary right now, so, in this commit, I plan to rename
> Andrew> source_styling_changed to just styling_changed, then, in the next
> Andrew> commit, I can use the same observer for both source styling, and
> Andrew> disassembler styling.
> 
> Andrew> There should be no user visible changes after this commit.
> 
> Looks good.  It seems to me that the observable was misnamed all along.

Thanks.  I pushed this patch with a small tweak to the commit message
so that I no longer talk about the "next" commit adding disassembler
styling.

Andrew

[PATCH 3/4] gdb: use python to colorize disassembler output

[Andrew Burgess]

This commit adds styling support to the disassembler output, as such
two new commands are added to GDB:

  set style disassembler on|off
  show style disassembler

In this commit I make use of the Python Pygments package to provide
the styling.  I did investigate making use of libsource-highlight,
however, I found the highlighting results to be inferior to those of
Pygments; only some mnemonics were highlighted, and highlighting of
register names such as r9d and r8d (on x86-64) was incorrect.

To enable disassembler highlighting via Pygments, I've added a new
extension language hook, which is then implemented for Python.  This
hook is very similar to the existing hook for source code
colorization.

One possibly odd choice I made with the new hook is to pass a
gdb.Architecture through, even though this is currently unused.  The
reason this argument is not used is that, currently, styling is
performed identically for all architectures.

However, even though the Python function used to perform styling of
disassembly output is not part of any undocumented API, I don't want
to close the door on a user overriding this function to provide
architecture specific styling.  To do this, the user would inevitably
require access to the gdb.Architecture, and so I decided to add this
field now.

The styling is applied within gdb_disassembler::print_insn, to achieve
this, gdb_disassembler now writes its output into a temporary buffer,
styling is then applied to the contents of this buffer.  Finally the
gdb_disassembler buffer is copied out to its final destination stream.

There's a new test to check that the disassembler output includes some
escape sequences, though I don't check for specific colours; the
precise colors will depend on which instructions are in the
disassembler output.

The only negative change with this commit relates to how addresses are
printed, in the case when the Python Pygments package is not
available.  Addresses are printed via calls to GDB's print_address
function.  Traditionally, this would provide styling for the address
and symbol name if the output ui_file* supported styling.  Now that we
want to apply styling after the disassembler has finished, all
disassembler output is written into a temporary string_file, which is
configured not to support styling.  As a result, the print_address
call no longer performs styling.

If Pygments is available, this isn't a huge problem, the output will
be fully styled one the disassembler has finished.  However, if
Pygments is not available, or fails for some reason, then, it is now
too late to go back and have print_address apply styling.  We have
lost all print_address styling in this case.

I don't know how much of a problem this is, for me, having the
disassembler fully styled is a big enough win.  But, if people see
this as a huge problem we can investigate mechanisms to restore the
print_address styling (for the case where Pygments is not available).
---
 gdb/NEWS                         |  6 +++
 gdb/cli/cli-style.c              | 26 +++++++++++
 gdb/cli/cli-style.h              |  3 ++
 gdb/disasm.c                     | 23 +++++++++-
 gdb/disasm.h                     |  6 +++
 gdb/doc/gdb.texinfo              | 15 +++++++
 gdb/extension-priv.h             |  6 +++
 gdb/extension.c                  | 20 +++++++++
 gdb/extension.h                  |  8 ++++
 gdb/python/lib/gdb/__init__.py   | 11 +++++
 gdb/python/python.c              | 74 ++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/style.exp | 46 +++++++++++++++++---
 12 files changed, 236 insertions(+), 8 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index d001a03145d..37c58c527a9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -32,6 +32,12 @@ maint show internal-warning backtrace
   internal-error, or an internal-warning.  This is on by default for
   internal-error and off by default for internal-warning.
 
+set style disassembly on|off
+show style disassembly
+  If GDB is compiled with Python support, and the Python Pygments
+  package is available, then, when this setting is on, disassembler
+  output will have styling applied.
+
 * Python API
 
   ** New function gdb.add_history(), which takes a gdb.Value object
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index 228fa698c13..d4e49e3df5b 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -38,6 +38,11 @@ bool cli_styling = true;
 
 bool source_styling = true;
 
+/* True if disassembler styling is enabled.  Note that this is only
+   consulted when cli_styling is true.  */
+
+bool disassembler_styling = true;
+
 /* Name of colors; must correspond to ui_file_style::basic_color.  */
 static const char * const cli_colors[] = {
   "none",
@@ -302,6 +307,18 @@ show_style_sources (struct ui_file *file, int from_tty,
     fprintf_filtered (file, _("Source code styling is disabled.\n"));
 }
 
+/* Implement 'show style disassembler'.  */
+
+static void
+show_style_disassembler (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  if (disassembler_styling)
+    fprintf_filtered (file, _("Disassembler output styling is enabled.\n"));
+  else
+    fprintf_filtered (file, _("Disassembler output styling is disabled.\n"));
+}
+
 void _initialize_cli_style ();
 void
 _initialize_cli_style ()
@@ -337,6 +354,15 @@ available if the appropriate extension is available at runtime."
 			   ), set_style_enabled, show_style_sources,
 			   &style_set_list, &style_show_list);
 
+  add_setshow_boolean_cmd ("disassembler", no_class, &disassembler_styling, _("\
+Set whether disassembler output styling is enabled."), _("\
+Show whether disassembler output styling is enabled."), _("\
+If enabled, disassembler output is styled.  Disassembler highlighting\n\
+requires the Python Pygments library, if this library is not available\n\
+then disassembler highlighting will not be possible."
+			   ), set_style_enabled, show_style_disassembler,
+			   &style_set_list, &style_show_list);
+
   file_name_style.add_setshow_commands (no_class, _("\
 Filename display styling.\n\
 Configure filename colors and display intensity."),
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 78bc2cd6f1e..5361a644a95 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -128,6 +128,9 @@ extern cli_style_option version_style;
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
+/* True if disassembler styling is enabled.  */
+extern bool disassembler_styling;
+
 /* True if styling is enabled.  */
 extern bool cli_styling;
 
diff --git a/gdb/disasm.c b/gdb/disasm.c
index c045dfc94a6..29755c9aa23 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -753,9 +753,10 @@ get_all_disassembler_options (struct gdbarch *gdbarch)
 gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 				    struct ui_file *file,
 				    di_read_memory_ftype read_memory_func)
-  : m_gdbarch (gdbarch)
+  : m_gdbarch (gdbarch),
+    m_dest (file)
 {
-  init_disassemble_info (&m_di, file, dis_asm_fprintf);
+  init_disassemble_info (&m_di, &m_buffer, dis_asm_fprintf);
   m_di.flavour = bfd_target_unknown_flavour;
   m_di.memory_error_func = dis_asm_memory_error;
   m_di.print_address_func = dis_asm_print_address;
@@ -789,9 +790,27 @@ gdb_disassembler::print_insn (CORE_ADDR memaddr,
 			      int *branch_delay_insns)
 {
   m_err_memaddr.reset ();
+  m_buffer.clear ();
 
   int length = gdbarch_print_insn (arch (), memaddr, &m_di);
 
+  /* If we have successfully disassembled an instruction, and styling is
+     on, and possible, then style the disassembler output.  */
+  if (length > 0 && disassembler_styling
+      && m_dest->can_emit_style_escape ())
+    {
+      gdb::optional<std::string> ext_contents;
+      ext_contents = ext_lang_colorize_disasm (m_buffer.string (), arch ());
+      if (ext_contents.has_value ())
+	m_buffer.string () = std::move (*ext_contents);
+    }
+
+  /* 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 ());
+
+  /* If the disassembler failed then report an appropriate error.  */
   if (length < 0)
     {
       if (m_err_memaddr.has_value ())
diff --git a/gdb/disasm.h b/gdb/disasm.h
index f6de33e3db8..0e768f767c1 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -82,6 +82,12 @@ class gdb_disassembler
      non-memory error.  */
   gdb::optional<CORE_ADDR> m_err_memaddr;
 
+  /* Disassembler output is built up into this buffer.  */
+  string_file m_buffer;
+
+  /* The stream to which disassembler output will be written.  */
+  ui_file *m_dest;
+
   static int dis_asm_fprintf (void *stream, const char *format, ...)
     ATTRIBUTE_PRINTF(2,3);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 631a7c03b31..6644bd517ff 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26071,6 +26071,21 @@
 
 @item show style sources
 Show the current state of source code styling.
+
+@item set style disassembly @samp{on|off}
+Enable or disable disassembly styling.  This affects whether
+disassembly output, such as the output of the @code{disassemble}
+command, is styled.  Disassembly styling only works if styling in
+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}.
+
+@item show style disassembly
+Show the current state of disassembly styling.
 @end table
 
 Subcommands of @code{set style} control specific forms of styling.
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index 77f23e0f911..b2150624dde 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -257,6 +257,12 @@ struct extension_language_ops
      or an empty option.  */
   gdb::optional<std::string> (*colorize) (const std::string &name,
 					  const std::string &contents);
+
+  /* Colorize a single line of disassembler output, CONTENT.  This should
+     either return colorized (using ANSI terminal escapes) version of the
+     contents, or an empty optional.  */
+  gdb::optional<std::string> (*colorize_disasm) (const std::string &content,
+						 gdbarch *gdbarch);
 };
 
 /* State necessary to restore a signal handler to its previous value.  */
diff --git a/gdb/extension.c b/gdb/extension.c
index 89ab29f3d1c..6862147ac2f 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -904,6 +904,26 @@ ext_lang_colorize (const std::string &filename, const std::string &contents)
   return result;
 }
 
+/* See extension.h.  */
+
+gdb::optional<std::string>
+ext_lang_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  gdb::optional<std::string> result;
+
+  for (const struct extension_language_defn *extlang : extension_languages)
+    {
+      if (extlang->ops == nullptr
+	  || extlang->ops->colorize_disasm == nullptr)
+	continue;
+      result = extlang->ops->colorize_disasm (content, gdbarch);
+      if (result.has_value ())
+	return result;
+    }
+
+  return result;
+}
+
 /* Called via an observer before gdb prints its prompt.
    Iterate over the extension languages giving them a chance to
    change the prompt.  The first one to change the prompt wins,
diff --git a/gdb/extension.h b/gdb/extension.h
index 2f2ca3e7743..e8d2fbc7fc3 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -319,6 +319,14 @@ extern void get_matching_xmethod_workers
 extern gdb::optional<std::string> ext_lang_colorize
   (const std::string &filename, const std::string &contents);
 
+/* Try to colorize a single line of disassembler output, CONTENT for
+   GDBARCH.  This will return either a colorized (using ANSI terminal
+   escapes) version of CONTENT, or an empty value if colorizing could not
+   be done.  */
+
+extern gdb::optional<std::string> ext_lang_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
+
 #if GDB_SELF_TEST
 namespace selftests {
 extern void (*hook_set_active_ext_lang) ();
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 7b6d8701548..6ab797c8c5e 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -243,8 +243,19 @@ try:
         except:
             return None
 
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip()
+        except:
+            return None
 
 except:
 
     def colorize(filename, contents):
         return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index c7b5e7faa8e..1bf32c36329 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -121,6 +121,8 @@ static enum ext_lang_rc gdbpy_before_prompt_hook
   (const struct extension_language_defn *, const char *current_gdb_prompt);
 static gdb::optional<std::string> gdbpy_colorize
   (const std::string &filename, const std::string &contents);
+static gdb::optional<std::string> gdbpy_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
 
 /* The interface between gdb proper and loading of python scripts.  */
 
@@ -162,6 +164,8 @@ static const struct extension_language_ops python_extension_ops =
   gdbpy_get_matching_xmethod_workers,
 
   gdbpy_colorize,
+
+  gdbpy_colorize_disasm,
 };
 
 /* The main struct describing GDB's interface to the Python
@@ -1181,6 +1185,76 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
   return std::string (PyBytes_AsString (host_str.get ()));
 }
 
+/* This is the extension_language_ops.colorize_disasm "method".  */
+
+static gdb::optional<std::string>
+gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  if (!gdb_python_initialized)
+    return {};
+
+  gdbpy_enter enter_py (get_current_arch (), current_language);
+
+  if (gdb_python_module == nullptr
+      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+    return {};
+
+  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+					    "colorize_disasm"));
+  if (hook == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyCallable_Check (hook.get ()))
+    return {};
+
+  gdbpy_ref<> content_arg (PyString_FromString (content.c_str ()));
+  if (content_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> gdbarch_arg (gdbarch_to_arch_object (gdbarch));
+  if (gdbarch_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> result (PyObject_CallFunctionObjArgs (hook.get (),
+						    content_arg.get (),
+						    gdbarch_arg.get (),
+						    nullptr));
+  if (result == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!gdbpy_is_string (result.get ()))
+    return {};
+
+  gdbpy_ref<> unic = python_string_to_unicode (result.get ());
+  if (unic == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+  gdbpy_ref<> host_str (PyUnicode_AsEncodedString (unic.get (),
+						   host_charset (),
+						   nullptr));
+  if (host_str == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  return std::string (PyBytes_AsString (host_str.get ()));
+}
+
 \f
 
 /* Printing.  */
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 91d3059612d..d1fa30fb5f0 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -182,12 +182,25 @@ proc run_style_tests { } {
 
 	gdb_test_no_output "set width 0"
 
-	set main [limited_style main function]
-	set func [limited_style some_called_function function]
-	# Somewhere should see the call to the function.
-	gdb_test "disassemble main" \
-	    [concat "Dump of assembler code for function $main:.*" \
-		 "[limited_style $hex address].*$func.*"]
+	# Disassembly highlighting is done by Python, so, if the
+	# required modules are not available we'll not get the full
+	# highlighting.
+	if { $::python_disassembly_highlighting } {
+	    # Check that the header line of the disassembly output is
+	    # styled correctly, the address at the start of the first
+	    # disassembly line is styled correctly, and that there is at
+	    # least one escape sequence in the disassembly output.
+	    set main [limited_style main function]
+	    gdb_test "disassemble main" \
+		[concat "Dump of assembler code for function $main:\\r\\n" \
+		     "\\s+[limited_style $hex address]\\s+<\\+$decimal>:\[^\\r\\n\]+\033\\\[${decimal}\[^\\r\\n\]+.*" ""]
+	} else {
+	    set main [limited_style main function]
+	    # Somewhere should see the call to the function.
+	    gdb_test "disassemble main" \
+		[concat "Dump of assembler code for function $main:.*" \
+		     "[limited_style $hex address].*<some_called_function>.*"]
+	}
 
 	set ifield [limited_style int_field variable]
 	set sfield [limited_style string_field variable]
@@ -312,6 +325,27 @@ proc test_startup_version_string { } {
     gdb_test "" "${vers}.*" "version is styled at startup"
 }
 
+# Check to see if the Python highlighting of disassembler output is
+# expected or not, this highlighting requires Python support in GDB,
+# and the Python pygments module to be available.
+clean_restart ${binfile}
+if {![skip_python_tests]} {
+    gdb_test_multiple "python import pygments" "" {
+	-re "ModuleNotFoundError: No module named 'pygments'.*$gdb_prompt $" {
+	    set python_disassembly_highlighting false
+	}
+	-re "ImportError: No module named pygments.*$gdb_prompt $" {
+	    set python_disassembly_highlighting false
+	}
+	-re "^python import pygments\r\n$gdb_prompt $" {
+	    set python_disassembly_highlighting true
+	}
+    }
+} else {
+    set python_disassembly_highlighting false
+}
+
+verbose -log "APB: run python tests? ${python_disassembly_highlighting}"
 
 # Run tests with all styles in their default state.
 with_test_prefix "all styles enabled" {
-- 
2.25.4

Re: [PATCH 3/4] gdb: use python to colorize disassembler output

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> In this commit I make use of the Python Pygments package to provide
Andrew> the styling.  I did investigate making use of libsource-highlight,
Andrew> however, I found the highlighting results to be inferior to those of
Andrew> Pygments; only some mnemonics were highlighted, and highlighting of
Andrew> register names such as r9d and r8d (on x86-64) was incorrect.

FWIW I think source highlight also only handles x86 assembly.

I didn't look to see if Pygments is any better about this... is it?

Andrew> One possibly odd choice I made with the new hook is to pass a
Andrew> gdb.Architecture through, even though this is currently unused.
...
Andrew> However, even though the Python function used to perform styling of
Andrew> disassembly output is not part of any undocumented API, I don't want
Andrew> to close the door on a user overriding this function to provide
Andrew> architecture specific styling.

Sounds very reasonable to me.

Andrew> I don't know how much of a problem this is, for me, having the
Andrew> disassembler fully styled is a big enough win.  But, if people see
Andrew> this as a huge problem we can investigate mechanisms to restore the
Andrew> print_address styling (for the case where Pygments is not available).

It does seem like a step backward when Pygments isn't available.

Maybe one idea would be to disable the hook on the first failure -- just
fall back to the gdb styling, with an output stream that accepts
styling, and re-disassemble the single failing instruction.  Would this
work?

Andrew> +  if (!gdbpy_is_string (result.get ()))
Andrew> +    return {};
Andrew> +
Andrew> +  gdbpy_ref<> unic = python_string_to_unicode (result.get ());
Andrew> +  if (unic == nullptr)

I think the call to gdbpy_is_string is maybe not needed, as
python_string_to_unicode already does type checking.

Andrew> +    {
Andrew> +      gdbpy_print_stack ();
Andrew> +      return {};
Andrew> +    }
Andrew> +  gdbpy_ref<> host_str (PyUnicode_AsEncodedString (unic.get (),
Andrew> +						   host_charset (),
Andrew> +						   nullptr));

.. though actually perhaps the whole sequence could be replace with
python_string_to_host_string.  Though then you may prefer to use a
unique_xmalloc_ptr rather than a std::string in the rest of the API.

Tom

Re: [PATCH 3/4] gdb: use python to colorize disassembler output

[Andrew Burgess]

Tom,

Thanks for the feedback.

* Tom Tromey <tom@tromey.com> [2021-10-27 14:38:04 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> In this commit I make use of the Python Pygments package to provide
> Andrew> the styling.  I did investigate making use of libsource-highlight,
> Andrew> however, I found the highlighting results to be inferior to those of
> Andrew> Pygments; only some mnemonics were highlighted, and highlighting of
> Andrew> register names such as r9d and r8d (on x86-64) was incorrect.
> 
> FWIW I think source highlight also only handles x86 assembly.
> 
> I didn't look to see if Pygments is any better about this... is it?

It's nothing special, but it doesn't seem to be restricted to looking
for specific mnemonics, at least it always seems to be highlighting
the mnemonic in x86-64 and risc-v code that I've thrown at it.  It
then highlights registers, immediates, and comments differently.

> 
> Andrew> One possibly odd choice I made with the new hook is to pass a
> Andrew> gdb.Architecture through, even though this is currently unused.
> ...
> Andrew> However, even though the Python function used to perform styling of
> Andrew> disassembly output is not part of any undocumented API, I don't want
> Andrew> to close the door on a user overriding this function to provide
> Andrew> architecture specific styling.
> 
> Sounds very reasonable to me.
> 
> Andrew> I don't know how much of a problem this is, for me, having the
> Andrew> disassembler fully styled is a big enough win.  But, if people see
> Andrew> this as a huge problem we can investigate mechanisms to restore the
> Andrew> print_address styling (for the case where Pygments is not available).
> 
> It does seem like a step backward when Pygments isn't available.
> 
> Maybe one idea would be to disable the hook on the first failure -- just
> fall back to the gdb styling, with an output stream that accepts
> styling, and re-disassemble the single failing instruction.  Would this
> work?

I've done this.  I'd be grateful for your thoughts on my proposed
solution, it relies on in-place new to recreate the string_file with
different properties, not sure what your thoughts are on this.

I'm sure there must be alternative solutions that don't require this
trick, if this seems too distasteful...

> 
> Andrew> +  if (!gdbpy_is_string (result.get ()))
> Andrew> +    return {};
> Andrew> +
> Andrew> +  gdbpy_ref<> unic = python_string_to_unicode (result.get ());
> Andrew> +  if (unic == nullptr)
> 
> I think the call to gdbpy_is_string is maybe not needed, as
> python_string_to_unicode already does type checking.
> 
> Andrew> +    {
> Andrew> +      gdbpy_print_stack ();
> Andrew> +      return {};
> Andrew> +    }
> Andrew> +  gdbpy_ref<> host_str (PyUnicode_AsEncodedString (unic.get (),
> Andrew> +						   host_charset (),
> Andrew> +						   nullptr));
> 
> .. though actually perhaps the whole sequence could be replace with
> python_string_to_host_string.  Though then you may prefer to use a
> unique_xmalloc_ptr rather than a std::string in the rest of the API.

I replaces the sequence with python_string_to_host_string as you
suggested.  I've left this function returning a std::string though.
The result is going to be placed into a string_file, which just wraps
a std::string anyway, so there doesn't seem like a huge benefit to
having this return anything other than std::string, and this way the
source code "colorize", and the disassembler "colorize_disasm" APIs
are pretty similar.

New patch is below.

Thanks,
Andrew

---

commit 468dc239e58de38634136691ebb509fc5df200be
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Mon Oct 25 17:26:57 2021 +0100

    gdb: use python to colorize disassembler output
    
    This commit adds styling support to the disassembler output, as such
    two new commands are added to GDB:
    
      set style disassembler on|off
      show style disassembler
    
    In this commit I make use of the Python Pygments package to provide
    the styling.  I did investigate making use of libsource-highlight,
    however, I found the highlighting results to be inferior to those of
    Pygments; only some mnemonics were highlighted, and highlighting of
    register names such as r9d and r8d (on x86-64) was incorrect.
    
    To enable disassembler highlighting via Pygments, I've added a new
    extension language hook, which is then implemented for Python.  This
    hook is very similar to the existing hook for source code
    colorization.
    
    One possibly odd choice I made with the new hook is to pass a
    gdb.Architecture through, even though this is currently unused.  The
    reason this argument is not used is that, currently, styling is
    performed identically for all architectures.
    
    However, even though the Python function used to perform styling of
    disassembly output is not part of any undocumented API, I don't want
    to close the door on a user overriding this function to provide
    architecture specific styling.  To do this, the user would inevitably
    require access to the gdb.Architecture, and so I decided to add this
    field now.
    
    The styling is applied within gdb_disassembler::print_insn, to achieve
    this, gdb_disassembler now writes its output into a temporary buffer,
    styling is then applied to the contents of this buffer.  Finally the
    gdb_disassembler buffer is copied out to its final destination stream.
    
    There's a new test to check that the disassembler output includes some
    escape sequences, though I don't check for specific colours; the
    precise colors will depend on which instructions are in the
    disassembler output.
    
    The only negative change with this commit relates to how addresses are
    printed, in the case when the Python Pygments package is not
    available.  Addresses are printed via calls to GDB's print_address
    function.  Traditionally, this would provide styling for the address
    and symbol name if the output ui_file* supported styling.  Now that we
    want to apply styling after the disassembler has finished, all
    disassembler output is written into a temporary string_file, which is
    configured not to support styling.  As a result, the print_address
    call no longer performs styling.
    
    If Pygments is available, this isn't a huge problem, the output will
    be fully styled one the disassembler has finished.  However, if
    Pygments is not available, or fails for some reason, then, it is now
    too late to go back and have print_address apply styling.  We have
    lost all print_address styling in this case.
    
    I don't know how much of a problem this is, for me, having the
    disassembler fully styled is a big enough win.  But, if people see
    this as a huge problem we can investigate mechanisms to restore the
    print_address styling (for the case where Pygments is not available).

diff --git a/gdb/NEWS b/gdb/NEWS
index d001a03145d..37c58c527a9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -32,6 +32,12 @@ maint show internal-warning backtrace
   internal-error, or an internal-warning.  This is on by default for
   internal-error and off by default for internal-warning.
 
+set style disassembly on|off
+show style disassembly
+  If GDB is compiled with Python support, and the Python Pygments
+  package is available, then, when this setting is on, disassembler
+  output will have styling applied.
+
 * Python API
 
   ** New function gdb.add_history(), which takes a gdb.Value object
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index 228fa698c13..d4e49e3df5b 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -38,6 +38,11 @@ bool cli_styling = true;
 
 bool source_styling = true;
 
+/* True if disassembler styling is enabled.  Note that this is only
+   consulted when cli_styling is true.  */
+
+bool disassembler_styling = true;
+
 /* Name of colors; must correspond to ui_file_style::basic_color.  */
 static const char * const cli_colors[] = {
   "none",
@@ -302,6 +307,18 @@ show_style_sources (struct ui_file *file, int from_tty,
     fprintf_filtered (file, _("Source code styling is disabled.\n"));
 }
 
+/* Implement 'show style disassembler'.  */
+
+static void
+show_style_disassembler (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  if (disassembler_styling)
+    fprintf_filtered (file, _("Disassembler output styling is enabled.\n"));
+  else
+    fprintf_filtered (file, _("Disassembler output styling is disabled.\n"));
+}
+
 void _initialize_cli_style ();
 void
 _initialize_cli_style ()
@@ -337,6 +354,15 @@ available if the appropriate extension is available at runtime."
 			   ), set_style_enabled, show_style_sources,
 			   &style_set_list, &style_show_list);
 
+  add_setshow_boolean_cmd ("disassembler", no_class, &disassembler_styling, _("\
+Set whether disassembler output styling is enabled."), _("\
+Show whether disassembler output styling is enabled."), _("\
+If enabled, disassembler output is styled.  Disassembler highlighting\n\
+requires the Python Pygments library, if this library is not available\n\
+then disassembler highlighting will not be possible."
+			   ), set_style_enabled, show_style_disassembler,
+			   &style_set_list, &style_show_list);
+
   file_name_style.add_setshow_commands (no_class, _("\
 Filename display styling.\n\
 Configure filename colors and display intensity."),
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 78bc2cd6f1e..5361a644a95 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -128,6 +128,9 @@ extern cli_style_option version_style;
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
+/* True if disassembler styling is enabled.  */
+extern bool disassembler_styling;
+
 /* True if styling is enabled.  */
 extern bool cli_styling;
 
diff --git a/gdb/disasm.c b/gdb/disasm.c
index c045dfc94a6..8f7ec00486e 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -753,9 +753,12 @@ get_all_disassembler_options (struct gdbarch *gdbarch)
 gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 				    struct ui_file *file,
 				    di_read_memory_ftype read_memory_func)
-  : m_gdbarch (gdbarch)
+  : m_gdbarch (gdbarch),
+    m_buffer (!use_ext_lang_colorization_p && disassembler_styling
+	      && file->can_emit_style_escape ()),
+    m_dest (file)
 {
-  init_disassemble_info (&m_di, file, dis_asm_fprintf);
+  init_disassemble_info (&m_di, &m_buffer, dis_asm_fprintf);
   m_di.flavour = bfd_target_unknown_flavour;
   m_di.memory_error_func = dis_asm_memory_error;
   m_di.print_address_func = dis_asm_print_address;
@@ -784,14 +787,66 @@ gdb_disassembler::~gdb_disassembler ()
   disassemble_free_target (&m_di);
 }
 
+/* See disasm.h.  */
+
+bool gdb_disassembler::use_ext_lang_colorization_p = true;
+
+/* See disasm.h.  */
+
 int
 gdb_disassembler::print_insn (CORE_ADDR memaddr,
 			      int *branch_delay_insns)
 {
   m_err_memaddr.reset ();
+  m_buffer.clear ();
 
   int length = gdbarch_print_insn (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 (length > 0 && disassembler_styling
+      && use_ext_lang_colorization_p
+      && m_dest->can_emit_style_escape ())
+    {
+      gdb::optional<std::string> ext_contents;
+      ext_contents = ext_lang_colorize_disasm (m_buffer.string (), arch ());
+      if (ext_contents.has_value ())
+	m_buffer.string () = std::move (*ext_contents);
+      else
+	{
+	  /* The extension language failed to add styling to the
+	     disassembly output.  Set the static flag so that next time we
+	     disassemble we don't even bother attempting to use the
+	     extension language for styling.  */
+	  use_ext_lang_colorization_p = false;
+
+	  /* The instruction we just disassembled, and the extension
+	     languages failed to highlight, might have otherwise had some
+	     minimal highlighting applied by GDB.  In regain that
+	     highlighting we need to recreate m_buffer, but this time with
+	     styling support.
+
+	     To do this we perform an in-place new, but this time turn on
+	     the styling support, then we can re-disassembly the
+	     instruction, and gain any minimal styling GDB might add.  */
+	  gdb_static_assert ((std::is_same<decltype (m_buffer),
+			      string_file>::value));
+	  gdb_assert (!m_buffer.term_out ());
+	  m_buffer.~string_file ();
+	  new (&m_buffer) string_file (true);
+	  length = gdbarch_print_insn (arch (), memaddr, &m_di);
+	  gdb_assert (length > 0);
+	}
+    }
+
+  /* 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 ());
+
+  /* If the disassembler failed then report an appropriate error.  */
   if (length < 0)
     {
       if (m_err_memaddr.has_value ())
diff --git a/gdb/disasm.h b/gdb/disasm.h
index f6de33e3db8..cb7f3768995 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -82,6 +82,31 @@ class gdb_disassembler
      non-memory error.  */
   gdb::optional<CORE_ADDR> m_err_memaddr;
 
+  /* 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 in general is turned on, and also, whether *m_dest supports
+     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.
+
+     This field will initially be true, but will be set to false if
+     ext_lang_colorize_disasm fails to add styling at any time.
+
+     If the extension language is going to add the styling then m_buffer
+     should be created without styling support, the extension language will
+     then add styling at the end of the disassembly process.
+
+     If the extension language is not going to add the styling, then we
+     create m_buffer with styling support, and GDB will add minimal styling
+     (currently just to addresses and symbols) as it goes.  */
+  static bool use_ext_lang_colorization_p;
+
   static int dis_asm_fprintf (void *stream, const char *format, ...)
     ATTRIBUTE_PRINTF(2,3);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 631a7c03b31..6644bd517ff 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26071,6 +26071,21 @@
 
 @item show style sources
 Show the current state of source code styling.
+
+@item set style disassembly @samp{on|off}
+Enable or disable disassembly styling.  This affects whether
+disassembly output, such as the output of the @code{disassemble}
+command, is styled.  Disassembly styling only works if styling in
+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}.
+
+@item show style disassembly
+Show the current state of disassembly styling.
 @end table
 
 Subcommands of @code{set style} control specific forms of styling.
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index 77f23e0f911..b2150624dde 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -257,6 +257,12 @@ struct extension_language_ops
      or an empty option.  */
   gdb::optional<std::string> (*colorize) (const std::string &name,
 					  const std::string &contents);
+
+  /* Colorize a single line of disassembler output, CONTENT.  This should
+     either return colorized (using ANSI terminal escapes) version of the
+     contents, or an empty optional.  */
+  gdb::optional<std::string> (*colorize_disasm) (const std::string &content,
+						 gdbarch *gdbarch);
 };
 
 /* State necessary to restore a signal handler to its previous value.  */
diff --git a/gdb/extension.c b/gdb/extension.c
index 89ab29f3d1c..6862147ac2f 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -904,6 +904,26 @@ ext_lang_colorize (const std::string &filename, const std::string &contents)
   return result;
 }
 
+/* See extension.h.  */
+
+gdb::optional<std::string>
+ext_lang_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  gdb::optional<std::string> result;
+
+  for (const struct extension_language_defn *extlang : extension_languages)
+    {
+      if (extlang->ops == nullptr
+	  || extlang->ops->colorize_disasm == nullptr)
+	continue;
+      result = extlang->ops->colorize_disasm (content, gdbarch);
+      if (result.has_value ())
+	return result;
+    }
+
+  return result;
+}
+
 /* Called via an observer before gdb prints its prompt.
    Iterate over the extension languages giving them a chance to
    change the prompt.  The first one to change the prompt wins,
diff --git a/gdb/extension.h b/gdb/extension.h
index 2f2ca3e7743..e8d2fbc7fc3 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -319,6 +319,14 @@ extern void get_matching_xmethod_workers
 extern gdb::optional<std::string> ext_lang_colorize
   (const std::string &filename, const std::string &contents);
 
+/* Try to colorize a single line of disassembler output, CONTENT for
+   GDBARCH.  This will return either a colorized (using ANSI terminal
+   escapes) version of CONTENT, or an empty value if colorizing could not
+   be done.  */
+
+extern gdb::optional<std::string> ext_lang_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
+
 #if GDB_SELF_TEST
 namespace selftests {
 extern void (*hook_set_active_ext_lang) ();
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 7b6d8701548..6ab797c8c5e 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -243,8 +243,19 @@ try:
         except:
             return None
 
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip()
+        except:
+            return None
 
 except:
 
     def colorize(filename, contents):
         return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index c7b5e7faa8e..d407ca66405 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -121,6 +121,8 @@ static enum ext_lang_rc gdbpy_before_prompt_hook
   (const struct extension_language_defn *, const char *current_gdb_prompt);
 static gdb::optional<std::string> gdbpy_colorize
   (const std::string &filename, const std::string &contents);
+static gdb::optional<std::string> gdbpy_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
 
 /* The interface between gdb proper and loading of python scripts.  */
 
@@ -162,6 +164,8 @@ static const struct extension_language_ops python_extension_ops =
   gdbpy_get_matching_xmethod_workers,
 
   gdbpy_colorize,
+
+  gdbpy_colorize_disasm,
 };
 
 /* The main struct describing GDB's interface to the Python
@@ -1181,6 +1185,69 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
   return std::string (PyBytes_AsString (host_str.get ()));
 }
 
+/* This is the extension_language_ops.colorize_disasm "method".  */
+
+static gdb::optional<std::string>
+gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  if (!gdb_python_initialized)
+    return {};
+
+  gdbpy_enter enter_py (get_current_arch (), current_language);
+
+  if (gdb_python_module == nullptr
+      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+    return {};
+
+  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+					    "colorize_disasm"));
+  if (hook == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyCallable_Check (hook.get ()))
+    return {};
+
+  gdbpy_ref<> content_arg (PyString_FromString (content.c_str ()));
+  if (content_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> gdbarch_arg (gdbarch_to_arch_object (gdbarch));
+  if (gdbarch_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> result (PyObject_CallFunctionObjArgs (hook.get (),
+						    content_arg.get (),
+						    gdbarch_arg.get (),
+						    nullptr));
+  if (result == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (result == Py_None)
+    return {};
+
+  gdb::unique_xmalloc_ptr<char> str
+    = python_string_to_host_string (result.get ());
+  if (str == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+ return std::string (str.get ());
+}
+
 \f
 
 /* Printing.  */
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 91d3059612d..4cf367ec82d 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -182,12 +182,25 @@ proc run_style_tests { } {
 
 	gdb_test_no_output "set width 0"
 
-	set main [limited_style main function]
-	set func [limited_style some_called_function function]
-	# Somewhere should see the call to the function.
-	gdb_test "disassemble main" \
-	    [concat "Dump of assembler code for function $main:.*" \
-		 "[limited_style $hex address].*$func.*"]
+	# Disassembly highlighting is done by Python, so, if the
+	# required modules are not available we'll not get the full
+	# highlighting.
+	if { $::python_disassembly_highlighting } {
+	    # Check that the header line of the disassembly output is
+	    # styled correctly, the address at the start of the first
+	    # disassembly line is styled correctly, and that there is at
+	    # least one escape sequence in the disassembly output.
+	    set main [limited_style main function]
+	    gdb_test "disassemble main" \
+		[concat "Dump of assembler code for function $main:\\r\\n" \
+		     "\\s+[limited_style $hex address]\\s+<\\+$decimal>:\[^\\r\\n\]+\033\\\[${decimal}\[^\\r\\n\]+.*" ""]
+	} else {
+	    set main [limited_style main function]
+	    # Somewhere should see the call to the function.
+	    gdb_test "disassemble main" \
+		[concat "Dump of assembler code for function $main:.*" \
+		     "[limited_style $hex address].*<some_called_function>.*"]
+	}
 
 	set ifield [limited_style int_field variable]
 	set sfield [limited_style string_field variable]
@@ -312,6 +325,25 @@ proc test_startup_version_string { } {
     gdb_test "" "${vers}.*" "version is styled at startup"
 }
 
+# Check to see if the Python highlighting of disassembler output is
+# expected or not, this highlighting requires Python support in GDB,
+# and the Python pygments module to be available.
+clean_restart ${binfile}
+if {![skip_python_tests]} {
+    gdb_test_multiple "python import pygments" "" {
+	-re "ModuleNotFoundError: No module named 'pygments'.*$gdb_prompt $" {
+	    set python_disassembly_highlighting false
+	}
+	-re "ImportError: No module named pygments.*$gdb_prompt $" {
+	    set python_disassembly_highlighting false
+	}
+	-re "^python import pygments\r\n$gdb_prompt $" {
+	    set python_disassembly_highlighting true
+	}
+    }
+} else {
+    set python_disassembly_highlighting false
+}
 
 # Run tests with all styles in their default state.
 with_test_prefix "all styles enabled" {

Re: [PATCH 3/4] gdb: use python to colorize disassembler output

[Andrew Burgess]

Ping!

Any thoughts?

Thanks,
Andrew

* Andrew Burgess <andrew.burgess@embecosm.com> [2021-10-28 17:28:31 +0100]:

> Tom,
> 
> Thanks for the feedback.
> 
> * Tom Tromey <tom@tromey.com> [2021-10-27 14:38:04 -0600]:
> 
> > >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> > 
> > Andrew> In this commit I make use of the Python Pygments package to provide
> > Andrew> the styling.  I did investigate making use of libsource-highlight,
> > Andrew> however, I found the highlighting results to be inferior to those of
> > Andrew> Pygments; only some mnemonics were highlighted, and highlighting of
> > Andrew> register names such as r9d and r8d (on x86-64) was incorrect.
> > 
> > FWIW I think source highlight also only handles x86 assembly.
> > 
> > I didn't look to see if Pygments is any better about this... is it?
> 
> It's nothing special, but it doesn't seem to be restricted to looking
> for specific mnemonics, at least it always seems to be highlighting
> the mnemonic in x86-64 and risc-v code that I've thrown at it.  It
> then highlights registers, immediates, and comments differently.
> 
> > 
> > Andrew> One possibly odd choice I made with the new hook is to pass a
> > Andrew> gdb.Architecture through, even though this is currently unused.
> > ...
> > Andrew> However, even though the Python function used to perform styling of
> > Andrew> disassembly output is not part of any undocumented API, I don't want
> > Andrew> to close the door on a user overriding this function to provide
> > Andrew> architecture specific styling.
> > 
> > Sounds very reasonable to me.
> > 
> > Andrew> I don't know how much of a problem this is, for me, having the
> > Andrew> disassembler fully styled is a big enough win.  But, if people see
> > Andrew> this as a huge problem we can investigate mechanisms to restore the
> > Andrew> print_address styling (for the case where Pygments is not available).
> > 
> > It does seem like a step backward when Pygments isn't available.
> > 
> > Maybe one idea would be to disable the hook on the first failure -- just
> > fall back to the gdb styling, with an output stream that accepts
> > styling, and re-disassemble the single failing instruction.  Would this
> > work?
> 
> I've done this.  I'd be grateful for your thoughts on my proposed
> solution, it relies on in-place new to recreate the string_file with
> different properties, not sure what your thoughts are on this.
> 
> I'm sure there must be alternative solutions that don't require this
> trick, if this seems too distasteful...
> 
> > 
> > Andrew> +  if (!gdbpy_is_string (result.get ()))
> > Andrew> +    return {};
> > Andrew> +
> > Andrew> +  gdbpy_ref<> unic = python_string_to_unicode (result.get ());
> > Andrew> +  if (unic == nullptr)
> > 
> > I think the call to gdbpy_is_string is maybe not needed, as
> > python_string_to_unicode already does type checking.
> > 
> > Andrew> +    {
> > Andrew> +      gdbpy_print_stack ();
> > Andrew> +      return {};
> > Andrew> +    }
> > Andrew> +  gdbpy_ref<> host_str (PyUnicode_AsEncodedString (unic.get (),
> > Andrew> +						   host_charset (),
> > Andrew> +						   nullptr));
> > 
> > .. though actually perhaps the whole sequence could be replace with
> > python_string_to_host_string.  Though then you may prefer to use a
> > unique_xmalloc_ptr rather than a std::string in the rest of the API.
> 
> I replaces the sequence with python_string_to_host_string as you
> suggested.  I've left this function returning a std::string though.
> The result is going to be placed into a string_file, which just wraps
> a std::string anyway, so there doesn't seem like a huge benefit to
> having this return anything other than std::string, and this way the
> source code "colorize", and the disassembler "colorize_disasm" APIs
> are pretty similar.
> 
> New patch is below.
> 
> Thanks,
> Andrew
> 
> ---
> 
> commit 468dc239e58de38634136691ebb509fc5df200be
> Author: Andrew Burgess <andrew.burgess@embecosm.com>
> Date:   Mon Oct 25 17:26:57 2021 +0100
> 
>     gdb: use python to colorize disassembler output
>     
>     This commit adds styling support to the disassembler output, as such
>     two new commands are added to GDB:
>     
>       set style disassembler on|off
>       show style disassembler
>     
>     In this commit I make use of the Python Pygments package to provide
>     the styling.  I did investigate making use of libsource-highlight,
>     however, I found the highlighting results to be inferior to those of
>     Pygments; only some mnemonics were highlighted, and highlighting of
>     register names such as r9d and r8d (on x86-64) was incorrect.
>     
>     To enable disassembler highlighting via Pygments, I've added a new
>     extension language hook, which is then implemented for Python.  This
>     hook is very similar to the existing hook for source code
>     colorization.
>     
>     One possibly odd choice I made with the new hook is to pass a
>     gdb.Architecture through, even though this is currently unused.  The
>     reason this argument is not used is that, currently, styling is
>     performed identically for all architectures.
>     
>     However, even though the Python function used to perform styling of
>     disassembly output is not part of any undocumented API, I don't want
>     to close the door on a user overriding this function to provide
>     architecture specific styling.  To do this, the user would inevitably
>     require access to the gdb.Architecture, and so I decided to add this
>     field now.
>     
>     The styling is applied within gdb_disassembler::print_insn, to achieve
>     this, gdb_disassembler now writes its output into a temporary buffer,
>     styling is then applied to the contents of this buffer.  Finally the
>     gdb_disassembler buffer is copied out to its final destination stream.
>     
>     There's a new test to check that the disassembler output includes some
>     escape sequences, though I don't check for specific colours; the
>     precise colors will depend on which instructions are in the
>     disassembler output.
>     
>     The only negative change with this commit relates to how addresses are
>     printed, in the case when the Python Pygments package is not
>     available.  Addresses are printed via calls to GDB's print_address
>     function.  Traditionally, this would provide styling for the address
>     and symbol name if the output ui_file* supported styling.  Now that we
>     want to apply styling after the disassembler has finished, all
>     disassembler output is written into a temporary string_file, which is
>     configured not to support styling.  As a result, the print_address
>     call no longer performs styling.
>     
>     If Pygments is available, this isn't a huge problem, the output will
>     be fully styled one the disassembler has finished.  However, if
>     Pygments is not available, or fails for some reason, then, it is now
>     too late to go back and have print_address apply styling.  We have
>     lost all print_address styling in this case.
>     
>     I don't know how much of a problem this is, for me, having the
>     disassembler fully styled is a big enough win.  But, if people see
>     this as a huge problem we can investigate mechanisms to restore the
>     print_address styling (for the case where Pygments is not available).
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index d001a03145d..37c58c527a9 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -32,6 +32,12 @@ maint show internal-warning backtrace
>    internal-error, or an internal-warning.  This is on by default for
>    internal-error and off by default for internal-warning.
>  
> +set style disassembly on|off
> +show style disassembly
> +  If GDB is compiled with Python support, and the Python Pygments
> +  package is available, then, when this setting is on, disassembler
> +  output will have styling applied.
> +
>  * Python API
>  
>    ** New function gdb.add_history(), which takes a gdb.Value object
> diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
> index 228fa698c13..d4e49e3df5b 100644
> --- a/gdb/cli/cli-style.c
> +++ b/gdb/cli/cli-style.c
> @@ -38,6 +38,11 @@ bool cli_styling = true;
>  
>  bool source_styling = true;
>  
> +/* True if disassembler styling is enabled.  Note that this is only
> +   consulted when cli_styling is true.  */
> +
> +bool disassembler_styling = true;
> +
>  /* Name of colors; must correspond to ui_file_style::basic_color.  */
>  static const char * const cli_colors[] = {
>    "none",
> @@ -302,6 +307,18 @@ show_style_sources (struct ui_file *file, int from_tty,
>      fprintf_filtered (file, _("Source code styling is disabled.\n"));
>  }
>  
> +/* Implement 'show style disassembler'.  */
> +
> +static void
> +show_style_disassembler (struct ui_file *file, int from_tty,
> +			 struct cmd_list_element *c, const char *value)
> +{
> +  if (disassembler_styling)
> +    fprintf_filtered (file, _("Disassembler output styling is enabled.\n"));
> +  else
> +    fprintf_filtered (file, _("Disassembler output styling is disabled.\n"));
> +}
> +
>  void _initialize_cli_style ();
>  void
>  _initialize_cli_style ()
> @@ -337,6 +354,15 @@ available if the appropriate extension is available at runtime."
>  			   ), set_style_enabled, show_style_sources,
>  			   &style_set_list, &style_show_list);
>  
> +  add_setshow_boolean_cmd ("disassembler", no_class, &disassembler_styling, _("\
> +Set whether disassembler output styling is enabled."), _("\
> +Show whether disassembler output styling is enabled."), _("\
> +If enabled, disassembler output is styled.  Disassembler highlighting\n\
> +requires the Python Pygments library, if this library is not available\n\
> +then disassembler highlighting will not be possible."
> +			   ), set_style_enabled, show_style_disassembler,
> +			   &style_set_list, &style_show_list);
> +
>    file_name_style.add_setshow_commands (no_class, _("\
>  Filename display styling.\n\
>  Configure filename colors and display intensity."),
> diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
> index 78bc2cd6f1e..5361a644a95 100644
> --- a/gdb/cli/cli-style.h
> +++ b/gdb/cli/cli-style.h
> @@ -128,6 +128,9 @@ extern cli_style_option version_style;
>  /* True if source styling is enabled.  */
>  extern bool source_styling;
>  
> +/* True if disassembler styling is enabled.  */
> +extern bool disassembler_styling;
> +
>  /* True if styling is enabled.  */
>  extern bool cli_styling;
>  
> diff --git a/gdb/disasm.c b/gdb/disasm.c
> index c045dfc94a6..8f7ec00486e 100644
> --- a/gdb/disasm.c
> +++ b/gdb/disasm.c
> @@ -753,9 +753,12 @@ get_all_disassembler_options (struct gdbarch *gdbarch)
>  gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
>  				    struct ui_file *file,
>  				    di_read_memory_ftype read_memory_func)
> -  : m_gdbarch (gdbarch)
> +  : m_gdbarch (gdbarch),
> +    m_buffer (!use_ext_lang_colorization_p && disassembler_styling
> +	      && file->can_emit_style_escape ()),
> +    m_dest (file)
>  {
> -  init_disassemble_info (&m_di, file, dis_asm_fprintf);
> +  init_disassemble_info (&m_di, &m_buffer, dis_asm_fprintf);
>    m_di.flavour = bfd_target_unknown_flavour;
>    m_di.memory_error_func = dis_asm_memory_error;
>    m_di.print_address_func = dis_asm_print_address;
> @@ -784,14 +787,66 @@ gdb_disassembler::~gdb_disassembler ()
>    disassemble_free_target (&m_di);
>  }
>  
> +/* See disasm.h.  */
> +
> +bool gdb_disassembler::use_ext_lang_colorization_p = true;
> +
> +/* See disasm.h.  */
> +
>  int
>  gdb_disassembler::print_insn (CORE_ADDR memaddr,
>  			      int *branch_delay_insns)
>  {
>    m_err_memaddr.reset ();
> +  m_buffer.clear ();
>  
>    int length = gdbarch_print_insn (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 (length > 0 && disassembler_styling
> +      && use_ext_lang_colorization_p
> +      && m_dest->can_emit_style_escape ())
> +    {
> +      gdb::optional<std::string> ext_contents;
> +      ext_contents = ext_lang_colorize_disasm (m_buffer.string (), arch ());
> +      if (ext_contents.has_value ())
> +	m_buffer.string () = std::move (*ext_contents);
> +      else
> +	{
> +	  /* The extension language failed to add styling to the
> +	     disassembly output.  Set the static flag so that next time we
> +	     disassemble we don't even bother attempting to use the
> +	     extension language for styling.  */
> +	  use_ext_lang_colorization_p = false;
> +
> +	  /* The instruction we just disassembled, and the extension
> +	     languages failed to highlight, might have otherwise had some
> +	     minimal highlighting applied by GDB.  In regain that
> +	     highlighting we need to recreate m_buffer, but this time with
> +	     styling support.
> +
> +	     To do this we perform an in-place new, but this time turn on
> +	     the styling support, then we can re-disassembly the
> +	     instruction, and gain any minimal styling GDB might add.  */
> +	  gdb_static_assert ((std::is_same<decltype (m_buffer),
> +			      string_file>::value));
> +	  gdb_assert (!m_buffer.term_out ());
> +	  m_buffer.~string_file ();
> +	  new (&m_buffer) string_file (true);
> +	  length = gdbarch_print_insn (arch (), memaddr, &m_di);
> +	  gdb_assert (length > 0);
> +	}
> +    }
> +
> +  /* 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 ());
> +
> +  /* If the disassembler failed then report an appropriate error.  */
>    if (length < 0)
>      {
>        if (m_err_memaddr.has_value ())
> diff --git a/gdb/disasm.h b/gdb/disasm.h
> index f6de33e3db8..cb7f3768995 100644
> --- a/gdb/disasm.h
> +++ b/gdb/disasm.h
> @@ -82,6 +82,31 @@ class gdb_disassembler
>       non-memory error.  */
>    gdb::optional<CORE_ADDR> m_err_memaddr;
>  
> +  /* 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 in general is turned on, and also, whether *m_dest supports
> +     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.
> +
> +     This field will initially be true, but will be set to false if
> +     ext_lang_colorize_disasm fails to add styling at any time.
> +
> +     If the extension language is going to add the styling then m_buffer
> +     should be created without styling support, the extension language will
> +     then add styling at the end of the disassembly process.
> +
> +     If the extension language is not going to add the styling, then we
> +     create m_buffer with styling support, and GDB will add minimal styling
> +     (currently just to addresses and symbols) as it goes.  */
> +  static bool use_ext_lang_colorization_p;
> +
>    static int dis_asm_fprintf (void *stream, const char *format, ...)
>      ATTRIBUTE_PRINTF(2,3);
>  
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 631a7c03b31..6644bd517ff 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -26071,6 +26071,21 @@
>  
>  @item show style sources
>  Show the current state of source code styling.
> +
> +@item set style disassembly @samp{on|off}
> +Enable or disable disassembly styling.  This affects whether
> +disassembly output, such as the output of the @code{disassemble}
> +command, is styled.  Disassembly styling only works if styling in
> +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}.
> +
> +@item show style disassembly
> +Show the current state of disassembly styling.
>  @end table
>  
>  Subcommands of @code{set style} control specific forms of styling.
> diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
> index 77f23e0f911..b2150624dde 100644
> --- a/gdb/extension-priv.h
> +++ b/gdb/extension-priv.h
> @@ -257,6 +257,12 @@ struct extension_language_ops
>       or an empty option.  */
>    gdb::optional<std::string> (*colorize) (const std::string &name,
>  					  const std::string &contents);
> +
> +  /* Colorize a single line of disassembler output, CONTENT.  This should
> +     either return colorized (using ANSI terminal escapes) version of the
> +     contents, or an empty optional.  */
> +  gdb::optional<std::string> (*colorize_disasm) (const std::string &content,
> +						 gdbarch *gdbarch);
>  };
>  
>  /* State necessary to restore a signal handler to its previous value.  */
> diff --git a/gdb/extension.c b/gdb/extension.c
> index 89ab29f3d1c..6862147ac2f 100644
> --- a/gdb/extension.c
> +++ b/gdb/extension.c
> @@ -904,6 +904,26 @@ ext_lang_colorize (const std::string &filename, const std::string &contents)
>    return result;
>  }
>  
> +/* See extension.h.  */
> +
> +gdb::optional<std::string>
> +ext_lang_colorize_disasm (const std::string &content, gdbarch *gdbarch)
> +{
> +  gdb::optional<std::string> result;
> +
> +  for (const struct extension_language_defn *extlang : extension_languages)
> +    {
> +      if (extlang->ops == nullptr
> +	  || extlang->ops->colorize_disasm == nullptr)
> +	continue;
> +      result = extlang->ops->colorize_disasm (content, gdbarch);
> +      if (result.has_value ())
> +	return result;
> +    }
> +
> +  return result;
> +}
> +
>  /* Called via an observer before gdb prints its prompt.
>     Iterate over the extension languages giving them a chance to
>     change the prompt.  The first one to change the prompt wins,
> diff --git a/gdb/extension.h b/gdb/extension.h
> index 2f2ca3e7743..e8d2fbc7fc3 100644
> --- a/gdb/extension.h
> +++ b/gdb/extension.h
> @@ -319,6 +319,14 @@ extern void get_matching_xmethod_workers
>  extern gdb::optional<std::string> ext_lang_colorize
>    (const std::string &filename, const std::string &contents);
>  
> +/* Try to colorize a single line of disassembler output, CONTENT for
> +   GDBARCH.  This will return either a colorized (using ANSI terminal
> +   escapes) version of CONTENT, or an empty value if colorizing could not
> +   be done.  */
> +
> +extern gdb::optional<std::string> ext_lang_colorize_disasm
> +  (const std::string &content, gdbarch *gdbarch);
> +
>  #if GDB_SELF_TEST
>  namespace selftests {
>  extern void (*hook_set_active_ext_lang) ();
> diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
> index 7b6d8701548..6ab797c8c5e 100644
> --- a/gdb/python/lib/gdb/__init__.py
> +++ b/gdb/python/lib/gdb/__init__.py
> @@ -243,8 +243,19 @@ try:
>          except:
>              return None
>  
> +    def colorize_disasm(content, gdbarch):
> +        # Don't want any errors.
> +        try:
> +            lexer = lexers.get_lexer_by_name("asm")
> +            formatter = formatters.TerminalFormatter()
> +            return highlight(content, lexer, formatter).rstrip()
> +        except:
> +            return None
>  
>  except:
>  
>      def colorize(filename, contents):
>          return None
> +
> +    def colorize_disasm(content, gdbarch):
> +        return None
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index c7b5e7faa8e..d407ca66405 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -121,6 +121,8 @@ static enum ext_lang_rc gdbpy_before_prompt_hook
>    (const struct extension_language_defn *, const char *current_gdb_prompt);
>  static gdb::optional<std::string> gdbpy_colorize
>    (const std::string &filename, const std::string &contents);
> +static gdb::optional<std::string> gdbpy_colorize_disasm
> +  (const std::string &content, gdbarch *gdbarch);
>  
>  /* The interface between gdb proper and loading of python scripts.  */
>  
> @@ -162,6 +164,8 @@ static const struct extension_language_ops python_extension_ops =
>    gdbpy_get_matching_xmethod_workers,
>  
>    gdbpy_colorize,
> +
> +  gdbpy_colorize_disasm,
>  };
>  
>  /* The main struct describing GDB's interface to the Python
> @@ -1181,6 +1185,69 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
>    return std::string (PyBytes_AsString (host_str.get ()));
>  }
>  
> +/* This is the extension_language_ops.colorize_disasm "method".  */
> +
> +static gdb::optional<std::string>
> +gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
> +{
> +  if (!gdb_python_initialized)
> +    return {};
> +
> +  gdbpy_enter enter_py (get_current_arch (), current_language);
> +
> +  if (gdb_python_module == nullptr
> +      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
> +    return {};
> +
> +  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
> +					    "colorize_disasm"));
> +  if (hook == nullptr)
> +    {
> +      gdbpy_print_stack ();
> +      return {};
> +    }
> +
> +  if (!PyCallable_Check (hook.get ()))
> +    return {};
> +
> +  gdbpy_ref<> content_arg (PyString_FromString (content.c_str ()));
> +  if (content_arg == nullptr)
> +    {
> +      gdbpy_print_stack ();
> +      return {};
> +    }
> +
> +  gdbpy_ref<> gdbarch_arg (gdbarch_to_arch_object (gdbarch));
> +  if (gdbarch_arg == nullptr)
> +    {
> +      gdbpy_print_stack ();
> +      return {};
> +    }
> +
> +  gdbpy_ref<> result (PyObject_CallFunctionObjArgs (hook.get (),
> +						    content_arg.get (),
> +						    gdbarch_arg.get (),
> +						    nullptr));
> +  if (result == nullptr)
> +    {
> +      gdbpy_print_stack ();
> +      return {};
> +    }
> +
> +  if (result == Py_None)
> +    return {};
> +
> +  gdb::unique_xmalloc_ptr<char> str
> +    = python_string_to_host_string (result.get ());
> +  if (str == nullptr)
> +    {
> +      gdbpy_print_stack ();
> +      return {};
> +    }
> +
> + return std::string (str.get ());
> +}
> +
>  \f
>  
>  /* Printing.  */
> diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
> index 91d3059612d..4cf367ec82d 100644
> --- a/gdb/testsuite/gdb.base/style.exp
> +++ b/gdb/testsuite/gdb.base/style.exp
> @@ -182,12 +182,25 @@ proc run_style_tests { } {
>  
>  	gdb_test_no_output "set width 0"
>  
> -	set main [limited_style main function]
> -	set func [limited_style some_called_function function]
> -	# Somewhere should see the call to the function.
> -	gdb_test "disassemble main" \
> -	    [concat "Dump of assembler code for function $main:.*" \
> -		 "[limited_style $hex address].*$func.*"]
> +	# Disassembly highlighting is done by Python, so, if the
> +	# required modules are not available we'll not get the full
> +	# highlighting.
> +	if { $::python_disassembly_highlighting } {
> +	    # Check that the header line of the disassembly output is
> +	    # styled correctly, the address at the start of the first
> +	    # disassembly line is styled correctly, and that there is at
> +	    # least one escape sequence in the disassembly output.
> +	    set main [limited_style main function]
> +	    gdb_test "disassemble main" \
> +		[concat "Dump of assembler code for function $main:\\r\\n" \
> +		     "\\s+[limited_style $hex address]\\s+<\\+$decimal>:\[^\\r\\n\]+\033\\\[${decimal}\[^\\r\\n\]+.*" ""]
> +	} else {
> +	    set main [limited_style main function]
> +	    # Somewhere should see the call to the function.
> +	    gdb_test "disassemble main" \
> +		[concat "Dump of assembler code for function $main:.*" \
> +		     "[limited_style $hex address].*<some_called_function>.*"]
> +	}
>  
>  	set ifield [limited_style int_field variable]
>  	set sfield [limited_style string_field variable]
> @@ -312,6 +325,25 @@ proc test_startup_version_string { } {
>      gdb_test "" "${vers}.*" "version is styled at startup"
>  }
>  
> +# Check to see if the Python highlighting of disassembler output is
> +# expected or not, this highlighting requires Python support in GDB,
> +# and the Python pygments module to be available.
> +clean_restart ${binfile}
> +if {![skip_python_tests]} {
> +    gdb_test_multiple "python import pygments" "" {
> +	-re "ModuleNotFoundError: No module named 'pygments'.*$gdb_prompt $" {
> +	    set python_disassembly_highlighting false
> +	}
> +	-re "ImportError: No module named pygments.*$gdb_prompt $" {
> +	    set python_disassembly_highlighting false
> +	}
> +	-re "^python import pygments\r\n$gdb_prompt $" {
> +	    set python_disassembly_highlighting true
> +	}
> +    }
> +} else {
> +    set python_disassembly_highlighting false
> +}
>  
>  # Run tests with all styles in their default state.
>  with_test_prefix "all styles enabled" {

[PATCH 4/4] gdb/python: move styling support to gdb.styling

[Andrew Burgess]

This commit moves the two Python functions that are used for styling
into a new module, gdb.styling, there's then a small update in
python.c so GDB can find the functions in their new location.

The motivation for this change is purely to try and reduce the clutter
in the top-level gdb module, and encapsulate related functions into
modules.  I did ponder documenting these functions as part of the
Python API, however, doing so would effectively "fix" the API, and I'm
still wondering if there's improvements that could be made, also, the
colorize function is only called in some cases now that GDB prefers
libsource-highlight, so it's not entirely sure how this would work as
part of a user facing API.

Still, despite these functions never having been part of a documented
API, it is possible that a user out there has overridden these to, in
some way, customize how GDB performs styling.  Moving the function as
I propose in this patch could break things for that user, however,
fixing this breakage is trivial, and, as these functions were never
documented, I don't think we should be obliged to not break user code
that relies on them.
---
 gdb/data-directory/Makefile.in |  1 +
 gdb/python/lib/gdb/__init__.py | 29 --------------------
 gdb/python/lib/gdb/styling.py  | 48 ++++++++++++++++++++++++++++++++++
 gdb/python/python.c            | 24 ++++++++++++-----
 4 files changed, 67 insertions(+), 35 deletions(-)
 create mode 100644 gdb/python/lib/gdb/styling.py

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 888325f974e..ab0733e29dc 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -74,6 +74,7 @@ PYTHON_FILE_LIST = \
 	gdb/frames.py \
 	gdb/printing.py \
 	gdb/prompt.py \
+	gdb/styling.py \
 	gdb/types.py \
 	gdb/unwinder.py \
 	gdb/xmethod.py \
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 6ab797c8c5e..8d63ae67fe8 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -230,32 +230,3 @@ def find_pc_line(pc):
     Return the gdb.Symtab_and_line object corresponding to the pc value."""
     return current_progspace().find_pc_line(pc)
 
-
-try:
-    from pygments import formatters, lexers, highlight
-
-    def colorize(filename, contents):
-        # Don't want any errors.
-        try:
-            lexer = lexers.get_lexer_for_filename(filename, stripnl=False)
-            formatter = formatters.TerminalFormatter()
-            return highlight(contents, lexer, formatter)
-        except:
-            return None
-
-    def colorize_disasm(content, gdbarch):
-        # Don't want any errors.
-        try:
-            lexer = lexers.get_lexer_by_name("asm")
-            formatter = formatters.TerminalFormatter()
-            return highlight(content, lexer, formatter).rstrip()
-        except:
-            return None
-
-except:
-
-    def colorize(filename, contents):
-        return None
-
-    def colorize_disasm(content, gdbarch):
-        return None
diff --git a/gdb/python/lib/gdb/styling.py b/gdb/python/lib/gdb/styling.py
new file mode 100644
index 00000000000..d03c0c7252a
--- /dev/null
+++ b/gdb/python/lib/gdb/styling.py
@@ -0,0 +1,48 @@
+# Styling related hooks.
+# Copyright (C) 2010-2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Utilities for styling."""
+
+import gdb
+
+try:
+    from pygments import formatters, lexers, highlight
+
+    def colorize(filename, contents):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_for_filename(filename, stripnl=False)
+            formatter = formatters.TerminalFormatter()
+            return highlight(contents, lexer, formatter)
+        except:
+            return None
+
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip()
+        except:
+            return None
+
+except:
+
+    def colorize(filename, contents):
+        return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 1bf32c36329..14f66554867 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1127,11 +1127,17 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  if (gdb_python_module == nullptr
-      || !PyObject_HasAttrString (gdb_python_module, "colorize"))
+  gdbpy_ref<> module (PyImport_ImportModule ("gdb.styling"));
+  if (module == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyObject_HasAttrString (module.get (), "colorize"))
     return {};
 
-  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module, "colorize"));
+  gdbpy_ref<> hook (PyObject_GetAttrString (module.get (), "colorize"));
   if (hook == nullptr)
     {
       gdbpy_print_stack ();
@@ -1195,11 +1201,17 @@ gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  if (gdb_python_module == nullptr
-      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+  gdbpy_ref<> module (PyImport_ImportModule ("gdb.styling"));
+  if (module == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyObject_HasAttrString (module.get (), "colorize_disasm"))
     return {};
 
-  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+  gdbpy_ref<> hook (PyObject_GetAttrString (module.get (),
 					    "colorize_disasm"));
   if (hook == nullptr)
     {
-- 
2.25.4

Re: [PATCH 4/4] gdb/python: move styling support to gdb.styling

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> This commit moves the two Python functions that are used for styling
Andrew> into a new module, gdb.styling, there's then a small update in
Andrew> python.c so GDB can find the functions in their new location.

Looks reasonable to me.

Andrew> Still, despite these functions never having been part of a documented
Andrew> API, it is possible that a user out there has overridden these to, in
Andrew> some way, customize how GDB performs styling.  Moving the function as
Andrew> I propose in this patch could break things for that user, however,
Andrew> fixing this breakage is trivial, and, as these functions were never
Andrew> documented, I don't think we should be obliged to not break user code
Andrew> that relies on them.

I agree, we should only be constrained to the documented API.

Tom

[PATCHv2 0/2] Disassembler Output Styling

[Andrew Burgess]

Since v1:

 - I pushed patches #1 and #2,

 - I rebased onto current master, minor conflicts resolved, no other
   changes.

 - New patch #2 (old patch #4) has been reviewed, so just patch #1
   needs review.

---

Andrew Burgess (2):
  gdb: use python to colorize disassembler output
  gdb/python: move styling support to gdb.styling

 gdb/NEWS                         |  6 +++
 gdb/cli/cli-style.c              | 26 ++++++++++
 gdb/cli/cli-style.h              |  3 ++
 gdb/data-directory/Makefile.in   |  1 +
 gdb/disasm.c                     | 59 +++++++++++++++++++++-
 gdb/disasm.h                     | 25 ++++++++++
 gdb/doc/gdb.texinfo              | 15 ++++++
 gdb/extension-priv.h             |  6 +++
 gdb/extension.c                  | 20 ++++++++
 gdb/extension.h                  |  8 +++
 gdb/python/lib/gdb/__init__.py   | 18 -------
 gdb/python/lib/gdb/styling.py    | 48 ++++++++++++++++++
 gdb/python/python.c              | 85 ++++++++++++++++++++++++++++++--
 gdb/testsuite/gdb.base/style.exp | 44 ++++++++++++++---
 14 files changed, 335 insertions(+), 29 deletions(-)
 create mode 100644 gdb/python/lib/gdb/styling.py

-- 
2.25.4

[PATCHv2 1/2] gdb: use python to colorize disassembler output

[Andrew Burgess]

From: Andrew Burgess <andrew.burgess@embecosm.com>

This commit adds styling support to the disassembler output, as such
two new commands are added to GDB:

  set style disassembler on|off
  show style disassembler

In this commit I make use of the Python Pygments package to provide
the styling.  I did investigate making use of libsource-highlight,
however, I found the highlighting results to be inferior to those of
Pygments; only some mnemonics were highlighted, and highlighting of
register names such as r9d and r8d (on x86-64) was incorrect.

To enable disassembler highlighting via Pygments, I've added a new
extension language hook, which is then implemented for Python.  This
hook is very similar to the existing hook for source code
colorization.

One possibly odd choice I made with the new hook is to pass a
gdb.Architecture through, even though this is currently unused.  The
reason this argument is not used is that, currently, styling is
performed identically for all architectures.

However, even though the Python function used to perform styling of
disassembly output is not part of any undocumented API, I don't want
to close the door on a user overriding this function to provide
architecture specific styling.  To do this, the user would inevitably
require access to the gdb.Architecture, and so I decided to add this
field now.

The styling is applied within gdb_disassembler::print_insn, to achieve
this, gdb_disassembler now writes its output into a temporary buffer,
styling is then applied to the contents of this buffer.  Finally the
gdb_disassembler buffer is copied out to its final destination stream.

There's a new test to check that the disassembler output includes some
escape sequences, though I don't check for specific colours; the
precise colors will depend on which instructions are in the
disassembler output.

The only negative change with this commit relates to how addresses are
printed, in the case when the Python Pygments package is not
available.  Addresses are printed via calls to GDB's print_address
function.  Traditionally, this would provide styling for the address
and symbol name if the output ui_file* supported styling.  Now that we
want to apply styling after the disassembler has finished, all
disassembler output is written into a temporary string_file, which is
configured not to support styling.  As a result, the print_address
call no longer performs styling.

If Pygments is available, this isn't a huge problem, the output will
be fully styled one the disassembler has finished.  However, if
Pygments is not available, or fails for some reason, then, it is now
too late to go back and have print_address apply styling.  We have
lost all print_address styling in this case.

I don't know how much of a problem this is, for me, having the
disassembler fully styled is a big enough win.  But, if people see
this as a huge problem we can investigate mechanisms to restore the
print_address styling (for the case where Pygments is not available).
---
 gdb/NEWS                         |  6 +++
 gdb/cli/cli-style.c              | 26 +++++++++++++
 gdb/cli/cli-style.h              |  3 ++
 gdb/disasm.c                     | 59 +++++++++++++++++++++++++++-
 gdb/disasm.h                     | 25 ++++++++++++
 gdb/doc/gdb.texinfo              | 15 +++++++
 gdb/extension-priv.h             |  6 +++
 gdb/extension.c                  | 20 ++++++++++
 gdb/extension.h                  |  8 ++++
 gdb/python/lib/gdb/__init__.py   | 11 ++++++
 gdb/python/python.c              | 67 ++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/style.exp | 44 ++++++++++++++++++---
 12 files changed, 282 insertions(+), 8 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index b0e4cce72ee..c62fed3f37f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -39,6 +39,12 @@ set logging enabled on|off
 show logging enabled
   These commands set or show whether logging is enabled or disabled.
 
+set style disassembly on|off
+show style disassembly
+  If GDB is compiled with Python support, and the Python Pygments
+  package is available, then, when this setting is on, disassembler
+  output will have styling applied.
+
 * Python API
 
   ** New function gdb.add_history(), which takes a gdb.Value object
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index a94b8c940bc..26abc3374ca 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -38,6 +38,11 @@ bool cli_styling = true;
 
 bool source_styling = true;
 
+/* True if disassembler styling is enabled.  Note that this is only
+   consulted when cli_styling is true.  */
+
+bool disassembler_styling = true;
+
 /* Name of colors; must correspond to ui_file_style::basic_color.  */
 static const char * const cli_colors[] = {
   "none",
@@ -301,6 +306,18 @@ show_style_sources (struct ui_file *file, int from_tty,
     fprintf_filtered (file, _("Source code styling is disabled.\n"));
 }
 
+/* Implement 'show style disassembler'.  */
+
+static void
+show_style_disassembler (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  if (disassembler_styling)
+    fprintf_filtered (file, _("Disassembler output styling is enabled.\n"));
+  else
+    fprintf_filtered (file, _("Disassembler output styling is disabled.\n"));
+}
+
 void _initialize_cli_style ();
 void
 _initialize_cli_style ()
@@ -337,6 +354,15 @@ available if the appropriate extension is available at runtime."
 			   ), set_style_enabled, show_style_sources,
 			   &style_set_list, &style_show_list);
 
+  add_setshow_boolean_cmd ("disassembler", no_class, &disassembler_styling, _("\
+Set whether disassembler output styling is enabled."), _("\
+Show whether disassembler output styling is enabled."), _("\
+If enabled, disassembler output is styled.  Disassembler highlighting\n\
+requires the Python Pygments library, if this library is not available\n\
+then disassembler highlighting will not be possible."
+			   ), set_style_enabled, show_style_disassembler,
+			   &style_set_list, &style_show_list);
+
   file_name_style.add_setshow_commands (no_class, _("\
 Filename display styling.\n\
 Configure filename colors and display intensity."),
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 78bc2cd6f1e..5361a644a95 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -128,6 +128,9 @@ extern cli_style_option version_style;
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
+/* True if disassembler styling is enabled.  */
+extern bool disassembler_styling;
+
 /* True if styling is enabled.  */
 extern bool cli_styling;
 
diff --git a/gdb/disasm.c b/gdb/disasm.c
index c045dfc94a6..8f7ec00486e 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -753,9 +753,12 @@ get_all_disassembler_options (struct gdbarch *gdbarch)
 gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 				    struct ui_file *file,
 				    di_read_memory_ftype read_memory_func)
-  : m_gdbarch (gdbarch)
+  : m_gdbarch (gdbarch),
+    m_buffer (!use_ext_lang_colorization_p && disassembler_styling
+	      && file->can_emit_style_escape ()),
+    m_dest (file)
 {
-  init_disassemble_info (&m_di, file, dis_asm_fprintf);
+  init_disassemble_info (&m_di, &m_buffer, dis_asm_fprintf);
   m_di.flavour = bfd_target_unknown_flavour;
   m_di.memory_error_func = dis_asm_memory_error;
   m_di.print_address_func = dis_asm_print_address;
@@ -784,14 +787,66 @@ gdb_disassembler::~gdb_disassembler ()
   disassemble_free_target (&m_di);
 }
 
+/* See disasm.h.  */
+
+bool gdb_disassembler::use_ext_lang_colorization_p = true;
+
+/* See disasm.h.  */
+
 int
 gdb_disassembler::print_insn (CORE_ADDR memaddr,
 			      int *branch_delay_insns)
 {
   m_err_memaddr.reset ();
+  m_buffer.clear ();
 
   int length = gdbarch_print_insn (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 (length > 0 && disassembler_styling
+      && use_ext_lang_colorization_p
+      && m_dest->can_emit_style_escape ())
+    {
+      gdb::optional<std::string> ext_contents;
+      ext_contents = ext_lang_colorize_disasm (m_buffer.string (), arch ());
+      if (ext_contents.has_value ())
+	m_buffer.string () = std::move (*ext_contents);
+      else
+	{
+	  /* The extension language failed to add styling to the
+	     disassembly output.  Set the static flag so that next time we
+	     disassemble we don't even bother attempting to use the
+	     extension language for styling.  */
+	  use_ext_lang_colorization_p = false;
+
+	  /* The instruction we just disassembled, and the extension
+	     languages failed to highlight, might have otherwise had some
+	     minimal highlighting applied by GDB.  In regain that
+	     highlighting we need to recreate m_buffer, but this time with
+	     styling support.
+
+	     To do this we perform an in-place new, but this time turn on
+	     the styling support, then we can re-disassembly the
+	     instruction, and gain any minimal styling GDB might add.  */
+	  gdb_static_assert ((std::is_same<decltype (m_buffer),
+			      string_file>::value));
+	  gdb_assert (!m_buffer.term_out ());
+	  m_buffer.~string_file ();
+	  new (&m_buffer) string_file (true);
+	  length = gdbarch_print_insn (arch (), memaddr, &m_di);
+	  gdb_assert (length > 0);
+	}
+    }
+
+  /* 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 ());
+
+  /* If the disassembler failed then report an appropriate error.  */
   if (length < 0)
     {
       if (m_err_memaddr.has_value ())
diff --git a/gdb/disasm.h b/gdb/disasm.h
index f6de33e3db8..cb7f3768995 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -82,6 +82,31 @@ class gdb_disassembler
      non-memory error.  */
   gdb::optional<CORE_ADDR> m_err_memaddr;
 
+  /* 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 in general is turned on, and also, whether *m_dest supports
+     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.
+
+     This field will initially be true, but will be set to false if
+     ext_lang_colorize_disasm fails to add styling at any time.
+
+     If the extension language is going to add the styling then m_buffer
+     should be created without styling support, the extension language will
+     then add styling at the end of the disassembly process.
+
+     If the extension language is not going to add the styling, then we
+     create m_buffer with styling support, and GDB will add minimal styling
+     (currently just to addresses and symbols) as it goes.  */
+  static bool use_ext_lang_colorization_p;
+
   static int dis_asm_fprintf (void *stream, const char *format, ...)
     ATTRIBUTE_PRINTF(2,3);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1a944b10ae5..458822577d7 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26080,6 +26080,21 @@
 
 @item show style sources
 Show the current state of source code styling.
+
+@item set style disassembly @samp{on|off}
+Enable or disable disassembly styling.  This affects whether
+disassembly output, such as the output of the @code{disassemble}
+command, is styled.  Disassembly styling only works if styling in
+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}.
+
+@item show style disassembly
+Show the current state of disassembly styling.
 @end table
 
 Subcommands of @code{set style} control specific forms of styling.
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index 77f23e0f911..b2150624dde 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -257,6 +257,12 @@ struct extension_language_ops
      or an empty option.  */
   gdb::optional<std::string> (*colorize) (const std::string &name,
 					  const std::string &contents);
+
+  /* Colorize a single line of disassembler output, CONTENT.  This should
+     either return colorized (using ANSI terminal escapes) version of the
+     contents, or an empty optional.  */
+  gdb::optional<std::string> (*colorize_disasm) (const std::string &content,
+						 gdbarch *gdbarch);
 };
 
 /* State necessary to restore a signal handler to its previous value.  */
diff --git a/gdb/extension.c b/gdb/extension.c
index 89ab29f3d1c..6862147ac2f 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -904,6 +904,26 @@ ext_lang_colorize (const std::string &filename, const std::string &contents)
   return result;
 }
 
+/* See extension.h.  */
+
+gdb::optional<std::string>
+ext_lang_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  gdb::optional<std::string> result;
+
+  for (const struct extension_language_defn *extlang : extension_languages)
+    {
+      if (extlang->ops == nullptr
+	  || extlang->ops->colorize_disasm == nullptr)
+	continue;
+      result = extlang->ops->colorize_disasm (content, gdbarch);
+      if (result.has_value ())
+	return result;
+    }
+
+  return result;
+}
+
 /* Called via an observer before gdb prints its prompt.
    Iterate over the extension languages giving them a chance to
    change the prompt.  The first one to change the prompt wins,
diff --git a/gdb/extension.h b/gdb/extension.h
index 2f2ca3e7743..e8d2fbc7fc3 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -319,6 +319,14 @@ extern void get_matching_xmethod_workers
 extern gdb::optional<std::string> ext_lang_colorize
   (const std::string &filename, const std::string &contents);
 
+/* Try to colorize a single line of disassembler output, CONTENT for
+   GDBARCH.  This will return either a colorized (using ANSI terminal
+   escapes) version of CONTENT, or an empty value if colorizing could not
+   be done.  */
+
+extern gdb::optional<std::string> ext_lang_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
+
 #if GDB_SELF_TEST
 namespace selftests {
 extern void (*hook_set_active_ext_lang) ();
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 7b6d8701548..6ab797c8c5e 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -243,8 +243,19 @@ try:
         except:
             return None
 
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip()
+        except:
+            return None
 
 except:
 
     def colorize(filename, contents):
         return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index d8a6a5978de..92619fdb3ad 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -121,6 +121,8 @@ static enum ext_lang_rc gdbpy_before_prompt_hook
   (const struct extension_language_defn *, const char *current_gdb_prompt);
 static gdb::optional<std::string> gdbpy_colorize
   (const std::string &filename, const std::string &contents);
+static gdb::optional<std::string> gdbpy_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
 
 /* The interface between gdb proper and loading of python scripts.  */
 
@@ -162,6 +164,8 @@ static const struct extension_language_ops python_extension_ops =
   gdbpy_get_matching_xmethod_workers,
 
   gdbpy_colorize,
+
+  gdbpy_colorize_disasm,
 };
 
 /* The main struct describing GDB's interface to the Python
@@ -1181,6 +1185,69 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
   return std::string (PyBytes_AsString (host_str.get ()));
 }
 
+/* This is the extension_language_ops.colorize_disasm "method".  */
+
+static gdb::optional<std::string>
+gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  if (!gdb_python_initialized)
+    return {};
+
+  gdbpy_enter enter_py (get_current_arch (), current_language);
+
+  if (gdb_python_module == nullptr
+      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+    return {};
+
+  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+					    "colorize_disasm"));
+  if (hook == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyCallable_Check (hook.get ()))
+    return {};
+
+  gdbpy_ref<> content_arg (PyString_FromString (content.c_str ()));
+  if (content_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> gdbarch_arg (gdbarch_to_arch_object (gdbarch));
+  if (gdbarch_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> result (PyObject_CallFunctionObjArgs (hook.get (),
+						    content_arg.get (),
+						    gdbarch_arg.get (),
+						    nullptr));
+  if (result == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (result == Py_None)
+    return {};
+
+  gdb::unique_xmalloc_ptr<char> str
+    = python_string_to_host_string (result.get ());
+  if (str == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+ return std::string (str.get ());
+}
+
 \f
 
 /* Printing.  */
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 91d3059612d..4cf367ec82d 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -182,12 +182,25 @@ proc run_style_tests { } {
 
 	gdb_test_no_output "set width 0"
 
-	set main [limited_style main function]
-	set func [limited_style some_called_function function]
-	# Somewhere should see the call to the function.
-	gdb_test "disassemble main" \
-	    [concat "Dump of assembler code for function $main:.*" \
-		 "[limited_style $hex address].*$func.*"]
+	# Disassembly highlighting is done by Python, so, if the
+	# required modules are not available we'll not get the full
+	# highlighting.
+	if { $::python_disassembly_highlighting } {
+	    # Check that the header line of the disassembly output is
+	    # styled correctly, the address at the start of the first
+	    # disassembly line is styled correctly, and that there is at
+	    # least one escape sequence in the disassembly output.
+	    set main [limited_style main function]
+	    gdb_test "disassemble main" \
+		[concat "Dump of assembler code for function $main:\\r\\n" \
+		     "\\s+[limited_style $hex address]\\s+<\\+$decimal>:\[^\\r\\n\]+\033\\\[${decimal}\[^\\r\\n\]+.*" ""]
+	} else {
+	    set main [limited_style main function]
+	    # Somewhere should see the call to the function.
+	    gdb_test "disassemble main" \
+		[concat "Dump of assembler code for function $main:.*" \
+		     "[limited_style $hex address].*<some_called_function>.*"]
+	}
 
 	set ifield [limited_style int_field variable]
 	set sfield [limited_style string_field variable]
@@ -312,6 +325,25 @@ proc test_startup_version_string { } {
     gdb_test "" "${vers}.*" "version is styled at startup"
 }
 
+# Check to see if the Python highlighting of disassembler output is
+# expected or not, this highlighting requires Python support in GDB,
+# and the Python pygments module to be available.
+clean_restart ${binfile}
+if {![skip_python_tests]} {
+    gdb_test_multiple "python import pygments" "" {
+	-re "ModuleNotFoundError: No module named 'pygments'.*$gdb_prompt $" {
+	    set python_disassembly_highlighting false
+	}
+	-re "ImportError: No module named pygments.*$gdb_prompt $" {
+	    set python_disassembly_highlighting false
+	}
+	-re "^python import pygments\r\n$gdb_prompt $" {
+	    set python_disassembly_highlighting true
+	}
+    }
+} else {
+    set python_disassembly_highlighting false
+}
 
 # Run tests with all styles in their default state.
 with_test_prefix "all styles enabled" {
-- 
2.25.4

Re: [PATCHv2 1/2] gdb: use python to colorize disassembler output

[Eli Zaretskii]

> Date: Thu, 25 Nov 2021 10:36:42 +0000
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <andrew.burgess@embecosm.com>
> 
>  gdb/NEWS                         |  6 +++
>  gdb/cli/cli-style.c              | 26 +++++++++++++
>  gdb/cli/cli-style.h              |  3 ++
>  gdb/disasm.c                     | 59 +++++++++++++++++++++++++++-
>  gdb/disasm.h                     | 25 ++++++++++++
>  gdb/doc/gdb.texinfo              | 15 +++++++
>  gdb/extension-priv.h             |  6 +++
>  gdb/extension.c                  | 20 ++++++++++
>  gdb/extension.h                  |  8 ++++
>  gdb/python/lib/gdb/__init__.py   | 11 ++++++
>  gdb/python/python.c              | 67 ++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.base/style.exp | 44 ++++++++++++++++++---
>  12 files changed, 282 insertions(+), 8 deletions(-)

Thanks, the documentation parts are OK.

[PATCHv2 2/2] gdb/python: move styling support to gdb.styling

[Andrew Burgess]

From: Andrew Burgess <andrew.burgess@embecosm.com>

This commit moves the two Python functions that are used for styling
into a new module, gdb.styling, there's then a small update in
python.c so GDB can find the functions in their new location.

The motivation for this change is purely to try and reduce the clutter
in the top-level gdb module, and encapsulate related functions into
modules.  I did ponder documenting these functions as part of the
Python API, however, doing so would effectively "fix" the API, and I'm
still wondering if there's improvements that could be made, also, the
colorize function is only called in some cases now that GDB prefers
libsource-highlight, so it's not entirely sure how this would work as
part of a user facing API.

Still, despite these functions never having been part of a documented
API, it is possible that a user out there has overridden these to, in
some way, customize how GDB performs styling.  Moving the function as
I propose in this patch could break things for that user, however,
fixing this breakage is trivial, and, as these functions were never
documented, I don't think we should be obliged to not break user code
that relies on them.
---
 gdb/data-directory/Makefile.in |  1 +
 gdb/python/lib/gdb/__init__.py | 29 --------------------
 gdb/python/lib/gdb/styling.py  | 48 ++++++++++++++++++++++++++++++++++
 gdb/python/python.c            | 24 ++++++++++++-----
 4 files changed, 67 insertions(+), 35 deletions(-)
 create mode 100644 gdb/python/lib/gdb/styling.py

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 888325f974e..ab0733e29dc 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -74,6 +74,7 @@ PYTHON_FILE_LIST = \
 	gdb/frames.py \
 	gdb/printing.py \
 	gdb/prompt.py \
+	gdb/styling.py \
 	gdb/types.py \
 	gdb/unwinder.py \
 	gdb/xmethod.py \
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 6ab797c8c5e..8d63ae67fe8 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -230,32 +230,3 @@ def find_pc_line(pc):
     Return the gdb.Symtab_and_line object corresponding to the pc value."""
     return current_progspace().find_pc_line(pc)
 
-
-try:
-    from pygments import formatters, lexers, highlight
-
-    def colorize(filename, contents):
-        # Don't want any errors.
-        try:
-            lexer = lexers.get_lexer_for_filename(filename, stripnl=False)
-            formatter = formatters.TerminalFormatter()
-            return highlight(contents, lexer, formatter)
-        except:
-            return None
-
-    def colorize_disasm(content, gdbarch):
-        # Don't want any errors.
-        try:
-            lexer = lexers.get_lexer_by_name("asm")
-            formatter = formatters.TerminalFormatter()
-            return highlight(content, lexer, formatter).rstrip()
-        except:
-            return None
-
-except:
-
-    def colorize(filename, contents):
-        return None
-
-    def colorize_disasm(content, gdbarch):
-        return None
diff --git a/gdb/python/lib/gdb/styling.py b/gdb/python/lib/gdb/styling.py
new file mode 100644
index 00000000000..d03c0c7252a
--- /dev/null
+++ b/gdb/python/lib/gdb/styling.py
@@ -0,0 +1,48 @@
+# Styling related hooks.
+# Copyright (C) 2010-2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Utilities for styling."""
+
+import gdb
+
+try:
+    from pygments import formatters, lexers, highlight
+
+    def colorize(filename, contents):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_for_filename(filename, stripnl=False)
+            formatter = formatters.TerminalFormatter()
+            return highlight(contents, lexer, formatter)
+        except:
+            return None
+
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip()
+        except:
+            return None
+
+except:
+
+    def colorize(filename, contents):
+        return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 92619fdb3ad..f80cd1f19db 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1127,11 +1127,17 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  if (gdb_python_module == nullptr
-      || !PyObject_HasAttrString (gdb_python_module, "colorize"))
+  gdbpy_ref<> module (PyImport_ImportModule ("gdb.styling"));
+  if (module == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyObject_HasAttrString (module.get (), "colorize"))
     return {};
 
-  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module, "colorize"));
+  gdbpy_ref<> hook (PyObject_GetAttrString (module.get (), "colorize"));
   if (hook == nullptr)
     {
       gdbpy_print_stack ();
@@ -1195,11 +1201,17 @@ gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  if (gdb_python_module == nullptr
-      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+  gdbpy_ref<> module (PyImport_ImportModule ("gdb.styling"));
+  if (module == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyObject_HasAttrString (module.get (), "colorize_disasm"))
     return {};
 
-  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+  gdbpy_ref<> hook (PyObject_GetAttrString (module.get (),
 					    "colorize_disasm"));
   if (hook == nullptr)
     {
-- 
2.25.4

Ping: [PATCHv2 0/2] Disassembler Output Styling

[Andrew Burgess]

Ping!

Eli, thanks for the docs review.

I'm still hoping for someone else to cast their eye over patch #1.
Patch #2 has already been given the OK by Tom in V1.

Thanks,
Andrew

* Andrew Burgess <aburgess@redhat.com> [2021-11-25 10:36:41 +0000]:

> Since v1:
> 
>  - I pushed patches #1 and #2,
> 
>  - I rebased onto current master, minor conflicts resolved, no other
>    changes.
> 
>  - New patch #2 (old patch #4) has been reviewed, so just patch #1
>    needs review.
> 
> ---
> 
> Andrew Burgess (2):
>   gdb: use python to colorize disassembler output
>   gdb/python: move styling support to gdb.styling
> 
>  gdb/NEWS                         |  6 +++
>  gdb/cli/cli-style.c              | 26 ++++++++++
>  gdb/cli/cli-style.h              |  3 ++
>  gdb/data-directory/Makefile.in   |  1 +
>  gdb/disasm.c                     | 59 +++++++++++++++++++++-
>  gdb/disasm.h                     | 25 ++++++++++
>  gdb/doc/gdb.texinfo              | 15 ++++++
>  gdb/extension-priv.h             |  6 +++
>  gdb/extension.c                  | 20 ++++++++
>  gdb/extension.h                  |  8 +++
>  gdb/python/lib/gdb/__init__.py   | 18 -------
>  gdb/python/lib/gdb/styling.py    | 48 ++++++++++++++++++
>  gdb/python/python.c              | 85 ++++++++++++++++++++++++++++++--
>  gdb/testsuite/gdb.base/style.exp | 44 ++++++++++++++---
>  14 files changed, 335 insertions(+), 29 deletions(-)
>  create mode 100644 gdb/python/lib/gdb/styling.py
> 
> -- 
> 2.25.4
> 

[PATCHv3 0/2] Disassembler Output Styling

[Andrew Burgess]

In this update, the biggest change, is that the controlling setting is
now 'set/show style disassembler enabled', that is I've added
'enabled' to the setting name.

My reason for this is that, in the future, I might want to add
additional disassembler styles, like:

  set style disassembler register ....
  set style disassembler mnemonic ....

And the new setting name leaves this possibility open, while the old
setting made this harder.

This version also includes an improved test, minor updates to the docs
to match the above change, and a rebase to current master.

Feedback welcome,

Thanks,
Andrew


---

Since v2:

  - Rebased onto current master,

  - Changed the name of the setting to 'set/show style disassembler
    enabled',

  - Updated the test to cover toggling this setting,

  - Updated the NEWS and docs to match the setting change,

  - Updated the commit message on patch #1, which I forgot to do in
    v2.

Since v1:

 - I pushed patches #1 and #2,

 - I rebased onto current master, minor conflicts resolved, no other
   changes.

 - New patch #2 (old patch #4) has been reviewed, so just patch #1
   needs review.


---

Andrew Burgess (2):
  gdb: use python to colorize disassembler output
  gdb/python: move styling support to gdb.styling

 gdb/NEWS                         |  6 +++
 gdb/cli/cli-style.c              | 44 +++++++++++++++++
 gdb/cli/cli-style.h              |  3 ++
 gdb/data-directory/Makefile.in   |  1 +
 gdb/disasm.c                     | 58 +++++++++++++++++++++-
 gdb/disasm.h                     | 25 ++++++++++
 gdb/doc/gdb.texinfo              | 15 ++++++
 gdb/extension-priv.h             |  6 +++
 gdb/extension.c                  | 20 ++++++++
 gdb/extension.h                  |  8 +++
 gdb/python/lib/gdb/__init__.py   | 18 -------
 gdb/python/lib/gdb/styling.py    | 48 ++++++++++++++++++
 gdb/python/python.c              | 85 ++++++++++++++++++++++++++++++--
 gdb/testsuite/gdb.base/style.exp | 82 +++++++++++++++++++++++++++++-
 14 files changed, 395 insertions(+), 24 deletions(-)
 create mode 100644 gdb/python/lib/gdb/styling.py

-- 
2.25.4

[PATCHv3 1/2] gdb: use python to colorize disassembler output

[Andrew Burgess]

From: Andrew Burgess <andrew.burgess@embecosm.com>

This commit adds styling support to the disassembler output, as such
two new commands are added to GDB:

  set style disassembler enabled on|off
  show style disassembler enabled

In this commit I make use of the Python Pygments package to provide
the styling.  I did investigate making use of libsource-highlight,
however, I found the highlighting results to be inferior to those of
Pygments; only some mnemonics were highlighted, and highlighting of
register names such as r9d and r8d (on x86-64) was incorrect.

To enable disassembler highlighting via Pygments, I've added a new
extension language hook, which is then implemented for Python.  This
hook is very similar to the existing hook for source code
colorization.

One possibly odd choice I made with the new hook is to pass a
gdb.Architecture through, even though this is currently unused.  The
reason this argument is not used is that, currently, styling is
performed identically for all architectures.

However, even though the Python function used to perform styling of
disassembly output is not part of any documented API, I don't want
to close the door on a user overriding this function to provide
architecture specific styling.  To do this, the user would inevitably
require access to the gdb.Architecture, and so I decided to add this
field now.

The styling is applied within gdb_disassembler::print_insn, to achieve
this, gdb_disassembler now writes its output into a temporary buffer,
styling is then applied to the contents of this buffer.  Finally the
gdb_disassembler buffer is copied out to its final destination stream.

There's a new test to check that the disassembler output includes some
escape sequences, though I don't check for specific colours; the
precise colors will depend on which instructions are in the
disassembler output, and, I guess, how pygments is configured.

The only negative change with this commit is how we currently style
addresses in GDB.

Currently, when the disassembler wants to print an address, we call
back into GDB, and GDB prints the address value using the `address`
styling, and the symbol name using `function` styling.  After this
commit, if pygments is used, then all disassembler styling is done
through pygments, and this include the address and symbol name parts
of the disassembler output.

I don't know how much of an issue this will be for people.  There's
already some precedent for this in GDB when we look at source styling.
For example, function names in styled source listings are not styled
using the `function` style, but instead, either GNU Source Highlight,
or pygments gets to decide how the function name should be styled.

If the Python pygments library is not present then GDB will continue
to behave as it always has, the disassembler output is mostly
unstyled, but the address and symbols are styled using the `address`
and `function` styles, as they are today.

However, if the user does `set style disassembler enabled off`, then
all disassembler styling is switched off.  This obviously covers the
use of pygments, but also includes the minimal styling done by GDB
when pygments is not available.
---
 gdb/NEWS                         |  6 +++
 gdb/cli/cli-style.c              | 44 +++++++++++++++++
 gdb/cli/cli-style.h              |  3 ++
 gdb/disasm.c                     | 58 +++++++++++++++++++++-
 gdb/disasm.h                     | 25 ++++++++++
 gdb/doc/gdb.texinfo              | 15 ++++++
 gdb/extension-priv.h             |  6 +++
 gdb/extension.c                  | 20 ++++++++
 gdb/extension.h                  |  8 ++++
 gdb/python/lib/gdb/__init__.py   | 11 +++++
 gdb/python/python.c              | 67 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/style.exp | 82 +++++++++++++++++++++++++++++++-
 12 files changed, 342 insertions(+), 3 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 13b66286876..bbcac8481ed 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -45,6 +45,12 @@ set logging enabled on|off
 show logging enabled
   These commands set or show whether logging is enabled or disabled.
 
+set style disassembler enabled on|off
+show style disassembler enabled
+  If GDB is compiled with Python support, and the Python Pygments
+  package is available, then, when this setting is on, disassembler
+  output will have styling applied.
+
 * Changed commands
 
 maint packet
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index a94b8c940bc..aae14512d9b 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -38,6 +38,11 @@ bool cli_styling = true;
 
 bool source_styling = true;
 
+/* True if disassembler styling is enabled.  Note that this is only
+   consulted when cli_styling is true.  */
+
+bool disassembler_styling = true;
+
 /* Name of colors; must correspond to ui_file_style::basic_color.  */
 static const char * const cli_colors[] = {
   "none",
@@ -274,6 +279,14 @@ cli_style_option::add_setshow_commands (enum command_class theclass,
 static cmd_list_element *style_set_list;
 static cmd_list_element *style_show_list;
 
+/* The command list for 'set style disassembler'.  */
+
+static cmd_list_element *style_disasm_set_list;
+
+/* The command list for 'show style disassembler'.  */
+
+static cmd_list_element *style_disasm_show_list;
+
 static void
 set_style_enabled  (const char *args, int from_tty, struct cmd_list_element *c)
 {
@@ -301,6 +314,18 @@ show_style_sources (struct ui_file *file, int from_tty,
     fprintf_filtered (file, _("Source code styling is disabled.\n"));
 }
 
+/* Implement 'show style disassembler'.  */
+
+static void
+show_style_disassembler (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  if (disassembler_styling)
+    fprintf_filtered (file, _("Disassembler output styling is enabled.\n"));
+  else
+    fprintf_filtered (file, _("Disassembler output styling is disabled.\n"));
+}
+
 void _initialize_cli_style ();
 void
 _initialize_cli_style ()
@@ -337,6 +362,25 @@ available if the appropriate extension is available at runtime."
 			   ), set_style_enabled, show_style_sources,
 			   &style_set_list, &style_show_list);
 
+  add_setshow_prefix_cmd ("disassembler", no_class,
+			  _("\
+Style-specific settings for the disassembler.\n\
+Configure various disassembler style-related variables."),
+			  _("\
+Style-specific settings for the disassembler.\n\
+Configure various disassembler style-related variables."),
+			  &style_disasm_set_list, &style_disasm_show_list,
+			  &style_set_list, &style_show_list);
+
+  add_setshow_boolean_cmd ("enabled", no_class, &disassembler_styling, _("\
+Set whether disassembler output styling is enabled."), _("\
+Show whether disassembler output styling is enabled."), _("\
+If enabled, disassembler output is styled.  Disassembler highlighting\n\
+requires the Python Pygments library, if this library is not available\n\
+then disassembler highlighting will not be possible."
+			   ), set_style_enabled, show_style_disassembler,
+			   &style_disasm_set_list, &style_disasm_show_list);
+
   file_name_style.add_setshow_commands (no_class, _("\
 Filename display styling.\n\
 Configure filename colors and display intensity."),
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 78bc2cd6f1e..5361a644a95 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -128,6 +128,9 @@ extern cli_style_option version_style;
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
+/* True if disassembler styling is enabled.  */
+extern bool disassembler_styling;
+
 /* True if styling is enabled.  */
 extern bool cli_styling;
 
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 4d1ee6893f5..8ab3998609c 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -782,9 +782,12 @@ get_all_disassembler_options (struct gdbarch *gdbarch)
 gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 				    struct ui_file *file,
 				    di_read_memory_ftype read_memory_func)
-  : m_gdbarch (gdbarch)
+  : m_gdbarch (gdbarch),
+    m_buffer (!use_ext_lang_colorization_p && disassembler_styling
+	      && file->can_emit_style_escape ()),
+    m_dest (file)
 {
-  init_disassemble_info (&m_di, file, dis_asm_fprintf);
+  init_disassemble_info (&m_di, &m_buffer, dis_asm_fprintf);
   m_di.flavour = bfd_target_unknown_flavour;
   m_di.memory_error_func = dis_asm_memory_error;
   m_di.print_address_func = dis_asm_print_address;
@@ -813,14 +816,65 @@ gdb_disassembler::~gdb_disassembler ()
   disassemble_free_target (&m_di);
 }
 
+/* See disasm.h.  */
+
+bool gdb_disassembler::use_ext_lang_colorization_p = true;
+
+/* See disasm.h.  */
+
 int
 gdb_disassembler::print_insn (CORE_ADDR memaddr,
 			      int *branch_delay_insns)
 {
   m_err_memaddr.reset ();
+  m_buffer.clear ();
 
   int length = gdbarch_print_insn (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 (length > 0 && disassembler_styling
+      && use_ext_lang_colorization_p
+      && m_dest->can_emit_style_escape ())
+    {
+      gdb::optional<std::string> ext_contents;
+      ext_contents = ext_lang_colorize_disasm (m_buffer.string (), arch ());
+      if (ext_contents.has_value ())
+	m_buffer.string () = std::move (*ext_contents);
+      else
+	{
+	  /* The extension language failed to add styling to the
+	     disassembly output.  Set the static flag so that next time we
+	     disassemble we don't even bother attempting to use the
+	     extension language for styling.  */
+	  use_ext_lang_colorization_p = 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
+	     need to recreate m_buffer, but this time with styling support.
+
+	     To do this we perform an in-place new, but this time turn on
+	     the styling support, then we can re-disassembly the
+	     instruction, and gain any minimal styling GDB might add.  */
+	  gdb_static_assert ((std::is_same<decltype (m_buffer),
+			      string_file>::value));
+	  gdb_assert (!m_buffer.term_out ());
+	  m_buffer.~string_file ();
+	  new (&m_buffer) string_file (true);
+	  length = gdbarch_print_insn (arch (), memaddr, &m_di);
+	  gdb_assert (length > 0);
+	}
+    }
+
+  /* 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 ());
+
+  /* If the disassembler failed then report an appropriate error.  */
   if (length < 0)
     {
       if (m_err_memaddr.has_value ())
diff --git a/gdb/disasm.h b/gdb/disasm.h
index f6de33e3db8..cb7f3768995 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -82,6 +82,31 @@ class gdb_disassembler
      non-memory error.  */
   gdb::optional<CORE_ADDR> m_err_memaddr;
 
+  /* 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 in general is turned on, and also, whether *m_dest supports
+     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.
+
+     This field will initially be true, but will be set to false if
+     ext_lang_colorize_disasm fails to add styling at any time.
+
+     If the extension language is going to add the styling then m_buffer
+     should be created without styling support, the extension language will
+     then add styling at the end of the disassembly process.
+
+     If the extension language is not going to add the styling, then we
+     create m_buffer with styling support, and GDB will add minimal styling
+     (currently just to addresses and symbols) as it goes.  */
+  static bool use_ext_lang_colorization_p;
+
   static int dis_asm_fprintf (void *stream, const char *format, ...)
     ATTRIBUTE_PRINTF(2,3);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d0c5bcf18e1..d411597de23 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26118,6 +26118,21 @@
 
 @item show style sources
 Show the current state of source code styling.
+
+@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}
+command, is styled.  Disassembler styling only works if styling in
+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}.
+
+@item show style disassembler enabled
+Show the current state of disassembler styling.
 @end table
 
 Subcommands of @code{set style} control specific forms of styling.
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index 77f23e0f911..b2150624dde 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -257,6 +257,12 @@ struct extension_language_ops
      or an empty option.  */
   gdb::optional<std::string> (*colorize) (const std::string &name,
 					  const std::string &contents);
+
+  /* Colorize a single line of disassembler output, CONTENT.  This should
+     either return colorized (using ANSI terminal escapes) version of the
+     contents, or an empty optional.  */
+  gdb::optional<std::string> (*colorize_disasm) (const std::string &content,
+						 gdbarch *gdbarch);
 };
 
 /* State necessary to restore a signal handler to its previous value.  */
diff --git a/gdb/extension.c b/gdb/extension.c
index 89ab29f3d1c..6862147ac2f 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -904,6 +904,26 @@ ext_lang_colorize (const std::string &filename, const std::string &contents)
   return result;
 }
 
+/* See extension.h.  */
+
+gdb::optional<std::string>
+ext_lang_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  gdb::optional<std::string> result;
+
+  for (const struct extension_language_defn *extlang : extension_languages)
+    {
+      if (extlang->ops == nullptr
+	  || extlang->ops->colorize_disasm == nullptr)
+	continue;
+      result = extlang->ops->colorize_disasm (content, gdbarch);
+      if (result.has_value ())
+	return result;
+    }
+
+  return result;
+}
+
 /* Called via an observer before gdb prints its prompt.
    Iterate over the extension languages giving them a chance to
    change the prompt.  The first one to change the prompt wins,
diff --git a/gdb/extension.h b/gdb/extension.h
index 2f2ca3e7743..e8d2fbc7fc3 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -319,6 +319,14 @@ extern void get_matching_xmethod_workers
 extern gdb::optional<std::string> ext_lang_colorize
   (const std::string &filename, const std::string &contents);
 
+/* Try to colorize a single line of disassembler output, CONTENT for
+   GDBARCH.  This will return either a colorized (using ANSI terminal
+   escapes) version of CONTENT, or an empty value if colorizing could not
+   be done.  */
+
+extern gdb::optional<std::string> ext_lang_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
+
 #if GDB_SELF_TEST
 namespace selftests {
 extern void (*hook_set_active_ext_lang) ();
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 7b6d8701548..6ab797c8c5e 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -243,8 +243,19 @@ try:
         except:
             return None
 
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip()
+        except:
+            return None
 
 except:
 
     def colorize(filename, contents):
         return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 82af012068b..7524489371d 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -121,6 +121,8 @@ static enum ext_lang_rc gdbpy_before_prompt_hook
   (const struct extension_language_defn *, const char *current_gdb_prompt);
 static gdb::optional<std::string> gdbpy_colorize
   (const std::string &filename, const std::string &contents);
+static gdb::optional<std::string> gdbpy_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
 
 /* The interface between gdb proper and loading of python scripts.  */
 
@@ -162,6 +164,8 @@ static const struct extension_language_ops python_extension_ops =
   gdbpy_get_matching_xmethod_workers,
 
   gdbpy_colorize,
+
+  gdbpy_colorize_disasm,
 };
 
 #endif /* HAVE_PYTHON */
@@ -1185,6 +1189,69 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
   return std::string (PyBytes_AsString (host_str.get ()));
 }
 
+/* This is the extension_language_ops.colorize_disasm "method".  */
+
+static gdb::optional<std::string>
+gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  if (!gdb_python_initialized)
+    return {};
+
+  gdbpy_enter enter_py (get_current_arch (), current_language);
+
+  if (gdb_python_module == nullptr
+      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+    return {};
+
+  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+					    "colorize_disasm"));
+  if (hook == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyCallable_Check (hook.get ()))
+    return {};
+
+  gdbpy_ref<> content_arg (PyString_FromString (content.c_str ()));
+  if (content_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> gdbarch_arg (gdbarch_to_arch_object (gdbarch));
+  if (gdbarch_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> result (PyObject_CallFunctionObjArgs (hook.get (),
+						    content_arg.get (),
+						    gdbarch_arg.get (),
+						    nullptr));
+  if (result == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (result == Py_None)
+    return {};
+
+  gdb::unique_xmalloc_ptr<char> str
+    = python_string_to_host_string (result.get ());
+  if (str == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+ return std::string (str.get ());
+}
+
 \f
 
 /* Printing.  */
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 5749f7b0d4c..076c5e24b31 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -13,6 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+load_lib gdb-python.exp
+
 # Test CLI output styling.
 
 standard_testfile
@@ -187,8 +189,16 @@ proc run_style_tests { } {
 
 	gdb_test_no_output "set width 0"
 
+	# If disassembler styling is being done by the Python pygments
+	# module, then we can't be sure how the 'some_called_function'
+	# symbol will be styled.  However, if pygments is not being
+	# used then we can know how the symbol name will be styled.
 	set main [limited_style main function]
-	set func [limited_style some_called_function function]
+	if { $::python_disassembly_styling } {
+	    set func "some_called_function"
+	} else {
+	    set func [limited_style some_called_function function]
+	}
 	# Somewhere should see the call to the function.
 	gdb_test "disassemble main" \
 	    [concat "Dump of assembler code for function $main:.*" \
@@ -304,6 +314,62 @@ proc run_style_tests { } {
     }
 }
 
+# Check that disassembler styling can be disabled.  The function that
+# we are disassembling has some minimal styling applied even if the
+# Python pygments module is not available, so, when we disable
+# disassembler styling, we should always see a change in output.
+proc test_disable_disassembler_styling { } {
+    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 $::binfile
+
+	set styled_hex [limited_style $::hex address]
+	set main [limited_style main function]
+
+	foreach_with_prefix disasm_styling { on off } {
+	    gdb_test_no_output "set style disassembler enabled ${disasm_styling}"
+
+	    set saw_header_line false
+	    set saw_styled_output_line false
+	    set saw_unstyled_output_line false
+	    gdb_test_multiple "disassemble main" "" {
+		-re "disassemble main\r\n" {
+		    exp_continue
+		}
+		-re "^Dump of assembler code for function $main:" {
+		    set saw_header_line true
+		    exp_continue
+		}
+		-re "^\\s+${styled_hex}\\s+<\[^>\]+>:\\s+\[^\r\n\033\]+\r\n" {
+		    set saw_unstyled_output_line true
+		    exp_continue
+		}
+		-re "^\\s+${styled_hex}\\s+<\[^>\]+>:\\s+\[^\r\n\]+\033\[^\r\n\]+\r\n" {
+		    set saw_styled_output_line true
+		    exp_continue
+		}
+		-re "^End of assembler dump\\.\r\n" {
+		    exp_continue
+		}
+		-re "^$::gdb_prompt $" {
+		    gdb_assert { $saw_header_line }
+		    if { $disasm_styling } {
+			gdb_assert { $saw_styled_output_line }
+			gdb_assert { !$saw_unstyled_output_line }
+		    } else {
+			gdb_assert { !$saw_styled_output_line }
+			gdb_assert { $saw_unstyled_output_line }
+		    }
+		}
+	    }
+	}
+    }
+}
+
 # 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).
@@ -317,6 +383,15 @@ proc test_startup_version_string { } {
     gdb_test "" "${vers}.*" "version is styled at startup"
 }
 
+# Check to see if the Python styling of disassembler output is
+# expected or not, this styling requires Python support in GDB, and
+# the Python pygments module to be available.
+clean_restart ${binfile}
+if {![skip_python_tests] && [gdb_py_module_available "pygments"]} {
+    set python_disassembly_styling true
+} else {
+    set python_disassembly_styling false
+}
 
 # Run tests with all styles in their default state.
 with_test_prefix "all styles enabled" {
@@ -333,5 +408,10 @@ foreach style { title file function highlight variable \
     }
 }
 
+# Check that the disassembler styling can be disabled.
+if { $python_disassembly_styling } {
+    test_disable_disassembler_styling
+}
+
 # Finally, check the styling of the version string during startup.
 test_startup_version_string
-- 
2.25.4

[PATCHv3 2/2] gdb/python: move styling support to gdb.styling

[Andrew Burgess]

From: Andrew Burgess <andrew.burgess@embecosm.com>

This commit moves the two Python functions that are used for styling
into a new module, gdb.styling, there's then a small update in
python.c so GDB can find the functions in their new location.

The motivation for this change is purely to try and reduce the clutter
in the top-level gdb module, and encapsulate related functions into
modules.  I did ponder documenting these functions as part of the
Python API, however, doing so would effectively "fix" the API, and I'm
still wondering if there's improvements that could be made, also, the
colorize function is only called in some cases now that GDB prefers
libsource-highlight, so it's not entirely sure how this would work as
part of a user facing API.

Still, despite these functions never having been part of a documented
API, it is possible that a user out there has overridden these to, in
some way, customize how GDB performs styling.  Moving the function as
I propose in this patch could break things for that user, however,
fixing this breakage is trivial, and, as these functions were never
documented, I don't think we should be obliged to not break user code
that relies on them.
---
 gdb/data-directory/Makefile.in |  1 +
 gdb/python/lib/gdb/__init__.py | 29 --------------------
 gdb/python/lib/gdb/styling.py  | 48 ++++++++++++++++++++++++++++++++++
 gdb/python/python.c            | 24 ++++++++++++-----
 4 files changed, 67 insertions(+), 35 deletions(-)
 create mode 100644 gdb/python/lib/gdb/styling.py

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 888325f974e..ab0733e29dc 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -74,6 +74,7 @@ PYTHON_FILE_LIST = \
 	gdb/frames.py \
 	gdb/printing.py \
 	gdb/prompt.py \
+	gdb/styling.py \
 	gdb/types.py \
 	gdb/unwinder.py \
 	gdb/xmethod.py \
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 6ab797c8c5e..8d63ae67fe8 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -230,32 +230,3 @@ def find_pc_line(pc):
     Return the gdb.Symtab_and_line object corresponding to the pc value."""
     return current_progspace().find_pc_line(pc)
 
-
-try:
-    from pygments import formatters, lexers, highlight
-
-    def colorize(filename, contents):
-        # Don't want any errors.
-        try:
-            lexer = lexers.get_lexer_for_filename(filename, stripnl=False)
-            formatter = formatters.TerminalFormatter()
-            return highlight(contents, lexer, formatter)
-        except:
-            return None
-
-    def colorize_disasm(content, gdbarch):
-        # Don't want any errors.
-        try:
-            lexer = lexers.get_lexer_by_name("asm")
-            formatter = formatters.TerminalFormatter()
-            return highlight(content, lexer, formatter).rstrip()
-        except:
-            return None
-
-except:
-
-    def colorize(filename, contents):
-        return None
-
-    def colorize_disasm(content, gdbarch):
-        return None
diff --git a/gdb/python/lib/gdb/styling.py b/gdb/python/lib/gdb/styling.py
new file mode 100644
index 00000000000..d03c0c7252a
--- /dev/null
+++ b/gdb/python/lib/gdb/styling.py
@@ -0,0 +1,48 @@
+# Styling related hooks.
+# Copyright (C) 2010-2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Utilities for styling."""
+
+import gdb
+
+try:
+    from pygments import formatters, lexers, highlight
+
+    def colorize(filename, contents):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_for_filename(filename, stripnl=False)
+            formatter = formatters.TerminalFormatter()
+            return highlight(contents, lexer, formatter)
+        except:
+            return None
+
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip()
+        except:
+            return None
+
+except:
+
+    def colorize(filename, contents):
+        return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 7524489371d..0a9b14917bf 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1131,11 +1131,17 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  if (gdb_python_module == nullptr
-      || !PyObject_HasAttrString (gdb_python_module, "colorize"))
+  gdbpy_ref<> module (PyImport_ImportModule ("gdb.styling"));
+  if (module == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyObject_HasAttrString (module.get (), "colorize"))
     return {};
 
-  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module, "colorize"));
+  gdbpy_ref<> hook (PyObject_GetAttrString (module.get (), "colorize"));
   if (hook == nullptr)
     {
       gdbpy_print_stack ();
@@ -1199,11 +1205,17 @@ gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  if (gdb_python_module == nullptr
-      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+  gdbpy_ref<> module (PyImport_ImportModule ("gdb.styling"));
+  if (module == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyObject_HasAttrString (module.get (), "colorize_disasm"))
     return {};
 
-  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+  gdbpy_ref<> hook (PyObject_GetAttrString (module.get (),
 					    "colorize_disasm"));
   if (hook == nullptr)
     {
-- 
2.25.4

[PATCHv4 0/2] Disassembler Output Styling

[Andrew Burgess]

Changes in v4:

  Rebased on to current master.  The colorize_disasm function is now
  passed a bytes object rather than a unicode string.  The return from
  colorize_disasm can be either a bytes object, or a unicode string.
  If GDB gets a unicode string, then it is converted to bytes using
  the current host_charset().

  There's no change in the documentation in this version, the
  documentation has already been reviewed in a previous version.

Changes in v3:

  In this update, the biggest change, is that the controlling setting is
  now 'set/show style disassembler enabled', that is I've added
  'enabled' to the setting name.
  
  My reason for this is that, in the future, I might want to add
  additional disassembler styles, like:
  
    set style disassembler register ....
    set style disassembler mnemonic ....
  
  And the new setting name leaves this possibility open, while the old
  setting made this harder.
  
  This version also includes an improved test, minor updates to the docs
  to match the above change, and a rebase to current master.

---

Andrew Burgess (2):
  gdb: use python to colorize disassembler output
  gdb/python: move styling support to gdb.styling

 gdb/NEWS                         |  6 ++
 gdb/cli/cli-style.c              | 44 ++++++++++++++
 gdb/cli/cli-style.h              |  3 +
 gdb/data-directory/Makefile.in   |  1 +
 gdb/disasm.c                     | 58 ++++++++++++++++++-
 gdb/disasm.h                     | 25 ++++++++
 gdb/doc/gdb.texinfo              | 15 +++++
 gdb/extension-priv.h             |  6 ++
 gdb/extension.c                  | 20 +++++++
 gdb/extension.h                  |  8 +++
 gdb/python/lib/gdb/__init__.py   | 18 ------
 gdb/python/lib/gdb/styling.py    | 49 ++++++++++++++++
 gdb/python/python.c              | 98 +++++++++++++++++++++++++++++++-
 gdb/testsuite/gdb.base/style.exp | 82 +++++++++++++++++++++++++-
 14 files changed, 409 insertions(+), 24 deletions(-)
 create mode 100644 gdb/python/lib/gdb/styling.py

-- 
2.25.4

[PATCHv4 1/2] gdb: use python to colorize disassembler output

[Andrew Burgess]

From: Andrew Burgess <andrew.burgess@embecosm.com>

This commit adds styling support to the disassembler output, as such
two new commands are added to GDB:

  set style disassembler enabled on|off
  show style disassembler enabled

In this commit I make use of the Python Pygments package to provide
the styling.  I did investigate making use of libsource-highlight,
however, I found the highlighting results to be inferior to those of
Pygments; only some mnemonics were highlighted, and highlighting of
register names such as r9d and r8d (on x86-64) was incorrect.

To enable disassembler highlighting via Pygments, I've added a new
extension language hook, which is then implemented for Python.  This
hook is very similar to the existing hook for source code
colorization.

One possibly odd choice I made with the new hook is to pass a
gdb.Architecture through, even though this is currently unused.  The
reason this argument is not used is that, currently, styling is
performed identically for all architectures.

However, even though the Python function used to perform styling of
disassembly output is not part of any documented API, I don't want
to close the door on a user overriding this function to provide
architecture specific styling.  To do this, the user would inevitably
require access to the gdb.Architecture, and so I decided to add this
field now.

The styling is applied within gdb_disassembler::print_insn, to achieve
this, gdb_disassembler now writes its output into a temporary buffer,
styling is then applied to the contents of this buffer.  Finally the
gdb_disassembler buffer is copied out to its final destination stream.

There's a new test to check that the disassembler output includes some
escape sequences, though I don't check for specific colours; the
precise colors will depend on which instructions are in the
disassembler output, and, I guess, how pygments is configured.

The only negative change with this commit is how we currently style
addresses in GDB.

Currently, when the disassembler wants to print an address, we call
back into GDB, and GDB prints the address value using the `address`
styling, and the symbol name using `function` styling.  After this
commit, if pygments is used, then all disassembler styling is done
through pygments, and this include the address and symbol name parts
of the disassembler output.

I don't know how much of an issue this will be for people.  There's
already some precedent for this in GDB when we look at source styling.
For example, function names in styled source listings are not styled
using the `function` style, but instead, either GNU Source Highlight,
or pygments gets to decide how the function name should be styled.

If the Python pygments library is not present then GDB will continue
to behave as it always has, the disassembler output is mostly
unstyled, but the address and symbols are styled using the `address`
and `function` styles, as they are today.

However, if the user does `set style disassembler enabled off`, then
all disassembler styling is switched off.  This obviously covers the
use of pygments, but also includes the minimal styling done by GDB
when pygments is not available.
---
 gdb/NEWS                         |  6 +++
 gdb/cli/cli-style.c              | 44 +++++++++++++++++
 gdb/cli/cli-style.h              |  3 ++
 gdb/disasm.c                     | 58 +++++++++++++++++++++-
 gdb/disasm.h                     | 25 ++++++++++
 gdb/doc/gdb.texinfo              | 15 ++++++
 gdb/extension-priv.h             |  6 +++
 gdb/extension.c                  | 20 ++++++++
 gdb/extension.h                  |  8 ++++
 gdb/python/lib/gdb/__init__.py   | 13 +++++
 gdb/python/python.c              | 80 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/style.exp | 82 +++++++++++++++++++++++++++++++-
 12 files changed, 357 insertions(+), 3 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index c26e15b530a..f097699ba60 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -64,6 +64,12 @@ set debug threads on|off
 show debug threads
   Print additional debug messages about thread creation and deletion.
 
+set style disassembler enabled on|off
+show style disassembler enabled
+  If GDB is compiled with Python support, and the Python Pygments
+  package is available, then, when this setting is on, disassembler
+  output will have styling applied.
+
 * Changed commands
 
 maint packet
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index 2fd00e9cc3e..6c1652d3986 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -38,6 +38,11 @@ bool cli_styling = true;
 
 bool source_styling = true;
 
+/* True if disassembler styling is enabled.  Note that this is only
+   consulted when cli_styling is true.  */
+
+bool disassembler_styling = true;
+
 /* Name of colors; must correspond to ui_file_style::basic_color.  */
 static const char * const cli_colors[] = {
   "none",
@@ -274,6 +279,14 @@ cli_style_option::add_setshow_commands (enum command_class theclass,
 static cmd_list_element *style_set_list;
 static cmd_list_element *style_show_list;
 
+/* The command list for 'set style disassembler'.  */
+
+static cmd_list_element *style_disasm_set_list;
+
+/* The command list for 'show style disassembler'.  */
+
+static cmd_list_element *style_disasm_show_list;
+
 static void
 set_style_enabled  (const char *args, int from_tty, struct cmd_list_element *c)
 {
@@ -301,6 +314,18 @@ show_style_sources (struct ui_file *file, int from_tty,
     fprintf_filtered (file, _("Source code styling is disabled.\n"));
 }
 
+/* Implement 'show style disassembler'.  */
+
+static void
+show_style_disassembler (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  if (disassembler_styling)
+    fprintf_filtered (file, _("Disassembler output styling is enabled.\n"));
+  else
+    fprintf_filtered (file, _("Disassembler output styling is disabled.\n"));
+}
+
 void _initialize_cli_style ();
 void
 _initialize_cli_style ()
@@ -337,6 +362,25 @@ available if the appropriate extension is available at runtime."
 			   ), set_style_enabled, show_style_sources,
 			   &style_set_list, &style_show_list);
 
+  add_setshow_prefix_cmd ("disassembler", no_class,
+			  _("\
+Style-specific settings for the disassembler.\n\
+Configure various disassembler style-related variables."),
+			  _("\
+Style-specific settings for the disassembler.\n\
+Configure various disassembler style-related variables."),
+			  &style_disasm_set_list, &style_disasm_show_list,
+			  &style_set_list, &style_show_list);
+
+  add_setshow_boolean_cmd ("enabled", no_class, &disassembler_styling, _("\
+Set whether disassembler output styling is enabled."), _("\
+Show whether disassembler output styling is enabled."), _("\
+If enabled, disassembler output is styled.  Disassembler highlighting\n\
+requires the Python Pygments library, if this library is not available\n\
+then disassembler highlighting will not be possible."
+			   ), set_style_enabled, show_style_disassembler,
+			   &style_disasm_set_list, &style_disasm_show_list);
+
   file_name_style.add_setshow_commands (no_class, _("\
 Filename display styling.\n\
 Configure filename colors and display intensity."),
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 3333c72f65a..f69df47098c 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -128,6 +128,9 @@ extern cli_style_option version_style;
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
+/* True if disassembler styling is enabled.  */
+extern bool disassembler_styling;
+
 /* True if styling is enabled.  */
 extern bool cli_styling;
 
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 724040faa92..cd6ce47f9fa 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -782,9 +782,12 @@ get_all_disassembler_options (struct gdbarch *gdbarch)
 gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 				    struct ui_file *file,
 				    di_read_memory_ftype read_memory_func)
-  : m_gdbarch (gdbarch)
+  : m_gdbarch (gdbarch),
+    m_buffer (!use_ext_lang_colorization_p && disassembler_styling
+	      && file->can_emit_style_escape ()),
+    m_dest (file)
 {
-  init_disassemble_info (&m_di, file, dis_asm_fprintf);
+  init_disassemble_info (&m_di, &m_buffer, dis_asm_fprintf);
   m_di.flavour = bfd_target_unknown_flavour;
   m_di.memory_error_func = dis_asm_memory_error;
   m_di.print_address_func = dis_asm_print_address;
@@ -813,14 +816,65 @@ gdb_disassembler::~gdb_disassembler ()
   disassemble_free_target (&m_di);
 }
 
+/* See disasm.h.  */
+
+bool gdb_disassembler::use_ext_lang_colorization_p = true;
+
+/* See disasm.h.  */
+
 int
 gdb_disassembler::print_insn (CORE_ADDR memaddr,
 			      int *branch_delay_insns)
 {
   m_err_memaddr.reset ();
+  m_buffer.clear ();
 
   int length = gdbarch_print_insn (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 (length > 0 && disassembler_styling
+      && use_ext_lang_colorization_p
+      && m_dest->can_emit_style_escape ())
+    {
+      gdb::optional<std::string> ext_contents;
+      ext_contents = ext_lang_colorize_disasm (m_buffer.string (), arch ());
+      if (ext_contents.has_value ())
+	m_buffer.string () = std::move (*ext_contents);
+      else
+	{
+	  /* The extension language failed to add styling to the
+	     disassembly output.  Set the static flag so that next time we
+	     disassemble we don't even bother attempting to use the
+	     extension language for styling.  */
+	  use_ext_lang_colorization_p = 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
+	     need to recreate m_buffer, but this time with styling support.
+
+	     To do this we perform an in-place new, but this time turn on
+	     the styling support, then we can re-disassembly the
+	     instruction, and gain any minimal styling GDB might add.  */
+	  gdb_static_assert ((std::is_same<decltype (m_buffer),
+			      string_file>::value));
+	  gdb_assert (!m_buffer.term_out ());
+	  m_buffer.~string_file ();
+	  new (&m_buffer) string_file (true);
+	  length = gdbarch_print_insn (arch (), memaddr, &m_di);
+	  gdb_assert (length > 0);
+	}
+    }
+
+  /* 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 ());
+
+  /* If the disassembler failed then report an appropriate error.  */
   if (length < 0)
     {
       if (m_err_memaddr.has_value ())
diff --git a/gdb/disasm.h b/gdb/disasm.h
index d739b57d898..3c14c50be3a 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -82,6 +82,31 @@ class gdb_disassembler
      non-memory error.  */
   gdb::optional<CORE_ADDR> m_err_memaddr;
 
+  /* 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 in general is turned on, and also, whether *m_dest supports
+     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.
+
+     This field will initially be true, but will be set to false if
+     ext_lang_colorize_disasm fails to add styling at any time.
+
+     If the extension language is going to add the styling then m_buffer
+     should be created without styling support, the extension language will
+     then add styling at the end of the disassembly process.
+
+     If the extension language is not going to add the styling, then we
+     create m_buffer with styling support, and GDB will add minimal styling
+     (currently just to addresses and symbols) as it goes.  */
+  static bool use_ext_lang_colorization_p;
+
   static int dis_asm_fprintf (void *stream, const char *format, ...)
     ATTRIBUTE_PRINTF(2,3);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 48873aa34fe..9403bd7c0cb 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26132,6 +26132,21 @@
 
 @item show style sources
 Show the current state of source code styling.
+
+@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}
+command, is styled.  Disassembler styling only works if styling in
+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}.
+
+@item show style disassembler enabled
+Show the current state of disassembler styling.
 @end table
 
 Subcommands of @code{set style} control specific forms of styling.
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index ed2121a127b..d9450b51231 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -257,6 +257,12 @@ struct extension_language_ops
      or an empty option.  */
   gdb::optional<std::string> (*colorize) (const std::string &name,
 					  const std::string &contents);
+
+  /* Colorize a single line of disassembler output, CONTENT.  This should
+     either return colorized (using ANSI terminal escapes) version of the
+     contents, or an empty optional.  */
+  gdb::optional<std::string> (*colorize_disasm) (const std::string &content,
+						 gdbarch *gdbarch);
 };
 
 /* State necessary to restore a signal handler to its previous value.  */
diff --git a/gdb/extension.c b/gdb/extension.c
index f04e928d33d..8f39b86e952 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -904,6 +904,26 @@ ext_lang_colorize (const std::string &filename, const std::string &contents)
   return result;
 }
 
+/* See extension.h.  */
+
+gdb::optional<std::string>
+ext_lang_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  gdb::optional<std::string> result;
+
+  for (const struct extension_language_defn *extlang : extension_languages)
+    {
+      if (extlang->ops == nullptr
+	  || extlang->ops->colorize_disasm == nullptr)
+	continue;
+      result = extlang->ops->colorize_disasm (content, gdbarch);
+      if (result.has_value ())
+	return result;
+    }
+
+  return result;
+}
+
 /* Called via an observer before gdb prints its prompt.
    Iterate over the extension languages giving them a chance to
    change the prompt.  The first one to change the prompt wins,
diff --git a/gdb/extension.h b/gdb/extension.h
index 64d7396f5b7..7eb89530c44 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -319,6 +319,14 @@ extern void get_matching_xmethod_workers
 extern gdb::optional<std::string> ext_lang_colorize
   (const std::string &filename, const std::string &contents);
 
+/* Try to colorize a single line of disassembler output, CONTENT for
+   GDBARCH.  This will return either a colorized (using ANSI terminal
+   escapes) version of CONTENT, or an empty value if colorizing could not
+   be done.  */
+
+extern gdb::optional<std::string> ext_lang_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
+
 #if GDB_SELF_TEST
 namespace selftests {
 extern void (*hook_set_active_ext_lang) ();
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 11a1b444bfd..8cbf1c0791d 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -243,7 +243,20 @@ try:
         except:
             return None
 
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip()
+        except:
+            return None
+
+
 except:
 
     def colorize(filename, contents):
         return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index e05b99c0bec..0d1a1256e9a 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -121,6 +121,8 @@ static enum ext_lang_rc gdbpy_before_prompt_hook
   (const struct extension_language_defn *, const char *current_gdb_prompt);
 static gdb::optional<std::string> gdbpy_colorize
   (const std::string &filename, const std::string &contents);
+static gdb::optional<std::string> gdbpy_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
 
 /* The interface between gdb proper and loading of python scripts.  */
 
@@ -162,6 +164,8 @@ static const struct extension_language_ops python_extension_ops =
   gdbpy_get_matching_xmethod_workers,
 
   gdbpy_colorize,
+
+  gdbpy_colorize_disasm,
 };
 
 #endif /* HAVE_PYTHON */
@@ -1185,6 +1189,82 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
   return std::string (PyBytes_AsString (host_str.get ()));
 }
 
+/* This is the extension_language_ops.colorize_disasm "method".  */
+
+static gdb::optional<std::string>
+gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  if (!gdb_python_initialized)
+    return {};
+
+  gdbpy_enter enter_py (get_current_arch (), current_language);
+
+  if (gdb_python_module == nullptr
+      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+    return {};
+
+  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+					    "colorize_disasm"));
+  if (hook == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyCallable_Check (hook.get ()))
+    return {};
+
+  gdbpy_ref<> content_arg (PyBytes_FromString (content.c_str ()));
+  if (content_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> gdbarch_arg (gdbarch_to_arch_object (gdbarch));
+  if (gdbarch_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> result (PyObject_CallFunctionObjArgs (hook.get (),
+						    content_arg.get (),
+						    gdbarch_arg.get (),
+						    nullptr));
+  if (result == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (result == Py_None)
+    return {};
+
+  if (PyBytes_Check (result.get ()))
+    return std::string (PyBytes_AsString (result.get ()));
+  else if (PyUnicode_Check (result.get ()))
+    {
+      gdbpy_ref<> bytes (PyUnicode_AsEncodedString (result.get (),
+						    host_charset (),
+						    nullptr));
+      if (bytes == nullptr)
+	{
+	  gdbpy_print_stack ();
+	  return {};
+	}
+
+      return std::string (PyBytes_AsString (bytes.get ()));
+    }
+  else
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       _("Unexpected type returned from gdb.colorize_disasm."));
+      gdbpy_print_stack ();
+      return {};
+    }
+}
+
 \f
 
 /* Printing.  */
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 2cd155d2cf2..68196d6e3e2 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -13,6 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+load_lib gdb-python.exp
+
 # Test CLI output styling.
 
 standard_testfile
@@ -187,8 +189,16 @@ proc run_style_tests { } {
 
 	gdb_test_no_output "set width 0"
 
+	# If disassembler styling is being done by the Python pygments
+	# module, then we can't be sure how the 'some_called_function'
+	# symbol will be styled.  However, if pygments is not being
+	# used then we can know how the symbol name will be styled.
 	set main [limited_style main function]
-	set func [limited_style some_called_function function]
+	if { $::python_disassembly_styling } {
+	    set func "some_called_function"
+	} else {
+	    set func [limited_style some_called_function function]
+	}
 	# Somewhere should see the call to the function.
 	gdb_test "disassemble main" \
 	    [concat "Dump of assembler code for function $main:.*" \
@@ -304,6 +314,62 @@ proc run_style_tests { } {
     }
 }
 
+# Check that disassembler styling can be disabled.  The function that
+# we are disassembling has some minimal styling applied even if the
+# Python pygments module is not available, so, when we disable
+# disassembler styling, we should always see a change in output.
+proc test_disable_disassembler_styling { } {
+    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 $::binfile
+
+	set styled_hex [limited_style $::hex address]
+	set main [limited_style main function]
+
+	foreach_with_prefix disasm_styling { on off } {
+	    gdb_test_no_output "set style disassembler enabled ${disasm_styling}"
+
+	    set saw_header_line false
+	    set saw_styled_output_line false
+	    set saw_unstyled_output_line false
+	    gdb_test_multiple "disassemble main" "" {
+		-re "disassemble main\r\n" {
+		    exp_continue
+		}
+		-re "^Dump of assembler code for function $main:" {
+		    set saw_header_line true
+		    exp_continue
+		}
+		-re "^\\s+${styled_hex}\\s+<\[^>\]+>:\\s+\[^\r\n\033\]+\r\n" {
+		    set saw_unstyled_output_line true
+		    exp_continue
+		}
+		-re "^\\s+${styled_hex}\\s+<\[^>\]+>:\\s+\[^\r\n\]+\033\[^\r\n\]+\r\n" {
+		    set saw_styled_output_line true
+		    exp_continue
+		}
+		-re "^End of assembler dump\\.\r\n" {
+		    exp_continue
+		}
+		-re "^$::gdb_prompt $" {
+		    gdb_assert { $saw_header_line }
+		    if { $disasm_styling } {
+			gdb_assert { $saw_styled_output_line }
+			gdb_assert { !$saw_unstyled_output_line }
+		    } else {
+			gdb_assert { !$saw_styled_output_line }
+			gdb_assert { $saw_unstyled_output_line }
+		    }
+		}
+	    }
+	}
+    }
+}
+
 # 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).
@@ -317,6 +383,15 @@ proc test_startup_version_string { } {
     gdb_test "" "${vers}.*" "version is styled at startup"
 }
 
+# Check to see if the Python styling of disassembler output is
+# expected or not, this styling requires Python support in GDB, and
+# the Python pygments module to be available.
+clean_restart ${binfile}
+if {![skip_python_tests] && [gdb_py_module_available "pygments"]} {
+    set python_disassembly_styling true
+} else {
+    set python_disassembly_styling false
+}
 
 # Run tests with all styles in their default state.
 with_test_prefix "all styles enabled" {
@@ -333,5 +408,10 @@ foreach style { title file function highlight variable \
     }
 }
 
+# Check that the disassembler styling can be disabled.
+if { $python_disassembly_styling } {
+    test_disable_disassembler_styling
+}
+
 # Finally, check the styling of the version string during startup.
 test_startup_version_string
-- 
2.25.4

Re: [PATCHv4 1/2] gdb: use python to colorize disassembler output

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> In this commit I make use of the Python Pygments package to provide
Andrew> the styling.  I did investigate making use of libsource-highlight,
Andrew> however, I found the highlighting results to be inferior to those of
Andrew> Pygments; only some mnemonics were highlighted, and highlighting of
Andrew> register names such as r9d and r8d (on x86-64) was incorrect.

I think it only really handles x86.

I like this patch and think it should go in.  I found one nit.  Also,
what's up with the libopcodes-based colorizing?  That seemed promising
to me.

Andrew> +  gdbpy_enter enter_py (get_current_arch (), current_language);

I think the parameters here can be removed.

Tom

Re: [PATCHv4 1/2] gdb: use python to colorize disassembler output

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2022-02-10 14:13:30 -0700]:

> >>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Andrew> In this commit I make use of the Python Pygments package to provide
> Andrew> the styling.  I did investigate making use of libsource-highlight,
> Andrew> however, I found the highlighting results to be inferior to those of
> Andrew> Pygments; only some mnemonics were highlighted, and highlighting of
> Andrew> register names such as r9d and r8d (on x86-64) was incorrect.
> 
> I think it only really handles x86.
> 
> I like this patch and think it should go in.  I found one nit.  Also,
> what's up with the libopcodes-based colorizing?  That seemed promising
> to me.

I have a patch ready to post for libopcodes styling, but I wanted to
land this one before posting that patch, just to avoid any confusion.

As the libopcodes change would still require each disassembler to be
updated, I imagine that would take some time.  So, until that was
completed, having this patch in would be helpful.

After that, I think this will still have use.  I imagine having a
switch to disable libopcode styling, and a user could (potentially)
override the default colorize_disasm to implement their own styling.

> Andrew> +  gdbpy_enter enter_py (get_current_arch (), current_language);

Good catch.  Fixed.

Turns out that some changes to ui_file.h forced me to change how I
assign a string into a string_file object.

In the patch below, the only significant change, that I think needs
review is the new method string_file::operator=.  I'd feel better if
another pair of eyes has looked at that before I push it.

Thanks,
Andrew

---

commit 107db2a932ccda80648668e67be227dd10f15e20
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Mon Oct 25 17:26:57 2021 +0100

    gdb: use python to colorize disassembler output
    
    This commit adds styling support to the disassembler output, as such
    two new commands are added to GDB:
    
      set style disassembler enabled on|off
      show style disassembler enabled
    
    In this commit I make use of the Python Pygments package to provide
    the styling.  I did investigate making use of libsource-highlight,
    however, I found the highlighting results to be inferior to those of
    Pygments; only some mnemonics were highlighted, and highlighting of
    register names such as r9d and r8d (on x86-64) was incorrect.
    
    To enable disassembler highlighting via Pygments, I've added a new
    extension language hook, which is then implemented for Python.  This
    hook is very similar to the existing hook for source code
    colorization.
    
    One possibly odd choice I made with the new hook is to pass a
    gdb.Architecture through, even though this is currently unused.  The
    reason this argument is not used is that, currently, styling is
    performed identically for all architectures.
    
    However, even though the Python function used to perform styling of
    disassembly output is not part of any documented API, I don't want
    to close the door on a user overriding this function to provide
    architecture specific styling.  To do this, the user would inevitably
    require access to the gdb.Architecture, and so I decided to add this
    field now.
    
    The styling is applied within gdb_disassembler::print_insn, to achieve
    this, gdb_disassembler now writes its output into a temporary buffer,
    styling is then applied to the contents of this buffer.  Finally the
    gdb_disassembler buffer is copied out to its final destination stream.
    
    There's a new test to check that the disassembler output includes some
    escape sequences, though I don't check for specific colours; the
    precise colors will depend on which instructions are in the
    disassembler output, and, I guess, how pygments is configured.
    
    The only negative change with this commit is how we currently style
    addresses in GDB.
    
    Currently, when the disassembler wants to print an address, we call
    back into GDB, and GDB prints the address value using the `address`
    styling, and the symbol name using `function` styling.  After this
    commit, if pygments is used, then all disassembler styling is done
    through pygments, and this include the address and symbol name parts
    of the disassembler output.
    
    I don't know how much of an issue this will be for people.  There's
    already some precedent for this in GDB when we look at source styling.
    For example, function names in styled source listings are not styled
    using the `function` style, but instead, either GNU Source Highlight,
    or pygments gets to decide how the function name should be styled.
    
    If the Python pygments library is not present then GDB will continue
    to behave as it always has, the disassembler output is mostly
    unstyled, but the address and symbols are styled using the `address`
    and `function` styles, as they are today.
    
    However, if the user does `set style disassembler enabled off`, then
    all disassembler styling is switched off.  This obviously covers the
    use of pygments, but also includes the minimal styling done by GDB
    when pygments is not available.

diff --git a/gdb/NEWS b/gdb/NEWS
index b4a515120db..619f6c48760 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -89,6 +89,12 @@ show suppress-cli-notifications
   the program being debugged stops (e.g., because of hitting a
   breakpoint, completing source-stepping, an interrupt, etc.).
 
+set style disassembler enabled on|off
+show style disassembler enabled
+  If GDB is compiled with Python support, and the Python Pygments
+  package is available, then, when this setting is on, disassembler
+  output will have styling applied.
+
 * Changed commands
 
 maint packet
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index 2fd00e9cc3e..6c1652d3986 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -38,6 +38,11 @@ bool cli_styling = true;
 
 bool source_styling = true;
 
+/* True if disassembler styling is enabled.  Note that this is only
+   consulted when cli_styling is true.  */
+
+bool disassembler_styling = true;
+
 /* Name of colors; must correspond to ui_file_style::basic_color.  */
 static const char * const cli_colors[] = {
   "none",
@@ -274,6 +279,14 @@ cli_style_option::add_setshow_commands (enum command_class theclass,
 static cmd_list_element *style_set_list;
 static cmd_list_element *style_show_list;
 
+/* The command list for 'set style disassembler'.  */
+
+static cmd_list_element *style_disasm_set_list;
+
+/* The command list for 'show style disassembler'.  */
+
+static cmd_list_element *style_disasm_show_list;
+
 static void
 set_style_enabled  (const char *args, int from_tty, struct cmd_list_element *c)
 {
@@ -301,6 +314,18 @@ show_style_sources (struct ui_file *file, int from_tty,
     fprintf_filtered (file, _("Source code styling is disabled.\n"));
 }
 
+/* Implement 'show style disassembler'.  */
+
+static void
+show_style_disassembler (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  if (disassembler_styling)
+    fprintf_filtered (file, _("Disassembler output styling is enabled.\n"));
+  else
+    fprintf_filtered (file, _("Disassembler output styling is disabled.\n"));
+}
+
 void _initialize_cli_style ();
 void
 _initialize_cli_style ()
@@ -337,6 +362,25 @@ available if the appropriate extension is available at runtime."
 			   ), set_style_enabled, show_style_sources,
 			   &style_set_list, &style_show_list);
 
+  add_setshow_prefix_cmd ("disassembler", no_class,
+			  _("\
+Style-specific settings for the disassembler.\n\
+Configure various disassembler style-related variables."),
+			  _("\
+Style-specific settings for the disassembler.\n\
+Configure various disassembler style-related variables."),
+			  &style_disasm_set_list, &style_disasm_show_list,
+			  &style_set_list, &style_show_list);
+
+  add_setshow_boolean_cmd ("enabled", no_class, &disassembler_styling, _("\
+Set whether disassembler output styling is enabled."), _("\
+Show whether disassembler output styling is enabled."), _("\
+If enabled, disassembler output is styled.  Disassembler highlighting\n\
+requires the Python Pygments library, if this library is not available\n\
+then disassembler highlighting will not be possible."
+			   ), set_style_enabled, show_style_disassembler,
+			   &style_disasm_set_list, &style_disasm_show_list);
+
   file_name_style.add_setshow_commands (no_class, _("\
 Filename display styling.\n\
 Configure filename colors and display intensity."),
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 3333c72f65a..f69df47098c 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -128,6 +128,9 @@ extern cli_style_option version_style;
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
+/* True if disassembler styling is enabled.  */
+extern bool disassembler_styling;
+
 /* True if styling is enabled.  */
 extern bool cli_styling;
 
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 44c702a7177..b4cde801cb0 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -782,9 +782,12 @@ get_all_disassembler_options (struct gdbarch *gdbarch)
 gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 				    struct ui_file *file,
 				    di_read_memory_ftype read_memory_func)
-  : m_gdbarch (gdbarch)
+  : m_gdbarch (gdbarch),
+    m_buffer (!use_ext_lang_colorization_p && disassembler_styling
+	      && file->can_emit_style_escape ()),
+    m_dest (file)
 {
-  init_disassemble_info (&m_di, file, dis_asm_fprintf);
+  init_disassemble_info (&m_di, &m_buffer, dis_asm_fprintf);
   m_di.flavour = bfd_target_unknown_flavour;
   m_di.memory_error_func = dis_asm_memory_error;
   m_di.print_address_func = dis_asm_print_address;
@@ -813,14 +816,65 @@ gdb_disassembler::~gdb_disassembler ()
   disassemble_free_target (&m_di);
 }
 
+/* See disasm.h.  */
+
+bool gdb_disassembler::use_ext_lang_colorization_p = true;
+
+/* See disasm.h.  */
+
 int
 gdb_disassembler::print_insn (CORE_ADDR memaddr,
 			      int *branch_delay_insns)
 {
   m_err_memaddr.reset ();
+  m_buffer.clear ();
 
   int length = gdbarch_print_insn (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 (length > 0 && disassembler_styling
+      && use_ext_lang_colorization_p
+      && m_dest->can_emit_style_escape ())
+    {
+      gdb::optional<std::string> ext_contents;
+      ext_contents = ext_lang_colorize_disasm (m_buffer.string (), arch ());
+      if (ext_contents.has_value ())
+	m_buffer = std::move (*ext_contents);
+      else
+	{
+	  /* The extension language failed to add styling to the
+	     disassembly output.  Set the static flag so that next time we
+	     disassemble we don't even bother attempting to use the
+	     extension language for styling.  */
+	  use_ext_lang_colorization_p = 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
+	     need to recreate m_buffer, but this time with styling support.
+
+	     To do this we perform an in-place new, but this time turn on
+	     the styling support, then we can re-disassembly the
+	     instruction, and gain any minimal styling GDB might add.  */
+	  gdb_static_assert ((std::is_same<decltype (m_buffer),
+			      string_file>::value));
+	  gdb_assert (!m_buffer.term_out ());
+	  m_buffer.~string_file ();
+	  new (&m_buffer) string_file (true);
+	  length = gdbarch_print_insn (arch (), memaddr, &m_di);
+	  gdb_assert (length > 0);
+	}
+    }
+
+  /* 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 ());
+
+  /* If the disassembler failed then report an appropriate error.  */
   if (length < 0)
     {
       if (m_err_memaddr.has_value ())
diff --git a/gdb/disasm.h b/gdb/disasm.h
index 359fb6a67fd..399afc5ae71 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -82,6 +82,31 @@ class gdb_disassembler
      non-memory error.  */
   gdb::optional<CORE_ADDR> m_err_memaddr;
 
+  /* 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 in general is turned on, and also, whether *m_dest supports
+     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.
+
+     This field will initially be true, but will be set to false if
+     ext_lang_colorize_disasm fails to add styling at any time.
+
+     If the extension language is going to add the styling then m_buffer
+     should be created without styling support, the extension language will
+     then add styling at the end of the disassembly process.
+
+     If the extension language is not going to add the styling, then we
+     create m_buffer with styling support, and GDB will add minimal styling
+     (currently just to addresses and symbols) as it goes.  */
+  static bool use_ext_lang_colorization_p;
+
   static int dis_asm_fprintf (void *stream, const char *format, ...)
     ATTRIBUTE_PRINTF(2,3);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8e7fb2d9020..7cd89912908 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26132,6 +26132,21 @@
 
 @item show style sources
 Show the current state of source code styling.
+
+@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}
+command, is styled.  Disassembler styling only works if styling in
+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}.
+
+@item show style disassembler enabled
+Show the current state of disassembler styling.
 @end table
 
 Subcommands of @code{set style} control specific forms of styling.
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index ed2121a127b..d9450b51231 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -257,6 +257,12 @@ struct extension_language_ops
      or an empty option.  */
   gdb::optional<std::string> (*colorize) (const std::string &name,
 					  const std::string &contents);
+
+  /* Colorize a single line of disassembler output, CONTENT.  This should
+     either return colorized (using ANSI terminal escapes) version of the
+     contents, or an empty optional.  */
+  gdb::optional<std::string> (*colorize_disasm) (const std::string &content,
+						 gdbarch *gdbarch);
 };
 
 /* State necessary to restore a signal handler to its previous value.  */
diff --git a/gdb/extension.c b/gdb/extension.c
index f04e928d33d..8f39b86e952 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -904,6 +904,26 @@ ext_lang_colorize (const std::string &filename, const std::string &contents)
   return result;
 }
 
+/* See extension.h.  */
+
+gdb::optional<std::string>
+ext_lang_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  gdb::optional<std::string> result;
+
+  for (const struct extension_language_defn *extlang : extension_languages)
+    {
+      if (extlang->ops == nullptr
+	  || extlang->ops->colorize_disasm == nullptr)
+	continue;
+      result = extlang->ops->colorize_disasm (content, gdbarch);
+      if (result.has_value ())
+	return result;
+    }
+
+  return result;
+}
+
 /* Called via an observer before gdb prints its prompt.
    Iterate over the extension languages giving them a chance to
    change the prompt.  The first one to change the prompt wins,
diff --git a/gdb/extension.h b/gdb/extension.h
index 64d7396f5b7..7eb89530c44 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -319,6 +319,14 @@ extern void get_matching_xmethod_workers
 extern gdb::optional<std::string> ext_lang_colorize
   (const std::string &filename, const std::string &contents);
 
+/* Try to colorize a single line of disassembler output, CONTENT for
+   GDBARCH.  This will return either a colorized (using ANSI terminal
+   escapes) version of CONTENT, or an empty value if colorizing could not
+   be done.  */
+
+extern gdb::optional<std::string> ext_lang_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
+
 #if GDB_SELF_TEST
 namespace selftests {
 extern void (*hook_set_active_ext_lang) ();
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 9734a0d9437..891f89093f1 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -264,7 +264,20 @@ try:
         except:
             return None
 
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip().encode()
+        except:
+            return None
+
+
 except:
 
     def colorize(filename, contents):
         return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 2e659ee6e14..cc80dc1daf9 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -121,6 +121,8 @@ static enum ext_lang_rc gdbpy_before_prompt_hook
   (const struct extension_language_defn *, const char *current_gdb_prompt);
 static gdb::optional<std::string> gdbpy_colorize
   (const std::string &filename, const std::string &contents);
+static gdb::optional<std::string> gdbpy_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
 
 /* The interface between gdb proper and loading of python scripts.  */
 
@@ -162,6 +164,8 @@ static const struct extension_language_ops python_extension_ops =
   gdbpy_get_matching_xmethod_workers,
 
   gdbpy_colorize,
+
+  gdbpy_colorize_disasm,
 };
 
 #endif /* HAVE_PYTHON */
@@ -1213,6 +1217,69 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
   return std::string (PyBytes_AsString (result.get ()));
 }
 
+/* This is the extension_language_ops.colorize_disasm "method".  */
+
+static gdb::optional<std::string>
+gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  if (!gdb_python_initialized)
+    return {};
+
+  gdbpy_enter enter_py;
+
+  if (gdb_python_module == nullptr
+      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+    return {};
+
+  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+					    "colorize_disasm"));
+  if (hook == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyCallable_Check (hook.get ()))
+    return {};
+
+  gdbpy_ref<> content_arg (PyBytes_FromString (content.c_str ()));
+  if (content_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> gdbarch_arg (gdbarch_to_arch_object (gdbarch));
+  if (gdbarch_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> result (PyObject_CallFunctionObjArgs (hook.get (),
+						    content_arg.get (),
+						    gdbarch_arg.get (),
+						    nullptr));
+  if (result == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (result == Py_None)
+    return {};
+
+  if (!PyBytes_Check (result.get ()))
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       _("Return value from gdb.colorize_disasm should be a bytes object or None."));
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  return std::string (PyBytes_AsString (result.get ()));
+}
+
 \f
 
 /* Printing.  */
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 2cd155d2cf2..68196d6e3e2 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -13,6 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+load_lib gdb-python.exp
+
 # Test CLI output styling.
 
 standard_testfile
@@ -187,8 +189,16 @@ proc run_style_tests { } {
 
 	gdb_test_no_output "set width 0"
 
+	# If disassembler styling is being done by the Python pygments
+	# module, then we can't be sure how the 'some_called_function'
+	# symbol will be styled.  However, if pygments is not being
+	# used then we can know how the symbol name will be styled.
 	set main [limited_style main function]
-	set func [limited_style some_called_function function]
+	if { $::python_disassembly_styling } {
+	    set func "some_called_function"
+	} else {
+	    set func [limited_style some_called_function function]
+	}
 	# Somewhere should see the call to the function.
 	gdb_test "disassemble main" \
 	    [concat "Dump of assembler code for function $main:.*" \
@@ -304,6 +314,62 @@ proc run_style_tests { } {
     }
 }
 
+# Check that disassembler styling can be disabled.  The function that
+# we are disassembling has some minimal styling applied even if the
+# Python pygments module is not available, so, when we disable
+# disassembler styling, we should always see a change in output.
+proc test_disable_disassembler_styling { } {
+    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 $::binfile
+
+	set styled_hex [limited_style $::hex address]
+	set main [limited_style main function]
+
+	foreach_with_prefix disasm_styling { on off } {
+	    gdb_test_no_output "set style disassembler enabled ${disasm_styling}"
+
+	    set saw_header_line false
+	    set saw_styled_output_line false
+	    set saw_unstyled_output_line false
+	    gdb_test_multiple "disassemble main" "" {
+		-re "disassemble main\r\n" {
+		    exp_continue
+		}
+		-re "^Dump of assembler code for function $main:" {
+		    set saw_header_line true
+		    exp_continue
+		}
+		-re "^\\s+${styled_hex}\\s+<\[^>\]+>:\\s+\[^\r\n\033\]+\r\n" {
+		    set saw_unstyled_output_line true
+		    exp_continue
+		}
+		-re "^\\s+${styled_hex}\\s+<\[^>\]+>:\\s+\[^\r\n\]+\033\[^\r\n\]+\r\n" {
+		    set saw_styled_output_line true
+		    exp_continue
+		}
+		-re "^End of assembler dump\\.\r\n" {
+		    exp_continue
+		}
+		-re "^$::gdb_prompt $" {
+		    gdb_assert { $saw_header_line }
+		    if { $disasm_styling } {
+			gdb_assert { $saw_styled_output_line }
+			gdb_assert { !$saw_unstyled_output_line }
+		    } else {
+			gdb_assert { !$saw_styled_output_line }
+			gdb_assert { $saw_unstyled_output_line }
+		    }
+		}
+	    }
+	}
+    }
+}
+
 # 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).
@@ -317,6 +383,15 @@ proc test_startup_version_string { } {
     gdb_test "" "${vers}.*" "version is styled at startup"
 }
 
+# Check to see if the Python styling of disassembler output is
+# expected or not, this styling requires Python support in GDB, and
+# the Python pygments module to be available.
+clean_restart ${binfile}
+if {![skip_python_tests] && [gdb_py_module_available "pygments"]} {
+    set python_disassembly_styling true
+} else {
+    set python_disassembly_styling false
+}
 
 # Run tests with all styles in their default state.
 with_test_prefix "all styles enabled" {
@@ -333,5 +408,10 @@ foreach style { title file function highlight variable \
     }
 }
 
+# Check that the disassembler styling can be disabled.
+if { $python_disassembly_styling } {
+    test_disable_disassembler_styling
+}
+
 # Finally, check the styling of the version string during startup.
 test_startup_version_string
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 3df9f936da5..d8bc3fb3e24 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -191,6 +191,14 @@ class string_file : public ui_file
     return ret;
   }
 
+  /* Set the internal buffer contents to STR.  Any existing contents are
+     discarded.  */
+  string_file &operator= (std::string &&str)
+  {
+    m_string = std::move (str);
+    return *this;
+  }
+
   /* Provide a few convenience methods with the same API as the
      underlying std::string.  */
   const char *data () const { return m_string.data (); }

Re: [PATCHv4 1/2] gdb: use python to colorize disassembler output

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> In the patch below, the only significant change, that I think needs
Andrew> review is the new method string_file::operator=.  I'd feel better if
Andrew> another pair of eyes has looked at that before I push it.

Looks good.
Thank you for doing this.

Tom

Re: [PATCHv4 1/2] gdb: use python to colorize disassembler output

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> In the patch below, the only significant change, that I think needs
> Andrew> review is the new method string_file::operator=.  I'd feel better if
> Andrew> another pair of eyes has looked at that before I push it.
>
> Looks good.
> Thank you for doing this.

Thanks, I pushed these patches.

Andrew

[PATCHv4 2/2] gdb/python: move styling support to gdb.styling

[Andrew Burgess]

From: Andrew Burgess <andrew.burgess@embecosm.com>

This commit moves the two Python functions that are used for styling
into a new module, gdb.styling, there's then a small update in
python.c so GDB can find the functions in their new location.

The motivation for this change is purely to try and reduce the clutter
in the top-level gdb module, and encapsulate related functions into
modules.  I did ponder documenting these functions as part of the
Python API, however, doing so would effectively "fix" the API, and I'm
still wondering if there's improvements that could be made, also, the
colorize function is only called in some cases now that GDB prefers
libsource-highlight, so it's not entirely sure how this would work as
part of a user facing API.

Still, despite these functions never having been part of a documented
API, it is possible that a user out there has overridden these to, in
some way, customize how GDB performs styling.  Moving the function as
I propose in this patch could break things for that user, however,
fixing this breakage is trivial, and, as these functions were never
documented, I don't think we should be obliged to not break user code
that relies on them.
---
 gdb/data-directory/Makefile.in |  1 +
 gdb/python/lib/gdb/__init__.py | 31 ---------------------
 gdb/python/lib/gdb/styling.py  | 49 ++++++++++++++++++++++++++++++++++
 gdb/python/python.c            | 24 ++++++++++++-----
 4 files changed, 68 insertions(+), 37 deletions(-)
 create mode 100644 gdb/python/lib/gdb/styling.py

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 6e219e09efc..b606fc654b5 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -74,6 +74,7 @@ PYTHON_FILE_LIST = \
 	gdb/frames.py \
 	gdb/printing.py \
 	gdb/prompt.py \
+	gdb/styling.py \
 	gdb/types.py \
 	gdb/unwinder.py \
 	gdb/xmethod.py \
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 8cbf1c0791d..880a2c093d2 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -229,34 +229,3 @@ def find_pc_line(pc):
     """find_pc_line (pc) -> Symtab_and_line.
     Return the gdb.Symtab_and_line object corresponding to the pc value."""
     return current_progspace().find_pc_line(pc)
-
-
-try:
-    from pygments import formatters, lexers, highlight
-
-    def colorize(filename, contents):
-        # Don't want any errors.
-        try:
-            lexer = lexers.get_lexer_for_filename(filename, stripnl=False)
-            formatter = formatters.TerminalFormatter()
-            return highlight(contents, lexer, formatter)
-        except:
-            return None
-
-    def colorize_disasm(content, gdbarch):
-        # Don't want any errors.
-        try:
-            lexer = lexers.get_lexer_by_name("asm")
-            formatter = formatters.TerminalFormatter()
-            return highlight(content, lexer, formatter).rstrip()
-        except:
-            return None
-
-
-except:
-
-    def colorize(filename, contents):
-        return None
-
-    def colorize_disasm(content, gdbarch):
-        return None
diff --git a/gdb/python/lib/gdb/styling.py b/gdb/python/lib/gdb/styling.py
new file mode 100644
index 00000000000..2e13913d870
--- /dev/null
+++ b/gdb/python/lib/gdb/styling.py
@@ -0,0 +1,49 @@
+# Styling related hooks.
+# Copyright (C) 2010-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Utilities for styling."""
+
+import gdb
+
+try:
+    from pygments import formatters, lexers, highlight
+
+    def colorize(filename, contents):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_for_filename(filename, stripnl=False)
+            formatter = formatters.TerminalFormatter()
+            return highlight(contents, lexer, formatter)
+        except:
+            return None
+
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip()
+        except:
+            return None
+
+
+except:
+
+    def colorize(filename, contents):
+        return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 0d1a1256e9a..810377b64a0 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1131,11 +1131,17 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  if (gdb_python_module == nullptr
-      || !PyObject_HasAttrString (gdb_python_module, "colorize"))
+  gdbpy_ref<> module (PyImport_ImportModule ("gdb.styling"));
+  if (module == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyObject_HasAttrString (module.get (), "colorize"))
     return {};
 
-  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module, "colorize"));
+  gdbpy_ref<> hook (PyObject_GetAttrString (module.get (), "colorize"));
   if (hook == nullptr)
     {
       gdbpy_print_stack ();
@@ -1199,11 +1205,17 @@ gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  if (gdb_python_module == nullptr
-      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+  gdbpy_ref<> module (PyImport_ImportModule ("gdb.styling"));
+  if (module == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyObject_HasAttrString (module.get (), "colorize_disasm"))
     return {};
 
-  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+  gdbpy_ref<> hook (PyObject_GetAttrString (module.get (),
 					    "colorize_disasm"));
   if (hook == nullptr)
     {
-- 
2.25.4

Re: [PATCHv4 2/2] gdb/python: move styling support to gdb.styling

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> From: Andrew Burgess <andrew.burgess@embecosm.com>
Andrew> This commit moves the two Python functions that are used for styling
Andrew> into a new module, gdb.styling, there's then a small update in
Andrew> python.c so GDB can find the functions in their new location.

Makes sense to me.

Andrew> Still, despite these functions never having been part of a documented
Andrew> API, it is possible that a user out there has overridden these to, in
Andrew> some way, customize how GDB performs styling.  Moving the function as
Andrew> I propose in this patch could break things for that user, however,
Andrew> fixing this breakage is trivial, and, as these functions were never
Andrew> documented, I don't think we should be obliged to not break user code
Andrew> that relies on them.

Also, in Python it's easy to have the code dynamically adjust, because
it can examine the module and change its behavior.  So this sort of
change is pretty easy to adapt to if need be.

Tom

Re: [PATCHv4 2/2] gdb/python: move styling support to gdb.styling

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> also, the
Andrew> colorize function is only called in some cases now that GDB prefers
Andrew> libsource-highlight, so it's not entirely sure how this would work as
Andrew> part of a user facing API.

Oops, I meant to respond to this too.  I think this could be changed in
some way, say a user-facing parameter, or some kind of Python hook.
I just didn't happen to need it and thought at the time that gdb should
prefer a GNU tool by default.

Tom

[PATCHv5 0/2] Disassembler Output Styling

[Andrew Burgess]

Changes in v5:

  Rebased onto current master.

  Simplified the colorize_disasm API in match my latest proposal here:
    https://sourceware.org/pipermail/gdb-patches/2022-January/185060.html
  That is, the colorize_disasm takes a bytes object, and is expected
  to return a bytes object, nothing else is allowed (ecept for None,
  which is a special case).

Changes in v4:

  Rebased on to current master.  The colorize_disasm function is now
  passed a bytes object rather than a unicode string.  The return from
  colorize_disasm can be either a bytes object, or a unicode string.
  If GDB gets a unicode string, then it is converted to bytes using
  the current host_charset().

  There's no change in the documentation in this version, the
  documentation has already been reviewed in a previous version.

Changes in v3:

  In this update, the biggest change, is that the controlling setting is
  now 'set/show style disassembler enabled', that is I've added
  'enabled' to the setting name.
  
  My reason for this is that, in the future, I might want to add
  additional disassembler styles, like:
  
    set style disassembler register ....
    set style disassembler mnemonic ....
  
  And the new setting name leaves this possibility open, while the old
  setting made this harder.
  
  This version also includes an improved test, minor updates to the docs
  to match the above change, and a rebase to current master.


---

Andrew Burgess (2):
  gdb: use python to colorize disassembler output
  gdb/python: move styling support to gdb.styling

 gdb/NEWS                         |  6 +++
 gdb/cli/cli-style.c              | 44 +++++++++++++++++
 gdb/cli/cli-style.h              |  3 ++
 gdb/data-directory/Makefile.in   |  1 +
 gdb/disasm.c                     | 58 +++++++++++++++++++++-
 gdb/disasm.h                     | 25 ++++++++++
 gdb/doc/gdb.texinfo              | 15 ++++++
 gdb/extension-priv.h             |  6 +++
 gdb/extension.c                  | 20 ++++++++
 gdb/extension.h                  |  8 +++
 gdb/python/lib/gdb/__init__.py   | 18 -------
 gdb/python/lib/gdb/styling.py    | 49 ++++++++++++++++++
 gdb/python/python.c              | 85 ++++++++++++++++++++++++++++++--
 gdb/testsuite/gdb.base/style.exp | 82 +++++++++++++++++++++++++++++-
 14 files changed, 396 insertions(+), 24 deletions(-)
 create mode 100644 gdb/python/lib/gdb/styling.py

-- 
2.25.4

[PATCHv5 1/2] gdb: use python to colorize disassembler output

[Andrew Burgess]

From: Andrew Burgess <andrew.burgess@embecosm.com>

This commit adds styling support to the disassembler output, as such
two new commands are added to GDB:

  set style disassembler enabled on|off
  show style disassembler enabled

In this commit I make use of the Python Pygments package to provide
the styling.  I did investigate making use of libsource-highlight,
however, I found the highlighting results to be inferior to those of
Pygments; only some mnemonics were highlighted, and highlighting of
register names such as r9d and r8d (on x86-64) was incorrect.

To enable disassembler highlighting via Pygments, I've added a new
extension language hook, which is then implemented for Python.  This
hook is very similar to the existing hook for source code
colorization.

One possibly odd choice I made with the new hook is to pass a
gdb.Architecture through, even though this is currently unused.  The
reason this argument is not used is that, currently, styling is
performed identically for all architectures.

However, even though the Python function used to perform styling of
disassembly output is not part of any documented API, I don't want
to close the door on a user overriding this function to provide
architecture specific styling.  To do this, the user would inevitably
require access to the gdb.Architecture, and so I decided to add this
field now.

The styling is applied within gdb_disassembler::print_insn, to achieve
this, gdb_disassembler now writes its output into a temporary buffer,
styling is then applied to the contents of this buffer.  Finally the
gdb_disassembler buffer is copied out to its final destination stream.

There's a new test to check that the disassembler output includes some
escape sequences, though I don't check for specific colours; the
precise colors will depend on which instructions are in the
disassembler output, and, I guess, how pygments is configured.

The only negative change with this commit is how we currently style
addresses in GDB.

Currently, when the disassembler wants to print an address, we call
back into GDB, and GDB prints the address value using the `address`
styling, and the symbol name using `function` styling.  After this
commit, if pygments is used, then all disassembler styling is done
through pygments, and this include the address and symbol name parts
of the disassembler output.

I don't know how much of an issue this will be for people.  There's
already some precedent for this in GDB when we look at source styling.
For example, function names in styled source listings are not styled
using the `function` style, but instead, either GNU Source Highlight,
or pygments gets to decide how the function name should be styled.

If the Python pygments library is not present then GDB will continue
to behave as it always has, the disassembler output is mostly
unstyled, but the address and symbols are styled using the `address`
and `function` styles, as they are today.

However, if the user does `set style disassembler enabled off`, then
all disassembler styling is switched off.  This obviously covers the
use of pygments, but also includes the minimal styling done by GDB
when pygments is not available.
---
 gdb/NEWS                         |  6 +++
 gdb/cli/cli-style.c              | 44 +++++++++++++++++
 gdb/cli/cli-style.h              |  3 ++
 gdb/disasm.c                     | 58 +++++++++++++++++++++-
 gdb/disasm.h                     | 25 ++++++++++
 gdb/doc/gdb.texinfo              | 15 ++++++
 gdb/extension-priv.h             |  6 +++
 gdb/extension.c                  | 20 ++++++++
 gdb/extension.h                  |  8 ++++
 gdb/python/lib/gdb/__init__.py   | 13 +++++
 gdb/python/python.c              | 67 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/style.exp | 82 +++++++++++++++++++++++++++++++-
 12 files changed, 344 insertions(+), 3 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 8c13cefb22f..bbd5daa5a38 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -81,6 +81,12 @@ maint show gnu-source-highlight enabled
   styling to a particular source file, then the Python Pygments
   library will be used instead.
 
+set style disassembler enabled on|off
+show style disassembler enabled
+  If GDB is compiled with Python support, and the Python Pygments
+  package is available, then, when this setting is on, disassembler
+  output will have styling applied.
+
 * Changed commands
 
 maint packet
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index 2fd00e9cc3e..6c1652d3986 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -38,6 +38,11 @@ bool cli_styling = true;
 
 bool source_styling = true;
 
+/* True if disassembler styling is enabled.  Note that this is only
+   consulted when cli_styling is true.  */
+
+bool disassembler_styling = true;
+
 /* Name of colors; must correspond to ui_file_style::basic_color.  */
 static const char * const cli_colors[] = {
   "none",
@@ -274,6 +279,14 @@ cli_style_option::add_setshow_commands (enum command_class theclass,
 static cmd_list_element *style_set_list;
 static cmd_list_element *style_show_list;
 
+/* The command list for 'set style disassembler'.  */
+
+static cmd_list_element *style_disasm_set_list;
+
+/* The command list for 'show style disassembler'.  */
+
+static cmd_list_element *style_disasm_show_list;
+
 static void
 set_style_enabled  (const char *args, int from_tty, struct cmd_list_element *c)
 {
@@ -301,6 +314,18 @@ show_style_sources (struct ui_file *file, int from_tty,
     fprintf_filtered (file, _("Source code styling is disabled.\n"));
 }
 
+/* Implement 'show style disassembler'.  */
+
+static void
+show_style_disassembler (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  if (disassembler_styling)
+    fprintf_filtered (file, _("Disassembler output styling is enabled.\n"));
+  else
+    fprintf_filtered (file, _("Disassembler output styling is disabled.\n"));
+}
+
 void _initialize_cli_style ();
 void
 _initialize_cli_style ()
@@ -337,6 +362,25 @@ available if the appropriate extension is available at runtime."
 			   ), set_style_enabled, show_style_sources,
 			   &style_set_list, &style_show_list);
 
+  add_setshow_prefix_cmd ("disassembler", no_class,
+			  _("\
+Style-specific settings for the disassembler.\n\
+Configure various disassembler style-related variables."),
+			  _("\
+Style-specific settings for the disassembler.\n\
+Configure various disassembler style-related variables."),
+			  &style_disasm_set_list, &style_disasm_show_list,
+			  &style_set_list, &style_show_list);
+
+  add_setshow_boolean_cmd ("enabled", no_class, &disassembler_styling, _("\
+Set whether disassembler output styling is enabled."), _("\
+Show whether disassembler output styling is enabled."), _("\
+If enabled, disassembler output is styled.  Disassembler highlighting\n\
+requires the Python Pygments library, if this library is not available\n\
+then disassembler highlighting will not be possible."
+			   ), set_style_enabled, show_style_disassembler,
+			   &style_disasm_set_list, &style_disasm_show_list);
+
   file_name_style.add_setshow_commands (no_class, _("\
 Filename display styling.\n\
 Configure filename colors and display intensity."),
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 3333c72f65a..f69df47098c 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -128,6 +128,9 @@ extern cli_style_option version_style;
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
+/* True if disassembler styling is enabled.  */
+extern bool disassembler_styling;
+
 /* True if styling is enabled.  */
 extern bool cli_styling;
 
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 724040faa92..cd6ce47f9fa 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -782,9 +782,12 @@ get_all_disassembler_options (struct gdbarch *gdbarch)
 gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 				    struct ui_file *file,
 				    di_read_memory_ftype read_memory_func)
-  : m_gdbarch (gdbarch)
+  : m_gdbarch (gdbarch),
+    m_buffer (!use_ext_lang_colorization_p && disassembler_styling
+	      && file->can_emit_style_escape ()),
+    m_dest (file)
 {
-  init_disassemble_info (&m_di, file, dis_asm_fprintf);
+  init_disassemble_info (&m_di, &m_buffer, dis_asm_fprintf);
   m_di.flavour = bfd_target_unknown_flavour;
   m_di.memory_error_func = dis_asm_memory_error;
   m_di.print_address_func = dis_asm_print_address;
@@ -813,14 +816,65 @@ gdb_disassembler::~gdb_disassembler ()
   disassemble_free_target (&m_di);
 }
 
+/* See disasm.h.  */
+
+bool gdb_disassembler::use_ext_lang_colorization_p = true;
+
+/* See disasm.h.  */
+
 int
 gdb_disassembler::print_insn (CORE_ADDR memaddr,
 			      int *branch_delay_insns)
 {
   m_err_memaddr.reset ();
+  m_buffer.clear ();
 
   int length = gdbarch_print_insn (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 (length > 0 && disassembler_styling
+      && use_ext_lang_colorization_p
+      && m_dest->can_emit_style_escape ())
+    {
+      gdb::optional<std::string> ext_contents;
+      ext_contents = ext_lang_colorize_disasm (m_buffer.string (), arch ());
+      if (ext_contents.has_value ())
+	m_buffer.string () = std::move (*ext_contents);
+      else
+	{
+	  /* The extension language failed to add styling to the
+	     disassembly output.  Set the static flag so that next time we
+	     disassemble we don't even bother attempting to use the
+	     extension language for styling.  */
+	  use_ext_lang_colorization_p = 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
+	     need to recreate m_buffer, but this time with styling support.
+
+	     To do this we perform an in-place new, but this time turn on
+	     the styling support, then we can re-disassembly the
+	     instruction, and gain any minimal styling GDB might add.  */
+	  gdb_static_assert ((std::is_same<decltype (m_buffer),
+			      string_file>::value));
+	  gdb_assert (!m_buffer.term_out ());
+	  m_buffer.~string_file ();
+	  new (&m_buffer) string_file (true);
+	  length = gdbarch_print_insn (arch (), memaddr, &m_di);
+	  gdb_assert (length > 0);
+	}
+    }
+
+  /* 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 ());
+
+  /* If the disassembler failed then report an appropriate error.  */
   if (length < 0)
     {
       if (m_err_memaddr.has_value ())
diff --git a/gdb/disasm.h b/gdb/disasm.h
index d739b57d898..3c14c50be3a 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -82,6 +82,31 @@ class gdb_disassembler
      non-memory error.  */
   gdb::optional<CORE_ADDR> m_err_memaddr;
 
+  /* 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 in general is turned on, and also, whether *m_dest supports
+     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.
+
+     This field will initially be true, but will be set to false if
+     ext_lang_colorize_disasm fails to add styling at any time.
+
+     If the extension language is going to add the styling then m_buffer
+     should be created without styling support, the extension language will
+     then add styling at the end of the disassembly process.
+
+     If the extension language is not going to add the styling, then we
+     create m_buffer with styling support, and GDB will add minimal styling
+     (currently just to addresses and symbols) as it goes.  */
+  static bool use_ext_lang_colorization_p;
+
   static int dis_asm_fprintf (void *stream, const char *format, ...)
     ATTRIBUTE_PRINTF(2,3);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index fe81687a66c..2a7cae62215 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26132,6 +26132,21 @@
 
 @item show style sources
 Show the current state of source code styling.
+
+@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}
+command, is styled.  Disassembler styling only works if styling in
+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}.
+
+@item show style disassembler enabled
+Show the current state of disassembler styling.
 @end table
 
 Subcommands of @code{set style} control specific forms of styling.
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index ed2121a127b..d9450b51231 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -257,6 +257,12 @@ struct extension_language_ops
      or an empty option.  */
   gdb::optional<std::string> (*colorize) (const std::string &name,
 					  const std::string &contents);
+
+  /* Colorize a single line of disassembler output, CONTENT.  This should
+     either return colorized (using ANSI terminal escapes) version of the
+     contents, or an empty optional.  */
+  gdb::optional<std::string> (*colorize_disasm) (const std::string &content,
+						 gdbarch *gdbarch);
 };
 
 /* State necessary to restore a signal handler to its previous value.  */
diff --git a/gdb/extension.c b/gdb/extension.c
index f04e928d33d..8f39b86e952 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -904,6 +904,26 @@ ext_lang_colorize (const std::string &filename, const std::string &contents)
   return result;
 }
 
+/* See extension.h.  */
+
+gdb::optional<std::string>
+ext_lang_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  gdb::optional<std::string> result;
+
+  for (const struct extension_language_defn *extlang : extension_languages)
+    {
+      if (extlang->ops == nullptr
+	  || extlang->ops->colorize_disasm == nullptr)
+	continue;
+      result = extlang->ops->colorize_disasm (content, gdbarch);
+      if (result.has_value ())
+	return result;
+    }
+
+  return result;
+}
+
 /* Called via an observer before gdb prints its prompt.
    Iterate over the extension languages giving them a chance to
    change the prompt.  The first one to change the prompt wins,
diff --git a/gdb/extension.h b/gdb/extension.h
index 64d7396f5b7..7eb89530c44 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -319,6 +319,14 @@ extern void get_matching_xmethod_workers
 extern gdb::optional<std::string> ext_lang_colorize
   (const std::string &filename, const std::string &contents);
 
+/* Try to colorize a single line of disassembler output, CONTENT for
+   GDBARCH.  This will return either a colorized (using ANSI terminal
+   escapes) version of CONTENT, or an empty value if colorizing could not
+   be done.  */
+
+extern gdb::optional<std::string> ext_lang_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
+
 #if GDB_SELF_TEST
 namespace selftests {
 extern void (*hook_set_active_ext_lang) ();
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 11a1b444bfd..1bfb12daef8 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -243,7 +243,20 @@ try:
         except:
             return None
 
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip().encode()
+        except:
+            return None
+
+
 except:
 
     def colorize(filename, contents):
         return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 4dcda53d9ab..1bdb4af33cb 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -121,6 +121,8 @@ static enum ext_lang_rc gdbpy_before_prompt_hook
   (const struct extension_language_defn *, const char *current_gdb_prompt);
 static gdb::optional<std::string> gdbpy_colorize
   (const std::string &filename, const std::string &contents);
+static gdb::optional<std::string> gdbpy_colorize_disasm
+  (const std::string &content, gdbarch *gdbarch);
 
 /* The interface between gdb proper and loading of python scripts.  */
 
@@ -162,6 +164,8 @@ static const struct extension_language_ops python_extension_ops =
   gdbpy_get_matching_xmethod_workers,
 
   gdbpy_colorize,
+
+  gdbpy_colorize_disasm,
 };
 
 #endif /* HAVE_PYTHON */
@@ -1195,6 +1199,69 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
   return std::string (PyBytes_AsString (host_str.get ()));
 }
 
+/* This is the extension_language_ops.colorize_disasm "method".  */
+
+static gdb::optional<std::string>
+gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
+{
+  if (!gdb_python_initialized)
+    return {};
+
+  gdbpy_enter enter_py (get_current_arch (), current_language);
+
+  if (gdb_python_module == nullptr
+      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+    return {};
+
+  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+					    "colorize_disasm"));
+  if (hook == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyCallable_Check (hook.get ()))
+    return {};
+
+  gdbpy_ref<> content_arg (PyBytes_FromString (content.c_str ()));
+  if (content_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> gdbarch_arg (gdbarch_to_arch_object (gdbarch));
+  if (gdbarch_arg == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  gdbpy_ref<> result (PyObject_CallFunctionObjArgs (hook.get (),
+						    content_arg.get (),
+						    gdbarch_arg.get (),
+						    nullptr));
+  if (result == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (result == Py_None)
+    return {};
+
+  if (!PyBytes_Check (result.get ()))
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       _("Return value from gdb.colorize_disasm should be a bytes object or None."));
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  return std::string (PyBytes_AsString (result.get ()));
+}
+
 \f
 
 /* Printing.  */
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 2cd155d2cf2..68196d6e3e2 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -13,6 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+load_lib gdb-python.exp
+
 # Test CLI output styling.
 
 standard_testfile
@@ -187,8 +189,16 @@ proc run_style_tests { } {
 
 	gdb_test_no_output "set width 0"
 
+	# If disassembler styling is being done by the Python pygments
+	# module, then we can't be sure how the 'some_called_function'
+	# symbol will be styled.  However, if pygments is not being
+	# used then we can know how the symbol name will be styled.
 	set main [limited_style main function]
-	set func [limited_style some_called_function function]
+	if { $::python_disassembly_styling } {
+	    set func "some_called_function"
+	} else {
+	    set func [limited_style some_called_function function]
+	}
 	# Somewhere should see the call to the function.
 	gdb_test "disassemble main" \
 	    [concat "Dump of assembler code for function $main:.*" \
@@ -304,6 +314,62 @@ proc run_style_tests { } {
     }
 }
 
+# Check that disassembler styling can be disabled.  The function that
+# we are disassembling has some minimal styling applied even if the
+# Python pygments module is not available, so, when we disable
+# disassembler styling, we should always see a change in output.
+proc test_disable_disassembler_styling { } {
+    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 $::binfile
+
+	set styled_hex [limited_style $::hex address]
+	set main [limited_style main function]
+
+	foreach_with_prefix disasm_styling { on off } {
+	    gdb_test_no_output "set style disassembler enabled ${disasm_styling}"
+
+	    set saw_header_line false
+	    set saw_styled_output_line false
+	    set saw_unstyled_output_line false
+	    gdb_test_multiple "disassemble main" "" {
+		-re "disassemble main\r\n" {
+		    exp_continue
+		}
+		-re "^Dump of assembler code for function $main:" {
+		    set saw_header_line true
+		    exp_continue
+		}
+		-re "^\\s+${styled_hex}\\s+<\[^>\]+>:\\s+\[^\r\n\033\]+\r\n" {
+		    set saw_unstyled_output_line true
+		    exp_continue
+		}
+		-re "^\\s+${styled_hex}\\s+<\[^>\]+>:\\s+\[^\r\n\]+\033\[^\r\n\]+\r\n" {
+		    set saw_styled_output_line true
+		    exp_continue
+		}
+		-re "^End of assembler dump\\.\r\n" {
+		    exp_continue
+		}
+		-re "^$::gdb_prompt $" {
+		    gdb_assert { $saw_header_line }
+		    if { $disasm_styling } {
+			gdb_assert { $saw_styled_output_line }
+			gdb_assert { !$saw_unstyled_output_line }
+		    } else {
+			gdb_assert { !$saw_styled_output_line }
+			gdb_assert { $saw_unstyled_output_line }
+		    }
+		}
+	    }
+	}
+    }
+}
+
 # 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).
@@ -317,6 +383,15 @@ proc test_startup_version_string { } {
     gdb_test "" "${vers}.*" "version is styled at startup"
 }
 
+# Check to see if the Python styling of disassembler output is
+# expected or not, this styling requires Python support in GDB, and
+# the Python pygments module to be available.
+clean_restart ${binfile}
+if {![skip_python_tests] && [gdb_py_module_available "pygments"]} {
+    set python_disassembly_styling true
+} else {
+    set python_disassembly_styling false
+}
 
 # Run tests with all styles in their default state.
 with_test_prefix "all styles enabled" {
@@ -333,5 +408,10 @@ foreach style { title file function highlight variable \
     }
 }
 
+# Check that the disassembler styling can be disabled.
+if { $python_disassembly_styling } {
+    test_disable_disassembler_styling
+}
+
 # Finally, check the styling of the version string during startup.
 test_startup_version_string
-- 
2.25.4

[PATCHv5 2/2] gdb/python: move styling support to gdb.styling

[Andrew Burgess]

From: Andrew Burgess <andrew.burgess@embecosm.com>

This commit moves the two Python functions that are used for styling
into a new module, gdb.styling, there's then a small update in
python.c so GDB can find the functions in their new location.

The motivation for this change is purely to try and reduce the clutter
in the top-level gdb module, and encapsulate related functions into
modules.  I did ponder documenting these functions as part of the
Python API, however, doing so would effectively "fix" the API, and I'm
still wondering if there's improvements that could be made, also, the
colorize function is only called in some cases now that GDB prefers
libsource-highlight, so it's not entirely sure how this would work as
part of a user facing API.

Still, despite these functions never having been part of a documented
API, it is possible that a user out there has overridden these to, in
some way, customize how GDB performs styling.  Moving the function as
I propose in this patch could break things for that user, however,
fixing this breakage is trivial, and, as these functions were never
documented, I don't think we should be obliged to not break user code
that relies on them.
---
 gdb/data-directory/Makefile.in |  1 +
 gdb/python/lib/gdb/__init__.py | 31 ---------------------
 gdb/python/lib/gdb/styling.py  | 49 ++++++++++++++++++++++++++++++++++
 gdb/python/python.c            | 24 ++++++++++++-----
 4 files changed, 68 insertions(+), 37 deletions(-)
 create mode 100644 gdb/python/lib/gdb/styling.py

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 6e219e09efc..b606fc654b5 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -74,6 +74,7 @@ PYTHON_FILE_LIST = \
 	gdb/frames.py \
 	gdb/printing.py \
 	gdb/prompt.py \
+	gdb/styling.py \
 	gdb/types.py \
 	gdb/unwinder.py \
 	gdb/xmethod.py \
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 1bfb12daef8..880a2c093d2 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -229,34 +229,3 @@ def find_pc_line(pc):
     """find_pc_line (pc) -> Symtab_and_line.
     Return the gdb.Symtab_and_line object corresponding to the pc value."""
     return current_progspace().find_pc_line(pc)
-
-
-try:
-    from pygments import formatters, lexers, highlight
-
-    def colorize(filename, contents):
-        # Don't want any errors.
-        try:
-            lexer = lexers.get_lexer_for_filename(filename, stripnl=False)
-            formatter = formatters.TerminalFormatter()
-            return highlight(contents, lexer, formatter)
-        except:
-            return None
-
-    def colorize_disasm(content, gdbarch):
-        # Don't want any errors.
-        try:
-            lexer = lexers.get_lexer_by_name("asm")
-            formatter = formatters.TerminalFormatter()
-            return highlight(content, lexer, formatter).rstrip().encode()
-        except:
-            return None
-
-
-except:
-
-    def colorize(filename, contents):
-        return None
-
-    def colorize_disasm(content, gdbarch):
-        return None
diff --git a/gdb/python/lib/gdb/styling.py b/gdb/python/lib/gdb/styling.py
new file mode 100644
index 00000000000..2991dac3d99
--- /dev/null
+++ b/gdb/python/lib/gdb/styling.py
@@ -0,0 +1,49 @@
+# Styling related hooks.
+# Copyright (C) 2010-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Utilities for styling."""
+
+import gdb
+
+try:
+    from pygments import formatters, lexers, highlight
+
+    def colorize(filename, contents):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_for_filename(filename, stripnl=False)
+            formatter = formatters.TerminalFormatter()
+            return highlight(contents, lexer, formatter)
+        except:
+            return None
+
+    def colorize_disasm(content, gdbarch):
+        # Don't want any errors.
+        try:
+            lexer = lexers.get_lexer_by_name("asm")
+            formatter = formatters.TerminalFormatter()
+            return highlight(content, lexer, formatter).rstrip().encode()
+        except:
+            return None
+
+
+except:
+
+    def colorize(filename, contents):
+        return None
+
+    def colorize_disasm(content, gdbarch):
+        return None
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 1bdb4af33cb..a0aa03e4aeb 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1141,11 +1141,17 @@ gdbpy_colorize (const std::string &filename, const std::string &contents)
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  if (gdb_python_module == nullptr
-      || !PyObject_HasAttrString (gdb_python_module, "colorize"))
+  gdbpy_ref<> module (PyImport_ImportModule ("gdb.styling"));
+  if (module == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyObject_HasAttrString (module.get (), "colorize"))
     return {};
 
-  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module, "colorize"));
+  gdbpy_ref<> hook (PyObject_GetAttrString (module.get (), "colorize"));
   if (hook == nullptr)
     {
       gdbpy_print_stack ();
@@ -1209,11 +1215,17 @@ gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  if (gdb_python_module == nullptr
-      || !PyObject_HasAttrString (gdb_python_module, "colorize_disasm"))
+  gdbpy_ref<> module (PyImport_ImportModule ("gdb.styling"));
+  if (module == nullptr)
+    {
+      gdbpy_print_stack ();
+      return {};
+    }
+
+  if (!PyObject_HasAttrString (module.get (), "colorize_disasm"))
     return {};
 
-  gdbpy_ref<> hook (PyObject_GetAttrString (gdb_python_module,
+  gdbpy_ref<> hook (PyObject_GetAttrString (module.get (),
 					    "colorize_disasm"));
   if (hook == nullptr)
     {
-- 
2.25.4

Re: [PATCHv5 0/2] Disassembler Output Styling

[Andrew Burgess]

Ping!

Any objections if I merge this?

Patch #2 was already given the OK right back in V1.

Patch #1 had some general points raised in V1, which I've addressed, but
nothing serious.  So I think this is probably OK to push, but I want to
give folk a change to complain before I go for it.

Thanks,
Andrew



Andrew Burgess <aburgess@redhat.com> writes:

> Changes in v5:
>
>   Rebased onto current master.
>
>   Simplified the colorize_disasm API in match my latest proposal here:
>     https://sourceware.org/pipermail/gdb-patches/2022-January/185060.html
>   That is, the colorize_disasm takes a bytes object, and is expected
>   to return a bytes object, nothing else is allowed (ecept for None,
>   which is a special case).
>
> Changes in v4:
>
>   Rebased on to current master.  The colorize_disasm function is now
>   passed a bytes object rather than a unicode string.  The return from
>   colorize_disasm can be either a bytes object, or a unicode string.
>   If GDB gets a unicode string, then it is converted to bytes using
>   the current host_charset().
>
>   There's no change in the documentation in this version, the
>   documentation has already been reviewed in a previous version.
>
> Changes in v3:
>
>   In this update, the biggest change, is that the controlling setting is
>   now 'set/show style disassembler enabled', that is I've added
>   'enabled' to the setting name.
>   
>   My reason for this is that, in the future, I might want to add
>   additional disassembler styles, like:
>   
>     set style disassembler register ....
>     set style disassembler mnemonic ....
>   
>   And the new setting name leaves this possibility open, while the old
>   setting made this harder.
>   
>   This version also includes an improved test, minor updates to the docs
>   to match the above change, and a rebase to current master.
>
>
> ---
>
> Andrew Burgess (2):
>   gdb: use python to colorize disassembler output
>   gdb/python: move styling support to gdb.styling
>
>  gdb/NEWS                         |  6 +++
>  gdb/cli/cli-style.c              | 44 +++++++++++++++++
>  gdb/cli/cli-style.h              |  3 ++
>  gdb/data-directory/Makefile.in   |  1 +
>  gdb/disasm.c                     | 58 +++++++++++++++++++++-
>  gdb/disasm.h                     | 25 ++++++++++
>  gdb/doc/gdb.texinfo              | 15 ++++++
>  gdb/extension-priv.h             |  6 +++
>  gdb/extension.c                  | 20 ++++++++
>  gdb/extension.h                  |  8 +++
>  gdb/python/lib/gdb/__init__.py   | 18 -------
>  gdb/python/lib/gdb/styling.py    | 49 ++++++++++++++++++
>  gdb/python/python.c              | 85 ++++++++++++++++++++++++++++++--
>  gdb/testsuite/gdb.base/style.exp | 82 +++++++++++++++++++++++++++++-
>  14 files changed, 396 insertions(+), 24 deletions(-)
>  create mode 100644 gdb/python/lib/gdb/styling.py
>
> -- 
> 2.25.4