Re: [PATCHv3 2/2] gdb/python: extend the Python Disassembler API to allow for styling

[Andrew Burgess <aburgess@redhat.com>]

Andrew Burgess <aburgess@redhat.com> writes:

> This commit extends the Python Disassembler API to allow for styling
> of the instructions.
>
> Before this commit the Python Disassembler API allowed the user to do
> two things:
>
>   - They could intercept instruction disassembly requests and return a
>     string of their choosing, this string then became the disassembled
>     instruction, or
>
>   - They could call builtin_disassemble, which would call back into
>     libopcode to perform the disassembly.  As libopcode printed the
>     instruction GDB would collect these print requests and build a
>     string.  This string was then returned from the builtin_disassemble
>     call, and the user could modify or extend this string as needed.
>
> Neither of these approaches allowed for, or preserved, disassembler
> styling, which is now available within libopcodes for many of the more
> popular architectures GDB supports.
>
> This commit aims to fill this gap.  After this commit a user will be
> able to do the following things:
>
>   - Implement a custom instruction disassembler entirely in Python
>     without calling back into libopcodes, the custom disassembler will
>     be able to return styling information such that GDB will display
>     the instruction fully styled.  All of GDB's existing style
>     settings will affect how instructions coming from the Python
>     disassembler are displayed in the expected manner.
>
>   - Call builtin_disassemble and receive a result that represents how
>     libopcode would like the instruction styled.  The user can then
>     adjust or extend the disassembled instruction before returning the
>     result to GDB.  Again, the instruction will be styled as expected.
>
> To achieve this I will add two new classes to GDB,
> DisassemblerTextPart and DisassemblerAddressPart.
>
> Within builtin_disassemble, instead of capturing the print calls from
> libopcodes and building a single string, we will now create either a
> text part or address part and store these parts in a vector.
>
> The DisassemblerTextPart will capture a small piece of text along with
> the associated style that should be used to display the text.  This
> corresponds to the disassembler calling
> disassemble_info::fprintf_styled_func, or for disassemblers that don't
> support styling disassemble_info::fprintf_func.
>
> The DisassemblerAddressPart is used when libopcodes requests that an
> address be printed, and takes care of printing the address and
> associated symbol, this corresponds to the disassembler calling
> disassemble_info::print_address_func.
>
> These parts are then placed within the DisassemblerResult when
> builtin_disassemble returns.
>
> Alternatively, the user can directly create parts by calling two new
> methods on the DisassembleInfo class: DisassembleInfo.text_part and
> DisassembleInfo.address_part.
>
> Having created these parts the user can then pass these parts when
> initializing a new DisassemblerResult object.
>
> Finally, when we return from Python to gdbpy_print_insn, one way or
> another, the result being returned will have a list of parts.  Back in
> GDB's C++ code we walk the list of parts and call back into GDB's core
> to display the disassembled instruction with the correct styling.
>
> The new API lives in parallel with the old API.  Any existing code
> that creates a DisassemblerResult using a single string immediately
> creates a single DisassemblerTextPart containing the entire
> instruction and gives this part the default text style.  This is also
> what happens if the user calls builtin_disassemble for an architecture
> that doesn't (yet) support libopcode styling.
>
> This matches up with what happens when the Python API is not involved,
> an architecture without disassembler styling support uses the old
> libopcodes printing API (the API that doesn't pass style info), and
> GDB just prints everything using the default text style.
>
> The reason that parts are created by calling methods on
> DisassembleInfo, rather than calling the class constructor directly,
> is DisassemblerAddressPart.  Ideally this part would only hold the
> address which the part represents, but in order to support backwards
> compatibility we need to be able to convert the
> DisassemblerAddressPart into a string.  To do that we need to call
> GDB's internal print_address function, and to do that we need an
> gdbarch.
>
> What this means is that the DisassemblerAddressPart needs to take a
> gdb.Architecture object at creation time.  The only valid place a user
> can pull this from is from the DisassembleInfo object, so having the
> DisassembleInfo act as a factory ensures that the correct gdbarch is
> passed over each time.  I implemented both solutions (the one
> presented here, and an alternative where parts could be constructed
> directly), and this felt like the cleanest solution.
>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Pushed the patch below to fix the formatting of a .py file.

Sorry for the noise.

Thanks,
Andrew

--

commit 66b8e6c7b8d3c137029f4f2ab8c5b798aa1cbdd7
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue May 16 11:59:45 2023 +0100

    gdb/testsuite: fix formatting of gdb.python/py-disasm.py
    
    Run black on gdb.python/py-disasm.py file and commit the changes.

diff --git a/gdb/testsuite/gdb.python/py-disasm.py b/gdb/testsuite/gdb.python/py-disasm.py
index ec6b0e8deca..67ba6756ea9 100644
--- a/gdb/testsuite/gdb.python/py-disasm.py
+++ b/gdb/testsuite/gdb.python/py-disasm.py
@@ -31,7 +31,7 @@ def builtin_disassemble_wrapper(info):
     assert len(result.parts) > 0
     tmp_str = ""
     for p in result.parts:
-        assert(p.string == str(p))
+        assert p.string == str(p)
         tmp_str += p.string
     assert tmp_str == result.string
     return result
@@ -586,9 +586,9 @@ class All_Text_Part_Styles(TestDisassembler):
         result = builtin_disassemble_wrapper(info)
         result = DisassemblerResult(length=result.length, parts=parts)
 
-        tmp_str = "";
+        tmp_str = ""
         for p in parts:
-            assert (p.string == str(p))
+            assert p.string == str(p)
             tmp_str += str(p)
         assert tmp_str == result.string