RE: GDB 8.0 release/branching 2017-03-20 update

Metzger, Markus T
2017-03-23T16:01:44Z

> -----Original Message-----
> From: Yao Qi [mailto:qiyaoltc@gmail.com]
> Sent: Wednesday, March 22, 2017 6:09 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org; Wiederhake, Tim
> <tim.wiederhake@intel.com>; xdje42@gmail.com; Joel Brobecker
> <brobecker@adacore.com>
> Subject: Re: GDB 8.0 release/branching 2017-03-20 update

Hello Yao,

> >> 1. BtraceInstruction.sal is not necessary. It can be got via
> >> gdb.find_pc_line(BtraceInstruction.pc).
> >
> > I asked Tim to put this into the Instruction object in order to support
> > future extensions.
> >
> > At the moment, we decode from live memory, and there, you're right
> > that we can get the SAL from the PC. If we wanted to support exec()
> > or dlclose() in the future, the trace would refer to instructions that are
> > no longer available in memory and GDB would not know about them.
> >
> > Having users get the SAL from the Instruction object gives us more
> > freedom to model this. We could, for example, decode from binary
> > files and leave GDB to represent the current program state. Not sure
> > whether this would suffice - probably not. But it sounded like a good
> > idea to have users get such information via the Instruction object from
> > the beginning. That's also why the Instruction object has a data function
> > to get the raw bytes of an instruction rather than having users read
> > it from memory.
> >
> >
> >> 2. BtraceInstruction has some attributes (pc, data, decoded, and size,)
> >> which are not btrace related.
> >> They should be moved to a new class gdb.Instruction and BtraceInstruction
> >> extends it.
> >>
> >> Instruction
> >> {
> >> pc, data, decode, size;
> >> };
> >
> > That makes sense if we're considering a Python API for GDB's disassembler.
> > I would extend it to include SAL for the above reasons so users don't need
> > to know from where they got the Instruction object.
>
> IMO, SAL only makes sense to RecordInstruction. I don't see why must put
> SAL into Instruction.

It is certainly not a must. It might make things easier (or just more convenient)
but if we want a minimal Instruction base class, we would only need pc, decode
(rename to disassembly?), and size.

Record instructions will add data and sal and at least record-btrace will add
is_speculative. In order to not complicate things too much I would add
is_speculative also to record-full (even if it is always false) and put everything
into a single RecordInstruction class for all record targets. OK?

> > I'm wondering how much of this discussion is really a question of sub-classing
> > and how much of it is about documentation. Would I even be able to tell the
> > difference in my python scripts that use the API?
>
> All the user visible stuff should be covered in this discussion. We
> need to mention that "gdb.RecordInstruction extends gdb.Instruction" in
> the document, so that we don't have to document gdb.Instruction
> attributes again in gdb.RecordInstruction.
>
> >
> > The duck typing argument makes sense to me and I'm not familiar enough
> > with implementing Python to have an opinion on how this should be done.
> > Do we have an expert on this in the GDB community?
> >
>
> The duck typing is about writing python code, but I raise my concerns in
> this thread from the view of controlling the API *interface* for
> different record methods by means of inheritance. With or without
> inheritance, the python script still can do duck typing, no problem, but
> we will end up having completely different ways of organizing these
> interface. Suppose we have three record methods, "btrace", "full" and
> "bar", in current approach, we need to document all the attributes for
> these three methods, although some of them are in common. Why don't we
> put these common attributes into a base class, and only document the
> base class *once*?

Makes sense. Is this how it is usually implemented in Python?

I liked about the design that we could have a record-(target-)specific internal
representation. IIRC we only store the thread's ptid and the instruction number
in that thread's instruction history. An Instruction object provided by some
future gdb.disassemble() function will need a different internal representation.
Will we be able to do this with a class hierarchy?

> Further, with such base class, we can guarantee that the client python
> scripts only access base class attributes are right even with the new
> record method may be added in the future. In current approach, we write
> such python code "print(hex(i.pc), i.sal, i.decoded)" just because
> BtraceInstruction and FullInstruction *happen* to have these
> attributes. What if a new record method "bar" have instruction trace
> which doesn't have attribute "decoded"?

They'd get a not-implemented exception.

I don't see how we could prevent that, though. If we added another
recording method that didn't provide one of the functions the old
recording method's instruction object provided, it will have to fail
somehow.

And we don't want scripts to restrict themselves to the Instruction
base class. If they are using record-btrace we do want them to get
the sal from the instruction object, for example.

Thanks,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928