Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble

[Doug Evans <dje at google dot com>]

On Tue, Feb 5, 2013 at 5:53 PM, Siva Chandra <sivachandra@google.com> wrote:
> On Tue, Feb 5, 2013 at 3:28 PM, Doug Evans <dje@google.com> wrote:
>> I like the idea, but for an API I wouldn't mind seeing something
>> a bit lower level.  E.g., skip the higher level disassembler entry
>> points in gdb (mixed source/assembly support and all that), and provide
>> more direct access to the disassembler.
>
> The only useful entry point currently available is gdb_disassembly and
> I do not think it is a bad entry point. Other disassembly functions in
> disasm.c are static.

Well, it begs the question what entrypoint into libopcodes does
gdb_disassembly use? :-)
Anyways, my point is gdb_disassembly is a bit high level for the
Python API for me.

> However, for the Python API, my patch provides
> only one option of whether to include or exclude opcodes in the
> disassembled output.

Today.

Someone will think "Oh look, I can just add a flag and get more features."
with no real consideration if that level in the API is the right place
for those features.

>> I didn't go through it with a fine toothed comb, but here are some questions.
>> 1) Can we remove the py_out global?
>
> At what level do you not want this to be global? I have made it static
> to the file in the attached patch.

This has been addressed elsewhere.

>> 2) It seems like this will export a lot of struct ui_out to the user.
>>    I'd rather provide disassembly without having to commit to supporting
>>    struct ui_out in Python.
>
> I am not very sure I understand this point. My patch does not expose
> anything about the struct ui_out to the user/Python API. Python ui_out
> is only a way to get results from GDB internals into a Python data
> structure. Also, this Python data structure does not depend on
> gdb_disassembly's display format.

Let's take a step back and assume one wants to add disassembly to Python,
without knowing anything about what gdb does and doesn't already provide.

If I ask to disassemble an instruction at a particular address I
expect to get back a string (with possibly an error thrown or some
such if the memory at that address isn't readable).
Let's see an enumeration of the other things one might want to get back.

- symbol name of a referenced address?
  - or at least the address so that the caller can do the symbol
lookup, which might be preferable
- raw bytes of the instruction?
- length of the instruction?
- anything else?

Hmmm, another thing occurs to me:
Global State In An API Is Bad.
What are the other inputs to disassembly, and should they be parameters?
- syntax?
  - for now we could use the CLI setting as the default
- architecture variant? (including endianness)
   - all aspects of this are presumably encoded in the
gdb.Architecture class so no need for an explicit parameter, included
here for completeness sake
- anything else?

If not, we're forcing the caller to do a "save current global state,
change it, catch the call to the disassembler so we can properly
restore state, restore global state" dance.
We need to move away from this in APIs (we're suffering from aspects
of this in gdb's internal APIs, let's learn this lesson and apply it).

> 2013-02-05  Siva Chandra Reddy  <sivachandra@google.com>
>
>         Add a new method 'disassemble' to gdb.Architecture class.
>         * Makefile.in: Add entries for the new file python/py-out.c
>         * python/py-arch.c (archpy_disassmble): Implementation of the
>         new method gdb.Architecture.disassemble.
>         (arch_object_methods): Add entry for the new method.
>         * python/py-out.c: Implementation of a Python ui_out.
>         * python/python-internal.h: Add declarations for new utility
>         functions.
>         * python/python.c (_initialize_python): Initialize Python
>         ui_out.