This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


I now have a patch (find it attached) which has been tweaked based on
comments from Doug.

Thanks Doug, for another round of detailed feedback.

2013-02-13  Siva Chandra Reddy  <sivachandra@google.com>

        Add a new method 'disassemble' to gdb.Architecture class.
        * python/py-arch.c (archpy_disassmble): Implementation of the
        new method gdb.Architecture.disassemble.
        (arch_object_methods): Add entry for the new method.

doc/

        * gdb.texinfo (Architectures In Python): Add description about
        the new method gdb.Architecture.disassemble.

testsuite/

        * gdb.python/py-arch.c: New test case
        * gdb.python/py-arch.exp: New tests to test
        gdb.Architecture.disassemble
        * gdb.python/Makefile.in: Add py-arch to the list of
        EXECUTABLES.

On Wed, Feb 13, 2013 at 12:42 PM, Doug Evans <dje@google.com> wrote:
> Reading beyond high seems counterintuitive, but I can understand why it's easiest if it works that way.

I made it that way because I used the way 'disassemble' command works
as my guideline for this Python disassemble feature. However, I made
Python API to disassemble instructions which have a start address in
[start, end], while the disassemble command does the same in [start,
end).

> Also, I can imagine a user wanting to specify a count instead of high.
> How about supporting both "high" and "count", with the default being high is unspecified and count=1?

I have now added a count argument.

> Including the function name and offset in the output feels wrong.
> It's trying to make the API call do too much.

Removed in the attached patch.

As I have mentioned above, I used the 'functionality' of the
'disassemble' command as a guide. Hence, I had put in whatever it has.
For my needs, all I want are 'addr' and 'asm', and the rest were only
good to have.

> Plus, the documentation needs to specify what "flavor" is used (e.g. intel vs att).
> I'd just add a line of texzt saying the flavor (or choose a better word) is the one specified by the CLI variable disassembly-flavor,
> and leave for another day adding an input parameter to specify the flavor.

Added a note to the docs.

I am not very sure if we should (in future) add the 'flavor' as an
argument to the disassemble method. I prefer if it were a global
function in the 'gdb' module. Again, this is my opinion as I
personally do not see why one would want to have assembly code from
different architectures in different flavors.

> Why call build_address_symbol here?
> To me it's trying to make this API call do too much.

Removed now.

> btw, what about filename,line isn't useful?

It is not that they are not useful, but that build_address_symbol does
not return useful values. I have verified this with experiments but
have not digged into the code to figure out why. There is a similar
not in disasm.c:dump_insns.

> So I can imagine 3 API routines here:
> - disassemble (pc)
> - get_function_and_offset (pc) [or some such]
> - get_file_and_line (pc) [assuming gdbpy_find_pc_line doesn't always do what one wants]

I agree. I will add the last two to my TODO list.

Thanks,
Siva Chandra

Attachment: arch_disassemble_patch_v6.txt
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]