* [PATCH 0/4] Disassembler Output Styling @ 2021-10-26 9:37 Andrew Burgess 2021-10-26 9:37 ` [PATCH 1/4] gdb/python: make some global variables static Andrew Burgess ` (4 more replies) 0 siblings, 5 replies; 36+ messages in thread From: Andrew Burgess @ 2021-10-26 9:37 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/4] gdb/python: make some global variables static 2021-10-26 9:37 [PATCH 0/4] Disassembler Output Styling Andrew Burgess @ 2021-10-26 9:37 ` Andrew Burgess 2021-10-27 20:20 ` Tom Tromey 2021-10-26 9:37 ` [PATCH 2/4] gdb: rename source_styling_changed observer Andrew Burgess ` (3 subsequent siblings) 4 siblings, 1 reply; 36+ messages in thread From: Andrew Burgess @ 2021-10-26 9:37 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/4] gdb/python: make some global variables static 2021-10-26 9:37 ` [PATCH 1/4] gdb/python: make some global variables static Andrew Burgess @ 2021-10-27 20:20 ` Tom Tromey 2021-11-25 10:12 ` Andrew Burgess 0 siblings, 1 reply; 36+ messages in thread From: Tom Tromey @ 2021-10-27 20:20 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/4] gdb/python: make some global variables static 2021-10-27 20:20 ` Tom Tromey @ 2021-11-25 10:12 ` Andrew Burgess 2021-11-25 15:02 ` Enze Li 0 siblings, 1 reply; 36+ messages in thread From: Andrew Burgess @ 2021-11-25 10:12 UTC (permalink / raw) To: Tom Tromey; +Cc: Andrew Burgess, gdb-patches * 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/4] gdb/python: make some global variables static 2021-11-25 10:12 ` Andrew Burgess @ 2021-11-25 15:02 ` Enze Li 2021-11-25 18:11 ` Andrew Burgess 0 siblings, 1 reply; 36+ messages in thread From: Enze Li @ 2021-11-25 15:02 UTC (permalink / raw) To: Andrew Burgess, Tom Tromey; +Cc: Andrew Burgess, gdb-patches 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/4] gdb/python: make some global variables static 2021-11-25 15:02 ` Enze Li @ 2021-11-25 18:11 ` Andrew Burgess 0 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2021-11-25 18:11 UTC (permalink / raw) To: Enze Li; +Cc: Tom Tromey, Andrew Burgess, gdb-patches * 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; ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/4] gdb: rename source_styling_changed observer 2021-10-26 9:37 [PATCH 0/4] Disassembler Output Styling Andrew Burgess 2021-10-26 9:37 ` [PATCH 1/4] gdb/python: make some global variables static Andrew Burgess @ 2021-10-26 9:37 ` Andrew Burgess 2021-10-27 20:22 ` Tom Tromey 2021-10-26 9:37 ` [PATCH 3/4] gdb: use python to colorize disassembler output Andrew Burgess ` (2 subsequent siblings) 4 siblings, 1 reply; 36+ messages in thread From: Andrew Burgess @ 2021-10-26 9:37 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/4] gdb: rename source_styling_changed observer 2021-10-26 9:37 ` [PATCH 2/4] gdb: rename source_styling_changed observer Andrew Burgess @ 2021-10-27 20:22 ` Tom Tromey 2021-11-25 10:17 ` Andrew Burgess 0 siblings, 1 reply; 36+ messages in thread From: Tom Tromey @ 2021-10-27 20:22 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/4] gdb: rename source_styling_changed observer 2021-10-27 20:22 ` Tom Tromey @ 2021-11-25 10:17 ` Andrew Burgess 0 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2021-11-25 10:17 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches * 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 3/4] gdb: use python to colorize disassembler output 2021-10-26 9:37 [PATCH 0/4] Disassembler Output Styling Andrew Burgess 2021-10-26 9:37 ` [PATCH 1/4] gdb/python: make some global variables static Andrew Burgess 2021-10-26 9:37 ` [PATCH 2/4] gdb: rename source_styling_changed observer Andrew Burgess @ 2021-10-26 9:37 ` Andrew Burgess 2021-10-27 20:38 ` Tom Tromey 2021-10-26 9:37 ` [PATCH 4/4] gdb/python: move styling support to gdb.styling Andrew Burgess 2021-11-25 10:36 ` [PATCHv2 0/2] Disassembler Output Styling Andrew Burgess 4 siblings, 1 reply; 36+ messages in thread From: Andrew Burgess @ 2021-10-26 9:37 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/4] gdb: use python to colorize disassembler output 2021-10-26 9:37 ` [PATCH 3/4] gdb: use python to colorize disassembler output Andrew Burgess @ 2021-10-27 20:38 ` Tom Tromey 2021-10-28 16:28 ` Andrew Burgess 0 siblings, 1 reply; 36+ messages in thread From: Tom Tromey @ 2021-10-27 20:38 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/4] gdb: use python to colorize disassembler output 2021-10-27 20:38 ` Tom Tromey @ 2021-10-28 16:28 ` Andrew Burgess 2021-11-22 14:44 ` Andrew Burgess 0 siblings, 1 reply; 36+ messages in thread From: Andrew Burgess @ 2021-10-28 16:28 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches 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" { ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/4] gdb: use python to colorize disassembler output 2021-10-28 16:28 ` Andrew Burgess @ 2021-11-22 14:44 ` Andrew Burgess 0 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2021-11-22 14:44 UTC (permalink / raw) To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches 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" { ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 4/4] gdb/python: move styling support to gdb.styling 2021-10-26 9:37 [PATCH 0/4] Disassembler Output Styling Andrew Burgess ` (2 preceding siblings ...) 2021-10-26 9:37 ` [PATCH 3/4] gdb: use python to colorize disassembler output Andrew Burgess @ 2021-10-26 9:37 ` Andrew Burgess 2021-10-27 20:39 ` Tom Tromey 2021-11-25 10:36 ` [PATCHv2 0/2] Disassembler Output Styling Andrew Burgess 4 siblings, 1 reply; 36+ messages in thread From: Andrew Burgess @ 2021-10-26 9:37 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 4/4] gdb/python: move styling support to gdb.styling 2021-10-26 9:37 ` [PATCH 4/4] gdb/python: move styling support to gdb.styling Andrew Burgess @ 2021-10-27 20:39 ` Tom Tromey 0 siblings, 0 replies; 36+ messages in thread From: Tom Tromey @ 2021-10-27 20:39 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv2 0/2] Disassembler Output Styling 2021-10-26 9:37 [PATCH 0/4] Disassembler Output Styling Andrew Burgess ` (3 preceding siblings ...) 2021-10-26 9:37 ` [PATCH 4/4] gdb/python: move styling support to gdb.styling Andrew Burgess @ 2021-11-25 10:36 ` Andrew Burgess 2021-11-25 10:36 ` [PATCHv2 1/2] gdb: use python to colorize disassembler output Andrew Burgess ` (3 more replies) 4 siblings, 4 replies; 36+ messages in thread From: Andrew Burgess @ 2021-11-25 10:36 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv2 1/2] gdb: use python to colorize disassembler output 2021-11-25 10:36 ` [PATCHv2 0/2] Disassembler Output Styling Andrew Burgess @ 2021-11-25 10:36 ` Andrew Burgess 2021-11-25 11:04 ` Eli Zaretskii 2021-11-25 10:36 ` [PATCHv2 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess ` (2 subsequent siblings) 3 siblings, 1 reply; 36+ messages in thread From: Andrew Burgess @ 2021-11-25 10:36 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCHv2 1/2] gdb: use python to colorize disassembler output 2021-11-25 10:36 ` [PATCHv2 1/2] gdb: use python to colorize disassembler output Andrew Burgess @ 2021-11-25 11:04 ` Eli Zaretskii 0 siblings, 0 replies; 36+ messages in thread From: Eli Zaretskii @ 2021-11-25 11:04 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches > 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. ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv2 2/2] gdb/python: move styling support to gdb.styling 2021-11-25 10:36 ` [PATCHv2 0/2] Disassembler Output Styling Andrew Burgess 2021-11-25 10:36 ` [PATCHv2 1/2] gdb: use python to colorize disassembler output Andrew Burgess @ 2021-11-25 10:36 ` Andrew Burgess 2021-12-06 14:32 ` Ping: [PATCHv2 0/2] Disassembler Output Styling Andrew Burgess 2021-12-13 14:12 ` [PATCHv3 " Andrew Burgess 3 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2021-11-25 10:36 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Ping: [PATCHv2 0/2] Disassembler Output Styling 2021-11-25 10:36 ` [PATCHv2 0/2] Disassembler Output Styling Andrew Burgess 2021-11-25 10:36 ` [PATCHv2 1/2] gdb: use python to colorize disassembler output Andrew Burgess 2021-11-25 10:36 ` [PATCHv2 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess @ 2021-12-06 14:32 ` Andrew Burgess 2021-12-13 14:12 ` [PATCHv3 " Andrew Burgess 3 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2021-12-06 14:32 UTC (permalink / raw) To: gdb-patches 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 > ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3 0/2] Disassembler Output Styling 2021-11-25 10:36 ` [PATCHv2 0/2] Disassembler Output Styling Andrew Burgess ` (2 preceding siblings ...) 2021-12-06 14:32 ` Ping: [PATCHv2 0/2] Disassembler Output Styling Andrew Burgess @ 2021-12-13 14:12 ` Andrew Burgess 2021-12-13 14:12 ` [PATCHv3 1/2] gdb: use python to colorize disassembler output Andrew Burgess ` (2 more replies) 3 siblings, 3 replies; 36+ messages in thread From: Andrew Burgess @ 2021-12-13 14:12 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3 1/2] gdb: use python to colorize disassembler output 2021-12-13 14:12 ` [PATCHv3 " Andrew Burgess @ 2021-12-13 14:12 ` Andrew Burgess 2021-12-13 14:12 ` [PATCHv3 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess 2022-01-11 14:30 ` [PATCHv4 0/2] Disassembler Output Styling Andrew Burgess 2 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2021-12-13 14:12 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3 2/2] gdb/python: move styling support to gdb.styling 2021-12-13 14:12 ` [PATCHv3 " Andrew Burgess 2021-12-13 14:12 ` [PATCHv3 1/2] gdb: use python to colorize disassembler output Andrew Burgess @ 2021-12-13 14:12 ` Andrew Burgess 2022-01-11 14:30 ` [PATCHv4 0/2] Disassembler Output Styling Andrew Burgess 2 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2021-12-13 14:12 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv4 0/2] Disassembler Output Styling 2021-12-13 14:12 ` [PATCHv3 " Andrew Burgess 2021-12-13 14:12 ` [PATCHv3 1/2] gdb: use python to colorize disassembler output Andrew Burgess 2021-12-13 14:12 ` [PATCHv3 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess @ 2022-01-11 14:30 ` Andrew Burgess 2022-01-11 14:31 ` [PATCHv4 1/2] gdb: use python to colorize disassembler output Andrew Burgess ` (2 more replies) 2 siblings, 3 replies; 36+ messages in thread From: Andrew Burgess @ 2022-01-11 14:30 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv4 1/2] gdb: use python to colorize disassembler output 2022-01-11 14:30 ` [PATCHv4 0/2] Disassembler Output Styling Andrew Burgess @ 2022-01-11 14:31 ` Andrew Burgess 2022-02-10 21:13 ` Tom Tromey 2022-01-11 14:31 ` [PATCHv4 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess 2022-01-21 16:26 ` [PATCHv5 0/2] Disassembler Output Styling Andrew Burgess 2 siblings, 1 reply; 36+ messages in thread From: Andrew Burgess @ 2022-01-11 14:31 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCHv4 1/2] gdb: use python to colorize disassembler output 2022-01-11 14:31 ` [PATCHv4 1/2] gdb: use python to colorize disassembler output Andrew Burgess @ 2022-02-10 21:13 ` Tom Tromey 2022-02-11 14:27 ` Andrew Burgess 0 siblings, 1 reply; 36+ messages in thread From: Tom Tromey @ 2022-02-10 21:13 UTC (permalink / raw) To: Andrew Burgess via Gdb-patches; +Cc: Andrew Burgess, Andrew Burgess >>>>> "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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCHv4 1/2] gdb: use python to colorize disassembler output 2022-02-10 21:13 ` Tom Tromey @ 2022-02-11 14:27 ` Andrew Burgess 2022-02-13 18:02 ` Tom Tromey 0 siblings, 1 reply; 36+ messages in thread From: Andrew Burgess @ 2022-02-11 14:27 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches * 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 (); } ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCHv4 1/2] gdb: use python to colorize disassembler output 2022-02-11 14:27 ` Andrew Burgess @ 2022-02-13 18:02 ` Tom Tromey 2022-02-14 11:22 ` Andrew Burgess 0 siblings, 1 reply; 36+ messages in thread From: Tom Tromey @ 2022-02-13 18:02 UTC (permalink / raw) To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCHv4 1/2] gdb: use python to colorize disassembler output 2022-02-13 18:02 ` Tom Tromey @ 2022-02-14 11:22 ` Andrew Burgess 0 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2022-02-14 11:22 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv4 2/2] gdb/python: move styling support to gdb.styling 2022-01-11 14:30 ` [PATCHv4 0/2] Disassembler Output Styling Andrew Burgess 2022-01-11 14:31 ` [PATCHv4 1/2] gdb: use python to colorize disassembler output Andrew Burgess @ 2022-01-11 14:31 ` Andrew Burgess 2022-02-10 21:15 ` Tom Tromey 2022-02-10 21:16 ` Tom Tromey 2022-01-21 16:26 ` [PATCHv5 0/2] Disassembler Output Styling Andrew Burgess 2 siblings, 2 replies; 36+ messages in thread From: Andrew Burgess @ 2022-01-11 14:31 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCHv4 2/2] gdb/python: move styling support to gdb.styling 2022-01-11 14:31 ` [PATCHv4 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess @ 2022-02-10 21:15 ` Tom Tromey 2022-02-10 21:16 ` Tom Tromey 1 sibling, 0 replies; 36+ messages in thread From: Tom Tromey @ 2022-02-10 21:15 UTC (permalink / raw) To: Andrew Burgess via Gdb-patches; +Cc: Andrew Burgess, Andrew Burgess >>>>> "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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCHv4 2/2] gdb/python: move styling support to gdb.styling 2022-01-11 14:31 ` [PATCHv4 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess 2022-02-10 21:15 ` Tom Tromey @ 2022-02-10 21:16 ` Tom Tromey 1 sibling, 0 replies; 36+ messages in thread From: Tom Tromey @ 2022-02-10 21:16 UTC (permalink / raw) To: Andrew Burgess via Gdb-patches; +Cc: Andrew Burgess, Andrew Burgess >>>>> "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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv5 0/2] Disassembler Output Styling 2022-01-11 14:30 ` [PATCHv4 0/2] Disassembler Output Styling Andrew Burgess 2022-01-11 14:31 ` [PATCHv4 1/2] gdb: use python to colorize disassembler output Andrew Burgess 2022-01-11 14:31 ` [PATCHv4 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess @ 2022-01-21 16:26 ` Andrew Burgess 2022-01-21 16:26 ` [PATCHv5 1/2] gdb: use python to colorize disassembler output Andrew Burgess ` (2 more replies) 2 siblings, 3 replies; 36+ messages in thread From: Andrew Burgess @ 2022-01-21 16:26 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv5 1/2] gdb: use python to colorize disassembler output 2022-01-21 16:26 ` [PATCHv5 0/2] Disassembler Output Styling Andrew Burgess @ 2022-01-21 16:26 ` Andrew Burgess 2022-01-21 16:26 ` [PATCHv5 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess 2022-02-03 20:32 ` [PATCHv5 0/2] Disassembler Output Styling Andrew Burgess 2 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2022-01-21 16:26 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv5 2/2] gdb/python: move styling support to gdb.styling 2022-01-21 16:26 ` [PATCHv5 0/2] Disassembler Output Styling Andrew Burgess 2022-01-21 16:26 ` [PATCHv5 1/2] gdb: use python to colorize disassembler output Andrew Burgess @ 2022-01-21 16:26 ` Andrew Burgess 2022-02-03 20:32 ` [PATCHv5 0/2] Disassembler Output Styling Andrew Burgess 2 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2022-01-21 16:26 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCHv5 0/2] Disassembler Output Styling 2022-01-21 16:26 ` [PATCHv5 0/2] Disassembler Output Styling Andrew Burgess 2022-01-21 16:26 ` [PATCHv5 1/2] gdb: use python to colorize disassembler output Andrew Burgess 2022-01-21 16:26 ` [PATCHv5 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess @ 2022-02-03 20:32 ` Andrew Burgess 2 siblings, 0 replies; 36+ messages in thread From: Andrew Burgess @ 2022-02-03 20:32 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2022-02-14 11:22 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-26 9:37 [PATCH 0/4] Disassembler Output Styling Andrew Burgess 2021-10-26 9:37 ` [PATCH 1/4] gdb/python: make some global variables static Andrew Burgess 2021-10-27 20:20 ` Tom Tromey 2021-11-25 10:12 ` Andrew Burgess 2021-11-25 15:02 ` Enze Li 2021-11-25 18:11 ` Andrew Burgess 2021-10-26 9:37 ` [PATCH 2/4] gdb: rename source_styling_changed observer Andrew Burgess 2021-10-27 20:22 ` Tom Tromey 2021-11-25 10:17 ` Andrew Burgess 2021-10-26 9:37 ` [PATCH 3/4] gdb: use python to colorize disassembler output Andrew Burgess 2021-10-27 20:38 ` Tom Tromey 2021-10-28 16:28 ` Andrew Burgess 2021-11-22 14:44 ` Andrew Burgess 2021-10-26 9:37 ` [PATCH 4/4] gdb/python: move styling support to gdb.styling Andrew Burgess 2021-10-27 20:39 ` Tom Tromey 2021-11-25 10:36 ` [PATCHv2 0/2] Disassembler Output Styling Andrew Burgess 2021-11-25 10:36 ` [PATCHv2 1/2] gdb: use python to colorize disassembler output Andrew Burgess 2021-11-25 11:04 ` Eli Zaretskii 2021-11-25 10:36 ` [PATCHv2 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess 2021-12-06 14:32 ` Ping: [PATCHv2 0/2] Disassembler Output Styling Andrew Burgess 2021-12-13 14:12 ` [PATCHv3 " Andrew Burgess 2021-12-13 14:12 ` [PATCHv3 1/2] gdb: use python to colorize disassembler output Andrew Burgess 2021-12-13 14:12 ` [PATCHv3 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess 2022-01-11 14:30 ` [PATCHv4 0/2] Disassembler Output Styling Andrew Burgess 2022-01-11 14:31 ` [PATCHv4 1/2] gdb: use python to colorize disassembler output Andrew Burgess 2022-02-10 21:13 ` Tom Tromey 2022-02-11 14:27 ` Andrew Burgess 2022-02-13 18:02 ` Tom Tromey 2022-02-14 11:22 ` Andrew Burgess 2022-01-11 14:31 ` [PATCHv4 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess 2022-02-10 21:15 ` Tom Tromey 2022-02-10 21:16 ` Tom Tromey 2022-01-21 16:26 ` [PATCHv5 0/2] Disassembler Output Styling Andrew Burgess 2022-01-21 16:26 ` [PATCHv5 1/2] gdb: use python to colorize disassembler output Andrew Burgess 2022-01-21 16:26 ` [PATCHv5 2/2] gdb/python: move styling support to gdb.styling Andrew Burgess 2022-02-03 20:32 ` [PATCHv5 0/2] Disassembler Output Styling Andrew Burgess
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).