[PATCHv5 3/5] gdb/python: implement the print_insn extension language hook

[Eli Zaretskii]

> Date: Fri,  6 May 2022 18:17:12 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <andrew.burgess@embecosm.com>
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 982f4a1a18c..ddbaff51f89 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -38,6 +38,40 @@ maintenance info line-table
>       This is the same format that GDB uses when printing address, symbol,
>       and offset information from the disassembler.
>  
> +  ** New Python API for wrapping GDB's disassembler:
> +
> +     - gdb.disassembler.register_disassembler(DISASSEMBLER, ARCH).
> +       DISASSEMBLER is a sub-class of gdb.disassembler.Disassembler.
> +       ARCH is either None or a string containing a bfd architecture
> +       name.  DISASSEMBLER is registered as a disassembler for
> +       architecture ARCH, or for all architectures if ARCH is None.
> +       The previous disassembler registered for ARCH is returned, this
> +       can be None if no previous disassembler was registered.
> +
> +     - gdb.disassembler.Disassembler is the class from which all
> +       disassemblers should inherit.  Its constructor takes a string,
> +       a name for the disassembler, which is currently only used in
> +       some debug output.  Sub-classes should override the __call__
> +       method to perform disassembly, invoking __call__ on this base
> +       class will raise an exception.
> +
> +     - gdb.disassembler.DisassembleInfo is the class used to describe
> +       a single disassembly request from GDB.  An instance of this
> +       class is passed to the __call__ method of
> +       gdb.disassembler.Disassembler and has the following read-only
> +       attributes: 'address', and 'architecture', as well as the
> +       following method: 'read_memory'.
> +
> +     - gdb.disassembler.builtin_disassemble(INFO, MEMORY_SOURCE),
> +       calls GDB's builtin disassembler on INFO, which is a
> +       gdb.disassembler.DisassembleInfo object.  MEMORY_SOURCE is
> +       optional, its default value is None.  If MEMORY_SOURCE is not
> +       None then it must be an object that has a 'read_memory' method.
> +
> +     - gdb.disassembler.DisassemblerResult is a class that can be used
> +       to wrap the result of a call to a Disassembler.  It has
> +       read-only attributes 'length' and 'string'.
> +
>  *** Changes in GDB 12

This part is OK.

> +@smallexample
> +(@value{GDBP}) show architecture
> +The target architecture is set to "auto" (currently "i386").
> +(@value{GDBP}) maint info python-disassemblers
> +Architecture        Disassember Name
> +i386                Disassembler_1	(Matches current architecture)
> +GLOBAL              Disassembler_2
> +(@value{GDBP}) set architecture arm
> +The target architecture is set to "arm".
> +(@value{GDBP}) maint info python-disassemblers
> +quit
> +Architecture        Disassember Name
> +i386                Disassembler_1
> +GLOBAL              Disassembler_2	(Matches current architecture)
> +@end smallexample

It is best to subdivide this long example into at least 2 groups using
@group..@end group.  This way, the example will not be broken between
2 pages in some arbitrary place.  My suggestion is to do this:

  +@smallexample
  + group
  +(@value{GDBP}) show architecture
  +The target architecture is set to "auto" (currently "i386").
  +(@value{GDBP}) maint info python-disassemblers
  +Architecture        Disassember Name
  +i386                Disassembler_1	(Matches current architecture)
  +GLOBAL              Disassembler_2
  +@end group
  +@group
  +(@value{GDBP}) set architecture arm
  +The target architecture is set to "arm".
  +(@value{GDBP}) maint info python-disassemblers
  +quit
  +Architecture        Disassember Name
  +i386                Disassembler_1
  +GLOBAL              Disassembler_2	(Matches current architecture)
  +@end group
  +@end smallexample

> +Consider, for example, and architecture with 2-byte and 4-byte
                          ^^^
I think you meant "an" there.

> +order to establish if the instruction is 2 or 4 bytes long.  If the
> +instruction is 4-bytes long then the disassembler might then read the
                               ^^^^                        ^^^^
Two "then" in a row; one of them is redundant.

> +@defun DisassemblerResult.__init__ (@var{length}, @var{string})
> +Initialise an instance of this class, @var{length} is the length of
   ^^^^^^^^^^
"Initialize".  We use the US spelling.

> +To see which disassemblers have been registered the @kbd{maint info
> +python-disassemblers} command can be used (@pxref{maint info
> +python-disassemblers}).

It is better to use active tense: "you can use ... to see which
disassemblers ...".

Thanks.